* possible recursive locking in ATM layer
@ 2006-07-04 15:59 Duncan Sands
2006-07-04 16:13 ` Arjan van de Ven
0 siblings, 1 reply; 5+ messages in thread
From: Duncan Sands @ 2006-07-04 15:59 UTC (permalink / raw)
To: linux-kernel; +Cc: Ingo Molnar, chas
Linux version 2.6.17-git22 (duncan@baldrick) (gcc version 4.0.3 (Ubuntu 4.0.3-1ubuntu5)) #20 PREEMPT Tue Jul 4 10:35:04 CEST 2006
[ 2381.598609] =============================================
[ 2381.619314] [ INFO: possible recursive locking detected ]
[ 2381.635497] ---------------------------------------------
[ 2381.651706] atmarpd/2696 is trying to acquire lock:
[ 2381.666354] (&skb_queue_lock_key){-+..}, at: [<c028c540>] skb_migrate+0x24/0x6c
[ 2381.688848]
[ 2381.688849] but task is already holding lock:
[ 2381.706406] (&skb_queue_lock_key){-+..}, at: [<c028c536>] skb_migrate+0x1a/0x6c
[ 2381.728898]
[ 2381.728900] other info that might help us debug this:
[ 2381.748534] 2 locks held by atmarpd/2696:
[ 2381.760560] #0: (ioctl_mutex){--..}, at: [<c028f814>] mutex_lock+0x1c/0x1f
[ 2381.782066] #1: (&skb_queue_lock_key){-+..}, at: [<c028c536>] skb_migrate+0x1a/0x6c
[ 2381.805935]
[ 2381.805937] stack backtrace:
[ 2381.819319] [<c010398b>] show_trace_log_lvl+0x54/0xfd
[ 2381.834838] [<c0104aaa>] show_trace+0xd/0x10
[ 2381.848009] [<c0104ac4>] dump_stack+0x17/0x1b
[ 2381.861436] [<c0129e46>] __lock_acquire+0x76c/0x9d3
[ 2381.876582] [<c012a38a>] lock_acquire+0x5e/0x80
[ 2381.890656] [<c029075f>] _spin_lock+0x23/0x32
[ 2381.904220] [<c028c540>] skb_migrate+0x24/0x6c
[ 2381.919084] [<c028db9d>] clip_ioctl+0x2a1/0x480
[ 2381.934248] [<c028a2e1>] vcc_ioctl+0x241/0x2f8
[ 2381.949142] [<c024475d>] sock_ioctl+0x191/0x1b7
[ 2381.964134] [<c015ddec>] do_ioctl+0x20/0x65
[ 2381.977330] [<c015e089>] vfs_ioctl+0x258/0x26b
[ 2381.991283] [<c015e0c6>] sys_ioctl+0x2a/0x44
[ 2382.004711] [<c0102791>] sysenter_past_esp+0x56/0x8d
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: possible recursive locking in ATM layer 2006-07-04 15:59 possible recursive locking in ATM layer Duncan Sands @ 2006-07-04 16:13 ` Arjan van de Ven 2006-07-05 10:23 ` Duncan Sands 2006-07-05 13:33 ` Avi Kivity 0 siblings, 2 replies; 5+ messages in thread From: Arjan van de Ven @ 2006-07-04 16:13 UTC (permalink / raw) To: Duncan Sands; +Cc: netdev, linux-kernel, Ingo Molnar, chas From: Arjan van de Ven <arjan@linux.intel.com> > Linux version 2.6.17-git22 (duncan@baldrick) (gcc version 4.0.3 (Ubuntu 4.0.3-1ubuntu5)) #20 PREEMPT Tue Jul 4 10:35:04 CEST 2006 > > [ 2381.598609] ============================================= > [ 2381.619314] [ INFO: possible recursive locking detected ] > [ 2381.635497] --------------------------------------------- > [ 2381.651706] atmarpd/2696 is trying to acquire lock: > [ 2381.666354] (&skb_queue_lock_key){-+..}, at: [<c028c540>] skb_migrate+0x24/0x6c > [ 2381.688848] ok this is a real potential deadlock in a way, it takes two locks of 2 skbuffs without doing any kind of lock ordering; I think the following patch should fix it. Just sort the lock taking order by address of the skb.. it's not pretty but it's the best this can do in a minimally invasive way. I still agree with the comment that this code shouldn't live in the atm layer... Signed-off-by: Arjan van de Ven <arjan@linux.intel.com> --- net/atm/ipcommon.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) Index: linux-2.6.17-mm6/net/atm/ipcommon.c =================================================================== --- linux-2.6.17-mm6.orig/net/atm/ipcommon.c +++ linux-2.6.17-mm6/net/atm/ipcommon.c @@ -25,8 +25,8 @@ /* * skb_migrate appends the list at "from" to "to", emptying "from" in the * process. skb_migrate is atomic with respect to all other skb operations on - * "from" and "to". Note that it locks both lists at the same time, so beware - * of potential deadlocks. + * "from" and "to". Note that it locks both lists at the same time, so to deal + * with the lock ordering, the locks are taken in address order. * * This function should live in skbuff.c or skbuff.h. */ @@ -39,8 +39,13 @@ void skb_migrate(struct sk_buff_head *fr struct sk_buff *skb_to = (struct sk_buff *) to; struct sk_buff *prev; - spin_lock_irqsave(&from->lock,flags); - spin_lock(&to->lock); + if (from<to) { + spin_lock_irqsave(&from->lock,flags); + spin_lock_nested(&to->lock, SINGLE_DEPTH_NESTING); + } else { + spin_lock_irqsave(&to->lock, flags); + spin_lock_nested(&from->lock, SINGLE_DEPTH_NESTING); + } prev = from->prev; from->next->prev = to->prev; prev->next = skb_to; ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: possible recursive locking in ATM layer 2006-07-04 16:13 ` Arjan van de Ven @ 2006-07-05 10:23 ` Duncan Sands 2006-07-05 13:33 ` Avi Kivity 1 sibling, 0 replies; 5+ messages in thread From: Duncan Sands @ 2006-07-05 10:23 UTC (permalink / raw) To: Arjan van de Ven; +Cc: netdev, linux-kernel, Ingo Molnar, chas > ok this is a real potential deadlock in a way, it takes two locks of 2 > skbuffs without doing any kind of lock ordering; I think the following > patch should fix it. Just sort the lock taking order by address of the > skb.. it's not pretty but it's the best this can do in a minimally > invasive way. The patch is effective. Thanks for making it! Best wishes, Duncan. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: possible recursive locking in ATM layer 2006-07-04 16:13 ` Arjan van de Ven 2006-07-05 10:23 ` Duncan Sands @ 2006-07-05 13:33 ` Avi Kivity 2006-07-05 13:42 ` Arjan van de Ven 1 sibling, 1 reply; 5+ messages in thread From: Avi Kivity @ 2006-07-05 13:33 UTC (permalink / raw) To: Arjan van de Ven; +Cc: Duncan Sands, netdev, linux-kernel, Ingo Molnar, chas Arjan van de Ven wrote: > > From: Arjan van de Ven <arjan@linux.intel.com> > > > Linux version 2.6.17-git22 (duncan@baldrick) (gcc version 4.0.3 > (Ubuntu 4.0.3-1ubuntu5)) #20 PREEMPT Tue Jul 4 10:35:04 CEST 2006 > > > > > [ 2381.598609] ============================================= > > [ 2381.619314] [ INFO: possible recursive locking detected ] > > [ 2381.635497] --------------------------------------------- > > [ 2381.651706] atmarpd/2696 is trying to acquire lock: > > [ 2381.666354] (&skb_queue_lock_key){-+..}, at: [<c028c540>] > skb_migrate+0x24/0x6c > > [ 2381.688848] > > > ok this is a real potential deadlock in a way, it takes two locks of 2 > skbuffs without doing any kind of lock ordering; I think the following > patch should fix it. Just sort the lock taking order by address of the > skb.. it's not pretty but it's the best this can do in a minimally > invasive way. > Isn't it a deadlock only if skb_migrate(a, b) and skb_migrate(b, a) can be called concurrently? -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: possible recursive locking in ATM layer 2006-07-05 13:33 ` Avi Kivity @ 2006-07-05 13:42 ` Arjan van de Ven 0 siblings, 0 replies; 5+ messages in thread From: Arjan van de Ven @ 2006-07-05 13:42 UTC (permalink / raw) To: Avi Kivity; +Cc: Duncan Sands, netdev, linux-kernel, Ingo Molnar, chas On Wed, 2006-07-05 at 16:33 +0300, Avi Kivity wrote: > Arjan van de Ven wrote: > > > > From: Arjan van de Ven <arjan@linux.intel.com> > > > > > Linux version 2.6.17-git22 (duncan@baldrick) (gcc version 4.0.3 > > (Ubuntu 4.0.3-1ubuntu5)) #20 PREEMPT Tue Jul 4 10:35:04 CEST 2006 > > > > > > > > [ 2381.598609] ============================================= > > > [ 2381.619314] [ INFO: possible recursive locking detected ] > > > [ 2381.635497] --------------------------------------------- > > > [ 2381.651706] atmarpd/2696 is trying to acquire lock: > > > [ 2381.666354] (&skb_queue_lock_key){-+..}, at: [<c028c540>] > > skb_migrate+0x24/0x6c > > > [ 2381.688848] > > > > > > ok this is a real potential deadlock in a way, it takes two locks of 2 > > skbuffs without doing any kind of lock ordering; I think the following > > patch should fix it. Just sort the lock taking order by address of the > > skb.. it's not pretty but it's the best this can do in a minimally > > invasive way. > > > > Isn't it a deadlock only if skb_migrate(a, b) and skb_migrate(b, a) can > be called concurrently? yes, well, and if there are no other double-takers... > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2006-07-05 13:42 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2006-07-04 15:59 possible recursive locking in ATM layer Duncan Sands 2006-07-04 16:13 ` Arjan van de Ven 2006-07-05 10:23 ` Duncan Sands 2006-07-05 13:33 ` Avi Kivity 2006-07-05 13:42 ` Arjan van de Ven
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.