On Tue, Feb 12, 2008 at 10:00:03PM -0800, David Miller wrote: > From: James Chapman <[EMAIL PROTECTED]> > Date: Tue, 12 Feb 2008 10:58:21 +0000 > > > Here is a trace from when we had _bh locks. > > The problem is that the pppol2tp code calls sk_dst_get() in software > interrupt context and that is not allowed.
Actually, this lockdep report probably warns of something else: sk_dst_lock which is seen in process context with softirqs enabled implicitly endangers some other (ppp_generic) locks, which depend on ppp_generic pch->upl read_lock, which is taken in softirq context. I can't see on this report any hardirqs dependancies, so it seems even blocking hard interrupts, just like in this patch, shouldn't solve this problem: if BHs were really disabled in exactly the same places, then it seems this warning should trigger in both variants. I studied long ago some similar problem with pppoe, and it looked like ppp_generic needed some fix, but now I've to recall or re-learn almost all and it needs more time... Anyway, there is a possibility, this warning isn't so dangerous. > sk_dst_get() grabs sk->sk_dst_lock without any BH enabling or > disabling. > > It can do that because the usage is to make all the lock > taking calls in user context, and in the packet processing > paths use __sk_dst_get(). BTW, does it mean that ip4_datagram_connect() can be used only in user context? > Probably what the pppol2tp code should do is use __sk_dst_check() > instead of sk_dst_get(). You then have to be able to handle > NULL returns, just like UDP sendmsg() does, which means you'll > need to cook up a routing lookup if __sk_dst_check() gives you > NULL because the route became obsolete. I think that without changing ppp_generic or changing ip_queue_xmit() to something else in pppol2tp this would be really hard to avoid such a warning (and maybe not very necessary). Regards, Jarek P. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
