Both a cleanup, as khubd open codes several facilities that are provided by workqueue, and a prepation for running port resume events in khubd context. Running port recovery in khubd context allows coalescing / ordering port recovery actions with device recovery and it guarantees that hub_events is idle for the duration of a recovery event. Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx> --- drivers/usb/core/hub.c | 135 +++++++++++------------------------------------- drivers/usb/core/hub.h | 4 - 2 files changed, 31 insertions(+), 108 deletions(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 48d5c8fff77f..83abaecfabfe 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; @@ -577,18 +569,12 @@ static int hub_port_status(struct usb_hub *hub, int port1, static void kick_khubd(struct usb_hub *hub) { - unsigned long flags; - - spin_lock_irqsave(&hub_event_lock, flags); - if (!hub->disconnected && list_empty(&hub->event_list)) { - list_add_tail(&hub->event_list, &hub_event_list); + struct usb_interface *intf = to_usb_interface(hub->intfdev); - /* 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); + /* Suppress autosuspend until khubd runs */ + usb_autopm_get_interface_no_resume(intf); + if (!queue_work(khubd_wq, &hub->event_work)) + usb_autopm_put_interface_async(intf); } void usb_kick_khubd(struct usb_device *hdev) @@ -1591,10 +1577,8 @@ fail_keep_maxchild: return ret; } -static void hub_release(struct kref *kref) +static void hub_release(struct usb_hub *hub) { - struct usb_hub *hub = container_of(kref, struct usb_hub, kref); - usb_put_intf(to_usb_interface(hub->intfdev)); kfree(hub); } @@ -1607,19 +1591,13 @@ static void hub_disconnect(struct usb_interface *intf) struct usb_device *hdev = interface_to_usbdev(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); - usb_autopm_put_interface_no_suspend(intf); - } - hub->disconnected = 1; - spin_unlock_irq(&hub_event_lock); - /* Disconnect all children and quiesce the hub */ hub->error = 0; hub_quiesce(hub, HUB_DISCONNECT); + if (cancel_work_sync(&hub->event_work)) + usb_autopm_put_interface(intf); + usb_set_intfdata (intf, NULL); for (i = 0; i < hdev->maxchild; i++) @@ -1636,9 +1614,10 @@ static void hub_disconnect(struct usb_interface *intf) kfree(hub->buffer); pm_suspend_ignore_children(&intf->dev, false); - kref_put(&hub->kref, hub_release); + hub_release(hub); } +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; @@ -1728,8 +1707,7 @@ descriptor_error: return -ENOMEM; } - 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); @@ -4662,45 +4640,17 @@ static int hub_handle_remote_wakeup(struct usb_hub *hub, unsigned int port, return connect_change; } -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; - u16 portstatus; - u16 portchange; - 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); + struct usb_hub *hub = container_of(w, struct usb_hub, event_work); + struct usb_device *hdev = hub->hdev; + struct device *hub_dev = hub->intfdev; + struct usb_interface *intf = to_usb_interface(hub_dev); + int i, ret, connect_change, wakeup_change; + u16 portstatus, portchange; + u16 hubstatus, hubchange; - 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... */ @@ -4710,8 +4660,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) { @@ -4909,31 +4857,8 @@ 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 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[] = { @@ -4973,11 +4898,12 @@ 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 */ + /* Fall through if workqueue creation failed */ usb_deregister(&hub_driver); printk(KERN_ERR "%s: can't start khubd\n", usbcore_name); @@ -4986,8 +4912,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 @@ -4996,7 +4920,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 419e7ac51c12..7547713d036a 100644 --- a/drivers/usb/core/hub.h +++ b/drivers/usb/core/hub.h @@ -27,7 +27,6 @@ struct usb_hub { struct device *intfdev; /* the "interface" device */ struct usb_device *hdev; - struct kref kref; struct urb *urb; /* for interrupt polling pipe */ /* buffer for urb ... with extra space in case of babble */ @@ -41,7 +40,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 */ @@ -67,7 +66,6 @@ struct usb_hub { unsigned limited_power:1; unsigned quiescing:1; - unsigned disconnected:1; unsigned quirk_check_port_auto_suspend:1; -- 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