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/)
On Mon, 29 May 2006 20:32:47 +0200
"Michal Piotrowski" <[email protected]> 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);
}
* Michal Piotrowski <[email protected]> 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) {
On 29/05/06, Ingo Molnar <[email protected]> wrote:
>
> * Michal Piotrowski <[email protected]> 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/)
At Mon, 29 May 2006 11:47:49 -0700,
Andrew Morton wrote:
>
> On Mon, 29 May 2006 20:32:47 +0200
> "Michal Piotrowski" <[email protected]> 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
At Mon, 29 May 2006 21:07:07 +0200,
Ingo Molnar wrote:
>
>
> * Michal Piotrowski <[email protected]> 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