Skip to content

Commit cde3788

Browse files
Matthias Kaehlckegregkh
authored andcommitted
usb: misc: onboard_hub: Move 'attach' work to the driver
Currently each onboard_hub platform device owns an 'attach' work, which is scheduled when the device probes. With this deadlocks have been reported on a Raspberry Pi 3 B+ [1], which has nested onboard hubs. The flow of the deadlock is something like this (with the onboard_hub driver built as a module) [2]: - USB root hub is instantiated - core hub driver calls onboard_hub_create_pdevs(), which creates the 'raw' platform device for the 1st level hub - 1st level hub is probed by the core hub driver - core hub driver calls onboard_hub_create_pdevs(), which creates the 'raw' platform device for the 2nd level hub - onboard_hub platform driver is registered - platform device for 1st level hub is probed - schedules 'attach' work - platform device for 2nd level hub is probed - schedules 'attach' work - onboard_hub USB driver is registered - device (and parent) lock of hub is held while the device is re-probed with the onboard_hub driver - 'attach' work (running in another thread) calls driver_attach(), which blocks on one of the hub device locks - onboard_hub_destroy_pdevs() is called by the core hub driver when one of the hubs is detached - destroying the pdevs invokes onboard_hub_remove(), which waits for the 'attach' work to complete - waits forever, since the 'attach' work can't acquire the device lock Use a single work struct for the driver instead of having a work struct per onboard hub platform driver instance. With that it isn't necessary to cancel the work in onboard_hub_remove(), which fixes the deadlock. The work is only cancelled when the driver is unloaded. [1] https://lore.kernel.org/r/[email protected]/ [2] https://lore.kernel.org/all/[email protected]/ Cc: [email protected] Fixes: 8bc0636 ("usb: misc: Add onboard_usb_hub driver") Link: https://lore.kernel.org/r/[email protected]/ Link: https://lore.kernel.org/all/[email protected]/ Reported-by: Stefan Wahren <[email protected]> Signed-off-by: Matthias Kaehlcke <[email protected]> Link: https://lore.kernel.org/r/20230110172954.v2.2.I16b51f32db0c32f8a8532900bfe1c70c8572881a@changeid Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent e585435 commit cde3788

File tree

1 file changed

+6
-6
lines changed

1 file changed

+6
-6
lines changed

drivers/usb/misc/onboard_usb_hub.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,10 @@
2727

2828
#include "onboard_usb_hub.h"
2929

30+
static void onboard_hub_attach_usb_driver(struct work_struct *work);
31+
3032
static struct usb_device_driver onboard_hub_usbdev_driver;
33+
static DECLARE_WORK(attach_usb_driver_work, onboard_hub_attach_usb_driver);
3134

3235
/************************** Platform driver **************************/
3336

@@ -45,7 +48,6 @@ struct onboard_hub {
4548
bool is_powered_on;
4649
bool going_away;
4750
struct list_head udev_list;
48-
struct work_struct attach_usb_driver_work;
4951
struct mutex lock;
5052
};
5153

@@ -271,8 +273,7 @@ static int onboard_hub_probe(struct platform_device *pdev)
271273
* This needs to be done deferred to avoid self-deadlocks on systems
272274
* with nested onboard hubs.
273275
*/
274-
INIT_WORK(&hub->attach_usb_driver_work, onboard_hub_attach_usb_driver);
275-
schedule_work(&hub->attach_usb_driver_work);
276+
schedule_work(&attach_usb_driver_work);
276277

277278
return 0;
278279
}
@@ -285,9 +286,6 @@ static int onboard_hub_remove(struct platform_device *pdev)
285286

286287
hub->going_away = true;
287288

288-
if (&hub->attach_usb_driver_work != current_work())
289-
cancel_work_sync(&hub->attach_usb_driver_work);
290-
291289
mutex_lock(&hub->lock);
292290

293291
/* unbind the USB devices to avoid dangling references to this device */
@@ -449,6 +447,8 @@ static void __exit onboard_hub_exit(void)
449447
{
450448
usb_deregister_device_driver(&onboard_hub_usbdev_driver);
451449
platform_driver_unregister(&onboard_hub_driver);
450+
451+
cancel_work_sync(&attach_usb_driver_work);
452452
}
453453
module_exit(onboard_hub_exit);
454454

0 commit comments

Comments
 (0)