lockdep whine in 2.6.26-rc2-mm1
Subsystems (2)
=============================================
[ 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
Driver 'sr' needs updating - please use bus_type methods sd 0:0:0:0: Attached scsi generic sg0 type 0
sr0: scsi3-mmc drive: 24x/24x writer dvd-ram cd/rw xa/form2 cdda tray Uniform CD-ROM driver Revision: 3.20
sr 3:0:0:0: Attached scsi CD-ROM sr0
sr 3:0:0:0: Attached scsi generic sg1 type 5
Full dmesg: http://userweb.kernel.org/~akpm/dmesg-t61p.txt
Config: http://userweb.kernel.org/~akpm/config-t61p.txt
Quoted 35 lines · Andrew Morton
=============================================
[ 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/0x57abut task is already holding lock:
(&cls->mutex){--..}, at: [<ffffffff811b6787>]class_interface_register+0x48/0xbdother info that might help us debug this:
1 lock held by modprobe/942:
#0: (&cls->mutex){--..}, at: [<ffffffff811b6787>]class_interface_register+0x48/0xbdstack 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
Quoted 8 lines
[<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 :)
David, any thoughts?
Quoted 1 line
Driver 'sr' needs updating - please use
bus_typemethods
This is not the same issue, last I looked Kay and James were working on resolving this.
thanks,
greg k-h
Quoted 45 lines · Greg KH
Quoted 35 lines · Andrew Morton
=============================================
[ 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/0x57abut task is already holding lock:
(&cls->mutex){--..}, at: [<ffffffff811b6787>]class_interface_register+0x48/0xbdother info that might help us debug this:
1 lock held by modprobe/942:
#0: (&cls->mutex){--..}, at: [<ffffffff811b6787>]class_interface_register+0x48/0xbdstack 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/0x14Quoted 8 lines
[<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
Quoted 3 lines
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?
Signature · 4 lines
Intel are signing my paycheques ... these opinions are still mine "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step."
Quoted 52 lines · Matthew Wilcox
Quoted 45 lines · Greg KH
Quoted 35 lines · Andrew Morton
=============================================
[ 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/0x57abut task is already holding lock:
(&cls->mutex){--..}, at: [<ffffffff811b6787>]class_interface_register+0x48/0xbdother info that might help us debug this:
1 lock held by modprobe/942:
#0: (&cls->mutex){--..}, at: [<ffffffff811b6787>]class_interface_register+0x48/0xbdstack 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/0x14Quoted 8 lines
[<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/0x80Quoted 3 lines
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.
Quoted 8 lines
class_interface_register()(at least in -rc2) holds&parent->sem(ie the result of callingclass_get()on the class) around calling ->add_dev. The class in this case is thesg_sysfs_class, so it's callingsg_add().sg_add()callsdevice_create()which callsdevice_add()which acquires the&dev->class->sem. The class in this case would _also_ seem to besg_sysfs_class.So why doesn't this deadlock today?
The classes are different here, first sdev_class, then sg_sysfs_class
Regards
dave
Quoted 60 lines · Dave Young
Quoted 59 lines · Matthew Wilcox
Quoted 45 lines · Greg KH
Quoted 35 lines · Andrew Morton
=============================================
[ 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/0x57abut task is already holding lock:
(&cls->mutex){--..}, at: [<ffffffff811b6787>]class_interface_register+0x48/0xbdother info that might help us debug this:
1 lock held by modprobe/942:
#0: (&cls->mutex){--..}, at: [<ffffffff811b6787>]class_interface_register+0x48/0xbdstack 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/0x14Quoted 8 lines
[<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/0x80Quoted 3 lines
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 callingclass_get()on the class) around calling ->add_dev. The class in this case is thesg_sysfs_class, so it's callingsg_add().sg_add()callsdevice_create()which callsdevice_add()which acquires the&dev->class->sem. The class in this case would _also_ seem to besg_sysfs_class.
Quoted 3 lines
Quoted 1 line
So why doesn't this deadlock today?
The classes are different here, first
sdev_class, thensg_sysfs_class
Greg, what about using mutex_lock_nested to silence lockdep? They are the only usage of class->mutex out of class.c
---drivers/base/core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
drivers/base/core.c +2 -2
@@ -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)
Quoted 1 line · Dave Young
Quoted 1 line
The classes are different here, first
sdev_class, thensg_sysfs_class
Oh ... right. I misread scsi_register_interface as class_register_interface.
Quoted 2 lines
Greg, what about using
mutex_lock_nestedto silence lockdep? They are the only usage ofclass->mutexout ofclass.c
I don't see how we prove that, for example, you can never take the sg_sysfs_class mutex and then take the sdev_class mutex.
There aren't /that/ many classes in the kernel (my laptop has 33 in /sys/class). How about we make each (sysfs) class its own (lockdep) class? That way we let lockdep do its job rather than telling it "nah, you're all right, this is good".
I don't have a copy of the sem->mutex conversion to hand, but I imagine you want to do something like:
- Add a struct lock_class_key to struct class
- In drivers/base/class.c use __mutex_init instead of mutex_init
I think that should be enough ...
Signature · 4 lines
Intel are signing my paycheques ... these opinions are still mine "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step."
Quoted 11 lines · Matthew Wilcox
Quoted 1 line · Dave Young
Quoted 1 line
The classes are different here, first
sdev_class, thensg_sysfs_classOh ... right. I misread
scsi_register_interfaceasclass_register_interface.Quoted 2 lines
Greg, what about using
mutex_lock_nestedto silence lockdep? They are the only usage ofclass->mutexout ofclass.cI don't see how we prove that, for example, you can never take the
sg_sysfs_classmutex and then take thesdev_classmutex.
Sorry for my delay. AFAIK, there's no this kind of use.
Quoted 6 lines
There aren't /that/ many classes in the kernel (my laptop has 33 in
/sys/class). How about we make each (sysfs) class its own (lockdep) class? That way we let lockdep do its job rather than telling it "nah, you're all right, this is good".
Quoted 7 lines
I don't have a copy of the
sem->mutexconversion to hand, but I imagine you want to do something like:- Add a struct
lock_class_keyto struct class
- Indrivers/base/class.cuse__mutex_initinstead ofmutex_initI think that should be enough ...
Yes, it's safer than mutex_lock_nested. If you all have no objection I can do the fix like this.
Thanks.
Quoted 23 lines · Dave Young
Quoted 11 lines · Matthew Wilcox
Quoted 1 line · Dave Young
Quoted 1 line
The classes are different here, first
sdev_class, thensg_sysfs_classOh ... right. I misread
scsi_register_interfaceasclass_register_interface.Quoted 2 lines
Greg, what about using
mutex_lock_nestedto silence lockdep? They are the only usage ofclass->mutexout ofclass.cI don't see how we prove that, for example, you can never take the
sg_sysfs_classmutex and then take thesdev_classmutex.Sorry for my delay. AFAIK, there's no this kind of use.
Quoted 9 lines
There aren't /that/ many classes in the kernel (my laptop has 33 in
/sys/class). How about we make each (sysfs) class its own (lockdep) class? That way we let lockdep do its job rather than telling it "nah, you're all right, this is good".I don't have a copy of the
sem->mutexconversion to hand, but I imagine you want to do something like:
Quoted 8 lines
Quoted 4 lines
- Add a struct
lock_class_keyto struct class
- Indrivers/base/class.cuse__mutex_initinstead ofmutex_initI think that should be enough ...
Yes, it's safer than
mutex_lock_nested. If you all have no objection I can do the fix like this.
I rechecked the class_interface use, the users are scsi and pcmcia.
The two classes could call device_add/del while doing class_interface_register/unregister is :sg_sysfs_classpcmcia_socket_class
So is it possible to reset their lock_class instead of do __mutex_init for all classes?
Quoted 35 lines · Dave Young
Quoted 23 lines · Dave Young
Quoted 11 lines · Matthew Wilcox
Quoted 1 line · Dave Young
Quoted 1 line
The classes are different here, first
sdev_class, thensg_sysfs_classOh ... right. I misread
scsi_register_interfaceasclass_register_interface.Quoted 2 lines
Greg, what about using
mutex_lock_nestedto silence lockdep? They are the only usage ofclass->mutexout ofclass.cI don't see how we prove that, for example, you can never take the
sg_sysfs_classmutex and then take thesdev_classmutex.Sorry for my delay. AFAIK, there's no this kind of use.
Quoted 9 lines
There aren't /that/ many classes in the kernel (my laptop has 33 in
/sys/class). How about we make each (sysfs) class its own (lockdep) class? That way we let lockdep do its job rather than telling it "nah, you're all right, this is good".I don't have a copy of the
sem->mutexconversion to hand, but I imagine you want to do something like:Quoted 8 lines
Quoted 4 lines
- Add a struct
lock_class_keyto struct class
- Indrivers/base/class.cuse__mutex_initinstead ofmutex_initI think that should be enough ...
Yes, it's safer than
mutex_lock_nested. If you all have no objection I can do the fix like this.I rechecked the
class_interfaceuse, the users are scsi and pcmcia.
Quoted 8 lines
The two classes could call device_add/del while doing class_interface_register/unregister is :
sg_sysfs_classpcmcia_socket_classSo is it possible to reset their
lock_classinstead of do__mutex_initfor all classes?
Like this?
1) Add macro :
+#define class_reclassify(class) \
+do { \
+ static struct lock_class_key class_key; \
+ lockdep_set_class_and_name(&(class)->mutex, &class_key, \
+ (class)->name); \
+} while (0)
2) Then in sg.c:
class_reclassify(sg_sysfs_class);
3) And in pcmcia/cs.cclass_reclassify(&pcmcia_socket_class);
I have built and boot tested the patch, but due to connecting gmail smtp failed so I cant send it via mutt today.
Quoted 8 lines · Dave Young
Quoted 7 lines · Dave Young
Quoted 6 lines · Matthew Wilcox
Quoted 1 line · Dave Young
Quoted 1 line
The classes are different here, first
sdev_class, thensg_sysfs_classOh ... right. I misread
scsi_register_interfaceasclass_register_interface.
Quoted 7 lines
Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
Greg, what about using
mutex_lock_nestedto silence lockdep? They are the only usage ofclass->mutexout ofclass.cI don't see how we prove that, for example, you can never take the
sg_sysfs_classmutex and then take thesdev_classmutex.Sorry for my delay. AFAIK, there's no this kind of use.
The question isn't whether there is or isn't this kind of use right now. The question is whether there might be this kind of use in the future, and if there is, whether we'd like lockdep to warn us.
Quoted 9 lines
I rechecked the
class_interfaceuse, the users are scsi and pcmcia.The two classes could call device_add/del while doing class_interface_register/unregister is :
sg_sysfs_classpcmcia_socket_classSo is it possible to reset their
lock_classinstead of do__mutex_initfor all classes?
Again, it's not about current usage, it's about future usage. Why do you want all sysfs classes to have the same lockdep class? There can be good reasons. For example, if two locks are conceptually the same, keeping them in the same class helps you find AB-BA problems earlier.
Is there a reason you don't like my idea?
Signature · 4 lines
Intel are signing my paycheques ... these opinions are still mine "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step."
Quoted 12 lines · Matthew Wilcox
Quoted 11 lines · Dave Young
Quoted 10 lines · Dave Young
Quoted 9 lines · Matthew Wilcox
Quoted 1 line · Dave Young
Quoted 1 line
The classes are different here, first
sdev_class, thensg_sysfs_classOh ... right. I misread
scsi_register_interfaceasclass_register_interface.Quoted 2 lines
Greg, what about using
mutex_lock_nestedto silence lockdep? They are the only usage ofclass->mutexout ofclass.c
Quoted 8 lines
Quoted 4 lines
Quoted 4 lines
Quoted 2 lines
I don't see how we prove that, for example, you can never take the
sg_sysfs_classmutex and then take thesdev_classmutex.Sorry for my delay. AFAIK, there's no this kind of use.
The question isn't whether there is or isn't this kind of use right now. The question is whether there might be this kind of use in the future, and if there is, whether we'd like lockdep to warn us.
In the future, IMHO, the class_interface should go away just as class_device. If that happened this problem would going away as well.
Quoted 11 lines
Quoted 10 lines
I rechecked the
class_interfaceuse, the users are scsi and pcmcia.The two classes could call device_add/del while doing class_interface_register/unregister is :
sg_sysfs_classpcmcia_socket_classSo is it possible to reset their
lock_classinstead of do__mutex_initfor all classes?
Quoted 6 lines
Again, it's not about current usage, it's about future usage. Why do you want all sysfs classes to have the same lockdep class? There can be good reasons. For example, if two locks are conceptually the same, keeping them in the same class helps you find AB-BA problems earlier.
Is there a reason you don't like my idea?
Your idea is good. But as I said above, for current situation there's no potential problems except class_interface usage, and the class_interface will go away
in the future.
So IMO It's not necessary to do this for all classes.
Regards
dave
Quoted 6 lines · Dave Young
Quoted 3 lines
The question isn't whether there is or isn't this kind of use right now. The question is whether there might be this kind of use in the future, and if there is, whether we'd like lockdep to warn us.
In the future, IMHO, the
class_interfaceshould go away just asclass_device. If that happened this problem would going away as well.
Go away ... and be replaced by something else, right? I mean, it's not like we don't have a need to bind two kinds of driver to the same device. So we're going to continue to have the situation where you can register a new kind of driver and want to create new devices, so we'll still have a nesting of some kind.
Quoted 6 lines
Your idea is good. But as I said above, for current situation there's no potential problems except
class_interfaceusage, and theclass_interfacewill go away
in the future.So IMO It's not necessary to do this for all classes.
Assuming your analysis is correct, of course.
Signature · 4 lines
Intel are signing my paycheques ... these opinions are still mine "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step."
Quoted 16 lines · Dave Young
Quoted 15 lines · Matthew Wilcox
Quoted 14 lines · Dave Young
Quoted 13 lines · Dave Young
Quoted 11 lines · Matthew Wilcox
Quoted 1 line · Dave Young
Quoted 1 line
The classes are different here, first
sdev_class, thensg_sysfs_classOh ... right. I misread
scsi_register_interfaceasclass_register_interface.Quoted 2 lines
Greg, what about using
mutex_lock_nestedto silence lockdep? They are the only usage ofclass->mutexout ofclass.cI don't see how we prove that, for example, you can never take the
sg_sysfs_classmutex and then take thesdev_classmutex.
Quoted 8 lines
Quoted 5 lines
Quoted 1 line
Quoted 1 line
Sorry for my delay. AFAIK, there's no this kind of use.
The question isn't whether there is or isn't this kind of use right now. The question is whether there might be this kind of use in the future, and if there is, whether we'd like lockdep to warn us.
In the future, IMHO, the
class_interfaceshould go away just asclass_device. If that happened this problem would going away as well.
Patches gladly accepted to do this, but what you will end up with is something just called a different name, yet doing the same functionality, so you are back at square one :(
thanks,
greg k-h
Quoted 23 lines · Greg KH
Quoted 22 lines · Dave Young
Quoted 20 lines · Matthew Wilcox
Quoted 8 lines · Dave Young
Quoted 7 lines · Dave Young
Quoted 6 lines · Matthew Wilcox
Quoted 1 line · Dave Young
Quoted 1 line
The classes are different here, first
sdev_class, thensg_sysfs_classOh ... right. I misread
scsi_register_interfaceasclass_register_interface.Quoted 7 lines
Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
Greg, what about using
mutex_lock_nestedto silence lockdep? They are the only usage ofclass->mutexout ofclass.cI don't see how we prove that, for example, you can never take the
sg_sysfs_classmutex and then take thesdev_classmutex.Sorry for my delay. AFAIK, there's no this kind of use.
The question isn't whether there is or isn't this kind of use right now. The question is whether there might be this kind of use in the future, and if there is, whether we'd like lockdep to warn us.
Quoted 6 lines
Quoted 2 lines
In the future, IMHO, the
class_interfaceshould go away just asclass_device. If that happened this problem would going away as well.Patches gladly accepted to do this, but what you will end up with is something just called a different name, yet doing the same functionality, so you are back at square one :(
Greg, do you have proposals about the class_interface removal before this thread? or ideas?
---
Regards
dave
Quoted 27 lines · Dave Young
Quoted 26 lines · Greg KH
Quoted 24 lines · Dave Young
Quoted 12 lines · Matthew Wilcox
Quoted 11 lines · Dave Young
Quoted 10 lines · Dave Young
Quoted 9 lines · Matthew Wilcox
Quoted 1 line · Dave Young
Quoted 1 line
The classes are different here, first
sdev_class, thensg_sysfs_classOh ... right. I misread
scsi_register_interfaceasclass_register_interface.Quoted 2 lines
Greg, what about using
mutex_lock_nestedto silence lockdep? They are the only usage ofclass->mutexout ofclass.cQuoted 8 lines
Quoted 4 lines
Quoted 4 lines
Quoted 2 lines
I don't see how we prove that, for example, you can never take the
sg_sysfs_classmutex and then take thesdev_classmutex.Sorry for my delay. AFAIK, there's no this kind of use.
The question isn't whether there is or isn't this kind of use right now. The question is whether there might be this kind of use in the future, and if there is, whether we'd like lockdep to warn us.
In the future, IMHO, the
class_interfaceshould go away just asclass_device. If that happened this problem would going away as well.
Quoted 6 lines
Quoted 3 lines
Patches gladly accepted to do this, but what you will end up with is something just called a different name, yet doing the same functionality, so you are back at square one :(
Greg, do you have proposals about the
class_interfaceremoval before this thread? or ideas?
Nope, I'm not doing it :)
But, like I said, you are going to end up with the same functionality, right? So you will have the same problem.
thanks,
greg k-h
Quoted 45 lines · Greg KH
Quoted 35 lines · Andrew Morton
=============================================
[ 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/0x57abut task is already holding lock:
(&cls->mutex){--..}, at: [<ffffffff811b6787>]class_interface_register+0x48/0xbdother info that might help us debug this:
1 lock held by modprobe/942:
#0: (&cls->mutex){--..}, at: [<ffffffff811b6787>]class_interface_register+0x48/0xbdstack 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/0x14Quoted 8 lines
[<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
Quoted 5 lines
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 :)
David, any thoughts?
Yes, put them into -mm is right.
As matthew said, the class_interface_register call add_dev, then device_add cause the issue.
Regards
dave