On Fri, 13 Apr 2012, Alan Stern wrote:
> Sarah:
>
> I just tracked this down. When you do
>
> echo 0 >/sys/.../bConfigurationValue
>
> for a hub that has children, usb_disconnect() deadlocks when it tries
> to acquire the bandwidth_mutex. This is because the task already holds
> the mutex -- it was acquired in usb_set_configuration() just before the
> call to usb_disable_device().
>
> That is, usb_disable_device() unbinds the hub driver, since its
> interface is going away. The hub driver, in hub_quiesce(), disconnects
> all the child devices.
>
> Any ideas on how to fix this? Maybe usb_disable_device() should be
> responsible for acquiring the bandwidth_mutex on its own, after it
> unbinds the interfaces, instead of making the caller acquire the mutex.
In other words, something like this patch. However I'm not sure how
well it would interact with your
fccf4e86200b8f5edd9a65da26f150e32ba79808 commit; it undoes several of
the changes you introduced.
Alan Stern
Index: usb-3.4/drivers/usb/core/hub.c
===================================================================
--- usb-3.4.orig/drivers/usb/core/hub.c
+++ usb-3.4/drivers/usb/core/hub.c
@@ -1667,7 +1667,6 @@ void usb_disconnect(struct usb_device **
{
struct usb_device *udev = *pdev;
int i;
- struct usb_hcd *hcd = bus_to_hcd(udev->bus);
/* mark the device as inactive, so any further urb submissions for
* this device (and any of its children) will fail immediately.
@@ -1690,9 +1689,7 @@ void usb_disconnect(struct usb_device **
* so that the hardware is now fully quiesced.
*/
dev_dbg (&udev->dev, "unregistering device\n");
- mutex_lock(hcd->bandwidth_mutex);
usb_disable_device(udev, 0);
- mutex_unlock(hcd->bandwidth_mutex);
usb_hcd_synchronize_unlinks(udev);
usb_remove_ep_devs(&udev->ep0);
Index: usb-3.4/drivers/usb/core/message.c
===================================================================
--- usb-3.4.orig/drivers/usb/core/message.c
+++ usb-3.4/drivers/usb/core/message.c
@@ -1189,7 +1189,9 @@ void usb_disable_device(struct usb_devic
usb_disable_endpoint(dev, i + USB_DIR_IN, false);
}
/* Remove endpoints from the host controller internal state */
+ mutex_lock(hcd->bandwidth_mutex);
usb_hcd_alloc_bandwidth(dev, NULL, NULL, NULL);
+ mutex_unlock(hcd->bandwidth_mutex);
/* Second pass: remove endpoint pointers */
}
for (i = skip_ep0; i < 16; ++i) {
@@ -1749,7 +1751,6 @@ free_interfaces:
/* if it's already configured, clear out old state first.
* getting rid of old interfaces means unbinding their drivers.
*/
- mutex_lock(hcd->bandwidth_mutex);
if (dev->state != USB_STATE_ADDRESS)
usb_disable_device(dev, 1); /* Skip ep0 */
@@ -1762,6 +1763,7 @@ free_interfaces:
* host controller will not allow submissions to dropped endpoints. If
* this call fails, the device state is unchanged.
*/
+ mutex_lock(hcd->bandwidth_mutex);
ret = usb_hcd_alloc_bandwidth(dev, cp, NULL, NULL);
if (ret < 0) {
mutex_unlock(hcd->bandwidth_mutex);
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
