[PATCH v2 0/4] usb: hub: convert khubd into workqueue

[PATCH v2 0/4] usb: hub: convert khubd into workqueue

[Petr Mladek]

The workqueue API is well defined and tested. It has many options
that could be used to tune the scheduling. The code is usually
easier and thus more safe. It allows to avoid the extra thread
in most cases. It has has clearly defined behavior vrt. system suspend.

This patchset converts khubd into the workqueue. It saves one thread,
lock, and list.

It  looks huge but the main change is in the first patch. The rest is
simple renaming of functions, comments and documentation.

Changes in v2:

	+ solved potential races:
	  	 + get hub->kref in kick_hub_wq()
		 + call usb_get_dev(hdev) in hub_probe()
		   and 	usb_put_dev(hdev) in hub_release()
		 + INIT_WORK only once in hub_probe()
	 + do not call cancel_work_sync() in hub_disconnect()
	   to keep it fast
	 + rename kick_khubd() to kick_hub_wq() already
	   in the first patch; IMHO, it is cleaner while
	   adding only very few changes
	   
I send all patches again. Especially, 2nd and 3rd patch
needed refresh.

Note that the while cycle in hub_events() was needed when using
the kthread. More events might appear in the list before
the thread is scheduled. This is why I kept the order of patches
as is to do not break bisection.

Thanks a lot Alan Stern for hint and guidance.

The patches can be applied either against Linus' tree or linux-next.


Petr Mladek (4):
  usb: hub: convert khubd into workqueue
  usb: hub: remove obsolete while cycle in hub_event()
  usb: hub: rename usb_kick_khubd() to usb_kick_hub_wq()
  usb: hub: rename khubd to hub_wq in documentation and comments

 Documentation/DocBook/usb.tmpl             |   2 +-
 Documentation/usb/WUSB-Design-overview.txt |   6 +-
 Documentation/usb/hotplug.txt              |   2 +-
 drivers/net/usb/usbnet.c                   |  14 +-
 drivers/usb/README                         |   2 +-
 drivers/usb/core/hcd.c                     |  14 +-
 drivers/usb/core/hub.c                     | 380 +++++++++++++----------------
 drivers/usb/core/hub.h                     |   2 +-
 drivers/usb/core/usb.h                     |   2 +-
 drivers/usb/host/ehci-fsl.c                |   2 +-
 drivers/usb/host/ehci-hcd.c                |   2 +-
 drivers/usb/host/ehci-hub.c                |   8 +-
 drivers/usb/host/fhci-hcd.c                |   6 +-
 drivers/usb/host/fotg210-hcd.c             |   8 +-
 drivers/usb/host/fusbh200-hcd.c            |   8 +-
 drivers/usb/host/isp1760-hcd.c             |   6 +-
 drivers/usb/host/ohci-hcd.c                |   6 +-
 drivers/usb/host/ohci-hub.c                |   4 +-
 drivers/usb/host/ohci-omap.c               |   2 +-
 drivers/usb/host/oxu210hp-hcd.c            |  10 +-
 drivers/usb/host/sl811-hcd.c               |   8 +-
 drivers/usb/host/xhci-hub.c                |   2 +-
 drivers/usb/host/xhci.c                    |   4 +-
 drivers/usb/misc/usbtest.c                 |   2 +-
 drivers/usb/musb/am35x.c                   |   1 +
 drivers/usb/musb/tusb6010.c                |   2 +-
 drivers/usb/phy/phy-fsl-usb.c              |   2 +-
 drivers/usb/phy/phy-isp1301-omap.c         |   2 +-
 drivers/usb/wusbcore/devconnect.c          |   6 +-
 drivers/usb/wusbcore/wa-hc.h               |   2 +-
 sound/usb/midi.c                           |   2 +-
 31 files changed, 240 insertions(+), 279 deletions(-)

-- 
1.8.4

[PATCH v2 1/4] usb: hub: convert khubd into workqueue

[Petr Mladek]

There is no need to have separate kthread for handling USB hub events.
It is more elegant to use the workqueue framework.

The workqueue is allocated as freezable because the original thread was
freezable as well.

struct usb_hub is passed via the work item. Therefore we do not need
hub_event_list.

Also hub_thread() is not longer needed. It would call only hub_events().
The rest of the code did manipulate the kthread and it is handled by the
workqueue framework now.

Therefore the work item is proceed directly by hub_events(). Only one item
is manipulated in one call, so the function is renamed to hub_event().
The obsolete block from the while cycle will be removed in a followup patch.
It helps to see the real changes here.

kick_khubd is renamed to kick_hub_wq() to make the function clear. And the
protection against races is done another way, see below.

hub_event_lock has been removed. It cannot longer be used to protect struct
usb_hub between hub_event() and hub_disconnect(). Instead we need to get
hub->kref already in kick_hub_wq().

We would need to call also usb_get_dev() in kick_hub_wq(). But better
solution is to move this to hub_probe() where the struct hub is allocated.
As a result, usb_put_dev() is called in hub_release(). By other words,
it is handled the same way like usb_get_intf() and usb_put_intf().

But back to the lock. It is not really needed for the other scenarios as well.
queue_work() returns whether it succeeded. We could revert the needed operations
accordingly. This is enough to avoid duplicity and inconsistencies.

Yes, the removed lock causes that there is not longer such a strong
synchronization between scheduling the work and manipulating
hub->disconnected.

But kick_hub_wq() must never be called together with hub_disconnect()
otherwise even the original code would have failed. Any callers are
responsible for this.

Therefore the only problem is that hub_disconnect() could be called in parallel
with hub_event(). But this was possible even in the past. struct usb_hub is
still guarded by hub->kref and released in hub_events() when needed.

Note that the source file is still full of the obsolete "khubd" strings.
Let's remove them in a follow up patch. This patch already is complex enough.

Thanks a lot Alan Stern <stern@rowland.harvard.edu> for very useful tips
and guidance.

Signed-off-by: Petr Mladek <pmladek@suse.cz>
---
 drivers/usb/core/hub.c | 163 +++++++++++++++++++------------------------------
 drivers/usb/core/hub.h |   2 +-
 2 files changed, 65 insertions(+), 100 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index d481c99a20d7..b67f454c1edb 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -22,9 +22,8 @@
 #include <linux/usb/hcd.h>
 #include <linux/usb/otg.h>
 #include <linux/usb/quirks.h>
-#include <linux/kthread.h>
+#include <linux/workqueue.h>
 #include <linux/mutex.h>
-#include <linux/freezer.h>
 #include <linux/random.h>
 #include <linux/pm_qos.h>
 
@@ -41,14 +40,9 @@
  * change to USB_STATE_NOTATTACHED even when the semaphore isn't held. */
 static DEFINE_SPINLOCK(device_state_lock);
 
-/* khubd's worklist and its lock */
-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;
+/* workqueue to process hub events */
+static struct workqueue_struct *hub_wq;
+static void hub_event(struct work_struct *work);
 
 /* synchronize hub-port add/remove and peering operations */
 DEFINE_MUTEX(usb_port_peer_mutex);
@@ -104,6 +98,7 @@ EXPORT_SYMBOL_GPL(ehci_cf_port_reset_rwsem);
 #define HUB_DEBOUNCE_STEP	  25
 #define HUB_DEBOUNCE_STABLE	 100
 
+static void hub_release(struct kref *kref);
 static int usb_reset_and_verify_device(struct usb_device *udev);
 
 static inline char *portspeed(struct usb_hub *hub, int portstatus)
@@ -575,20 +570,31 @@ static int hub_port_status(struct usb_hub *hub, int port1,
 	return ret;
 }
 
-static void kick_khubd(struct usb_hub *hub)
+static void kick_hub_wq(struct usb_hub *hub)
 {
-	unsigned long	flags;
+	struct usb_interface *intf;
 
-	spin_lock_irqsave(&hub_event_lock, flags);
-	if (!hub->disconnected && list_empty(&hub->event_list)) {
-		list_add_tail(&hub->event_list, &hub_event_list);
+	if (hub->disconnected || work_pending(&hub->events))
+		return;
 
-		/* Suppress autosuspend until khubd runs */
-		usb_autopm_get_interface_no_resume(
-				to_usb_interface(hub->intfdev));
-		wake_up(&khubd_wait);
-	}
-	spin_unlock_irqrestore(&hub_event_lock, flags);
+	intf = to_usb_interface(hub->intfdev);
+	/*
+	 * Suppress autosuspend until the event is proceed.
+	 *
+	 * Be careful and make sure that the symmetric operation is
+	 * always called. We are here only when there is no pending
+	 * work for this hub. Therefore put the interface either when
+	 * the new work is called or when it is canceled.
+	 */
+	kref_get(&hub->kref);
+	usb_autopm_get_interface_no_resume(intf);
+
+	if (queue_work(hub_wq, &hub->events))
+		return;
+
+	/* the work has already been scheduled */
+	usb_autopm_put_interface_async(intf);
+	kref_put(&hub->kref, hub_release);
 }
 
 void usb_kick_khubd(struct usb_device *hdev)
@@ -596,7 +602,7 @@ void usb_kick_khubd(struct usb_device *hdev)
 	struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
 
 	if (hub)
-		kick_khubd(hub);
+		kick_hub_wq(hub);
 }
 
 /*
@@ -618,7 +624,7 @@ void usb_wakeup_notification(struct usb_device *hdev,
 	hub = usb_hub_to_struct_hub(hdev);
 	if (hub) {
 		set_bit(portnum, hub->wakeup_bits);
-		kick_khubd(hub);
+		kick_hub_wq(hub);
 	}
 }
 EXPORT_SYMBOL_GPL(usb_wakeup_notification);
@@ -658,7 +664,7 @@ static void hub_irq(struct urb *urb)
 	hub->nerrors = 0;
 
 	/* Something happened, let khubd figure it out */
-	kick_khubd(hub);
+	kick_hub_wq(hub);
 
 resubmit:
 	if (hub->quiescing)
@@ -972,7 +978,7 @@ static void hub_port_logical_disconnect(struct usb_hub *hub, int port1)
 	 */
 
 	set_bit(port1, hub->change_bits);
-	kick_khubd(hub);
+	kick_hub_wq(hub);
 }
 
 /**
@@ -1240,7 +1246,7 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type)
 				&hub->leds, LED_CYCLE_PERIOD);
 
 	/* Scan all ports that need attention */
-	kick_khubd(hub);
+	kick_hub_wq(hub);
 
 	/* Allow autosuspend if it was suppressed */
 	if (type <= HUB_INIT3)
@@ -1634,6 +1640,7 @@ static void hub_release(struct kref *kref)
 {
 	struct usb_hub *hub = container_of(kref, struct usb_hub, kref);
 
+	usb_put_dev(hub->hdev);
 	usb_put_intf(to_usb_interface(hub->intfdev));
 	kfree(hub);
 }
@@ -1646,14 +1653,11 @@ 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);
-	}
+	/*
+	 * Stop adding new hub events. We do not want to block here and thus
+	 * will not try to remove any pending work item.
+	 */
 	hub->disconnected = 1;
-	spin_unlock_irq(&hub_event_lock);
 
 	/* Disconnect all children and quiesce the hub */
 	hub->error = 0;
@@ -1793,12 +1797,13 @@ descriptor_error:
 	}
 
 	kref_init(&hub->kref);
-	INIT_LIST_HEAD(&hub->event_list);
 	hub->intfdev = &intf->dev;
 	hub->hdev = hdev;
 	INIT_DELAYED_WORK(&hub->leds, led_work);
 	INIT_DELAYED_WORK(&hub->init_work, NULL);
+	INIT_WORK(&hub->events, hub_event);
 	usb_get_intf(intf);
+	usb_get_dev(hdev);
 
 	usb_set_intfdata (intf, hub);
 	intf->needs_remote_wakeup = 1;
@@ -4993,9 +4998,8 @@ static void port_event(struct usb_hub *hub, int port1)
 }
 
 
-static void hub_events(void)
+static void hub_event(struct work_struct *work)
 {
-	struct list_head *tmp;
 	struct usb_device *hdev;
 	struct usb_interface *intf;
 	struct usb_hub *hub;
@@ -5004,32 +5008,13 @@ static void hub_events(void)
 	u16 hubchange;
 	int i, ret;
 
-	/*
-	 *  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);
+	/* temporary keep the block to show real changes in this commit */
+	{
+		hub = container_of(work, struct usb_hub, events);
 		hdev = hub->hdev;
-		usb_get_dev(hdev);
-		spin_unlock_irq(&hub_event_lock);
-
 		hub_dev = hub->intfdev;
 		intf = to_usb_interface(hub_dev);
+
 		dev_dbg(hub_dev, "state %d ports %d chg %04x evt %04x\n",
 				hdev->state, hdev->maxchild,
 				/* NOTE: expects max 15 ports... */
@@ -5040,25 +5025,25 @@ static void hub_events(void)
 		 * disconnected while waiting for the lock to succeed. */
 		usb_lock_device(hdev);
 		if (unlikely(hub->disconnected))
-			goto loop_disconnected;
+			goto out_hdev_lock;
 
 		/* If the hub has died, clean up after it */
 		if (hdev->state == USB_STATE_NOTATTACHED) {
 			hub->error = -ENODEV;
 			hub_quiesce(hub, HUB_DISCONNECT);
-			goto loop;
+			goto out_hdev_lock;
 		}
 
 		/* Autoresume */
 		ret = usb_autopm_get_interface(intf);
 		if (ret) {
 			dev_dbg(hub_dev, "Can't autoresume: %d\n", ret);
-			goto loop;
+			goto out_hdev_lock;
 		}
 
 		/* If this is an inactive hub, do nothing */
 		if (hub->quiescing)
-			goto loop_autopm;
+			goto out_autopm;
 
 		if (hub->error) {
 			dev_dbg (hub_dev, "resetting for error %d\n",
@@ -5068,7 +5053,7 @@ static void hub_events(void)
 			if (ret) {
 				dev_dbg (hub_dev,
 					"error resetting hub: %d\n", ret);
-				goto loop_autopm;
+				goto out_autopm;
 			}
 
 			hub->nerrors = 0;
@@ -5130,40 +5115,15 @@ static void hub_events(void)
 			}
 		}
 
- loop_autopm:
+ out_autopm:
 		/* Balance the usb_autopm_get_interface() above */
 		usb_autopm_put_interface_no_suspend(intf);
- loop:
-		/* Balance the usb_autopm_get_interface_no_resume() in
-		 * kick_khubd() and allow autosuspend.
-		 */
-		usb_autopm_put_interface(intf);
- loop_disconnected:
+ out_hdev_lock:
 		usb_unlock_device(hdev);
-		usb_put_dev(hdev);
+		/* Ballance the stuff in kick_hub_wq() and allow autosuspend */
+		usb_autopm_put_interface(intf);
 		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;
+	}
 }
 
 static const struct usb_device_id hub_id_table[] = {
@@ -5203,21 +5163,26 @@ int usb_hub_init(void)
 		return -1;
 	}
 
-	khubd_task = kthread_run(hub_thread, NULL, "khubd");
-	if (!IS_ERR(khubd_task))
+	/*
+	 * The workqueue 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.
+	 */
+	hub_wq = alloc_workqueue("hub", WQ_FREEZABLE, 0);
+	if (hub_wq)
 		return 0;
 
 	/* Fall through if kernel_thread failed */
 	usb_deregister(&hub_driver);
-	printk(KERN_ERR "%s: can't start khubd\n", usbcore_name);
+	pr_err("%s: can't allocate workqueue for hub\n", usbcore_name);
 
 	return -1;
 }
 
 void usb_hub_cleanup(void)
 {
-	kthread_stop(khubd_task);
-
+	destroy_workqueue(hub_wq);
 	/*
 	 * Hub resources are freed for us by usb_deregister. It calls
 	 * usb_driver_purge on every device which in turn calls that
diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index c77d8778af4b..688817fb3246 100644
--- a/drivers/usb/core/hub.h
+++ b/drivers/usb/core/hub.h
@@ -41,7 +41,6 @@ struct usb_hub {
 	int			error;		/* last reported error */
 	int			nerrors;	/* track consecutive errors */
 
-	struct list_head	event_list;	/* 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 */
@@ -77,6 +76,7 @@ struct usb_hub {
 	u8			indicator[USB_MAXCHILDREN];
 	struct delayed_work	leds;
 	struct delayed_work	init_work;
+	struct work_struct      events;
 	struct usb_port		**ports;
 };
 
-- 
1.8.4

[PATCH v2 2/4] usb: hub: remove obsolete while cycle in hub_event()

[Petr Mladek]

The USB hub events are proceed by workqueue instead of kthread now.
The result is that hub_event() function processes only one event.
The block from the while cycle was not removed earlier to show the real
 changes when switching to the workqueue.

When touching the code, it fixes also formatting of dev_err() and dev_dbg()
calls to make checkpatch.pl happy :-)

Signed-off-by: Petr Mladek <pmladek@suse.cz>
---
 drivers/usb/core/hub.c | 199 ++++++++++++++++++++++++-------------------------
 1 file changed, 96 insertions(+), 103 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index b67f454c1edb..b6b526a2e20d 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -4997,7 +4997,6 @@ static void port_event(struct usb_hub *hub, int port1)
 		hub_port_connect_change(hub, port1, portstatus, portchange);
 }
 
-
 static void hub_event(struct work_struct *work)
 {
 	struct usb_device *hdev;
@@ -5008,122 +5007,116 @@ static void hub_event(struct work_struct *work)
 	u16 hubchange;
 	int i, ret;
 
-	/* temporary keep the block to show real changes in this commit */
-	{
-		hub = container_of(work, struct usb_hub, events);
-		hdev = hub->hdev;
-		hub_dev = hub->intfdev;
-		intf = to_usb_interface(hub_dev);
-
-		dev_dbg(hub_dev, "state %d ports %d chg %04x evt %04x\n",
-				hdev->state, hdev->maxchild,
-				/* NOTE: expects max 15 ports... */
-				(u16) hub->change_bits[0],
-				(u16) hub->event_bits[0]);
-
-		/* 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 out_hdev_lock;
-
-		/* If the hub has died, clean up after it */
-		if (hdev->state == USB_STATE_NOTATTACHED) {
-			hub->error = -ENODEV;
-			hub_quiesce(hub, HUB_DISCONNECT);
-			goto out_hdev_lock;
-		}
+	hub = container_of(work, struct usb_hub, events);
+	hdev = hub->hdev;
+	hub_dev = hub->intfdev;
+	intf = to_usb_interface(hub_dev);
+
+	dev_dbg(hub_dev, "state %d ports %d chg %04x evt %04x\n",
+		hdev->state, hdev->maxchild,
+		/* NOTE: expects max 15 ports... */
+		(u16) hub->change_bits[0],
+		(u16) hub->event_bits[0]);
+
+	/* 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 out_hdev_lock;
+
+	/* If the hub has died, clean up after it */
+	if (hdev->state == USB_STATE_NOTATTACHED) {
+		hub->error = -ENODEV;
+		hub_quiesce(hub, HUB_DISCONNECT);
+		goto out_hdev_lock;
+	}
+
+	/* Autoresume */
+	ret = usb_autopm_get_interface(intf);
+	if (ret) {
+		dev_dbg(hub_dev, "Can't autoresume: %d\n", ret);
+		goto out_hdev_lock;
+	}
 
-		/* Autoresume */
-		ret = usb_autopm_get_interface(intf);
-		if (ret) {
-			dev_dbg(hub_dev, "Can't autoresume: %d\n", ret);
-			goto out_hdev_lock;
-		}
+	/* If this is an inactive hub, do nothing */
+	if (hub->quiescing)
+		goto out_autopm;
+
+	if (hub->error) {
+		dev_dbg(hub_dev, "resetting for error %d\n", hub->error);
 
-		/* If this is an inactive hub, do nothing */
-		if (hub->quiescing)
+		ret = usb_reset_device(hdev);
+		if (ret) {
+			dev_dbg(hub_dev, "error resetting hub: %d\n", ret);
 			goto out_autopm;
+		}
 
-		if (hub->error) {
-			dev_dbg (hub_dev, "resetting for error %d\n",
-				hub->error);
+		hub->nerrors = 0;
+		hub->error = 0;
+	}
 
-			ret = usb_reset_device(hdev);
-			if (ret) {
-				dev_dbg (hub_dev,
-					"error resetting hub: %d\n", ret);
-				goto out_autopm;
-			}
+	/* deal with port status changes */
+	for (i = 1; i <= hdev->maxchild; i++) {
+		struct usb_port *port_dev = hub->ports[i - 1];
 
-			hub->nerrors = 0;
-			hub->error = 0;
+		if (test_bit(i, hub->event_bits)
+		    || test_bit(i, hub->change_bits)
+		    || test_bit(i, hub->wakeup_bits)) {
+			/*
+			 * The get_noresume and barrier ensure that if
+			 * the port was in the process of resuming, we
+			 * flush that work and keep the port active for
+			 * the duration of the port_event().  However,
+			 * if the port is runtime pm suspended
+			 * (powered-off), we leave it in that state, run
+			 * an abbreviated port_event(), and move on.
+			 */
+			pm_runtime_get_noresume(&port_dev->dev);
+			pm_runtime_barrier(&port_dev->dev);
+			usb_lock_port(port_dev);
+			port_event(hub, i);
+			usb_unlock_port(port_dev);
+			pm_runtime_put_sync(&port_dev->dev);
 		}
+	}
 
-		/* deal with port status changes */
-		for (i = 1; i <= hdev->maxchild; i++) {
-			struct usb_port *port_dev = hub->ports[i - 1];
-
-			if (test_bit(i, hub->event_bits)
-					|| test_bit(i, hub->change_bits)
-					|| test_bit(i, hub->wakeup_bits)) {
-				/*
-				 * The get_noresume and barrier ensure that if
-				 * the port was in the process of resuming, we
-				 * flush that work and keep the port active for
-				 * the duration of the port_event().  However,
-				 * if the port is runtime pm suspended
-				 * (powered-off), we leave it in that state, run
-				 * an abbreviated port_event(), and move on.
-				 */
-				pm_runtime_get_noresume(&port_dev->dev);
-				pm_runtime_barrier(&port_dev->dev);
-				usb_lock_port(port_dev);
-				port_event(hub, i);
-				usb_unlock_port(port_dev);
-				pm_runtime_put_sync(&port_dev->dev);
-			}
+	/* deal with hub status changes */
+	if (test_and_clear_bit(0, hub->event_bits) == 0)
+		;	/* do nothing */
+	else if (hub_hub_status(hub, &hubstatus, &hubchange) < 0)
+		dev_err(hub_dev, "get_hub_status failed\n");
+	else {
+		if (hubchange & HUB_CHANGE_LOCAL_POWER) {
+			dev_dbg(hub_dev, "power change\n");
+			clear_hub_feature(hdev, C_HUB_LOCAL_POWER);
+			if (hubstatus & HUB_STATUS_LOCAL_POWER)
+				/* FIXME: Is this always true? */
+				hub->limited_power = 1;
+			else
+				hub->limited_power = 0;
 		}
+		if (hubchange & HUB_CHANGE_OVERCURRENT) {
+			u16 status = 0;
+			u16 unused;
 
-		/* deal with hub status changes */
-		if (test_and_clear_bit(0, hub->event_bits) == 0)
-			;	/* do nothing */
-		else if (hub_hub_status(hub, &hubstatus, &hubchange) < 0)
-			dev_err (hub_dev, "get_hub_status failed\n");
-		else {
-			if (hubchange & HUB_CHANGE_LOCAL_POWER) {
-				dev_dbg (hub_dev, "power change\n");
-				clear_hub_feature(hdev, C_HUB_LOCAL_POWER);
-				if (hubstatus & HUB_STATUS_LOCAL_POWER)
-					/* FIXME: Is this always true? */
-					hub->limited_power = 1;
-				else
-					hub->limited_power = 0;
-			}
-			if (hubchange & HUB_CHANGE_OVERCURRENT) {
-				u16 status = 0;
-				u16 unused;
-
-				dev_dbg(hub_dev, "over-current change\n");
-				clear_hub_feature(hdev, C_HUB_OVER_CURRENT);
-				msleep(500);	/* Cool down */
-				hub_power_on(hub, true);
-				hub_hub_status(hub, &status, &unused);
-				if (status & HUB_STATUS_OVERCURRENT)
-					dev_err(hub_dev, "over-current "
-						"condition\n");
-			}
+			dev_dbg(hub_dev, "over-current change\n");
+			clear_hub_feature(hdev, C_HUB_OVER_CURRENT);
+			msleep(500);	/* Cool down */
+			hub_power_on(hub, true);
+			hub_hub_status(hub, &status, &unused);
+			if (status & HUB_STATUS_OVERCURRENT)
+				dev_err(hub_dev, "over-current condition\n");
 		}
+	}
 
  out_autopm:
-		/* Balance the usb_autopm_get_interface() above */
-		usb_autopm_put_interface_no_suspend(intf);
+	/* Balance the usb_autopm_get_interface() above */
+	usb_autopm_put_interface_no_suspend(intf);
  out_hdev_lock:
-		usb_unlock_device(hdev);
-		/* Ballance the stuff in kick_hub_wq() and allow autosuspend */
-		usb_autopm_put_interface(intf);
-		kref_put(&hub->kref, hub_release);
-	}
+	usb_unlock_device(hdev);
+	/* Ballance the stuff in kick_hub_wq() and allow autosuspend */
+	usb_autopm_put_interface(intf);
+	kref_put(&hub->kref, hub_release);
 }
 
 static const struct usb_device_id hub_id_table[] = {
-- 
1.8.4

[PATCH v2 3/4] usb: hub: rename usb_kick_khubd() to usb_kick_hub_wq()

[Petr Mladek]

USB hub started to use a workqueue instead of kthread. Let's make it clear from
the function names.

Signed-off-by: Petr Mladek <pmladek@suse.cz>
---
 drivers/usb/core/hcd.c | 4 ++--
 drivers/usb/core/hub.c | 2 +-
 drivers/usb/core/usb.h | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 487abcfcccd8..4bec044a786c 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2386,7 +2386,7 @@ void usb_hc_died (struct usb_hcd *hcd)
 		/* make khubd clean up old urbs and devices */
 		usb_set_device_state (hcd->self.root_hub,
 				USB_STATE_NOTATTACHED);
-		usb_kick_khubd (hcd->self.root_hub);
+		usb_kick_hub_wq(hcd->self.root_hub);
 	}
 	if (usb_hcd_is_primary_hcd(hcd) && hcd->shared_hcd) {
 		hcd = hcd->shared_hcd;
@@ -2396,7 +2396,7 @@ void usb_hc_died (struct usb_hcd *hcd)
 			/* make khubd clean up old urbs and devices */
 			usb_set_device_state(hcd->self.root_hub,
 					USB_STATE_NOTATTACHED);
-			usb_kick_khubd(hcd->self.root_hub);
+			usb_kick_hub_wq(hcd->self.root_hub);
 		}
 	}
 	spin_unlock_irqrestore (&hcd_root_hub_lock, flags);
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index b6b526a2e20d..8d86350cb0a2 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -597,7 +597,7 @@ static void kick_hub_wq(struct usb_hub *hub)
 	kref_put(&hub->kref, hub_release);
 }
 
-void usb_kick_khubd(struct usb_device *hdev)
+void usb_kick_hub_wq(struct usb_device *hdev)
 {
 	struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
 
diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
index d9d08720c386..b1b34d0557c9 100644
--- a/drivers/usb/core/usb.h
+++ b/drivers/usb/core/usb.h
@@ -48,7 +48,7 @@ static inline unsigned usb_get_max_power(struct usb_device *udev,
 	return c->desc.bMaxPower * mul;
 }
 
-extern void usb_kick_khubd(struct usb_device *dev);
+extern void usb_kick_hub_wq(struct usb_device *dev);
 extern int usb_match_one_id_intf(struct usb_device *dev,
 				 struct usb_host_interface *intf,
 				 const struct usb_device_id *id);
-- 
1.8.4

[PATCH v2 4/4] usb: hub: rename khubd to hub_wq in documentation and comments

[Petr Mladek]

USB hub has started to use a workqueue instead of kthread. Let's update
the documentation and comments here and there.

This patch mostly just replaces "khubd" with "hub_wq". There are only few
exceptions where the whole sentence was updated. These more complicated
changes can be found in the following files:

	   Documentation/usb/hotplug.txt
	   drivers/net/usb/usbnet.c
	   drivers/usb/core/hcd.c
	   drivers/usb/host/ohci-hcd.c
	   drivers/usb/host/xhci.c

Signed-off-by: Petr Mladek <pmladek@suse.cz>
---
 Documentation/DocBook/usb.tmpl             |  2 +-
 Documentation/usb/WUSB-Design-overview.txt |  6 ++--
 Documentation/usb/hotplug.txt              |  2 +-
 drivers/net/usb/usbnet.c                   | 14 ++++++----
 drivers/usb/README                         |  2 +-
 drivers/usb/core/hcd.c                     | 10 +++----
 drivers/usb/core/hub.c                     | 44 +++++++++++++++---------------
 drivers/usb/host/ehci-fsl.c                |  2 +-
 drivers/usb/host/ehci-hcd.c                |  2 +-
 drivers/usb/host/ehci-hub.c                |  8 +++---
 drivers/usb/host/fhci-hcd.c                |  6 ++--
 drivers/usb/host/fotg210-hcd.c             |  8 +++---
 drivers/usb/host/fusbh200-hcd.c            |  8 +++---
 drivers/usb/host/isp1760-hcd.c             |  6 ++--
 drivers/usb/host/ohci-hcd.c                |  6 ++--
 drivers/usb/host/ohci-hub.c                |  4 +--
 drivers/usb/host/ohci-omap.c               |  2 +-
 drivers/usb/host/oxu210hp-hcd.c            | 10 +++----
 drivers/usb/host/sl811-hcd.c               |  8 +++---
 drivers/usb/host/xhci-hub.c                |  2 +-
 drivers/usb/host/xhci.c                    |  4 +--
 drivers/usb/misc/usbtest.c                 |  2 +-
 drivers/usb/musb/am35x.c                   |  1 +
 drivers/usb/musb/tusb6010.c                |  2 +-
 drivers/usb/phy/phy-fsl-usb.c              |  2 +-
 drivers/usb/phy/phy-isp1301-omap.c         |  2 +-
 drivers/usb/wusbcore/devconnect.c          |  6 ++--
 drivers/usb/wusbcore/wa-hc.h               |  2 +-
 sound/usb/midi.c                           |  2 +-
 29 files changed, 89 insertions(+), 86 deletions(-)

diff --git a/Documentation/DocBook/usb.tmpl b/Documentation/DocBook/usb.tmpl
index 85fc0e28576f..4cd5b2cd0f3d 100644
--- a/Documentation/DocBook/usb.tmpl
+++ b/Documentation/DocBook/usb.tmpl
@@ -593,7 +593,7 @@ for (;;) {
 	    Each device has one control endpoint (endpoint zero)
 	    which supports a limited RPC style RPC access.
 	    Devices are configured
-	    by khubd (in the kernel) setting a device-wide
+	    by hub_wq (in the kernel) setting a device-wide
 	    <emphasis>configuration</emphasis> that affects things
 	    like power consumption and basic functionality.
 	    The endpoints are part of USB <emphasis>interfaces</emphasis>,
diff --git a/Documentation/usb/WUSB-Design-overview.txt b/Documentation/usb/WUSB-Design-overview.txt
index 1cd07c017cf6..9d08f179a7ca 100644
--- a/Documentation/usb/WUSB-Design-overview.txt
+++ b/Documentation/usb/WUSB-Design-overview.txt
@@ -317,7 +317,7 @@ HC picks the /DN_Connect/ out (nep module sends to notif.c for delivery
 into /devconnect/). This process starts the authentication process for
 the device. First we allocate a /fake port/ and assign an
 unauthenticated address (128 to 255--what we really do is
-0x80 | fake_port_idx). We fiddle with the fake port status and /khubd/
+0x80 | fake_port_idx). We fiddle with the fake port status and /hub_wq/
 sees a new connection, so he moves on to enable the fake port with a reset.
 
 So now we are in the reset path -- we know we have a non-yet enumerated
@@ -326,7 +326,7 @@ device with an unauthorized address; we ask user space to authenticate
 exchange (FIXME: not yet done) and issue a /set address 0/ to bring the
 device to the default state. Device is authenticated.
 
-From here, the USB stack takes control through the usb_hcd ops. khubd
+From here, the USB stack takes control through the usb_hcd ops. hub_wq
 has seen the port status changes, as we have been toggling them. It will
 start enumerating and doing transfers through usb_hcd->urb_enqueue() to
 read descriptors and move our data.
@@ -340,7 +340,7 @@ Keep Alive IE; it responds with a /DN_Alive/ pong during the DNTS (this
 arrives to us as a notification through
 devconnect.c:wusb_handle_dn_alive(). If a device times out, we
 disconnect it from the system (cleaning up internal information and
-toggling the bits in the fake hub port, which kicks khubd into removing
+toggling the bits in the fake hub port, which kicks hub_wq into removing
 the rest of the stuff).
 
 This is done through devconnect:__wusb_check_devs(), which will scan the
diff --git a/Documentation/usb/hotplug.txt b/Documentation/usb/hotplug.txt
index a80b0e9a7a0b..5b243f315b2c 100644
--- a/Documentation/usb/hotplug.txt
+++ b/Documentation/usb/hotplug.txt
@@ -58,7 +58,7 @@ USB POLICY AGENT
 
 The USB subsystem currently invokes /sbin/hotplug when USB devices
 are added or removed from system.  The invocation is done by the kernel
-hub daemon thread [khubd], or else as part of root hub initialization
+hub workqueue [hub_wq], or else as part of root hub initialization
 (done by init, modprobe, kapmd, etc).  Its single command line parameter
 is the string "usb", and it passes these environment variables:
 
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 5173821a9575..20615bbd693b 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -69,8 +69,9 @@
 // reawaken network queue this soon after stopping; else watchdog barks
 #define TX_TIMEOUT_JIFFIES	(5*HZ)
 
-// throttle rx/tx briefly after some faults, so khubd might disconnect()
-// us (it polls at HZ/4 usually) before we report too many false errors.
+/* throttle rx/tx briefly after some faults, so hub_wq might disconnect()
+ * us (it polls at HZ/4 usually) before we report too many false errors.
+ */
 #define THROTTLE_JIFFIES	(HZ/8)
 
 // between wakeups
@@ -595,9 +596,9 @@ static void rx_complete (struct urb *urb)
 			  "rx shutdown, code %d\n", urb_status);
 		goto block;
 
-	/* we get controller i/o faults during khubd disconnect() delays.
+	/* we get controller i/o faults during hub_wq disconnect() delays.
 	 * throttle down resubmits, to avoid log floods; just temporarily,
-	 * so we still recover when the fault isn't a khubd delay.
+	 * so we still recover when the fault isn't a hub_wq delay.
 	 */
 	case -EPROTO:
 	case -ETIME:
@@ -1185,8 +1186,9 @@ static void tx_complete (struct urb *urb)
 		case -ESHUTDOWN:		// hardware gone
 			break;
 
-		// like rx, tx gets controller i/o faults during khubd delays
-		// and so it uses the same throttling mechanism.
+		/* like rx, tx gets controller i/o faults during hub_wq
+		 * delays and so it uses the same throttling mechanism.
+		 */
 		case -EPROTO:
 		case -ETIME:
 		case -EILSEQ:
diff --git a/drivers/usb/README b/drivers/usb/README
index 284f46b3e1cc..2144e7dbfa41 100644
--- a/drivers/usb/README
+++ b/drivers/usb/README
@@ -24,7 +24,7 @@ Here is a list of what each subdirectory here is, and what is contained in
 them.
 
 core/		- This is for the core USB host code, including the
-		  usbfs files and the hub class driver ("khubd").
+		  usbfs files and the hub class driver ("hub_wq").
 
 host/		- This is for USB host controller drivers.  This
 		  includes UHCI, OHCI, EHCI, and others that might
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 4bec044a786c..d2607a973a5c 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2301,7 +2301,7 @@ EXPORT_SYMBOL_GPL(usb_hcd_resume_root_hub);
  * Context: in_interrupt()
  *
  * Starts enumeration, with an immediate reset followed later by
- * khubd identifying and possibly configuring the device.
+ * hub_wq identifying and possibly configuring the device.
  * This is needed by OTG controller drivers, where it helps meet
  * HNP protocol timing requirements for starting a port reset.
  *
@@ -2320,7 +2320,7 @@ int usb_bus_start_enum(struct usb_bus *bus, unsigned port_num)
 	if (port_num && hcd->driver->start_port_reset)
 		status = hcd->driver->start_port_reset(hcd, port_num);
 
-	/* run khubd shortly after (first) root port reset finishes;
+	/* allocate hub_wq shortly after (first) root port reset finishes;
 	 * it may issue others, until at least 50 msecs have passed.
 	 */
 	if (status == 0)
@@ -2383,7 +2383,7 @@ void usb_hc_died (struct usb_hcd *hcd)
 	if (hcd->rh_registered) {
 		clear_bit(HCD_FLAG_POLL_RH, &hcd->flags);
 
-		/* make khubd clean up old urbs and devices */
+		/* make hub_wq clean up old urbs and devices */
 		usb_set_device_state (hcd->self.root_hub,
 				USB_STATE_NOTATTACHED);
 		usb_kick_hub_wq(hcd->self.root_hub);
@@ -2393,7 +2393,7 @@ void usb_hc_died (struct usb_hcd *hcd)
 		if (hcd->rh_registered) {
 			clear_bit(HCD_FLAG_POLL_RH, &hcd->flags);
 
-			/* make khubd clean up old urbs and devices */
+			/* make hub_wq clean up old urbs and devices */
 			usb_set_device_state(hcd->self.root_hub,
 					USB_STATE_NOTATTACHED);
 			usb_kick_hub_wq(hcd->self.root_hub);
@@ -2655,7 +2655,7 @@ int usb_add_hcd(struct usb_hcd *hcd,
 	set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
 
 	/* HC is in reset state, but accessible.  Now do the one-time init,
-	 * bottom up so that hcds can customize the root hubs before khubd
+	 * bottom up so that hcds can customize the root hubs before hub_wq
 	 * starts talking to them.  (Note, bus id is assigned early too.)
 	 */
 	if ((retval = hcd_buffer_create(hcd)) != 0) {
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 8d86350cb0a2..f8ba3bb3bc00 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -651,7 +651,7 @@ static void hub_irq(struct urb *urb)
 		hub->error = status;
 		/* FALL THROUGH */
 
-	/* let khubd handle things */
+	/* let hub_wq handle things */
 	case 0:			/* we got data:  port status changed */
 		bits = 0;
 		for (i = 0; i < urb->actual_length; ++i)
@@ -663,7 +663,7 @@ static void hub_irq(struct urb *urb)
 
 	hub->nerrors = 0;
 
-	/* Something happened, let khubd figure it out */
+	/* Something happened, let hub_wq figure it out */
 	kick_hub_wq(hub);
 
 resubmit:
@@ -694,7 +694,7 @@ hub_clear_tt_buffer (struct usb_device *hdev, u16 devinfo, u16 tt)
 }
 
 /*
- * enumeration blocks khubd for a long time. we use keventd instead, since
+ * enumeration blocks hub_wq for a long time. we use keventd instead, since
  * long blocking there is the exception, not the rule.  accordingly, HCDs
  * talking to TTs must queue control transfers (not just bulk and iso), so
  * both can talk to the same hub concurrently.
@@ -960,7 +960,7 @@ static int hub_port_disable(struct usb_hub *hub, int port1, int set_state)
 
 /*
  * Disable a port and mark a logical connect-change event, so that some
- * time later khubd will disconnect() any existing usb_device on the port
+ * time later hub_wq will disconnect() any existing usb_device on the port
  * and will re-enumerate if there actually is a device attached.
  */
 static void hub_port_logical_disconnect(struct usb_hub *hub, int port1)
@@ -973,7 +973,7 @@ static void hub_port_logical_disconnect(struct usb_hub *hub, int port1)
 	 *  - SRP saves power that way
 	 *  - ... new call, TBD ...
 	 * That's easy if this hub can switch power per-port, and
-	 * khubd reactivates the port later (timer, SRP, etc).
+	 * hub_wq reactivates the port later (timer, SRP, etc).
 	 * Powerdown must be optional, because of reset/DFU.
 	 */
 
@@ -986,7 +986,7 @@ static void hub_port_logical_disconnect(struct usb_hub *hub, int port1)
  * @udev: device to be disabled and removed
  * Context: @udev locked, must be able to sleep.
  *
- * After @udev's port has been disabled, khubd is notified and it will
+ * After @udev's port has been disabled, hub_wq is notified and it will
  * see that the device has been disconnected.  When the device is
  * physically unplugged and something is plugged in, the events will
  * be received and processed normally.
@@ -1106,7 +1106,7 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type)
  init2:
 
 	/*
-	 * Check each port and set hub->change_bits to let khubd know
+	 * Check each port and set hub->change_bits to let hub_wq know
 	 * which ports need attention.
 	 */
 	for (port1 = 1; port1 <= hdev->maxchild; ++port1) {
@@ -1173,7 +1173,7 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type)
 			clear_bit(port1, hub->removed_bits);
 
 		if (!udev || udev->state == USB_STATE_NOTATTACHED) {
-			/* Tell khubd to disconnect the device or
+			/* Tell hub_wq to disconnect the device or
 			 * check for a new connection
 			 */
 			if (udev || (portstatus & USB_PORT_STAT_CONNECTION) ||
@@ -1186,7 +1186,7 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type)
 				USB_SS_PORT_LS_U0;
 			/* The power session apparently survived the resume.
 			 * If there was an overcurrent or suspend change
-			 * (i.e., remote wakeup request), have khubd
+			 * (i.e., remote wakeup request), have hub_wq
 			 * take care of it.  Look at the port link state
 			 * for USB 3.0 hubs, since they don't have a suspend
 			 * change bit, and they don't set the port link change
@@ -1207,7 +1207,7 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type)
 				set_bit(port1, hub->change_bits);
 
 		} else {
-			/* The power session is gone; tell khubd */
+			/* The power session is gone; tell hub_wq */
 			usb_set_device_state(udev, USB_STATE_NOTATTACHED);
 			set_bit(port1, hub->change_bits);
 		}
@@ -1215,10 +1215,10 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type)
 
 	/* If no port-status-change flags were set, we don't need any
 	 * debouncing.  If flags were set we can try to debounce the
-	 * ports all at once right now, instead of letting khubd do them
+	 * ports all at once right now, instead of letting hub_wq do them
 	 * one at a time later on.
 	 *
-	 * If any port-status changes do occur during this delay, khubd
+	 * If any port-status changes do occur during this delay, hub_wq
 	 * will see them later and handle them normally.
 	 */
 	if (need_debounce_delay) {
@@ -1279,7 +1279,7 @@ static void hub_quiesce(struct usb_hub *hub, enum hub_quiescing_type type)
 
 	cancel_delayed_work_sync(&hub->init_work);
 
-	/* khubd and related activity won't re-trigger */
+	/* hub_wq and related activity won't re-trigger */
 	hub->quiescing = 1;
 
 	if (type != HUB_SUSPEND) {
@@ -1290,7 +1290,7 @@ static void hub_quiesce(struct usb_hub *hub, enum hub_quiescing_type type)
 		}
 	}
 
-	/* Stop khubd and related activity */
+	/* Stop hub_wq and related activity */
 	usb_kill_urb(hub->urb);
 	if (hub->has_indicators)
 		cancel_delayed_work_sync(&hub->leds);
@@ -1612,7 +1612,7 @@ static int hub_configure(struct usb_hub *hub,
 	if (ret < 0)
 		goto fail;
 
-	/* Update the HCD's internal representation of this hub before khubd
+	/* Update the HCD's internal representation of this hub before hub_wq
 	 * starts getting port status changes for devices under the hub.
 	 */
 	if (hcd->driver->update_hub_device) {
@@ -2042,7 +2042,7 @@ static void choose_devnum(struct usb_device *udev)
 	int		devnum;
 	struct usb_bus	*bus = udev->bus;
 
-	/* If khubd ever becomes multithreaded, this will need a lock */
+	/* If hub_wq ever becomes multithreaded, this will need a lock */
 	if (udev->wusb) {
 		devnum = udev->portnum + 1;
 		BUG_ON(test_bit(devnum, bus->devmap.devicemap));
@@ -3075,7 +3075,7 @@ static unsigned wakeup_enabled_descendants(struct usb_device *udev)
  * Once VBUS drop breaks the circuit, the port it's using has to go through
  * normal re-enumeration procedures, starting with enabling VBUS power.
  * Other than re-initializing the hub (plug/unplug, except for root hubs),
- * Linux (2.6) currently has NO mechanisms to initiate that:  no khubd
+ * Linux (2.6) currently has NO mechanisms to initiate that:  no hub_wq
  * timer, no SRP, no requests through sysfs.
  *
  * If Runtime PM isn't enabled or used, non-SuperSpeed devices may not get
@@ -3217,7 +3217,7 @@ static int finish_port_resume(struct usb_device *udev)
 	/* usb ch9 identifies four variants of SUSPENDED, based on what
 	 * state the device resumes to.  Linux currently won't see the
 	 * first two on the host side; they'd be inside hub_port_init()
-	 * during many timeouts, but khubd can't suspend until later.
+	 * during many timeouts, but hub_wq can't suspend until later.
 	 */
 	usb_set_device_state(udev, udev->actconfig
 			? USB_STATE_CONFIGURED
@@ -3582,7 +3582,7 @@ static int hub_suspend(struct usb_interface *intf, pm_message_t msg)
 
 	dev_dbg(&intf->dev, "%s\n", __func__);
 
-	/* stop khubd and related activity */
+	/* stop hub_wq and related activity */
 	hub_quiesce(hub, HUB_SUSPEND);
 	return 0;
 }
@@ -4975,10 +4975,10 @@ static void port_event(struct usb_hub *hub, int port1)
 	 * On disconnect USB3 protocol ports transit from U0 to
 	 * SS.Inactive to Rx.Detect. If this happens a warm-
 	 * reset is not needed, but a (re)connect may happen
-	 * before khubd runs and sees the disconnect, and the
+	 * before hub_wq runs and sees the disconnect, and the
 	 * device may be an unknown state.
 	 *
-	 * If the port went through SS.Inactive without khubd
+	 * If the port went through SS.Inactive without hub_wq
 	 * seeing it the C_LINK_STATE change flag will be set,
 	 * and we reset the dev to put it in a known state.
 	 */
@@ -5283,7 +5283,7 @@ static int descriptors_changed(struct usb_device *udev,
  * former operating configuration.  If the reset fails, or the device's
  * descriptors change from their values before the reset, or the original
  * configuration and altsettings cannot be restored, a flag will be set
- * telling khubd to pretend the device has been disconnected and then
+ * telling hub_wq to pretend the device has been disconnected and then
  * re-connected.  All drivers will be unbound, and the device will be
  * re-enumerated and probed all over again.
  *
diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c
index cf2734b532a7..3d84b6a41dae 100644
--- a/drivers/usb/host/ehci-fsl.c
+++ b/drivers/usb/host/ehci-fsl.c
@@ -627,7 +627,7 @@ static int ehci_start_port_reset(struct usb_hcd *hcd, unsigned port)
 	if (!(status & PORT_CONNECT))
 		return -ENODEV;
 
-	/* khubd will finish the reset later */
+	/* hub_wq will finish the reset later */
 	if (ehci_is_TDI(ehci)) {
 		writel(PORT_RESET |
 		       (status & ~(PORT_CSC | PORT_PEC | PORT_OCC)),
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 81cda09b47e3..0ed9b1d5921f 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -788,7 +788,7 @@ static irqreturn_t ehci_irq (struct usb_hcd *hcd)
 				continue;
 
 			/* start 20 msec resume signaling from this port,
-			 * and make khubd collect PORT_STAT_C_SUSPEND to
+			 * and make hub_wq collect PORT_STAT_C_SUSPEND to
 			 * stop that signaling.  Use 5 ms extra for safety,
 			 * like usb_port_resume() does.
 			 */
diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c
index 6130b7574908..7ccb3ccf8e86 100644
--- a/drivers/usb/host/ehci-hub.c
+++ b/drivers/usb/host/ehci-hub.c
@@ -656,7 +656,7 @@ ehci_hub_status_data (struct usb_hcd *hcd, char *buf)
 
 		/*
 		 * Return status information even for ports with OWNER set.
-		 * Otherwise khubd wouldn't see the disconnect event when a
+		 * Otherwise hub_wq wouldn't see the disconnect event when a
 		 * high-speed device is switched over to the companion
 		 * controller by the user.
 		 */
@@ -902,7 +902,7 @@ int ehci_hub_control(
 
 		/*
 		 * Even if OWNER is set, so the port is owned by the
-		 * companion controller, khubd needs to be able to clear
+		 * companion controller, hub_wq needs to be able to clear
 		 * the port-change status bits (especially
 		 * USB_PORT_STAT_C_CONNECTION).
 		 */
@@ -1000,7 +1000,7 @@ int ehci_hub_control(
 			 * However, not all EHCI implementations do this
 			 * automatically, even if they _do_ support per-port
 			 * power switching; they're allowed to just limit the
-			 * current.  khubd will turn the power back on.
+			 * current.  hub_wq will turn the power back on.
 			 */
 			if (((temp & PORT_OC) || (ehci->need_oc_pp_cycle))
 					&& HCS_PPC(ehci->hcs_params)) {
@@ -1085,7 +1085,7 @@ int ehci_hub_control(
 		}
 
 		/*
-		 * Even if OWNER is set, there's no harm letting khubd
+		 * Even if OWNER is set, there's no harm letting hub_wq
 		 * see the wPortStatus values (they should all be 0 except
 		 * for PORT_POWER anyway).
 		 */
diff --git a/drivers/usb/host/fhci-hcd.c b/drivers/usb/host/fhci-hcd.c
index 1cf68eaf2ed8..a1a1ef521436 100644
--- a/drivers/usb/host/fhci-hcd.c
+++ b/drivers/usb/host/fhci-hcd.c
@@ -360,12 +360,12 @@ static int fhci_start(struct usb_hcd *hcd)
 	hcd->state = HC_STATE_RUNNING;
 
 	/*
-	 * From here on, khubd concurrently accesses the root
+	 * From here on, hub_wq concurrently accesses the root
 	 * hub; drivers will be talking to enumerated devices.
-	 * (On restart paths, khubd already knows about the root
+	 * (On restart paths, hub_wq already knows about the root
 	 * hub and could find work as soon as we wrote FLAG_CF.)
 	 *
-	 * Before this point the HC was idle/ready.  After, khubd
+	 * Before this point the HC was idle/ready.  After, hub_wq
 	 * and device drivers may start it running.
 	 */
 	fhci_usb_enable(fhci);
diff --git a/drivers/usb/host/fotg210-hcd.c b/drivers/usb/host/fotg210-hcd.c
index adcd2050dced..3de1278677d0 100644
--- a/drivers/usb/host/fotg210-hcd.c
+++ b/drivers/usb/host/fotg210-hcd.c
@@ -1483,7 +1483,7 @@ fotg210_hub_status_data(struct usb_hcd *hcd, char *buf)
 
 	/*
 	 * Return status information even for ports with OWNER set.
-	 * Otherwise khubd wouldn't see the disconnect event when a
+	 * Otherwise hub_wq wouldn't see the disconnect event when a
 	 * high-speed device is switched over to the companion
 	 * controller by the user.
 	 */
@@ -1572,7 +1572,7 @@ static int fotg210_hub_control(
 
 		/*
 		 * Even if OWNER is set, so the port is owned by the
-		 * companion controller, khubd needs to be able to clear
+		 * companion controller, hub_wq needs to be able to clear
 		 * the port-change status bits (especially
 		 * USB_PORT_STAT_C_CONNECTION).
 		 */
@@ -1723,7 +1723,7 @@ static int fotg210_hub_control(
 		}
 
 		/*
-		 * Even if OWNER is set, there's no harm letting khubd
+		 * Even if OWNER is set, there's no harm letting hub_wq
 		 * see the wPortStatus values (they should all be 0 except
 		 * for PORT_POWER anyway).
 		 */
@@ -5445,7 +5445,7 @@ static irqreturn_t fotg210_irq(struct usb_hcd *hcd)
 				fotg210->reset_done[0] == 0) {
 
 			/* start 20 msec resume signaling from this port,
-			 * and make khubd collect PORT_STAT_C_SUSPEND to
+			 * and make hub_wq collect PORT_STAT_C_SUSPEND to
 			 * stop that signaling.  Use 5 ms extra for safety,
 			 * like usb_port_resume() does.
 			 */
diff --git a/drivers/usb/host/fusbh200-hcd.c b/drivers/usb/host/fusbh200-hcd.c
index ba9499060f63..abe42f31559f 100644
--- a/drivers/usb/host/fusbh200-hcd.c
+++ b/drivers/usb/host/fusbh200-hcd.c
@@ -1441,7 +1441,7 @@ fusbh200_hub_status_data (struct usb_hcd *hcd, char *buf)
 
 	/*
 	 * Return status information even for ports with OWNER set.
-	 * Otherwise khubd wouldn't see the disconnect event when a
+	 * Otherwise hub_wq wouldn't see the disconnect event when a
 	 * high-speed device is switched over to the companion
 	 * controller by the user.
 	 */
@@ -1530,7 +1530,7 @@ static int fusbh200_hub_control (
 
 		/*
 		 * Even if OWNER is set, so the port is owned by the
-		 * companion controller, khubd needs to be able to clear
+		 * companion controller, hub_wq needs to be able to clear
 		 * the port-change status bits (especially
 		 * USB_PORT_STAT_C_CONNECTION).
 		 */
@@ -1678,7 +1678,7 @@ static int fusbh200_hub_control (
 		}
 
 		/*
-		 * Even if OWNER is set, there's no harm letting khubd
+		 * Even if OWNER is set, there's no harm letting hub_wq
 		 * see the wPortStatus values (they should all be 0 except
 		 * for PORT_POWER anyway).
 		 */
@@ -5355,7 +5355,7 @@ static irqreturn_t fusbh200_irq (struct usb_hcd *hcd)
 				fusbh200->reset_done[0] == 0) {
 
 			/* start 20 msec resume signaling from this port,
-			 * and make khubd collect PORT_STAT_C_SUSPEND to
+			 * and make hub_wq collect PORT_STAT_C_SUSPEND to
 			 * stop that signaling.  Use 5 ms extra for safety,
 			 * like usb_port_resume() does.
 			 */
diff --git a/drivers/usb/host/isp1760-hcd.c b/drivers/usb/host/isp1760-hcd.c
index 51a0ae9cdd1d..e752c3098f38 100644
--- a/drivers/usb/host/isp1760-hcd.c
+++ b/drivers/usb/host/isp1760-hcd.c
@@ -1760,7 +1760,7 @@ static int isp1760_hub_status_data(struct usb_hcd *hcd, char *buf)
 
 	/*
 	 * Return status information even for ports with OWNER set.
-	 * Otherwise khubd wouldn't see the disconnect event when a
+	 * Otherwise hub_wq wouldn't see the disconnect event when a
 	 * high-speed device is switched over to the companion
 	 * controller by the user.
 	 */
@@ -1871,7 +1871,7 @@ static int isp1760_hub_control(struct usb_hcd *hcd, u16 typeReq,
 
 		/*
 		 * Even if OWNER is set, so the port is owned by the
-		 * companion controller, khubd needs to be able to clear
+		 * companion controller, hub_wq needs to be able to clear
 		 * the port-change status bits (especially
 		 * USB_PORT_STAT_C_CONNECTION).
 		 */
@@ -2000,7 +2000,7 @@ static int isp1760_hub_control(struct usb_hcd *hcd, u16 typeReq,
 					reg_read32(hcd->regs, HC_PORTSC1));
 		}
 		/*
-		 * Even if OWNER is set, there's no harm letting khubd
+		 * Even if OWNER is set, there's no harm letting hub_wq
 		 * see the wPortStatus values (they should all be 0 except
 		 * for PORT_POWER anyway).
 		 */
diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
index 46987735a2e3..d664edabf14e 100644
--- a/drivers/usb/host/ohci-hcd.c
+++ b/drivers/usb/host/ohci-hcd.c
@@ -632,7 +632,7 @@ retry:
 		return -EOVERFLOW;
 	}
 
-	/* use rhsc irqs after khubd is fully initialized */
+	/* use rhsc irqs after hub_wq is allocated */
 	set_bit(HCD_FLAG_POLL_RH, &hcd->flags);
 	hcd->uses_new_polling = 1;
 
@@ -909,8 +909,8 @@ static irqreturn_t ohci_irq (struct usb_hcd *hcd)
 		 * choices for RHSC.  Many followed the spec; RHSC triggers
 		 * on an edge, like setting and maybe clearing a port status
 		 * change bit.  With others it's level-triggered, active
-		 * until khubd clears all the port status change bits.  We'll
-		 * always disable it here and rely on polling until khubd
+		 * until hub_wq clears all the port status change bits.  We'll
+		 * always disable it here and rely on polling until hub_wq
 		 * re-enables it.
 		 */
 		ohci_writel(ohci, OHCI_INTR_RHSC, &regs->intrdisable);
diff --git a/drivers/usb/host/ohci-hub.c b/drivers/usb/host/ohci-hub.c
index 17d32b0ea565..0aa17c937115 100644
--- a/drivers/usb/host/ohci-hub.c
+++ b/drivers/usb/host/ohci-hub.c
@@ -585,7 +585,7 @@ static int ohci_start_port_reset (struct usb_hcd *hcd, unsigned port)
 	if (!(status & RH_PS_CCS))
 		return -ENODEV;
 
-	/* khubd will finish the reset later */
+	/* hub_wq will finish the reset later */
 	ohci_writel(ohci, RH_PS_PRS, &ohci->regs->roothub.portstatus [port]);
 	return 0;
 }
@@ -610,7 +610,7 @@ static int ohci_start_port_reset (struct usb_hcd *hcd, unsigned port)
 /* wrap-aware logic morphed from <linux/jiffies.h> */
 #define tick_before(t1,t2) ((s16)(((s16)(t1))-((s16)(t2))) < 0)
 
-/* called from some task, normally khubd */
+/* called from some task, normally hub_wq */
 static inline int root_port_reset (struct ohci_hcd *ohci, unsigned port)
 {
 	__hc32 __iomem *portstat = &ohci->regs->roothub.portstatus [port];
diff --git a/drivers/usb/host/ohci-omap.c b/drivers/usb/host/ohci-omap.c
index c923cafcaca7..de9428362503 100644
--- a/drivers/usb/host/ohci-omap.c
+++ b/drivers/usb/host/ohci-omap.c
@@ -283,7 +283,7 @@ static int ohci_omap_reset(struct usb_hcd *hcd)
 		ohci_to_hcd(ohci)->power_budget = 0;
 	}
 
-	/* FIXME khubd hub requests should manage power switching */
+	/* FIXME hub_wq hub requests should manage power switching */
 	omap_ohci_transceiver_power(1);
 
 	/* board init will have already handled HMC and mux setup.
diff --git a/drivers/usb/host/oxu210hp-hcd.c b/drivers/usb/host/oxu210hp-hcd.c
index da5fb0e3c363..4fe79a2d71a9 100644
--- a/drivers/usb/host/oxu210hp-hcd.c
+++ b/drivers/usb/host/oxu210hp-hcd.c
@@ -2046,7 +2046,7 @@ static void intr_deschedule(struct oxu_hcd *oxu, struct ehci_qh *qh)
 
 	/* simple/paranoid:  always delay, expecting the HC needs to read
 	 * qh->hw_next or finish a writeback after SPLIT/CSPLIT ... and
-	 * expect khubd to clean up after any CSPLITs we won't issue.
+	 * expect hub_wq to clean up after any CSPLITs we won't issue.
 	 * active high speed queues may need bigger delays...
 	 */
 	if (list_empty(&qh->qtd_list)
@@ -2501,7 +2501,7 @@ static irqreturn_t oxu210_hcd_irq(struct usb_hcd *hcd)
 				continue;
 
 			/* start 20 msec resume signaling from this port,
-			 * and make khubd collect PORT_STAT_C_SUSPEND to
+			 * and make hub_wq collect PORT_STAT_C_SUSPEND to
 			 * stop that signaling.
 			 */
 			oxu->reset_done[i] = jiffies + msecs_to_jiffies(20);
@@ -3119,7 +3119,7 @@ static int oxu_hub_status_data(struct usb_hcd *hcd, char *buf)
 
 		/*
 		 * Return status information even for ports with OWNER set.
-		 * Otherwise khubd wouldn't see the disconnect event when a
+		 * Otherwise hub_wq wouldn't see the disconnect event when a
 		 * high-speed device is switched over to the companion
 		 * controller by the user.
 		 */
@@ -3194,7 +3194,7 @@ static int oxu_hub_control(struct usb_hcd *hcd, u16 typeReq,
 
 		/*
 		 * Even if OWNER is set, so the port is owned by the
-		 * companion controller, khubd needs to be able to clear
+		 * companion controller, hub_wq needs to be able to clear
 		 * the port-change status bits (especially
 		 * USB_PORT_STAT_C_CONNECTION).
 		 */
@@ -3336,7 +3336,7 @@ static int oxu_hub_control(struct usb_hcd *hcd, u16 typeReq,
 		}
 
 		/*
-		 * Even if OWNER is set, there's no harm letting khubd
+		 * Even if OWNER is set, there's no harm letting hub_wq
 		 * see the wPortStatus values (they should all be 0 except
 		 * for PORT_POWER anyway).
 		 */
diff --git a/drivers/usb/host/sl811-hcd.c b/drivers/usb/host/sl811-hcd.c
index a517151867af..ad0c348e68e9 100644
--- a/drivers/usb/host/sl811-hcd.c
+++ b/drivers/usb/host/sl811-hcd.c
@@ -674,7 +674,7 @@ retry:
 			sl811->next_periodic = sl811->periodic[index];
 	}
 
-	/* khubd manages debouncing and wakeup */
+	/* hub_wq manages debouncing and wakeup */
 	if (irqstat & SL11H_INTMASK_INSRMV) {
 		sl811->stat_insrmv++;
 
@@ -714,7 +714,7 @@ retry:
 #endif
 
 		/* port status seems weird until after reset, so
-		 * force the reset and make khubd clean up later.
+		 * force the reset and make hub_wq clean up later.
 		 */
 		if (irqstat & SL11H_INTMASK_RD)
 			sl811->port1 &= ~USB_PORT_STAT_CONNECTION;
@@ -1079,7 +1079,7 @@ sl811h_hub_status_data(struct usb_hcd *hcd, char *buf)
 	if (!(sl811->port1 & (0xffff << 16)))
 		return 0;
 
-	/* tell khubd port 1 changed */
+	/* tell hub_wq port 1 changed */
 	*buf = (1 << 1);
 	return 1;
 }
@@ -1196,7 +1196,7 @@ sl811h_timer(unsigned long _sl811)
 		sl811_write(sl811, SL811_EP_A(SL11H_HOSTCTLREG),
 				SL11H_HCTLMASK_ARM);
 
-		/* khubd provides debounce delay */
+		/* hub_wq provides debounce delay */
 	} else {
 		sl811->ctrl1 = 0;
 	}
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 69aece31143a..0cf49ffa902e 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -892,7 +892,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 			/*
 			 * Turn on ports, even if there isn't per-port switching.
 			 * HC will report connect events even before this is set.
-			 * However, khubd will ignore the roothub events until
+			 * However, hub_wq will ignore the roothub events until
 			 * the roothub is registered.
 			 */
 			writel(temp | PORT_POWER, port_array[wIndex]);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index c4a8fca8ae93..5dd120280818 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3761,8 +3761,8 @@ disable_slot:
 /*
  * Issue an Address Device command and optionally send a corresponding
  * SetAddress request to the device.
- * We should be protected by the usb_address0_mutex in khubd's hub_port_init, so
- * we should only issue and wait on one address command at the same time.
+ * We should be protected by the usb_address0_mutex in hub_wq's hub_port_init,
+ * so we should only issue and wait on one address command at the same time.
  */
 static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev,
 			     enum xhci_setup_dev setup)
diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
index 829f446064ea..b43337a8c24e 100644
--- a/drivers/usb/misc/usbtest.c
+++ b/drivers/usb/misc/usbtest.c
@@ -1989,7 +1989,7 @@ static int test_unaligned_bulk(
  *
  * WARNING:  Because usbfs grabs udev->dev.sem before calling this ioctl(),
  * it locks out usbcore in certain code paths.  Notably, if you disconnect
- * the device-under-test, khubd will wait block forever waiting for the
+ * the device-under-test, hub_wq will wait block forever waiting for the
  * ioctl to complete ... so that usb_disconnect() can abort the pending
  * urbs and then call usbtest_disconnect().  To abort a test, you're best
  * off just killing the userspace task and waiting for it to exit.
diff --git a/drivers/usb/musb/am35x.c b/drivers/usb/musb/am35x.c
index 0a34dd859555..a2735df24cc6 100644
--- a/drivers/usb/musb/am35x.c
+++ b/drivers/usb/musb/am35x.c
@@ -1,3 +1,4 @@
+
 /*
  * Texas Instruments AM35x "glue layer"
  *
diff --git a/drivers/usb/musb/tusb6010.c b/drivers/usb/musb/tusb6010.c
index 7dfc6cb732c9..2daa779f1382 100644
--- a/drivers/usb/musb/tusb6010.c
+++ b/drivers/usb/musb/tusb6010.c
@@ -433,7 +433,7 @@ static void musb_do_idle(unsigned long _musb)
 	if (!musb->is_active) {
 		u32	wakeups;
 
-		/* wait until khubd handles port change status */
+		/* wait until hub_wq handles port change status */
 		if (is_host_active(musb) && (musb->port1_status >> 16))
 			goto done;
 
diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c
index 2b0f968d9325..f1ea5990a50a 100644
--- a/drivers/usb/phy/phy-fsl-usb.c
+++ b/drivers/usb/phy/phy-fsl-usb.c
@@ -609,7 +609,7 @@ static int fsl_otg_set_host(struct usb_otg *otg, struct usb_bus *host)
 		otg->host->otg_port = fsl_otg_initdata.otg_port;
 		otg->host->is_b_host = otg_dev->fsm.id;
 		/*
-		 * must leave time for khubd to finish its thing
+		 * must leave time for hub_wq to finish its thing
 		 * before yanking the host driver out from under it,
 		 * so suspend the host after a short delay.
 		 */
diff --git a/drivers/usb/phy/phy-isp1301-omap.c b/drivers/usb/phy/phy-isp1301-omap.c
index 69e49be8866b..8eea56d3ded6 100644
--- a/drivers/usb/phy/phy-isp1301-omap.c
+++ b/drivers/usb/phy/phy-isp1301-omap.c
@@ -1011,7 +1011,7 @@ static void isp_update_otg(struct isp1301 *isp, u8 stat)
 				break;
 			case OTG_STATE_A_WAIT_VFALL:
 				state = OTG_STATE_A_IDLE;
-				/* khubd may take a while to notice and
+				/* hub_wq may take a while to notice and
 				 * handle this disconnect, so don't go
 				 * to B_IDLE quite yet.
 				 */
diff --git a/drivers/usb/wusbcore/devconnect.c b/drivers/usb/wusbcore/devconnect.c
index 0677139c6065..3f4f5fbded55 100644
--- a/drivers/usb/wusbcore/devconnect.c
+++ b/drivers/usb/wusbcore/devconnect.c
@@ -329,7 +329,7 @@ void wusbhc_devconnect_ack(struct wusbhc *wusbhc, struct wusb_dn_connect *dnc,
 	port->wusb_dev = wusb_dev;
 	port->status |= USB_PORT_STAT_CONNECTION;
 	port->change |= USB_PORT_STAT_C_CONNECTION;
-	/* Now the port status changed to connected; khubd will
+	/* Now the port status changed to connected; hub_wq will
 	 * pick the change up and try to reset the port to bring it to
 	 * the enabled state--so this process returns up to the stack
 	 * and it calls back into wusbhc_rh_port_reset().
@@ -343,7 +343,7 @@ error_unlock:
 /*
  * Disconnect a Wireless USB device from its fake port
  *
- * Marks the port as disconnected so that khubd can pick up the change
+ * Marks the port as disconnected so that hub_wq can pick up the change
  * and drops our knowledge about the device.
  *
  * Assumes there is a device connected
@@ -379,7 +379,7 @@ static void __wusbhc_dev_disconnect(struct wusbhc *wusbhc,
 		wusbhc_gtk_rekey(wusbhc);
 
 	/* The Wireless USB part has forgotten about the device already; now
-	 * khubd's timer will pick up the disconnection and remove the USB
+	 * hub_wq's timer will pick up the disconnection and remove the USB
 	 * device from the system
 	 */
 }
diff --git a/drivers/usb/wusbcore/wa-hc.h b/drivers/usb/wusbcore/wa-hc.h
index f2a8d29e17b9..edc7267157f3 100644
--- a/drivers/usb/wusbcore/wa-hc.h
+++ b/drivers/usb/wusbcore/wa-hc.h
@@ -64,7 +64,7 @@
  *
  * Note much of the activity is difficult to follow. For example a
  * device connect goes to devconnect, which will cause the "fake" root
- * hub port to show a connect and stop there. Then khubd will notice
+ * hub port to show a connect and stop there. Then hub_wq will notice
  * and call into the rh.c:hwahc_rc_port_reset() code to authenticate
  * the device (and this might require user intervention) and enable
  * the port.
diff --git a/sound/usb/midi.c b/sound/usb/midi.c
index 7b166c2be0f7..b2b6f398a4e1 100644
--- a/sound/usb/midi.c
+++ b/sound/usb/midi.c
@@ -64,7 +64,7 @@
 /* #define DUMP_PACKETS */
 
 /*
- * how long to wait after some USB errors, so that khubd can disconnect() us
+ * how long to wait after some USB errors, so that hub_wq can disconnect() us
  * without too many spurious errors
  */
 #define ERROR_DELAY_JIFFIES (HZ / 10)
-- 
1.8.4

Re: [PATCH v2 1/4] usb: hub: convert khubd into workqueue

[Alan Stern]

On Wed, 17 Sep 2014, Petr Mladek wrote:

> There is no need to have separate kthread for handling USB hub events.
> It is more elegant to use the workqueue framework.
> 
> The workqueue is allocated as freezable because the original thread was
> freezable as well.

I've got a few comments about style.

> -static void kick_khubd(struct usb_hub *hub)
> +static void kick_hub_wq(struct usb_hub *hub)
>  {
> -	unsigned long	flags;
> +	struct usb_interface *intf;
>  
> -	spin_lock_irqsave(&hub_event_lock, flags);
> -	if (!hub->disconnected && list_empty(&hub->event_list)) {
> -		list_add_tail(&hub->event_list, &hub_event_list);
> +	if (hub->disconnected || work_pending(&hub->events))
> +		return;
>  
> -		/* Suppress autosuspend until khubd runs */
> -		usb_autopm_get_interface_no_resume(
> -				to_usb_interface(hub->intfdev));
> -		wake_up(&khubd_wait);
> -	}
> -	spin_unlock_irqrestore(&hub_event_lock, flags);
> +	intf = to_usb_interface(hub->intfdev);
> +	/*
> +	 * Suppress autosuspend until the event is proceed.
> +	 *
> +	 * Be careful and make sure that the symmetric operation is
> +	 * always called. We are here only when there is no pending
> +	 * work for this hub. Therefore put the interface either when
> +	 * the new work is called or when it is canceled.
> +	 */
> +	kref_get(&hub->kref);
> +	usb_autopm_get_interface_no_resume(intf);

This looks a little weird.  The assignment to intf and the call to
usb_autopm_get_interface_no_resume naturally go together.  So why
put the big comment and the kref_get call in between them?  Put the
comment first, then the assigment and the autopm call, and then the
kref_get.

> @@ -5130,40 +5115,15 @@ static void hub_events(void)
>  			}
>  		}
>  
> - loop_autopm:
> + out_autopm:
>  		/* Balance the usb_autopm_get_interface() above */
>  		usb_autopm_put_interface_no_suspend(intf);
> - loop:
> -		/* Balance the usb_autopm_get_interface_no_resume() in
> -		 * kick_khubd() and allow autosuspend.
> -		 */
> -		usb_autopm_put_interface(intf);
> - loop_disconnected:
> + out_hdev_lock:
>  		usb_unlock_device(hdev);
> -		usb_put_dev(hdev);
> +		/* Ballance the stuff in kick_hub_wq() and allow autosuspend */
> +		usb_autopm_put_interface(intf);

You misspelled "Balance".  Also, there should be a blank line before
this comment.

> @@ -5203,21 +5163,26 @@ int usb_hub_init(void)
>  		return -1;
>  	}
>  
> -	khubd_task = kthread_run(hub_thread, NULL, "khubd");
> -	if (!IS_ERR(khubd_task))
> +	/*
> +	 * The workqueue 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.
> +	 */
> +	hub_wq = alloc_workqueue("hub", WQ_FREEZABLE, 0);

Is "hub" really a good name for the workqueue?  If somebody reads it,
will they know what sort of hub it refers to?  Would "usb_hub" or even
"usb_hub_wq" be better?

>  void usb_hub_cleanup(void)
>  {
> -       kthread_stop(khubd_task);
> -
> +       destroy_workqueue(hub_wq);
>         /*

Don't get rid of this blank line.

More to come...

Alan Stern

Re: [PATCH v2 2/4] usb: hub: remove obsolete while cycle in hub_event()

[Alan Stern]

On Wed, 17 Sep 2014, Petr Mladek wrote:

> The USB hub events are proceed by workqueue instead of kthread now.
> The result is that hub_event() function processes only one event.
> The block from the while cycle was not removed earlier to show the real
>  changes when switching to the workqueue.
> 
> When touching the code, it fixes also formatting of dev_err() and dev_dbg()
> calls to make checkpatch.pl happy :-)

More sylistic comments...

> +	hub = container_of(work, struct usb_hub, events);
> +	hdev = hub->hdev;
> +	hub_dev = hub->intfdev;
> +	intf = to_usb_interface(hub_dev);
> +
> +	dev_dbg(hub_dev, "state %d ports %d chg %04x evt %04x\n",
> +		hdev->state, hdev->maxchild,
> +		/* NOTE: expects max 15 ports... */
> +		(u16) hub->change_bits[0],
> +		(u16) hub->event_bits[0]);

In this file, continuation lines are indented by 2 tab stops beyond the 
starting line, not one.

> +		if (test_bit(i, hub->event_bits)
> +		    || test_bit(i, hub->change_bits)
> +		    || test_bit(i, hub->wakeup_bits)) {

And not four spaces.

Alan Stern

Re: [PATCH v2 4/4] usb: hub: rename khubd to hub_wq in documentation and comments

[Alan Stern]

On Wed, 17 Sep 2014, Petr Mladek wrote:

> USB hub has started to use a workqueue instead of kthread. Let's update
> the documentation and comments here and there.
> 
> This patch mostly just replaces "khubd" with "hub_wq". There are only few
> exceptions where the whole sentence was updated. These more complicated
> changes can be found in the following files:
> 
> 	   Documentation/usb/hotplug.txt
> 	   drivers/net/usb/usbnet.c
> 	   drivers/usb/core/hcd.c
> 	   drivers/usb/host/ohci-hcd.c
> 	   drivers/usb/host/xhci.c

Okay, here's a real issue.

> @@ -2042,7 +2042,7 @@ static void choose_devnum(struct usb_device *udev)
>  	int		devnum;
>  	struct usb_bus	*bus = udev->bus;
>  
> -	/* If khubd ever becomes multithreaded, this will need a lock */
> +	/* If hub_wq ever becomes multithreaded, this will need a lock */
>  	if (udev->wusb) {
>  		devnum = udev->portnum + 1;
>  		BUG_ON(test_bit(devnum, bus->devmap.devicemap));

You probably didn't notice when changing this comment.  But in fact,
workqueues _are_ multithreaded.  Therefore you need to add a lock to 
this routine.

Still, apart from these relatively minor issues, the series looks good.

Alan Stern

Re: [PATCH v2 4/4] usb: hub: rename khubd to hub_wq in documentation and comments

[Tejun Heo]

Hello, Alan, Petr.

On Wed, Sep 17, 2014 at 01:36:26PM -0400, Alan Stern wrote:
> > -	/* If khubd ever becomes multithreaded, this will need a lock */
> > +	/* If hub_wq ever becomes multithreaded, this will need a lock */
> >  	if (udev->wusb) {
> >  		devnum = udev->portnum + 1;
> >  		BUG_ON(test_bit(devnum, bus->devmap.devicemap));
> 
> You probably didn't notice when changing this comment.  But in fact,
> workqueues _are_ multithreaded.  Therefore you need to add a lock to 
> this routine.

Haven't read the code but if this function is called from a single
work_struct, workqueue guarantees that there's only single thread of
execution at any given time.  A work item is never executed
concurrently no matter what.

Thanks.

-- 
tejun

Re: [PATCH v2 4/4] usb: hub: rename khubd to hub_wq in documentation and comments

[Alan Stern]

On Thu, 18 Sep 2014, Tejun Heo wrote:

> Hello, Alan, Petr.
> 
> On Wed, Sep 17, 2014 at 01:36:26PM -0400, Alan Stern wrote:
> > > -	/* If khubd ever becomes multithreaded, this will need a lock */
> > > +	/* If hub_wq ever becomes multithreaded, this will need a lock */
> > >  	if (udev->wusb) {
> > >  		devnum = udev->portnum + 1;
> > >  		BUG_ON(test_bit(devnum, bus->devmap.devicemap));
> > 
> > You probably didn't notice when changing this comment.  But in fact,
> > workqueues _are_ multithreaded.  Therefore you need to add a lock to 
> > this routine.
> 
> Haven't read the code but if this function is called from a single
> work_struct, workqueue guarantees that there's only single thread of
> execution at any given time.  A work item is never executed
> concurrently no matter what.

This routine can be called from multiple work_structs, because a USB 
bus can have multiple hubs.

Alan Stern

Re: [PATCH v2 4/4] usb: hub: rename khubd to hub_wq in documentation and comments

[Petr Mládek]

On Thu 18-09-14 10:24:23, Alan Stern wrote:
> On Thu, 18 Sep 2014, Tejun Heo wrote:
> 
> > Hello, Alan, Petr.
> > 
> > On Wed, Sep 17, 2014 at 01:36:26PM -0400, Alan Stern wrote:
> > > > -	/* If khubd ever becomes multithreaded, this will need a lock */
> > > > +	/* If hub_wq ever becomes multithreaded, this will need a lock */
> > > >  	if (udev->wusb) {
> > > >  		devnum = udev->portnum + 1;
> > > >  		BUG_ON(test_bit(devnum, bus->devmap.devicemap));
> > > 
> > > You probably didn't notice when changing this comment.  But in fact,
> > > workqueues _are_ multithreaded.  Therefore you need to add a lock to 
> > > this routine.
> 
> > Haven't read the code but if this function is called from a single
> > work_struct, workqueue guarantees that there's only single thread of
> > execution at any given time.  A work item is never executed
> > concurrently no matter what.
> 
> This routine can be called from multiple work_structs, because a USB 
> bus can have multiple hubs.

The easiest solution would be to allocate the work queue with
the flag WQ_UNBOUND and max_active = 1. It will force serialization
of all work items.

Alternatively, we might add the locking. But to be honest, I am not
sure that I am brave enough to do so.


First, I am not sure if this is the only location that might be
affected by the parallel execution of hub_event().

Second, there are used so many locks and the code is so complex that I
would need many days and maybe weeks to understand it.


Well, if we assume that this is the only problematic location, here
are the ideas how to prevent the parallel execution:

1. Use some brand new lock, e.g. call it hub_devnum_lock, and do:

   static void choose_devnum(struct usb_device *udev)
   {
	spin_lock_irq(&hub_devnum_lock);
	[...]
	spin_unlock_irq(&hub_event_lock);
   }

   This looks clean but it creates another lock.


2. Alternatively, we could use an existing global lock the same way,
   for example usb_bus_list_lock.

   But this looks like a hack and I do not like it much.


3. Alternatively, it seems the the function affects one
   "struct usb_device" and one "struct usb_bus". It might
   be enough to take the appropriate locks for these
   structures.

   This would mean to take two locks. It would be slower
   and we would need to make sure that it does not cause
   a dead lock.


Best Regards,
Petr

Re: [PATCH v2 4/4] usb: hub: rename khubd to hub_wq in documentation and comments

[Tejun Heo]

On Thu, Sep 18, 2014 at 06:15:02PM +0200, Petr Mládek wrote:
> The easiest solution would be to allocate the work queue with
> the flag WQ_UNBOUND and max_active = 1. It will force serialization
> of all work items.

Please use alloc_ordered_workqueue() for that purpose.  WQ_UNBOUND +
max_active == 1 no longer guarantees single thread of execution.

Thanks.

-- 
tejun

Re: [PATCH v2 4/4] usb: hub: rename khubd to hub_wq in documentation and comments

[Alan Stern]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: TEXT/PLAIN; charset=UTF-8, Size: 2599 bytes --]

On Thu, 18 Sep 2014, Petr [iso-8859-1] Mládek wrote:

> > This routine can be called from multiple work_structs, because a USB 
> > bus can have multiple hubs.
> 
> The easiest solution would be to allocate the work queue with
> the flag WQ_UNBOUND and max_active = 1. It will force serialization
> of all work items.
> 
> Alternatively, we might add the locking. But to be honest, I am not
> sure that I am brave enough to do so.
> 
> 
> First, I am not sure if this is the only location that might be
> affected by the parallel execution of hub_event().
> 
> Second, there are used so many locks and the code is so complex that I
> would need many days and maybe weeks to understand it.

In the past I have considered making khubd multithreaded.  But it
didn't seem especially important.  Apart from initial device discovery
while the system is starting up (which works out well enough now, even  
if it could be faster) there usually is activity on only one USB hub
and port at a time.

On the whole, I would be happy if we can simply guarantee that
max_active never gets higher than 1.  As Tejun recommended,
alloc_ordered_workqueue should be enough.

> Well, if we assume that this is the only problematic location, here
> are the ideas how to prevent the parallel execution:
> 
> 1. Use some brand new lock, e.g. call it hub_devnum_lock, and do:
> 
>    static void choose_devnum(struct usb_device *udev)
>    {
> 	spin_lock_irq(&hub_devnum_lock);
> 	[...]
> 	spin_unlock_irq(&hub_event_lock);
>    }
> 
>    This looks clean but it creates another lock.

That would be okay.  It shouldn't be a spinlock; a mutex would be
better.  And adding a new lock doesn't hurt if it is private to this
one routine, since this is a leaf routine.

Or if you want, you could reuse udev->bus->usb_address0_mutex.  That 
would make sense, because that mutex is already associated with device 
address assignment.

> 2. Alternatively, we could use an existing global lock the same way,
>    for example usb_bus_list_lock.
> 
>    But this looks like a hack and I do not like it much.
> 
> 
> 3. Alternatively, it seems the the function affects one
>    "struct usb_device" and one "struct usb_bus". It might
>    be enough to take the appropriate locks for these
>    structures.
> 
>    This would mean to take two locks. It would be slower
>    and we would need to make sure that it does not cause
>    a dead lock.

The right answer is to use either a private mutex or the bus-specific
usb_address0_mutex.  As far as I know, this is the only place that 
relies on khubd being a single thread.

Alan Stern