i2c: suppress lockdep warning on delete_device
Subsystems (2)
PATCH 1/1 i2c: suppress lockdep warning on delete_device
i2c: suppress lockdep warning on delete_device
Since commit 846f99749ab68bbc7f75c74fec305de675b1a1bf the following lockdep warning is thrown in case i2c device is removed (via delete_device sysfs attribute) which contains subdevices (e.g. i2c multiplexer):
=============================================
[ INFO: possible recursive locking detected ]
3.8.7-0-sampleversion-fct #8 Tainted: G O
---------------------------------------------
bash/3743 is trying to acquire lock:
(s_active#110){++++.+}, at: [<ffffffff802b3048>] sysfs_hash_and_remove+0x58/0xc8
but task is already holding lock:
(s_active#110){++++.+}, at: [<ffffffff802b3cb8>] sysfs_write_file+0xc8/0x208
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
---- lock(s_active#110);
lock(s_active#110);
*** DEADLOCK ***
May be due to missing lock nesting notation
4 locks held by bash/3743:
#0: (&buffer->mutex){+.+.+.}, at: [<ffffffff802b3c3c>] sysfs_write_file+0x4c/0x208
#1: (s_active#110){++++.+}, at: [<ffffffff802b3cb8>] sysfs_write_file+0xc8/0x208
#2: (&adap->userspace_clients_lock/1){+.+.+.}, at: [<ffffffff80454a18>] i2c_sysfs_delete_device+0x90/0x238
#3: (&__lockdep_no_validate__){......}, at: [<ffffffff803dcc24>] device_release_driver+0x24/0x48stack backtrace: Call Trace: [<ffffffff80575cc8>] dump_stack+0x8/0x34 [<ffffffff801b50fc>] __lock_acquire+0x161c/0x2110 [<ffffffff801b5c3c>] lock_acquire+0x4c/0x70 [<ffffffff802b60cc>] sysfs_addrm_finish+0x19c/0x1e0 [<ffffffff802b3048>] sysfs_hash_and_remove+0x58/0xc8 [<ffffffff802b7d8c>] sysfs_remove_group+0x64/0x148 [<ffffffff803d990c>] device_remove_attrs+0x9c/0x1a8 [<ffffffff803d9b1c>] device_del+0x104/0x1d8 [<ffffffff803d9c18>] device_unregister+0x28/0x70 [<ffffffff8045505c>] i2c_del_adapter+0x1cc/0x328 [<ffffffff8045802c>] i2c_del_mux_adapter+0x14/0x38 [<ffffffffc025c108>] pca954x_remove+0x90/0xe0 [pca954x] [<ffffffff804542f8>] i2c_device_remove+0x80/0xe8 [<ffffffff803dca9c>] __device_release_driver+0x74/0xf8 [<ffffffff803dcc2c>] device_release_driver+0x2c/0x48 [<ffffffff803dbc14>] bus_remove_device+0x13c/0x1d8 [<ffffffff803d9b24>] device_del+0x10c/0x1d8 [<ffffffff803d9c18>] device_unregister+0x28/0x70 [<ffffffff80454b08>] i2c_sysfs_delete_device+0x180/0x238 [<ffffffff802b3cd4>] sysfs_write_file+0xe4/0x208 [<ffffffff8023ddc4>] vfs_write+0xbc/0x160 [<ffffffff8023df6c>] SyS_write+0x54/0xd8 [<ffffffff8013d424>] handle_sys64+0x44/0x64
The problem is already known for USB and PCI subsystems. The reason is that delete_device attribute is defined statically in i2c-core.c and used for all devices in i2c subsystem.
Discussion of original USB problem:
http://lkml.indiana.edu/hypermail/linux/kernel/1204.3/01160.html
Commit 356c05d58af05d582e634b54b40050c73609617b introduced new macro to suppress lockdep warnings for this special case and included workaround for USB code.
LKML discussion of the workaround:
http://lkml.indiana.edu/hypermail/linux/kernel/1205.1/03634.html
As i2c case is in principle the same, the same workaround could be used here.
---
drivers/i2c/i2c-core.c +2 -1
@@ -892,7 +892,8 @@ i2c_sysfs_delete_device(struct device *d } static DEVICE_ATTR(new_device, S_IWUSR, NULL, i2c_sysfs_new_device); -static DEVICE_ATTR(delete_device, S_IWUSR, NULL, i2c_sysfs_delete_device); +static DEVICE_ATTR_IGNORE_LOCKDEP(delete_device, S_IWUSR, NULL, + i2c_sysfs_delete_device);static struct attribute *i2c_adapter_attrs[] = { &dev_attr_name.attr,
Quoted 5 lines · Alexander Sverdlin
i2c: suppress lockdep warning on
delete_deviceSince commit
846f99749ab68bbc7f75c74fec305de675b1a1bfthe following lockdep warning is thrown in case i2c device is removed (viadelete_devicesysfs attribute) which contains subdevices (e.g. i2c multiplexer):
Can you explain this in a little more detail? Exactly what does the delete_device attribute do, when called for device D?
That is, what does a write to /sys/bus/i2c/devices/D/delete_device do?
Alan Stern
Hi!
Quoted 10 lines · ext Alan Stern
Quoted 5 lines
i2c: suppress lockdep warning on
delete_deviceSince commit
846f99749ab68bbc7f75c74fec305de675b1a1bfthe following lockdep warning is thrown in case i2c device is removed (viadelete_devicesysfs attribute) which contains subdevices (e.g. i2c multiplexer):Can you explain this in a little more detail? Exactly what does the
delete_deviceattribute do, when called for device D?That is, what does a write to
/sys/bus/i2c/devices/D/delete_devicedo?
Like USB with its hubs, I2C structure could be tree-like, with I2C multiplexers. delete_device attribute removes I2C device from the subsystem, which is usually not a problem, except the case when device is a multiplexer and sub-devices should be removed first. Here we have the problem: holding a "s_active" lock on "delete_device" attribute of a parent (multiplexer) we need to delete children, but they were created with the same static attribute "delete_device".
The safety of this operation is exactly the same as in USB case... Any more clever lockdep annotation is exactly not so easy...
Signature · 2 lines
Best regards, Alexander Sverdlin.
Quoted 21 lines · Alexander Sverdlin
Hi!
Quoted 10 lines · ext Alan Stern
Quoted 5 lines
i2c: suppress lockdep warning on
delete_deviceSince commit
846f99749ab68bbc7f75c74fec305de675b1a1bfthe following lockdep warning is thrown in case i2c device is removed (viadelete_devicesysfs attribute) which contains subdevices (e.g. i2c multiplexer):Can you explain this in a little more detail? Exactly what does the
delete_deviceattribute do, when called for device D?That is, what does a write to
/sys/bus/i2c/devices/D/delete_devicedo?Like USB with its hubs, I2C structure could be tree-like, with I2C multiplexers.
delete_deviceattribute removes I2C device from the subsystem, which is usually not a problem, except the case when device is a multiplexer and sub-devices should be removed first. Here we have the problem: holding a "s_active" lock on "delete_device" attribute of a parent (multiplexer) we need to delete children, but they were created with the same static attribute "delete_device".
Quoted 2 lines
The safety of this operation is exactly the same as in USB case... Any more clever lockdep annotation is exactly not so easy...
If I understand you correctly, if D is an I2C multiplexer then writing to /sys/bus/i2c/devices/D/delete_device will get rid of all of D's children (and their descendants) and will also get rid of D itself -- right?
That's _not_ what the USB attributes do. For example, if D is a USB hub then writing 0 to /sys/bus/usb/devices/D/bConfigurationValue will get rid of all D's descendants but will not get rid of D.
Instead of doing it this way, the attribute method should call device_schedule_callback(). See commit d9a9cdfb078d.
Alan Stern
Hi!
Quoted 21 lines · ext Alan Stern
Quoted 18 lines
Quoted 10 lines
Quoted 5 lines
i2c: suppress lockdep warning on
delete_deviceSince commit
846f99749ab68bbc7f75c74fec305de675b1a1bfthe following lockdep warning is thrown in case i2c device is removed (viadelete_devicesysfs attribute) which contains subdevices (e.g. i2c multiplexer):Can you explain this in a little more detail? Exactly what does the
delete_deviceattribute do, when called for device D?That is, what does a write to
/sys/bus/i2c/devices/D/delete_devicedo?Like USB with its hubs, I2C structure could be tree-like, with I2C multiplexers.
delete_deviceattribute removes I2C device from the subsystem, which is usually not a problem, except the case when device is a multiplexer and sub-devices should be removed first. Here we have the problem: holding a "s_active" lock on "delete_device" attribute of a parent (multiplexer) we need to delete children, but they were created with the same static attribute "delete_device".Quoted 2 lines
The safety of this operation is exactly the same as in USB case... Any more clever lockdep annotation is exactly not so easy...
Quoted 8 lines
If I understand you correctly, if D is an I2C multiplexer then writing to
/sys/bus/i2c/devices/D/delete_devicewill get rid of all of D's children (and their descendants) and will also get rid of D itself -- right?That's _not_ what the USB attributes do. For example, if D is a USB hub then writing 0 to
/sys/bus/usb/devices/D/bConfigurationValuewill get rid of all D's descendants but will not get rid of D.
Well, seems that what I've described above wasn't precise enough... Actually only "bus controllers" have "delete_device" attribute. Simple devices on the bus do not have it. User write an address of the slave device to this attribute to delete it. This works fine for simple devices. In case of multiplexer (which is in turn also a "bus controller") there are nested "delete_device" attributes. So it's only possible to delete child nodes and not the controller itself, writing to its "delete_device". So from my POV, it fits to USB concept...
Quoted 2 lines
Instead of doing it this way, the attribute method should call
device_schedule_callback(). See commitd9a9cdfb078d.
Signature · 2 lines
Best regards, Alexander Sverdlin.
Quoted 29 lines · Alexander Sverdlin
Hi!
Quoted 26 lines · ext Alan Stern
Quoted 18 lines
Quoted 10 lines
Quoted 5 lines
i2c: suppress lockdep warning on
delete_deviceSince commit
846f99749ab68bbc7f75c74fec305de675b1a1bfthe following lockdep warning is thrown in case i2c device is removed (viadelete_devicesysfs attribute) which contains subdevices (e.g. i2c multiplexer):Can you explain this in a little more detail? Exactly what does the
delete_deviceattribute do, when called for device D?That is, what does a write to
/sys/bus/i2c/devices/D/delete_devicedo?Like USB with its hubs, I2C structure could be tree-like, with I2C multiplexers.
delete_deviceattribute removes I2C device from the subsystem, which is usually not a problem, except the case when device is a multiplexer and sub-devices should be removed first. Here we have the problem: holding a "s_active" lock on "delete_device" attribute of a parent (multiplexer) we need to delete children, but they were created with the same static attribute "delete_device".Quoted 2 lines
The safety of this operation is exactly the same as in USB case... Any more clever lockdep annotation is exactly not so easy...
If I understand you correctly, if D is an I2C multiplexer then writing to
/sys/bus/i2c/devices/D/delete_devicewill get rid of all of D's children (and their descendants) and will also get rid of D itself -- right?
Quoted 5 lines
Quoted 3 lines
That's _not_ what the USB attributes do. For example, if D is a USB hub then writing 0 to
/sys/bus/usb/devices/D/bConfigurationValuewill get rid of all D's descendants but will not get rid of D.Well, seems that what I've described above wasn't precise enough...
That's why I asked what "exactly" the attribute does. :-)
Quoted 6 lines
Actually only "bus controllers" have "
delete_device" attribute. Simple devices on the bus do not have it. User write an address of the slave device to this attribute to delete it. This works fine for simple devices. In case of multiplexer (which is in turn also a "bus controller") there are nested "delete_device" attributes. So it's only possible to delete child nodes and not the controller itself, writing to its "delete_device". So from my POV, it fits to USB concept...
Okay. If writing to the attribute doesn't delete the object which the attribute is attached to, then your patch is the correct approach.
Alan Stern
Wolfram Sang
AppliedQuoted 5 lines · Alexander Sverdlin
i2c: suppress lockdep warning on
delete_deviceSince commit
846f99749ab68bbc7f75c74fec305de675b1a1bfthe following lockdep warning is thrown in case i2c device is removed (viadelete_devicesysfs attribute) which contains subdevices (e.g. i2c multiplexer):
Applied to for-current with Alan's ack, thanks!
I couldn't apply the patch due to whitespace problems. I fixed them here because I wanted to have the patch in my very soon pull request. But please have a look.