lockdep report from 2.6.20.5-rc1

lockdep report from 2.6.20.5-rc1

[Dave Jones]

=======================================================
[ INFO: possible circular locking dependency detected ]
2.6.20-1.2933.fc6debug #1
-------------------------------------------------------
swapper/0 is trying to acquire lock:
 (&tbl->lock){-+-+}, at: [<c05d5664>] neigh_lookup+0x43/0xa2

but task is already holding lock:
 (&list->lock#4){-+..}, at: [<c05d65c8>] neigh_proxy_process+0x20/0xc2

which lock already depends on the new lock.


the existing dependency chain (in reverse order) is:

-> #2 (&list->lock#4){-+..}:
       [<c043f4f2>] __lock_acquire+0x913/0xa43
       [<c043f933>] lock_acquire+0x56/0x6f
       [<c06325df>] _spin_lock_irqsave+0x34/0x44
       [<c05cbb87>] skb_dequeue+0x12/0x43
       [<c05cc9d4>] skb_queue_purge+0x14/0x1b
       [<c05d5b70>] neigh_update+0x349/0x3a5
       [<c060cd37>] arp_process+0x4d1/0x50a
       [<c060ce53>] arp_rcv+0xe3/0x100
       [<c05d0e43>] netif_receive_skb+0x2db/0x35a
       [<c05d2806>] process_backlog+0x95/0xf6
       [<c05d29ed>] net_rx_action+0xa1/0x1a8
       [<c042c1f6>] __do_softirq+0x6f/0xe2
       [<c04063c6>] do_softirq+0x61/0xd0
       [<ffffffff>] 0xffffffff

-> #1 (&n->lock){-+-+}:
       [<c043f4f2>] __lock_acquire+0x913/0xa43
       [<c043f933>] lock_acquire+0x56/0x6f
       [<c063231e>] _write_lock+0x2b/0x38
       [<c05d74b7>] neigh_periodic_timer+0x99/0x138
       [<c042f053>] run_timer_softirq+0x104/0x168
       [<c042c1f6>] __do_softirq+0x6f/0xe2
       [<c04063c6>] do_softirq+0x61/0xd0
       [<ffffffff>] 0xffffffff

-> #0 (&tbl->lock){-+-+}:
       [<c043f3f3>] __lock_acquire+0x814/0xa43
       [<c043f933>] lock_acquire+0x56/0x6f
       [<c06323d6>] _read_lock_bh+0x30/0x3d
       [<c05d5664>] neigh_lookup+0x43/0xa2
       [<c05d6317>] neigh_event_ns+0x2c/0x7a
       [<c060cbec>] arp_process+0x386/0x50a
       [<c060ce78>] parp_redo+0x8/0xa
       [<c05d660e>] neigh_proxy_process+0x66/0xc2
       [<c042f053>] run_timer_softirq+0x104/0x168
       [<c042c1f6>] __do_softirq+0x6f/0xe2
       [<c04063c6>] do_softirq+0x61/0xd0
       [<ffffffff>] 0xffffffff

other info that might help us debug this:

1 lock held by swapper/0:
 #0:  (&list->lock#4){-+..}, at: [<c05d65c8>] neigh_proxy_process+0x20/0xc2

stack backtrace:
 [<c04051dd>] show_trace_log_lvl+0x1a/0x2f
 [<c0405782>] show_trace+0x12/0x14
 [<c0405806>] dump_stack+0x16/0x18
 [<c043dcf5>] print_circular_bug_tail+0x5f/0x68
 [<c043f3f3>] __lock_acquire+0x814/0xa43
 [<c043f933>] lock_acquire+0x56/0x6f
 [<c06323d6>] _read_lock_bh+0x30/0x3d
 [<c05d5664>] neigh_lookup+0x43/0xa2
 [<c05d6317>] neigh_event_ns+0x2c/0x7a
 [<c060cbec>] arp_process+0x386/0x50a
 [<c060ce78>] parp_redo+0x8/0xa
 [<c05d660e>] neigh_proxy_process+0x66/0xc2
 [<c042f053>] run_timer_softirq+0x104/0x168
 [<c042c1f6>] __do_softirq+0x6f/0xe2
 [<c04063c6>] do_softirq+0x61/0xd0
 =======================


-- 
http://www.codemonkey.org.uk

[RFT] proxy arp deadlock possible

[Stephen Hemminger]

Thanks Dave, there is a classic AB BA deadlock here.
We should break the dependency like this.

Could someone who uses proxy ARP test this?
-----

Need to drop one of the locks to avoid deadlock between
proxy arp and incoming arp receive.
---
 net/core/neighbour.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index cfc6001..d6b3a62 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -1212,6 +1212,7 @@ static void neigh_proxy_process(unsigned
 	unsigned long now = jiffies;
 	struct sk_buff *skb;
 
+rescan:
 	spin_lock(&tbl->proxy_queue.lock);
 
 	skb = tbl->proxy_queue.next;
@@ -1224,13 +1225,20 @@ static void neigh_proxy_process(unsigned
 		if (tdif <= 0) {
 			struct net_device *dev = back->dev;
 			__skb_unlink(back, &tbl->proxy_queue);
+
+			/* drop lock to avoid deadlock */
+			spin_unlock(&tbl->proxy_queue.lock);
+
 			if (tbl->proxy_redo && netif_running(dev))
 				tbl->proxy_redo(back);
 			else
 				kfree_skb(back);
 
 			dev_put(dev);
-		} else if (!sched_next || tdif < sched_next)
+			goto rescan;
+		}
+
+		if (!sched_next || tdif < sched_next)
 			sched_next = tdif;
 	}
 	del_timer(&tbl->proxy_timer);
-- 
1.4.1

Re: [RFT] proxy arp deadlock possible

[Herbert Xu]

Stephen Hemminger <shemminger@linux-foundation.org> wrote:
> Thanks Dave, there is a classic AB BA deadlock here.
> We should break the dependency like this.
> 
> Could someone who uses proxy ARP test this?

Sorry Stephen, this isn't necessary.  The lockdep thing is
simply confused here.  It's treating tbl->proxy_queue as the
same thing as neigh->arp_queue when they're clearly different.

I'm disappointed that after all this time lockdep is still
producing bogus reports like this.  I'm sure we've been
through this particular issue many times already.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [RFT] proxy arp deadlock possible

[Arjan van de Ven]

On Thu, 2007-04-05 at 10:44 +1000, Herbert Xu wrote:
> Stephen Hemminger <shemminger@linux-foundation.org> wrote:
> > Thanks Dave, there is a classic AB BA deadlock here.
> > We should break the dependency like this.
> > 
> > Could someone who uses proxy ARP test this?
> 
> Sorry Stephen, this isn't necessary.  The lockdep thing is
> simply confused here.  It's treating tbl->proxy_queue as the
> same thing as neigh->arp_queue when they're clearly different.
> 
> I'm disappointed that after all this time lockdep is still
> producing bogus reports like this.  I'm sure we've been
> through this particular issue many times already.


what's the exact lockdep output here?

-- 
if you want to mail me at work (you don't), use arjan (at) linux.intel.com
Test the interaction between Linux and your BIOS via http://www.linuxfirmwarekit.org

Re: [RFT] proxy arp deadlock possible

[Dave Jones]

On Wed, Apr 04, 2007 at 06:10:42PM -0700, Arjan van de Ven wrote:
 > On Thu, 2007-04-05 at 10:44 +1000, Herbert Xu wrote:
 > > Stephen Hemminger <shemminger@linux-foundation.org> wrote:
 > > > Thanks Dave, there is a classic AB BA deadlock here.
 > > > We should break the dependency like this.
 > > > 
 > > > Could someone who uses proxy ARP test this?
 > > 
 > > Sorry Stephen, this isn't necessary.  The lockdep thing is
 > > simply confused here.  It's treating tbl->proxy_queue as the
 > > same thing as neigh->arp_queue when they're clearly different.
 > > 
 > > I'm disappointed that after all this time lockdep is still
 > > producing bogus reports like this.  I'm sure we've been
 > > through this particular issue many times already.
 > 
 > 
 > what's the exact lockdep output here?

http://www.mail-archive.com/netdev@vger.kernel.org/msg35266.html

	Dave

-- 
http://www.codemonkey.org.uk

Re: [RFT] proxy arp deadlock possible

[Herbert Xu]

On Wed, Apr 04, 2007 at 06:10:42PM -0700, Arjan van de Ven wrote:
> 
> what's the exact lockdep output here?

Here's the original report:

=======================================================
[ INFO: possible circular locking dependency detected ]
2.6.20-1.2933.fc6debug #1
-------------------------------------------------------
swapper/0 is trying to acquire lock:
 (&tbl->lock){-+-+}, at: [<c05d5664>] neigh_lookup+0x43/0xa2

but task is already holding lock:
 (&list->lock#4){-+..}, at: [<c05d65c8>] neigh_proxy_process+0x20/0xc2

which lock already depends on the new lock.


the existing dependency chain (in reverse order) is:

-> #2 (&list->lock#4){-+..}:
       [<c043f4f2>] __lock_acquire+0x913/0xa43
       [<c043f933>] lock_acquire+0x56/0x6f
       [<c06325df>] _spin_lock_irqsave+0x34/0x44
       [<c05cbb87>] skb_dequeue+0x12/0x43
       [<c05cc9d4>] skb_queue_purge+0x14/0x1b
       [<c05d5b70>] neigh_update+0x349/0x3a5
       [<c060cd37>] arp_process+0x4d1/0x50a
       [<c060ce53>] arp_rcv+0xe3/0x100
       [<c05d0e43>] netif_receive_skb+0x2db/0x35a
       [<c05d2806>] process_backlog+0x95/0xf6
       [<c05d29ed>] net_rx_action+0xa1/0x1a8
       [<c042c1f6>] __do_softirq+0x6f/0xe2
       [<c04063c6>] do_softirq+0x61/0xd0
       [<ffffffff>] 0xffffffff

-> #1 (&n->lock){-+-+}:
       [<c043f4f2>] __lock_acquire+0x913/0xa43
       [<c043f933>] lock_acquire+0x56/0x6f
       [<c063231e>] _write_lock+0x2b/0x38
       [<c05d74b7>] neigh_periodic_timer+0x99/0x138
       [<c042f053>] run_timer_softirq+0x104/0x168
       [<c042c1f6>] __do_softirq+0x6f/0xe2
       [<c04063c6>] do_softirq+0x61/0xd0
       [<ffffffff>] 0xffffffff

-> #0 (&tbl->lock){-+-+}:
       [<c043f3f3>] __lock_acquire+0x814/0xa43
       [<c043f933>] lock_acquire+0x56/0x6f
       [<c06323d6>] _read_lock_bh+0x30/0x3d
       [<c05d5664>] neigh_lookup+0x43/0xa2
       [<c05d6317>] neigh_event_ns+0x2c/0x7a
       [<c060cbec>] arp_process+0x386/0x50a
       [<c060ce78>] parp_redo+0x8/0xa
       [<c05d660e>] neigh_proxy_process+0x66/0xc2
       [<c042f053>] run_timer_softirq+0x104/0x168
       [<c042c1f6>] __do_softirq+0x6f/0xe2
       [<c04063c6>] do_softirq+0x61/0xd0
       [<ffffffff>] 0xffffffff

other info that might help us debug this:

1 lock held by swapper/0:
 #0:  (&list->lock#4){-+..}, at: [<c05d65c8>] neigh_proxy_process+0x20/0xc2

stack backtrace:
 [<c04051dd>] show_trace_log_lvl+0x1a/0x2f
 [<c0405782>] show_trace+0x12/0x14
 [<c0405806>] dump_stack+0x16/0x18
 [<c043dcf5>] print_circular_bug_tail+0x5f/0x68
 [<c043f3f3>] __lock_acquire+0x814/0xa43
 [<c043f933>] lock_acquire+0x56/0x6f
 [<c06323d6>] _read_lock_bh+0x30/0x3d
 [<c05d5664>] neigh_lookup+0x43/0xa2
 [<c05d6317>] neigh_event_ns+0x2c/0x7a
 [<c060cbec>] arp_process+0x386/0x50a
 [<c060ce78>] parp_redo+0x8/0xa
 [<c05d660e>] neigh_proxy_process+0x66/0xc2
 [<c042f053>] run_timer_softirq+0x104/0x168
 [<c042c1f6>] __do_softirq+0x6f/0xe2
 [<c04063c6>] do_softirq+0x61/0xd0
 =======================

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt