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