usb(4) has a per-instance "event" thread, and there is also
a single task thread that runs all other usb_tasks.

usb_tasks are added to a tailq and the task thread runs
the tasks one at a time.

the per-usb "event" threads only run root hub explorations.
these are responsible for determining when a device has been
added or removed and starts the appropriate driver attach/
detach sequence.

now, when a driver's detach routine is run, it removes any
usb_tasks the driver has added.  this is necessary because
the task may actually try to do something with the hardware,
which is not available if it has been removed.

the problem is, with one thread running the tasks, and
a different thread attaching and detaching devices, when a
device is removed, there's really no guarantee that a detach,
and thus task removal, will happen before a task for that
device is run.

so this diff turns hub explorations into usb_tasks, and runs
them from the task thread.  to make sure exploration tasks
run before other tasks, it uses two tailqs: one for hub
explorations, and one for other tasks.  in this way, we can
be more sure that hub explorations happen before other
tasks, and reduces the chance of running "stale" tasks.

comments?

-- 
[email protected]
SDF Public Access UNIX System - http://sdf.lonestar.org

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       21 Aug 2010 03:27:04 -0000
@@ -95,18 +95,19 @@ 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_hub_tasks;
+TAILQ_HEAD(, usb_task) usb_device_tasks;
+volatile int usb_run_tasks;
+volatile int discover_pending = 0;
 
-volatile int threads_pending = 0;
-
 void   usb_discover(void *);
-void   usb_create_event_thread(void *);
-void   usb_event_thread(void *);
+void   usb_first_discover(void *);
+void   usb_create_task_thread(void *);
 void   usb_task_thread(void *);
 struct proc *usb_task_thread_proc = NULL;
 
@@ -201,6 +202,8 @@ usb_attach(struct device *parent, struct device *self,
                return;
        }
 
+       usb_init_task(&sc->sc_explore_task, usb_discover, sc);
+
        err = usbd_new_device(&sc->sc_dev, sc->sc_bus, 0, speed, 0,
                  &sc->sc_port);
        if (!err) {
@@ -230,30 +233,32 @@ usb_attach(struct device *parent, struct device *self,
                sc->sc_bus->use_polling--;
 
        config_pending_incr();
-       kthread_create_deferred(usb_create_event_thread, sc);
+       kthread_create_deferred(usb_first_discover, 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_hub_tasks);
+       TAILQ_INIT(&usb_device_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 lasst usb is detached */
+void
+usb_end_tasks(void)
+{
+       usb_run_tasks = 0;
+}
 
-       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 +271,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_discover)
+                       TAILQ_INSERT_TAIL(&usb_hub_tasks, task, next);
+               else
+                       TAILQ_INSERT_TAIL(&usb_device_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,21 +290,27 @@ 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_discover)
+                       TAILQ_REMOVE(&usb_hub_tasks, task, next);
+               else
+                       TAILQ_REMOVE(&usb_device_tasks, task, next);
                task->onqueue = 0;
        }
        splx(s);
 }
 
 void
-usb_event_thread(void *arg)
+usb_first_discover(void *arg)
 {
        struct usb_softc *sc = arg;
        int pwrdly;
 
-       DPRINTF(("usb_event_thread: start\n"));
+       if (sc->sc_bus->usbrev == USBREV_2_0)
+               discover_pending++;
 
        /* Wait for power to come good. */
        pwrdly = sc->sc_bus->root_hub->hub->hubdesc.bPwrOn2PwrGood * 
@@ -305,41 +318,18 @@ usb_event_thread(void *arg)
        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);
+       while (sc->sc_bus->usbrev != USBREV_2_0 && discover_pending)
+               (void)tsleep((void *)&discover_pending, PWAIT, "config", 0);
 
        /* Make sure first discover does something. */
-       sc->sc_bus->needs_explore = 1;
        usb_discover(sc);
        config_pending_decr();
 
        /* Wake up any companions waiting for handover before their probes. */
        if (sc->sc_bus->usbrev == USBREV_2_0) {
-               threads_pending--;
-               wakeup((void *)&threads_pending);
+               discover_pending--;
+               wakeup((void *)&discover_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
@@ -350,22 +340,43 @@ 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) {
+               s = splusb();
+               while (usb_run_tasks &&
+                   (!TAILQ_EMPTY(&usb_hub_tasks) ||
+                   !TAILQ_EMPTY(&usb_device_tasks))) {
+
+                       task = TAILQ_FIRST(&usb_hub_tasks);
+                       while (task != NULL && usb_run_tasks) {
+                               TAILQ_REMOVE(&usb_hub_tasks, task, next);
+                               task->onqueue = 0;
+                               splx(s);
+                               task->fun(task->arg);
+                               s = splusb();
+                               task = TAILQ_FIRST(&usb_hub_tasks);
+                       }
+
+                       task = TAILQ_FIRST(&usb_device_tasks);
+                       while (task != NULL && usb_run_tasks) {
+                               TAILQ_REMOVE(&usb_device_tasks, task, next);
+                               task->onqueue = 0;
+                               splx(s);
+                               task->fun(task->arg);
+                               s = splusb();
+                               if (!TAILQ_EMPTY(&usb_hub_tasks))
+                                       break;
+                               task = TAILQ_FIRST(&usb_device_tasks);
+                       }
                }
-               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();
+               splx(s);
+
+               if (usb_run_tasks) {
+                       /* XXX Is not timing out too optimistic? */
+                       tsleep(&usb_run_tasks, PWAIT, "usbtsk", 0);
+                       DPRINTFN(2,("%s: woke up\n", __func__));
                }
        }
+       kthread_exit(0);
 }
 
 int
@@ -678,18 +689,16 @@ usb_discover(void *v)
         * 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);
-       }
 }
 
 void
 usb_needs_explore(usbd_device_handle dev)
 {
-       DPRINTFN(2,("usb_needs_explore\n"));
-       dev->bus->needs_explore = 1;
-       wakeup(&dev->bus->needs_explore);
+       DPRINTFN(2, ("%s: %s\n", dev->bus->usbctl->sc_dev.dv_xname, __func__));
+       if (!dev->bus->usbctl->sc_dying)
+               usb_add_task(dev, &dev->bus->usbctl->sc_explore_task);
 }
 
 void
@@ -697,8 +706,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);
 }
 
 /* Called at splusb() */
@@ -822,14 +830,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     21 Aug 2010 03:27:05 -0000
@@ -71,15 +72,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  21 Aug 2010 03:27:05 -0000
@@ -103,7 +103,6 @@ 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 */
        char                    use_polling;
        struct usb_softc       *usbctl;
        struct usb_device_stats stats;
@@ -216,6 +215,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);

Reply via email to