For reliable port power management we want to be sure that khubd is only
taking actions on active ports. Towards that end we need to mark the
ports us undergoing a power transition and stop khubd actions on the
port. The existing hub->busy_bits attempt this notification, but it is
not synchronized with power transitions.
When we go to disable port power flush khubd to make sure it never takes
action on powered down port, or races actions against a port in the
middle of transitioning. This requires that khubd never trigger a
synchronous suspend otherwise it will deadlock, hence the conversions to
the async pm_runtime_put interfaces.
Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
Cc: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
Cc: Sarah Sharp <sarah.a.sharp@xxxxxxxxxxxxxxx>
Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
---
drivers/usb/core/hub.c | 118 ++++++++++++-----------------------------------
drivers/usb/core/hub.h | 3 -
drivers/usb/core/port.c | 30 ++++++++----
3 files changed, 51 insertions(+), 100 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 2e0812eff0ae..3324cf5fd253 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>
@@ -53,14 +52,7 @@ static inline int hub_is_superspeed(struct usb_device *hdev)
* 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;
+static struct workqueue_struct *khubd_wq;
/* cycle leds on hubs that aren't blinking for attention */
static bool blinkenlights = 0;
@@ -575,20 +567,18 @@ static int hub_port_status(struct usb_hub *hub, int port1,
return ret;
}
+static void hub_release(struct kref *);
static void kick_khubd(struct usb_hub *hub)
{
- unsigned long flags;
+ struct usb_interface *intf = to_usb_interface(hub->intfdev);
- 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);
+ /* Suppress autosuspend until khubd runs */
+ 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);
}
void usb_kick_khubd(struct usb_device *hdev)
@@ -959,7 +949,7 @@ int usb_remove_device(struct usb_device *udev)
usb_autopm_get_interface(intf);
set_bit(udev->portnum, hub->removed_bits);
hub_port_logical_disconnect(hub, udev->portnum);
- usb_autopm_put_interface(intf);
+ usb_autopm_put_interface_async(intf);
return 0;
}
@@ -1576,13 +1566,10 @@ static void hub_disconnect(struct usb_interface *intf)
int i;
/* 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);
+ if (cancel_work_sync(&hub->event_work)) {
usb_autopm_put_interface_no_suspend(intf);
+ kref_put(&hub->kref, hub_release);
}
- hub->disconnected = 1;
- spin_unlock_irq(&hub_event_lock);
/* Disconnect all children and quiesce the hub */
hub->error = 0;
@@ -1606,6 +1593,7 @@ static void hub_disconnect(struct usb_interface *intf)
kref_put(&hub->kref, hub_release);
}
+static void hub_events(struct work_struct *);
static int hub_probe(struct usb_interface *intf, const struct usb_device_id *id)
{
struct usb_host_interface *desc;
@@ -1696,7 +1684,7 @@ descriptor_error:
}
kref_init(&hub->kref);
- INIT_LIST_HEAD(&hub->event_list);
+ INIT_WORK(&hub->event_work, hub_events);
hub->intfdev = &intf->dev;
hub->hdev = hdev;
INIT_DELAYED_WORK(&hub->leds, led_work);
@@ -2342,7 +2330,7 @@ int usb_new_device(struct usb_device *udev)
(void) usb_create_ep_devs(&udev->dev, &udev->ep0, udev);
usb_mark_last_busy(udev);
- pm_runtime_put_sync_autosuspend(&udev->dev);
+ pm_runtime_put_autosuspend(&udev->dev);
return err;
fail:
@@ -4574,13 +4562,12 @@ static int hub_handle_remote_wakeup(struct usb_hub *hub, unsigned int port,
return connect_change;
}
-static void hub_events(void)
+static void hub_events(struct work_struct *w)
{
- struct list_head *tmp;
- struct usb_device *hdev;
- struct usb_interface *intf;
- struct usb_hub *hub;
- struct device *hub_dev;
+ struct usb_hub *hub = container_of(w, struct usb_hub, event_work);
+ struct device *hub_dev = hub->intfdev;
+ struct usb_device *hdev = hub->hdev;
+ struct usb_interface *intf = to_usb_interface(hub_dev);
u16 hubstatus;
u16 hubchange;
u16 portstatus;
@@ -4588,31 +4575,7 @@ static void hub_events(void)
int i, ret;
int connect_change, wakeup_change;
- /*
- * 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... */
@@ -4622,8 +4585,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) {
@@ -4818,34 +4779,14 @@ static void hub_events(void)
usb_autopm_put_interface_no_suspend(intf);
loop:
/* Balance the usb_autopm_get_interface_no_resume() in
- * kick_khubd() and allow autosuspend.
+ * kick_khubd() and allow autosuspend (asynchronously since
+ * suspension tries to flush khubd)
*/
- usb_autopm_put_interface(intf);
- loop_disconnected:
+ usb_autopm_put_interface_async(intf);
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 intefering 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);
}
static const struct usb_device_id hub_id_table[] = {
@@ -4885,8 +4826,9 @@ int usb_hub_init(void)
return -1;
}
- khubd_task = kthread_run(hub_thread, NULL, "khubd");
- if (!IS_ERR(khubd_task))
+ khubd_wq = create_freezable_workqueue("khubd");
+
+ if (khubd_wq)
return 0;
/* Fall through if kernel_thread failed */
@@ -4898,7 +4840,7 @@ int usb_hub_init(void)
void usb_hub_cleanup(void)
{
- kthread_stop(khubd_task);
+ destroy_workqueue(khubd_wq);
/*
* Hub resources are freed for us by usb_deregister. It calls
diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index 3eb51068c39f..915f76fe64c7 100644
--- a/drivers/usb/core/hub.h
+++ b/drivers/usb/core/hub.h
@@ -44,7 +44,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 */
@@ -68,7 +68,6 @@ struct usb_hub {
unsigned limited_power:1;
unsigned quiescing:1;
- unsigned disconnected:1;
unsigned quirk_check_port_auto_suspend:1;
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index 89f451fa0ab8..bef75e5adff2 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -109,8 +109,6 @@ static int usb_port_runtime_poweron(struct device *dev)
if (is_power_policy_on(port_dev))
return 0;
- set_bit(port1, hub->busy_bits);
-
retval = usb_set_port_feature(hdev, port1, USB_PORT_FEAT_POWER);
if (port_dev->child && !retval) {
/*
@@ -127,29 +125,41 @@ static int usb_port_runtime_poweron(struct device *dev)
retval = 0;
}
- clear_bit(port1, hub->busy_bits);
return retval;
}
static int usb_port_runtime_poweroff(struct device *dev)
{
+ static const int features[] = { USB_PORT_FEAT_POWER,
+ USB_PORT_FEAT_C_CONNECTION,
+ USB_PORT_FEAT_C_ENABLE };
struct usb_port *port_dev = to_usb_port(dev);
struct usb_device *hdev = to_usb_device(dev->parent->parent);
struct usb_hub *hub = usb_hub_to_struct_hub(hdev);
int port1 = port_dev->portnum;
- int retval;
-
+ int retval, i;
if (!hub)
return -EINVAL;
if (is_power_policy_on(port_dev))
return 0;
- set_bit(port1, hub->busy_bits);
- retval = usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_POWER);
- usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_CONNECTION);
- usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_ENABLE);
- clear_bit(port1, hub->busy_bits);
+ pm_runtime_set_suspended(&port_dev->dev);
+ flush_work(&hub->event_work);
+
+ for (i = 0; i < ARRAY_SIZE(features); i++) {
+ retval = usb_clear_port_feature(hdev, port1, features[i]);
+ if (retval)
+ break;
+ }
+
+ /* unwind on error */
+ for (; i >= 0 && i < ARRAY_SIZE(features); i--)
+ usb_set_port_feature(hdev, port1, features[i]);
+
+ /* returning an error will set our status to active, otherwise the
+ * early pm_runtime_set_suspended plus flush stands
+ */
return retval;
}
--
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
