forked from Rust-for-Linux/linux
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit 64dd6ed
power: supply: core: Fix extension related lockdep warning
Since commit 6037802 ("power: supply: core: implement extension API")
there is the following ABBA deadlock (simplified) between the LED trigger
code and the power-supply code:
1) When registering a power-supply class device, power_supply_register()
calls led_trigger_register() from power_supply_create_triggers() in
a scoped_guard(rwsem_read, &psy->extensions_sem) context.
led_trigger_register() then in turn takes a LED subsystem lock.
So here we have the following locking order:
* Read-lock extensions_sem
* Lock LED subsystem lock(s)
2) When registering a LED class device, with its default trigger set
to a power-supply LED trigger (which has already been registered)
The LED class code calls power_supply_led_trigger_activate() when
setting up the default trigger. power_supply_led_trigger_activate()
calls power_supply_get_property() to determine the initial value of
to assign to the LED and that read-locks extensions_sem. So now we
have the following locking order:
* Lock LED subsystem lock(s)
* Read-lock extensions_sem
Fixing this is easy, there is no need to hold the extensions_sem when
calling power_supply_create_triggers() since all triggers are always
created rather then checking for the presence of certain attributes as
power_supply_add_hwmon_sysfs() does. Move power_supply_create_triggers()
out of the guard block to fix this.
Here is the lockdep report fixed by this change:
[ 31.249343] ======================================================
[ 31.249378] WARNING: possible circular locking dependency detected
[ 31.249413] 6.13.0-rc6+ Rust-for-Linux#251 Tainted: G C E
[ 31.249440] ------------------------------------------------------
[ 31.249471] (udev-worker)/553 is trying to acquire lock:
[ 31.249501] ffff892adbcaf660 (&psy->extensions_sem){.+.+}-{4:4}, at: power_supply_get_property.part.0+0x22/0x150
[ 31.249574]
but task is already holding lock:
[ 31.249603] ffff892adbc0bad0 (&led_cdev->trigger_lock){+.+.}-{4:4}, at: led_trigger_set_default+0x34/0xe0
[ 31.249657]
which lock already depends on the new lock.
[ 31.249696]
the existing dependency chain (in reverse order) is:
[ 31.249735]
-> Rust-for-Linux#2 (&led_cdev->trigger_lock){+.+.}-{4:4}:
[ 31.249778] down_write+0x3b/0xd0
[ 31.249803] led_trigger_set_default+0x34/0xe0
[ 31.249833] led_classdev_register_ext+0x311/0x3a0
[ 31.249863] input_leds_connect+0x1dc/0x2a0
[ 31.249889] input_attach_handler.isra.0+0x75/0x90
[ 31.249921] input_register_device.cold+0xa1/0x150
[ 31.249955] hidinput_connect+0x8a2/0xb80
[ 31.249982] hid_connect+0x582/0x5c0
[ 31.250007] hid_hw_start+0x3f/0x60
[ 31.250030] hid_device_probe+0x122/0x1f0
[ 31.250053] really_probe+0xde/0x340
[ 31.250080] __driver_probe_device+0x78/0x110
[ 31.250105] driver_probe_device+0x1f/0xa0
[ 31.250132] __device_attach_driver+0x85/0x110
[ 31.250160] bus_for_each_drv+0x78/0xc0
[ 31.250184] __device_attach+0xb0/0x1b0
[ 31.250207] bus_probe_device+0x94/0xb0
[ 31.250230] device_add+0x64a/0x860
[ 31.250252] hid_add_device+0xe5/0x240
[ 31.250279] usbhid_probe+0x4dc/0x620
[ 31.250303] usb_probe_interface+0xe4/0x2a0
[ 31.250329] really_probe+0xde/0x340
[ 31.250353] __driver_probe_device+0x78/0x110
[ 31.250377] driver_probe_device+0x1f/0xa0
[ 31.250404] __device_attach_driver+0x85/0x110
[ 31.250431] bus_for_each_drv+0x78/0xc0
[ 31.250455] __device_attach+0xb0/0x1b0
[ 31.250478] bus_probe_device+0x94/0xb0
[ 31.250501] device_add+0x64a/0x860
[ 31.250523] usb_set_configuration+0x606/0x8a0
[ 31.250552] usb_generic_driver_probe+0x3e/0x60
[ 31.250579] usb_probe_device+0x3d/0x120
[ 31.250605] really_probe+0xde/0x340
[ 31.250629] __driver_probe_device+0x78/0x110
[ 31.250653] driver_probe_device+0x1f/0xa0
[ 31.250680] __device_attach_driver+0x85/0x110
[ 31.250707] bus_for_each_drv+0x78/0xc0
[ 31.250731] __device_attach+0xb0/0x1b0
[ 31.250753] bus_probe_device+0x94/0xb0
[ 31.250776] device_add+0x64a/0x860
[ 31.250798] usb_new_device.cold+0x141/0x38f
[ 31.250828] hub_event+0x1166/0x1980
[ 31.250854] process_one_work+0x20f/0x580
[ 31.250879] worker_thread+0x1d1/0x3b0
[ 31.250904] kthread+0xee/0x120
[ 31.250926] ret_from_fork+0x30/0x50
[ 31.250954] ret_from_fork_asm+0x1a/0x30
[ 31.250982]
-> Rust-for-Linux#1 (triggers_list_lock){++++}-{4:4}:
[ 31.251022] down_write+0x3b/0xd0
[ 31.251045] led_trigger_register+0x40/0x1b0
[ 31.251074] power_supply_register_led_trigger+0x88/0x150
[ 31.251107] power_supply_create_triggers+0x55/0xe0
[ 31.251135] __power_supply_register.part.0+0x34e/0x4a0
[ 31.251164] devm_power_supply_register+0x70/0xc0
[ 31.251190] bq27xxx_battery_setup+0x1a1/0x6d0 [bq27xxx_battery]
[ 31.251235] bq27xxx_battery_i2c_probe+0xe5/0x17f [bq27xxx_battery_i2c]
[ 31.251272] i2c_device_probe+0x125/0x2b0
[ 31.251299] really_probe+0xde/0x340
[ 31.251324] __driver_probe_device+0x78/0x110
[ 31.251348] driver_probe_device+0x1f/0xa0
[ 31.251375] __driver_attach+0xba/0x1c0
[ 31.251398] bus_for_each_dev+0x6b/0xb0
[ 31.251421] bus_add_driver+0x111/0x1f0
[ 31.251445] driver_register+0x6e/0xc0
[ 31.251470] i2c_register_driver+0x41/0xb0
[ 31.251498] do_one_initcall+0x5e/0x3a0
[ 31.251522] do_init_module+0x60/0x220
[ 31.251550] __do_sys_init_module+0x15f/0x190
[ 31.251575] do_syscall_64+0x93/0x180
[ 31.251598] entry_SYSCALL_64_after_hwframe+0x76/0x7e
[ 31.251629]
-> #0 (&psy->extensions_sem){.+.+}-{4:4}:
[ 31.251668] __lock_acquire+0x13ce/0x21c0
[ 31.251694] lock_acquire+0xcf/0x2e0
[ 31.251719] down_read+0x3e/0x170
[ 31.251741] power_supply_get_property.part.0+0x22/0x150
[ 31.251774] power_supply_update_leds+0x8d/0x230
[ 31.251804] power_supply_led_trigger_activate+0x18/0x20
[ 31.251837] led_trigger_set+0x1fc/0x300
[ 31.251863] led_trigger_set_default+0x90/0xe0
[ 31.251892] led_classdev_register_ext+0x311/0x3a0
[ 31.251921] devm_led_classdev_multicolor_register_ext+0x6e/0xb80 [led_class_multicolor]
[ 31.251969] ktd202x_probe+0x464/0x5c0 [leds_ktd202x]
[ 31.252002] i2c_device_probe+0x125/0x2b0
[ 31.252027] really_probe+0xde/0x340
[ 31.252052] __driver_probe_device+0x78/0x110
[ 31.252076] driver_probe_device+0x1f/0xa0
[ 31.252103] __driver_attach+0xba/0x1c0
[ 31.252125] bus_for_each_dev+0x6b/0xb0
[ 31.252148] bus_add_driver+0x111/0x1f0
[ 31.252172] driver_register+0x6e/0xc0
[ 31.252197] i2c_register_driver+0x41/0xb0
[ 31.252225] do_one_initcall+0x5e/0x3a0
[ 31.252248] do_init_module+0x60/0x220
[ 31.252274] __do_sys_init_module+0x15f/0x190
[ 31.253986] do_syscall_64+0x93/0x180
[ 31.255826] entry_SYSCALL_64_after_hwframe+0x76/0x7e
[ 31.257614]
other info that might help us debug this:
[ 31.257619] Chain exists of:
&psy->extensions_sem --> triggers_list_lock --> &led_cdev->trigger_lock
[ 31.257630] Possible unsafe locking scenario:
[ 31.257632] CPU0 CPU1
[ 31.257633] ---- ----
[ 31.257634] lock(&led_cdev->trigger_lock);
[ 31.257637] lock(triggers_list_lock);
[ 31.257640] lock(&led_cdev->trigger_lock);
[ 31.257643] rlock(&psy->extensions_sem);
[ 31.257646]
*** DEADLOCK ***
[ 31.289433] 4 locks held by (udev-worker)/553:
[ 31.289443] #0: ffff892ad9658108 (&dev->mutex){....}-{4:4}, at: __driver_attach+0xaf/0x1c0
[ 31.289463] Rust-for-Linux#1: ffff892adbc0bbc8 (&led_cdev->led_access){+.+.}-{4:4}, at: led_classdev_register_ext+0x1c7/0x3a0
[ 31.289476] Rust-for-Linux#2: ffffffffad0e30b0 (triggers_list_lock){++++}-{4:4}, at: led_trigger_set_default+0x2c/0xe0
[ 31.289487] Rust-for-Linux#3: ffff892adbc0bad0 (&led_cdev->trigger_lock){+.+.}-{4:4}, at: led_trigger_set_default+0x34/0xe0
Fixes: 6037802 ("power: supply: core: implement extension API")
Cc: Thomas Weißschuh <[email protected]>
Cc: Armin Wolf <[email protected]>
Signed-off-by: Hans de Goede <[email protected]>
Reviewed-by: Thomas Weißschuh <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Sebastian Reichel <[email protected]>1 parent 3fb3cb4 commit 64dd6edCopy full SHA for 64dd6ed
1 file changed
+4
-4
lines changeddrivers/power/supply/power_supply_core.c
Copy file name to clipboardexpand all lines: drivers/power/supply/power_supply_core.c+4-4
Original file line number | Diff line number | Diff line change | |
---|---|---|---|
| |||
1592 | 1592 |
| |
1593 | 1593 |
| |
1594 | 1594 |
| |
1595 |
| - | |
1596 |
| - | |
1597 |
| - | |
1598 |
| - | |
| 1595 | + | |
| 1596 | + | |
| 1597 | + | |
1599 | 1598 |
| |
| 1599 | + | |
1600 | 1600 |
| |
1601 | 1601 |
| |
1602 | 1602 |
| |
|
0 commit comments