new diff. integrated matthew's comments and task thread
loop and:
* put the hub explore task on the task queue, don't run it
directly (except in usb_attach()). add comments about why.
* make sure high speed hubs explore before companion low/
full speed hubs
* handle config_pending_{inrc,decr} correctly
* stop interrupts from starting an explore if the first explore
hasn't happened yet
* instead of using both "discover" and "explore" for the same
idea, just use "explore"
* usb_hub_tasks and usb_device_tasks are bad names. use
usb_explore_tasks for the explore tasks and usb_tasks for
the "generic" task tailq.
I've tested this on several machines with several different
usb configurations at boot (and of course plugging/unplugging
devices at random after boot). afaics, everything is working
properly. bonus: device attachment order is more predictible.
--
[email protected]
SDF Public Access UNIX System - http://sdf.lonestar.org
Index: uhub.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/uhub.c,v
retrieving revision 1.52
diff -u -p uhub.c
--- uhub.c 13 Nov 2009 18:06:57 -0000 1.52
+++ uhub.c 22 Aug 2010 21:28:17 -0000
@@ -586,5 +586,5 @@ uhub_intr(usbd_xfer_handle xfer, usbd_private_handle a
if (status == USBD_STALLED)
usbd_clear_endpoint_stall_async(sc->sc_ipipe);
else if (status == USBD_NORMAL_COMPLETION)
- usb_needs_explore(sc->sc_hub);
+ usb_needs_explore(sc->sc_hub, 0);
}
Index: usb.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/usb.c,v
retrieving revision 1.62
diff -u -p usb.c
--- usb.c 9 Nov 2009 17:53:39 -0000 1.62
+++ usb.c 22 Aug 2010 21:28:17 -0000
@@ -81,8 +81,7 @@ extern int ehcidebug;
#endif
/*
* 0 - do usual exploration
- * 1 - do not use timeout exploration
- * >1 - do no exploration
+ * !0 - do no exploration
*/
int usb_noexplore = 0;
#else
@@ -95,18 +94,20 @@ struct usb_softc {
usbd_bus_handle sc_bus; /* USB controller */
struct usbd_port sc_port; /* dummy port for root hub */
- struct proc *sc_event_thread;
+ struct usb_task sc_explore_task;
char sc_dying;
};
-TAILQ_HEAD(, usb_task) usb_all_tasks;
+TAILQ_HEAD(, usb_task) usb_explore_tasks;
+TAILQ_HEAD(, usb_task) usb_tasks;
-volatile int threads_pending = 0;
+int usb_run_tasks;
+int explore_pending;
-void usb_discover(void *);
-void usb_create_event_thread(void *);
-void usb_event_thread(void *);
+void usb_explore(void *);
+void usb_first_explore(void *);
+void usb_create_task_thread(void *);
void usb_task_thread(void *);
struct proc *usb_task_thread_proc = NULL;
@@ -161,7 +162,7 @@ usb_attach(struct device *parent, struct device *self,
int speed;
struct usb_event ue;
- DPRINTF(("usbd_attach\n"));
+ DPRINTF(("usb_attach\n"));
usbd_init();
sc->sc_bus = aux;
@@ -189,6 +190,11 @@ usb_attach(struct device *parent, struct device *self,
if (cold)
sc->sc_bus->use_polling++;
+ /* Don't let hub interrupts cause explore until ready. */
+ sc->sc_bus->config_pending = 1;
+
+ usb_init_task(&sc->sc_explore_task, usb_explore, sc);
+
ue.u.ue_ctrlr.ue_bus = sc->sc_dev.dv_unit;
usb_add_event(USB_EVENT_CTRLR_ATTACH, &ue);
@@ -229,31 +235,40 @@ usb_attach(struct device *parent, struct device *self,
if (cold)
sc->sc_bus->use_polling--;
- config_pending_incr();
- kthread_create_deferred(usb_create_event_thread, sc);
+ if (!sc->sc_dying) {
+ config_pending_incr();
+ kthread_create_deferred(usb_first_explore, sc);
+ }
}
+/*
+ * Called by usbd_init when first usb is attached.
+ */
void
-usb_create_event_thread(void *arg)
+usb_begin_tasks(void)
{
- struct usb_softc *sc = arg;
- static int created = 0;
+ TAILQ_INIT(&usb_explore_tasks);
+ TAILQ_INIT(&usb_tasks);
+ usb_run_tasks = 1;
+ kthread_create_deferred(usb_create_task_thread, NULL);
+}
- if (sc->sc_bus->usbrev == USBREV_2_0)
- threads_pending++;
+/*
+ * Called by usbd_finish when last usb is detached.
+ */
+void
+usb_end_tasks(void)
+{
+ usb_run_tasks = 0;
+ wakeup(&usb_run_tasks);
+}
- if (kthread_create(usb_event_thread, sc, &sc->sc_event_thread,
- "%s", sc->sc_dev.dv_xname))
- panic("unable to create event thread for %s",
- sc->sc_dev.dv_xname);
-
- if (!created) {
- created = 1;
- TAILQ_INIT(&usb_all_tasks);
- if (kthread_create(usb_task_thread, NULL,
- &usb_task_thread_proc, "usbtask"))
- panic("unable to create usb task thread");
- }
+void
+usb_create_task_thread(void *arg)
+{
+ if (kthread_create(usb_task_thread, NULL,
+ &usb_task_thread_proc, "usbtask"))
+ panic("unable to create usb task thread");
}
/*
@@ -266,15 +281,17 @@ usb_add_task(usbd_device_handle dev, struct usb_task *
{
int s;
+ DPRINTFN(2,("%s: task=%p onqueue=%d\n", __func__, task, task->onqueue));
+
s = splusb();
if (!task->onqueue) {
- DPRINTFN(2,("usb_add_task: task=%p\n", task));
- TAILQ_INSERT_TAIL(&usb_all_tasks, task, next);
+ if (task->fun == usb_explore)
+ TAILQ_INSERT_TAIL(&usb_explore_tasks, task, next);
+ else
+ TAILQ_INSERT_TAIL(&usb_tasks, task, next);
task->onqueue = 1;
- } else {
- DPRINTFN(3,("usb_add_task: task=%p on q\n", task));
}
- wakeup(&usb_all_tasks);
+ wakeup(&usb_run_tasks);
splx(s);
}
@@ -283,63 +300,45 @@ usb_rem_task(usbd_device_handle dev, struct usb_task *
{
int s;
+ DPRINTFN(2,("%s: task=%p onqueue=%d\n", __func__, task, task->onqueue));
+
s = splusb();
if (task->onqueue) {
- TAILQ_REMOVE(&usb_all_tasks, task, next);
+ if (task->fun == usb_explore)
+ TAILQ_REMOVE(&usb_explore_tasks, task, next);
+ else
+ TAILQ_REMOVE(&usb_tasks, task, next);
task->onqueue = 0;
}
splx(s);
}
void
-usb_event_thread(void *arg)
+usb_first_explore(void *arg)
{
struct usb_softc *sc = arg;
int pwrdly;
- DPRINTF(("usb_event_thread: start\n"));
+ if (sc->sc_bus->usbrev == USBREV_2_0)
+ explore_pending++;
/* Wait for power to come good. */
pwrdly = sc->sc_bus->root_hub->hub->hubdesc.bPwrOn2PwrGood *
UHD_PWRON_FACTOR + USB_EXTRA_POWER_UP_TIME;
usb_delay_ms(sc->sc_bus, pwrdly);
- /* USB1 threads wait for USB2 threads to finish their first probe. */
- while (sc->sc_bus->usbrev != USBREV_2_0 && threads_pending)
- (void)tsleep((void *)&threads_pending, PWAIT, "config", 0);
+ /* USB1 waits for USB2 to finish their first probe. */
+ while (sc->sc_bus->usbrev != USBREV_2_0 && explore_pending)
+ (void)tsleep((void *)&explore_pending, PWAIT, "config", 0);
- /* Make sure first discover does something. */
- sc->sc_bus->needs_explore = 1;
- usb_discover(sc);
- config_pending_decr();
+ /* Add first explore task to the queue. */
+ usb_needs_explore(sc->sc_bus->root_hub, 1);
/* Wake up any companions waiting for handover before their probes. */
if (sc->sc_bus->usbrev == USBREV_2_0) {
- threads_pending--;
- wakeup((void *)&threads_pending);
+ explore_pending--;
+ wakeup((void *)&explore_pending);
}
-
- while (!sc->sc_dying) {
-#ifdef USB_DEBUG
- if (usb_noexplore < 2)
-#endif
- usb_discover(sc);
-#ifdef USB_DEBUG
- (void)tsleep(&sc->sc_bus->needs_explore, PWAIT, "usbevt",
- usb_noexplore ? 0 : hz * 60);
-#else
- (void)tsleep(&sc->sc_bus->needs_explore, PWAIT, "usbevt",
- hz * 60);
-#endif
- DPRINTFN(2,("usb_event_thread: woke up\n"));
- }
- sc->sc_event_thread = NULL;
-
- /* In case parent is waiting for us to exit. */
- wakeup(sc);
-
- DPRINTF(("usb_event_thread: exit\n"));
- kthread_exit(0);
}
void
@@ -351,21 +350,23 @@ usb_task_thread(void *arg)
DPRINTF(("usb_task_thread: start\n"));
s = splusb();
- for (;;) {
- task = TAILQ_FIRST(&usb_all_tasks);
- if (task == NULL) {
- tsleep(&usb_all_tasks, PWAIT, "usbtsk", 0);
- task = TAILQ_FIRST(&usb_all_tasks);
+ while (usb_run_tasks) {
+ if ((task = TAILQ_FIRST(&usb_explore_tasks)) != NULL)
+ TAILQ_REMOVE(&usb_explore_tasks, task, next);
+ else if ((task = TAILQ_FIRST(&usb_tasks)) != NULL)
+ TAILQ_REMOVE(&usb_tasks, task, next);
+ else {
+ tsleep(&usb_run_tasks, PWAIT, "usbtsk", 0);
+ continue;
}
- DPRINTFN(2,("usb_task_thread: woke up task=%p\n", task));
- if (task != NULL) {
- TAILQ_REMOVE(&usb_all_tasks, task, next);
- task->onqueue = 0;
- splx(s);
- task->fun(task->arg);
- s = splusb();
- }
+ task->onqueue = 0;
+ splx(s);
+ task->fun(task->arg);
+ s = splusb();
}
+ splx(s);
+
+ kthread_exit(0);
}
int
@@ -662,34 +663,46 @@ usbkqfilter(dev_t dev, struct knote *kn)
return (0);
}
-/* Explore device tree from the root. */
+/*
+ * Explore device tree from the root. We need mutual exclusion to this
+ * hub while traversing the device tree, but this is guaranteed since this
+ * function is only called from the task thread, with one exception:
+ * usb_attach() calls this function, but there shouldn't be anything else
+ * trying to explore this hub at that time.
+ */
void
-usb_discover(void *v)
+usb_explore(void *v)
{
struct usb_softc *sc = v;
- DPRINTFN(2,("usb_discover\n"));
+ DPRINTFN(2,("%s: %s\n", __func__, sc->sc_dev.dv_xname));
#ifdef USB_DEBUG
- if (usb_noexplore > 1)
+ if (usb_noexplore)
return;
#endif
- /*
- * We need mutual exclusion while traversing the device tree,
- * but this is guaranteed since this function is only called
- * from the event thread for the controller.
- */
- while (sc->sc_bus->needs_explore && !sc->sc_dying) {
- sc->sc_bus->needs_explore = 0;
+
+ if (!sc->sc_dying)
sc->sc_bus->root_hub->hub->explore(sc->sc_bus->root_hub);
+
+ if (sc->sc_bus->config_pending) {
+ DPRINTF(("%s: %s: first explore done\n", __func__,
+ sc->sc_dev.dv_xname));
+ config_pending_decr();
+ sc->sc_bus->config_pending = 0;
}
}
void
-usb_needs_explore(usbd_device_handle dev)
+usb_needs_explore(usbd_device_handle dev, int first_explore)
{
- DPRINTFN(2,("usb_needs_explore\n"));
- dev->bus->needs_explore = 1;
- wakeup(&dev->bus->needs_explore);
+ DPRINTFN(3,("%s: %s\n", dev->bus->usbctl->sc_dev.dv_xname, __func__));
+
+ if (dev->bus->config_pending && !first_explore) {
+ DPRINTF(("%s: %s: not exploring before first explore\n",
+ __func__, dev->bus->usbctl->sc_dev.dv_xname));
+ return;
+ }
+ usb_add_task(dev, &dev->bus->usbctl->sc_explore_task);
}
void
@@ -697,8 +710,7 @@ usb_needs_reattach(usbd_device_handle dev)
{
DPRINTFN(2,("usb_needs_reattach\n"));
dev->powersrc->reattach = 1;
- dev->bus->needs_explore = 1;
- wakeup(&dev->bus->needs_explore);
+ usb_needs_explore(dev, 0);
}
/* Called at splusb() */
@@ -822,14 +834,7 @@ usb_detach(struct device *self, int flags)
if (sc->sc_port.device != NULL)
usb_disconnect_port(&sc->sc_port, self);
- /* Kill off event thread. */
- if (sc->sc_event_thread != NULL) {
- wakeup(&sc->sc_bus->needs_explore);
- if (tsleep(sc, PWAIT, "usbdet", hz * 60))
- printf("%s: event thread didn't die\n",
- sc->sc_dev.dv_xname);
- DPRINTF(("usb_detach: event thread dead\n"));
- }
+ usb_rem_task(sc->sc_bus->root_hub, &sc->sc_explore_task);
usbd_finish();
Index: usbdi.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/usbdi.c,v
retrieving revision 1.38
diff -u -p usbdi.c
--- usbdi.c 5 Mar 2010 17:28:54 -0000 1.38
+++ usbdi.c 22 Aug 2010 21:28:17 -0000
@@ -71,15 +71,18 @@ struct rwlock usbpalock;
void
usbd_init(void)
{
- if (usbd_nbuses == 0)
+ if (usbd_nbuses == 0) {
rw_init(&usbpalock, "usbpalock");
+ usb_begin_tasks();
+ }
usbd_nbuses++;
}
void
usbd_finish(void)
{
- --usbd_nbuses;
+ if (--usbd_nbuses == 0)
+ usb_end_tasks();
}
static __inline int
Index: usbdivar.h
===================================================================
RCS file: /cvs/src/sys/dev/usb/usbdivar.h,v
retrieving revision 1.37
diff -u -p usbdivar.h
--- usbdivar.h 12 Nov 2009 20:16:37 -0000 1.37
+++ usbdivar.h 22 Aug 2010 21:28:17 -0000
@@ -103,7 +103,7 @@ struct usbd_bus {
/* Filled by usb driver */
struct usbd_device *root_hub;
usbd_device_handle devices[USB_MAX_DEVICES];
- char needs_explore;/* a hub a signalled a change */
+ int config_pending;
char use_polling;
struct usb_softc *usbctl;
struct usb_device_stats stats;
@@ -216,6 +216,8 @@ struct usbd_xfer {
void usbd_init(void);
void usbd_finish(void);
+void usb_begin_tasks(void);
+void usb_end_tasks(void);
#ifdef USB_DEBUG
void usbd_dump_iface(struct usbd_interface *iface);
@@ -247,7 +249,7 @@ void usb_transfer_complete(usbd_xfer_handle
xfer);
void usb_disconnect_port(struct usbd_port *up, struct device *);
/* Routines from usb.c */
-void usb_needs_explore(usbd_device_handle);
+void usb_needs_explore(usbd_device_handle, int);
void usb_needs_reattach(usbd_device_handle);
void usb_schedsoftintr(struct usbd_bus *);
