Bug 171220

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.2.1) Gecko/20030225

Description of problem:
Stratus can heavily exercise hotplug functionality in both USB and PCI space.  While doing this, we hit a deadlock in khubd and were able to take a dump of memory.  Analyzing the dump with crash showed us that khubd had wedged itself via hub_events(...) in which locktree is called, thereby taking a serialize semaphore, and via a 2-part error path, usb_disconnect is later called, which tries to take the same semaphore.  A visual code inspection can uncover this as well.  Here is the sequence:

starting with hub_events, called from khubd...

                /* Lock the device, then check to see if we were
                 * disconnected while waiting for the lock to succeed. */
                if (locktree(hdev) < 0)        <- if successful, hdev->semaphore is taken.
                        break;
                if (hdev->state != USB_STATE_CONFIGURED ||
                                !hdev->actconfig ||
                                hub != usb_get_intfdata(
                                        hdev->actconfig->interface[0]))
                        goto loop;

                if (hub->error) {        <- error path condition #1
                        dev_dbg (hub_dev, "resetting for error %d\n",
                                hub->error);

                        if (hub_reset(hub)) {    <- error path condition #2
                                dev_dbg (hub_dev,
                                        "can't reset; disconnecting\n");
                                hub_start_disconnect(hdev);

And here is hub_start_disconnect:

/* caller has locked the hub */
/* FIXME!  This routine should be subsumed into hub_reset */
static void hub_start_disconnect(struct usb_device *hdev)
{
        struct usb_device *parent = hdev->parent;
        int i;

        /* Find the device pointer to disconnect */
        if (parent) {
                for (i = 0; i < parent->maxchild; i++) {
                        if (parent->children[i] == hdev) {
                                usb_disconnect(&parent->children[i]);
                                return;
                        }
                }
        }

        dev_err(&hdev->dev, "cannot disconnect hub!\n");
}

Even the comment above hub_start_disconnect points out that the device is locked. But usb_disconnect is going to take the same lock:

void usb_disconnect(struct usb_device **pdev)
{
        struct usb_device       *udev = *pdev;
        int                     i;

        if (!udev) {
                pr_debug ("%s nodev\n", __FUNCTION__);
                return;
        }

        /* mark the device as inactive, so any further urb submissions for
         * this device (and any of its children) will fail immediately.
         * this quiesces everyting except pending urbs.
         */
        usb_set_device_state(udev, USB_STATE_NOTATTACHED);

        /* lock the bus list on behalf of HCDs unregistering their root hubs */
        if (!udev->parent)
                down(&usb_bus_list_lock);
        down(&udev->serialize);    <- and here we are deadlocked because locktree did this already.


Version-Release number of selected component (if applicable):
kernel-2.6.9-22 (and earlier)

How reproducible:
Sometimes

Steps to Reproduce:
To hit this deadlock, one must first have a hub plugged into the host controller.   It shouldn't matter what is plugged into the hub itself.

It's not easy to take the error path described above. In our hardware, we have to use a hotplug disconnect on the PCI slot that hosts the ohci root hub at just the right time.  But when we do take the error path, khubd deadlocks itself.
  

Actual Results:  deadlock

Expected Results:  no deadlock

Additional info: