Both a cleanup, as khubd open codes several facilities that are provided
by workqueue, and an enabling step for flushing initial port discovery
operations.  A do { } while (0) loop in hub_event() is used to minimize
code thrash.

Signed-off-by: Dan Williams <dan.j.willi...@intel.com>
---
 drivers/usb/core/hub.c |  146 ++++++++++++++++--------------------------------
 drivers/usb/core/hub.h |    2 -
 2 files changed, 51 insertions(+), 97 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 3c109924d2ae..e0518af66af9 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -22,11 +22,10 @@
 #include <linux/usb/hcd.h>
 #include <linux/usb/otg.h>
 #include <linux/usb/quirks.h>
-#include <linux/kthread.h>
 #include <linux/mutex.h>
-#include <linux/freezer.h>
 #include <linux/random.h>
 #include <linux/pm_qos.h>
+#include <linux/workqueue.h>
 
 #include <asm/uaccess.h>
 #include <asm/byteorder.h>
@@ -41,14 +40,10 @@
  * change to USB_STATE_NOTATTACHED even when the semaphore isn't held. */
 static DEFINE_SPINLOCK(device_state_lock);
 
-/* khubd's worklist and its lock */
+/* sync event queueing vs hub disconnects */
 static DEFINE_SPINLOCK(hub_event_lock);
-static LIST_HEAD(hub_event_list);      /* List of hubs needing servicing */
 
-/* Wakes up khubd */
-static DECLARE_WAIT_QUEUE_HEAD(khubd_wait);
-
-static struct task_struct *khubd_task;
+static struct workqueue_struct *khubd_wq;
 
 /* cycle leds on hubs that aren't blinking for attention */
 static bool blinkenlights = 0;
@@ -559,18 +554,28 @@ static int hub_port_status(struct usb_hub *hub, int port1,
        return ret;
 }
 
+static void hub_release(struct kref *kref)
+{
+       struct usb_hub *hub = container_of(kref, struct usb_hub, kref);
+
+       usb_put_intf(to_usb_interface(hub->intfdev));
+       kfree(hub);
+}
+
 static void kick_khubd(struct usb_hub *hub)
 {
-       unsigned long   flags;
+       struct usb_interface *intf = to_usb_interface(hub->intfdev);
+       unsigned long flags;
 
+       /* Suppress autosuspend until khubd runs */
        spin_lock_irqsave(&hub_event_lock, flags);
-       if (!hub->disconnected && list_empty(&hub->event_list)) {
-               list_add_tail(&hub->event_list, &hub_event_list);
-
-               /* Suppress autosuspend until khubd runs */
-               usb_autopm_get_interface_no_resume(
-                               to_usb_interface(hub->intfdev));
-               wake_up(&khubd_wait);
+       if (!hub->disconnected) {
+               usb_autopm_get_interface_no_resume(intf);
+               kref_get(&hub->kref);
+               if (!queue_work(khubd_wq, &hub->event_work)) {
+                       usb_autopm_put_interface_async(intf);
+                       kref_put(&hub->kref, hub_release);
+               }
        }
        spin_unlock_irqrestore(&hub_event_lock, flags);
 }
@@ -1584,14 +1589,6 @@ fail_keep_maxchild:
        return ret;
 }
 
-static void hub_release(struct kref *kref)
-{
-       struct usb_hub *hub = container_of(kref, struct usb_hub, kref);
-
-       usb_put_intf(to_usb_interface(hub->intfdev));
-       kfree(hub);
-}
-
 static unsigned highspeed_hubs;
 
 static void hub_disconnect(struct usb_interface *intf)
@@ -1600,13 +1597,13 @@ static void hub_disconnect(struct usb_interface *intf)
        struct usb_device *hdev = interface_to_usbdev(intf);
        int port1;
 
-       /* Take the hub off the event list and don't let it be added again */
-       spin_lock_irq(&hub_event_lock);
-       if (!list_empty(&hub->event_list)) {
-               list_del_init(&hub->event_list);
-               usb_autopm_put_interface_no_suspend(intf);
-       }
+       /*
+        * Disable hub event processing, note that we can't flush the work
+        * since we may be holding a device lock that khubd wants to acquire
+        * (lockdep is prevented from flagging this)
+        */
        hub->disconnected = 1;
+       spin_lock_irq(&hub_event_lock);
        spin_unlock_irq(&hub_event_lock);
 
        /* Disconnect all children and quiesce the hub */
@@ -1636,6 +1633,8 @@ static void hub_disconnect(struct usb_interface *intf)
        kref_put(&hub->kref, hub_release);
 }
 
+static void hub_event(struct work_struct *);
+
 static int hub_probe(struct usb_interface *intf, const struct usb_device_id 
*id)
 {
        struct usb_host_interface *desc;
@@ -1726,7 +1725,7 @@ descriptor_error:
        }
 
        kref_init(&hub->kref);
-       INIT_LIST_HEAD(&hub->event_list);
+       INIT_WORK(&hub->event_work, hub_event);
        hub->intfdev = &intf->dev;
        hub->hdev = hdev;
        INIT_DELAYED_WORK(&hub->leds, led_work);
@@ -4828,42 +4827,16 @@ static void port_event(struct usb_hub *hub, int port1)
 }
 
 
-static void hub_events(void)
+static void hub_event(struct work_struct *w)
 {
-       struct list_head *tmp;
-       struct usb_device *hdev;
-       struct usb_interface *intf;
-       struct usb_hub *hub;
-       struct device *hub_dev;
-       u16 hubstatus;
-       u16 hubchange;
+       struct usb_hub *hub = container_of(w, struct usb_hub, event_work);
        int i, ret;
+       u16 hubstatus, hubchange;
+       struct usb_device *hdev = hub->hdev;
+       struct device *hub_dev = hub->intfdev;
+       struct usb_interface *intf = to_usb_interface(hub_dev);
 
-       /*
-        *  We restart the list every time to avoid a deadlock with
-        * deleting hubs downstream from this one. This should be
-        * safe since we delete the hub from the event list.
-        * Not the most efficient, but avoids deadlocks.
-        */
-       while (1) {
-
-               /* Grab the first entry at the beginning of the list */
-               spin_lock_irq(&hub_event_lock);
-               if (list_empty(&hub_event_list)) {
-                       spin_unlock_irq(&hub_event_lock);
-                       break;
-               }
-
-               tmp = hub_event_list.next;
-               list_del_init(tmp);
-
-               hub = list_entry(tmp, struct usb_hub, event_list);
-               kref_get(&hub->kref);
-               spin_unlock_irq(&hub_event_lock);
-
-               hdev = hub->hdev;
-               hub_dev = hub->intfdev;
-               intf = to_usb_interface(hub_dev);
+       do {
                dev_dbg(hub_dev, "state %d ports %d chg %04x evt %04x\n",
                                hdev->state, hdev->maxchild,
                                /* NOTE: expects max 15 ports... */
@@ -4873,8 +4846,6 @@ static void hub_events(void)
                /* Lock the device, then check to see if we were
                 * disconnected while waiting for the lock to succeed. */
                usb_lock_device(hdev);
-               if (unlikely(hub->disconnected))
-                       goto loop_disconnected;
 
                /* If the hub has died, clean up after it */
                if (hdev->state == USB_STATE_NOTATTACHED) {
@@ -4963,31 +4934,9 @@ static void hub_events(void)
                 * kick_khubd() and allow autosuspend.
                 */
                usb_autopm_put_interface(intf);
- loop_disconnected:
                usb_unlock_device(hdev);
-               kref_put(&hub->kref, hub_release);
-
-       } /* end while (1) */
-}
-
-static int hub_thread(void *__unused)
-{
-       /* khubd needs to be freezable to avoid interfering with USB-PERSIST
-        * port handover.  Otherwise it might see that a full-speed device
-        * was gone before the EHCI controller had handed its port over to
-        * the companion full-speed controller.
-        */
-       set_freezable();
-
-       do {
-               hub_events();
-               wait_event_freezable(khubd_wait,
-                               !list_empty(&hub_event_list) ||
-                               kthread_should_stop());
-       } while (!kthread_should_stop() || !list_empty(&hub_event_list));
-
-       pr_debug("%s: khubd exiting\n", usbcore_name);
-       return 0;
+       } while (0);
+       kref_put(&hub->kref, hub_release);
 }
 
 static const struct usb_device_id hub_id_table[] = {
@@ -5027,11 +4976,17 @@ int usb_hub_init(void)
                return -1;
        }
 
-       khubd_task = kthread_run(hub_thread, NULL, "khubd");
-       if (!IS_ERR(khubd_task))
+       /* khubd needs to be freezable to avoid interfering with USB-PERSIST
+        * port handover.  Otherwise it might see that a full-speed device
+        * was gone before the EHCI controller had handed its port over to
+        * the companion full-speed controller.
+        */
+       khubd_wq = create_freezable_workqueue("khubd");
+
+       if (khubd_wq)
                return 0;
 
-       /* Fall through if kernel_thread failed */
+       /* Fall through if workqueue creation failed */
        usb_deregister(&hub_driver);
        printk(KERN_ERR "%s: can't start khubd\n", usbcore_name);
 
@@ -5040,8 +4995,6 @@ int usb_hub_init(void)
 
 void usb_hub_cleanup(void)
 {
-       kthread_stop(khubd_task);
-
        /*
         * Hub resources are freed for us by usb_deregister. It calls
         * usb_driver_purge on every device which in turn calls that
@@ -5050,7 +5003,8 @@ void usb_hub_cleanup(void)
         * individual hub resources. -greg
         */
        usb_deregister(&hub_driver);
-} /* usb_hub_cleanup() */
+       destroy_workqueue(khubd_wq);
+}
 
 static int descriptors_changed(struct usb_device *udev,
                struct usb_device_descriptor *old_device_descriptor,
diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index 16d03486e8fc..96a058c8d85f 100644
--- a/drivers/usb/core/hub.h
+++ b/drivers/usb/core/hub.h
@@ -41,7 +41,7 @@ struct usb_hub {
        int                     error;          /* last reported error */
        int                     nerrors;        /* track consecutive errors */
 
-       struct list_head        event_list;     /* hubs w/data or errs ready */
+       struct work_struct      event_work;     /* hubs w/data or errs ready */
        unsigned long           event_bits[1];  /* status change bitmask */
        unsigned long           change_bits[1]; /* ports with logical connect
                                                        status change */

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to