Re: LOCKDEP complaints in l2tp_xmit_skb()

Eric Dumazet
2012-06-28T11:22:34Z

On Thu, 2012-06-28 at 10:57 +0200, Eric Dumazet wrote:
> On Thu, 2012-06-28 at 08:56 +0200, Eric Dumazet wrote:
>
> > [PATCH] net: Qdisc busylock gets its own lockdep class
> >
> > Tom Parkin reported following LOCKDEP splat :
> ..
> >
> > Instruct lockdep that each Qdisc busylock is independant, or else
> > bonding or various tunnels can trigger a splat.
> >
> > Reported-by: Tom Parkin <tparkin@katalix.com>
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > ---
>
> I reproduced the bug using a bond0 device, adding a qdisc on it,
> (one Qdisc on bond0, and a Qdisc on the slave too)
>
> Problem with this patch is I have following message :
>
> BUG: key ffff88..... not in .data!
>
> No more LOCKDEP splat, but patch not good as is.

I tested the alternative following patch with my bonding setup,
could you test it with l2tp ?

net/core/dev.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 6df2140..d265c67 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2412,13 +2412,13 @@ static inline int __dev_xmit_skb(struct sk_buff *skb, struct Qdisc *q,

qdisc_bstats_update(q, skb);

- if (sch_direct_xmit(skb, q, dev, txq, root_lock)) {
- if (unlikely(contended)) {
- spin_unlock(&q->busylock);
- contended = false;
- }
+ if (unlikely(contended)) {
+ spin_unlock(&q->busylock);
+ contended = false;
+ }
+ if (sch_direct_xmit(skb, q, dev, txq, root_lock))
__qdisc_run(q);
- } else
+ else
qdisc_run_end(q);

rc = NET_XMIT_SUCCESS;