On Thu, May 15, 2008 at 01:47:02PM +0800, Dave Young wrote: > On Thu, May 15, 2008 at 12:24 AM, Matthew Wilcox <matthew@xxxxxx> wrote: > > On Wed, May 14, 2008 at 08:56:39AM -0700, Greg KH wrote: > >> On Wed, May 14, 2008 at 12:09:33AM -0700, Andrew Morton wrote: > >> > > >> > ============================================= > >> > [ INFO: possible recursive locking detected ] > >> > 2.6.26-rc2-mm1 #15 > >> > --------------------------------------------- > >> > modprobe/942 is trying to acquire lock: > >> > (&cls->mutex){--..}, at: [<ffffffff811b431e>] device_add+0x43d/0x57a > >> > > >> > but task is already holding lock: > >> > (&cls->mutex){--..}, at: [<ffffffff811b6787>] class_interface_register+0x48/0xbd > >> > > >> > other info that might help us debug this: > >> > 1 lock held by modprobe/942: > >> > #0: (&cls->mutex){--..}, at: [<ffffffff811b6787>] class_interface_register+0x48/0xbd > >> > > >> > stack backtrace: > >> > Pid: 942, comm: modprobe Not tainted 2.6.26-rc2-mm1 #15 > >> > > >> > Call Trace: > >> > [<ffffffff81056be1>] __lock_acquire+0x90d/0xc50 > >> > [<ffffffff8100c50f>] ? restore_args+0x0/0x30 > >> > [<ffffffff811b431e>] ? device_add+0x43d/0x57a > >> > [<ffffffff81057276>] lock_acquire+0x91/0xb7 > >> > [<ffffffff811b431e>] ? device_add+0x43d/0x57a > >> > [<ffffffff812b23ab>] mutex_lock_nested+0xf2/0x278 > >> > [<ffffffff811b431e>] ? device_add+0x43d/0x57a > >> > [<ffffffff812b3acd>] ? _spin_unlock+0x23/0x28 > >> > [<ffffffff811b431e>] device_add+0x43d/0x57a > >> > [<ffffffff811b4471>] device_register+0x16/0x1b > >> > [<ffffffff811b4555>] device_create+0xdf/0x112 > >> > [<ffffffff81055fdc>] ? trace_hardirqs_on+0xd/0xf > >> > [<ffffffff81055fdc>] ? trace_hardirqs_on+0xd/0xf > >> > [<ffffffff812b1fe1>] ? mutex_unlock+0x9/0xb > >> > [<ffffffff811b79b7>] ? kobj_map+0x113/0x124 > >> > [<ffffffff810ac0f5>] ? exact_lock+0x0/0x14 > >> > [<ffffffff810abd09>] ? exact_match+0x0/0x9 > >> > [<ffffffffa00ec448>] :sg:sg_add+0x2a3/0x3bd > >> > [<ffffffff811b67b6>] class_interface_register+0x77/0xbd > >> > [<ffffffffa005d869>] :scsi_mod:scsi_register_interface+0x11/0x13 > >> > [<ffffffffa00d50a3>] :sg:init_sg+0xa3/0x155 > >> > [<ffffffff8105ea8f>] sys_init_module+0x1823/0x197a > >> > [<ffffffff810c45bc>] ? seq_release+0x0/0x56 > >> > [<ffffffff8100bebb>] system_call_after_swapgs+0x7b/0x80 > >> > >> I'm guessing that this is due to David's "change the semaphore to a > >> mutex" patch that you have in your tree, but I refused to take as I was > >> worried about just this issue :) > > > > Hm, I thought I saw the same patch from Arjan months ago ... anyway. > > > > class_interface_register() (at least in -rc2) holds &parent->sem (ie the > > result of calling class_get() on the class) around calling ->add_dev. > > The class in this case is the sg_sysfs_class, so it's calling sg_add(). > > sg_add() calls device_create() which calls device_add() which acquires > > the &dev->class->sem. The class in this case would _also_ seem to be > > sg_sysfs_class. > > > > So why doesn't this deadlock today? > > The classes are different here, first sdev_class, then sg_sysfs_class Greg, what about using mutex_lock_nested to silence lockdep? They are the only usage of class->mutex out of class.c Signed-off-by: Dave Young <hidave.darkstar@xxxxxxxxx> --- drivers/base/core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff -upr linux/drivers/base/core.c linux.new/drivers/base/core.c --- linux/drivers/base/core.c 2008-05-15 16:48:31.000000000 +0800 +++ linux.new/drivers/base/core.c 2008-05-15 16:47:59.000000000 +0800 @@ -888,7 +888,7 @@ int device_add(struct device *dev) klist_add_tail(&dev->knode_parent, &parent->klist_children); if (dev->class) { - mutex_lock(&dev->class->mutex); + mutex_lock_nested(&dev->class->mutex, SINGLE_DEPTH_NESTING); /* tie the class to the device */ list_add_tail(&dev->node, &dev->class->devices); @@ -997,7 +997,7 @@ void device_del(struct device *dev) if (dev->class) { device_remove_class_symlinks(dev); - mutex_lock(&dev->class->mutex); + mutex_lock_nested(&dev->class->mutex, SINGLE_DEPTH_NESTING); /* notify any interfaces that the device is now gone */ list_for_each_entry(class_intf, &dev->class->interfaces, node) if (class_intf->remove_dev) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html