2.6.17-rc4-mm3-lockdep BUG: possible deadlock detected!

2.6.17-rc4-mm3-lockdep BUG: possible deadlock detected!

[Michal Piotrowski]

Hi,

I get this with Ingo's lockdep patch from
http://people.redhat.com/mingo/generic-irq-subsystem/

====================================
[ BUG: possible deadlock detected! ]
------------------------------------
modprobe/592 is trying to acquire lock:
 (&grp->list_mutex){----}, at: [<fd9ee555>]
snd_seq_port_connect+0xc0/0x337 [snd_seq]

but task is already holding lock:
 (&grp->list_mutex){----}, at: [<fd9ee4fb>]
snd_seq_port_connect+0x66/0x337 [snd_seq]

which could potentially lead to deadlocks!

other info that might help us debug this:
1 locks held by modprobe/592:
 #0:  (&grp->list_mutex){----}, at: [<fd9ee4fb>]
snd_seq_port_connect+0x66/0x337 [snd_seq]

stack backtrace:
 <c1003f36> show_trace+0xd/0xf  <c1004449> dump_stack+0x17/0x19
 <c1039792> __lockdep_acquire+0xa1a/0xa39  <c1039bed> lockdep_acquire+0x69/0x82
 <fd9ee570> snd_seq_port_connect+0xdb/0x337 [snd_seq]  <fd9ea5f5>
snd_seq_ioctl_subscribe_port+0x96/0xe6 [snd_seq]
 <fd9e91d4> snd_seq_do_ioctl+0x5d/0x68 [snd_seq]  <fd9e921c>
snd_seq_kernel_client_ctl+0x2d/0x3b [snd_seq]
 <f8877274> snd_seq_oss_create_client+0x12d/0x142 [snd_seq_oss]
<f88770f7> alsa_seq_oss_init+0xf7/0x147 [snd_seq_oss]
 <c104116d> sys_init_module+0xa6/0x230  <c11efa8b> sysenter_past_esp+0x54/0x8d

Here is dmesg http://www.stardust.webpages.pl/files/lockdep/2.6.17-rc4-mm3-lockdep1/lockdep-dmesg

Here is config
http://www.stardust.webpages.pl/files/lockdep/2.6.17-rc4-mm3-lockdep1/lockdep-config

Regards,
Michal

-- 
Michal K. K. Piotrowski
LTG - Linux Testers Group
(http://www.stardust.webpages.pl/ltg/wiki/)

Re: 2.6.17-rc4-mm3-lockdep BUG: possible deadlock detected!

[Andrew Morton]

On Mon, 29 May 2006 20:32:47 +0200
"Michal Piotrowski" <michal.k.k.piotrowski@gmail.com> wrote:

> I get this with Ingo's lockdep patch from
> http://people.redhat.com/mingo/generic-irq-subsystem/
> 
> ====================================
> [ BUG: possible deadlock detected! ]
> ------------------------------------
> modprobe/592 is trying to acquire lock:
>  (&grp->list_mutex){----}, at: [<fd9ee555>]
> snd_seq_port_connect+0xc0/0x337 [snd_seq]
> 
> but task is already holding lock:
>  (&grp->list_mutex){----}, at: [<fd9ee4fb>]
> snd_seq_port_connect+0x66/0x337 [snd_seq]
> 
> which could potentially lead to deadlocks!
> 
> other info that might help us debug this:
> 1 locks held by modprobe/592:
>  #0:  (&grp->list_mutex){----}, at: [<fd9ee4fb>]
> snd_seq_port_connect+0x66/0x337 [snd_seq]

yes,

        down_write(&src->list_mutex);
        down_write(&dest->list_mutex);

I wonder if there's anything which prevents another task from concurrently
coming in and trying to perform the opposite connection and causing a
deadlock.

The way we normally handle this is

	if (src < dest) {
		down_write(&src->list_mutex);
		down_write(&dest->list_mutex);
	} else {
		down_write(&dest->list_mutex);
		down_write(&src->list_mutex);
	}

Re: 2.6.17-rc4-mm3-lockdep BUG: possible deadlock detected!

[Ingo Molnar]

* Michal Piotrowski <michal.k.k.piotrowski@gmail.com> wrote:

> I get this with Ingo's lockdep patch from
> http://people.redhat.com/mingo/generic-irq-subsystem/

sigh, that patchset is not released yet ... it showed up in the genirq 
directory accidentally. (will release it later today)

> ====================================
> [ BUG: possible deadlock detected! ]
> ------------------------------------

at first sight this looks like a rare case of nested locking not yet 
covered by the lock validator. Could you try the patch below, to 
correctly express this locking construct to the lock validator?

Btw., beyond this false positive, i dont see how the lock ordering 
between ports is guaranteed - maybe there's some implicit rule that 
enforces it. And the whole grp->list_lock and grp->list_mutex lock use 
seems quite fragile - using list_lock in atomic contexts and list_mutex 
in schedulable contexts?

	Ingo

Index: linux/sound/core/seq/seq_ports.c
===================================================================
--- linux.orig/sound/core/seq/seq_ports.c
+++ linux/sound/core/seq/seq_ports.c
@@ -518,7 +518,7 @@ int snd_seq_port_connect(struct snd_seq_
 	atomic_set(&subs->ref_count, 2);
 
 	down_write(&src->list_mutex);
-	down_write(&dest->list_mutex);
+	down_write_nested(&dest->list_mutex, SINGLE_DEPTH_NESTING);
 
 	exclusive = info->flags & SNDRV_SEQ_PORT_SUBS_EXCLUSIVE ? 1 : 0;
 	err = -EBUSY;
@@ -591,7 +591,7 @@ int snd_seq_port_disconnect(struct snd_s
 	unsigned long flags;
 
 	down_write(&src->list_mutex);
-	down_write(&dest->list_mutex);
+	down_write_nested(&dest->list_mutex, SINGLE_DEPTH_NESTING);
 
 	/* look for the connection */
 	list_for_each(p, &src->list_head) {

Re: 2.6.17-rc4-mm3-lockdep BUG: possible deadlock detected!

[Michal Piotrowski]

On 29/05/06, Ingo Molnar <mingo@elte.hu> wrote:
>
> * Michal Piotrowski <michal.k.k.piotrowski@gmail.com> wrote:
>
> > I get this with Ingo's lockdep patch from
> > http://people.redhat.com/mingo/generic-irq-subsystem/
>
> sigh, that patchset is not released yet ... it showed up in the genirq
> directory accidentally. (will release it later today)

Ok. So I'll wait with reporting that
http://www.stardust.webpages.pl/files/lockdep/2.6.17-rc4-mm3-lockdep1/lockdep-dmesg2
:)

>
> > ====================================
> > [ BUG: possible deadlock detected! ]
> > ------------------------------------
>
> at first sight this looks like a rare case of nested locking not yet
> covered by the lock validator. Could you try the patch below, to
> correctly express this locking construct to the lock validator?

Problem fixed. Thanks!

>
> Btw., beyond this false positive, i dont see how the lock ordering
> between ports is guaranteed - maybe there's some implicit rule that
> enforces it. And the whole grp->list_lock and grp->list_mutex lock use
> seems quite fragile - using list_lock in atomic contexts and list_mutex
> in schedulable contexts?
>
>         Ingo
>

Regards,
Michal

-- 
Michal K. K. Piotrowski
LTG - Linux Testers Group
(http://www.stardust.webpages.pl/ltg/wiki/)

Re: 2.6.17-rc4-mm3-lockdep BUG: possible deadlock detected!

[Takashi Iwai]

At Mon, 29 May 2006 11:47:49 -0700,
Andrew Morton wrote:
> 
> On Mon, 29 May 2006 20:32:47 +0200
> "Michal Piotrowski" <michal.k.k.piotrowski@gmail.com> wrote:
> 
> > I get this with Ingo's lockdep patch from
> > http://people.redhat.com/mingo/generic-irq-subsystem/
> > 
> > ====================================
> > [ BUG: possible deadlock detected! ]
> > ------------------------------------
> > modprobe/592 is trying to acquire lock:
> >  (&grp->list_mutex){----}, at: [<fd9ee555>]
> > snd_seq_port_connect+0xc0/0x337 [snd_seq]
> > 
> > but task is already holding lock:
> >  (&grp->list_mutex){----}, at: [<fd9ee4fb>]
> > snd_seq_port_connect+0x66/0x337 [snd_seq]
> > 
> > which could potentially lead to deadlocks!
> > 
> > other info that might help us debug this:
> > 1 locks held by modprobe/592:
> >  #0:  (&grp->list_mutex){----}, at: [<fd9ee4fb>]
> > snd_seq_port_connect+0x66/0x337 [snd_seq]
> 
> yes,
> 
>         down_write(&src->list_mutex);
>         down_write(&dest->list_mutex);
> 
> I wonder if there's anything which prevents another task from concurrently
> coming in and trying to perform the opposite connection and causing a
> deadlock.

I'm not 100% sure but I don't think the deadlock occurs because the
object has actually two locks, namely,

	port.c_src.list_mutex
	port.c_dst.list_mutex

so the above is equivalent with

	down(port_A.c_src.list_mutex);
	down(port_B.c_dst.list_mutex);

When the opposite connection is performed concurrently, different
locks are taken:

	down(port_B.c_src.list_mutex);
	down(port_A.c_dst.list_mutex);

In the code, there is another place that using down_write() (in
clear_subscriber_list()), but it's already after closing accesses to
that port, hence there can be no race there.


Takashi

Re: [Alsa-devel] 2.6.17-rc4-mm3-lockdep BUG: possible deadlock detected!

[Takashi Iwai]

At Mon, 29 May 2006 21:07:07 +0200,
Ingo Molnar wrote:
> 
> 
> * Michal Piotrowski <michal.k.k.piotrowski@gmail.com> wrote:
> 
> > I get this with Ingo's lockdep patch from
> > http://people.redhat.com/mingo/generic-irq-subsystem/
> 
> sigh, that patchset is not released yet ... it showed up in the genirq 
> directory accidentally. (will release it later today)
> 
> > ====================================
> > [ BUG: possible deadlock detected! ]
> > ------------------------------------
> 
> at first sight this looks like a rare case of nested locking not yet 
> covered by the lock validator. Could you try the patch below, to 
> correctly express this locking construct to the lock validator?
> 
> Btw., beyond this false positive, i dont see how the lock ordering 
> between ports is guaranteed - maybe there's some implicit rule that 
> enforces it.

As mentioned in another post, different locks are used depending
whether it's source or destination.  Thus the confliction doesn't
occur in the reverse order.

>  And the whole grp->list_lock and grp->list_mutex lock use 
> seems quite fragile - using list_lock in atomic contexts and list_mutex 
> in schedulable contexts?

Yes, exactly.  read_lock(grp->list_lock) is used in seq_clientmgr.c
for the atomic contexts to follow the linked list.


Takashi