[PATCH v5 15/16] usb: convert khubd to a workqueue

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.williams@xxxxxxxxx>
---
 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux