On 26/05/14(Mon) 13:46, Martin Pieuchot wrote:
> It is currently possible to trigger a race between the thread doing
> DVACT_QUIESCE and the USB thread exploring the buses.
> 
> This race is really easy to reproduce if you have a lot of controllers
> and you try to suspend just after resuming.  In the best case, it blows
> your kernel during suspend, in the worst case it blows it at resume :)
> 
> So here's a diff to avoid such race by deferring the action of detaching
> the root hub to the USB thread doing exploration.
> 
> I'd appreciate if people having troubles with suspend/resume could try
> this diff an report back.

Previous diff was lacking the header chunk, please use this one instead.

Martin

Index: usb.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/usb/usb.c,v
retrieving revision 1.96
diff -u -p -r1.96 usb.c
--- usb.c       11 May 2014 16:33:21 -0000      1.96
+++ usb.c       26 May 2014 11:19:31 -0000
@@ -260,11 +260,15 @@ usb_attach_roothub(struct usb_softc *sc)
 void
 usb_detach_roothub(struct usb_softc *sc)
 {
-       /* Make all devices disconnect. */
-       if (sc->sc_port.device != NULL)
-               usb_disconnect_port(&sc->sc_port, (struct device *)sc);
+       /*
+        * To avoid races with the usb task thread, mark the root hub
+        * as disconnecting and schedule an exploration task to detach
+        * it.
+        */
+       sc->sc_bus->flags |= USB_BUS_DISCONNECTING;
+       usb_needs_explore(sc->sc_bus->root_hub, 0);
 
-       usb_rem_wait_task(sc->sc_bus->root_hub, &sc->sc_explore_task);
+       usb_wait_task(sc->sc_bus->root_hub, &sc->sc_explore_task);
 
        sc->sc_bus->root_hub = NULL;
 }
@@ -840,7 +844,18 @@ usb_explore(void *v)
                        usb_delay_ms(sc->sc_bus, pwrdly - waited_ms);
        }
 
-       sc->sc_bus->root_hub->hub->explore(sc->sc_bus->root_hub);
+       if (sc->sc_bus->flags & USB_BUS_DISCONNECTING) {
+               /* Prevent new tasks from being scheduled. */
+               sc->sc_bus->dying = 1;
+
+               /* Make all devices disconnect. */
+               if (sc->sc_port.device != NULL)
+                       usb_disconnect_port(&sc->sc_port, (struct device *)sc);
+
+               sc->sc_bus->flags &= ~USB_BUS_DISCONNECTING;
+       } else {
+               sc->sc_bus->root_hub->hub->explore(sc->sc_bus->root_hub);
+       }
 
        if (sc->sc_bus->flags & USB_BUS_CONFIG_PENDING) {
                DPRINTF(("%s: %s: first explore done\n", __func__,
@@ -896,7 +911,6 @@ usb_activate(struct device *self, int ac
 
        switch (act) {
        case DVACT_QUIESCE:
-               sc->sc_bus->dying = 1;
                if (sc->sc_bus->root_hub != NULL)
                        usb_detach_roothub(sc);
                break;
@@ -914,10 +928,6 @@ usb_activate(struct device *self, int ac
                        usb_needs_explore(sc->sc_bus->root_hub, 0);
                sc->sc_bus->use_polling--;
                break;
-       case DVACT_DEACTIVATE:
-               rv = config_activate_children(self, act);
-               sc->sc_bus->dying = 1;
-               break;
        default:
                rv = config_activate_children(self, act);
                break;
@@ -929,10 +939,6 @@ int
 usb_detach(struct device *self, int flags)
 {
        struct usb_softc *sc = (struct usb_softc *)self;
-
-       DPRINTF(("usb_detach: start\n"));
-
-       sc->sc_bus->dying = 1;
 
        if (sc->sc_bus->root_hub != NULL) {
                usb_detach_roothub(sc);
Index: uhub.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/usb/uhub.c,v
retrieving revision 1.66
diff -u -p -r1.66 uhub.c
--- uhub.c      11 Mar 2014 10:24:42 -0000      1.66
+++ uhub.c      26 May 2014 11:19:31 -0000
@@ -546,6 +546,9 @@ uhub_intr(struct usbd_xfer *xfer, void *
 {
        struct uhub_softc *sc = addr;
 
+       if (usbd_is_dying(sc->sc_hub))
+               return;
+
        DPRINTF("%s: intr status=%d\n", sc->sc_dev.dv_xname, status);
        if (status == USBD_STALLED)
                usbd_clear_endpoint_stall_async(sc->sc_ipipe);
Index: ehci.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/usb/ehci.c,v
retrieving revision 1.155
diff -u -p -r1.155 ehci.c
--- ehci.c      16 May 2014 19:00:18 -0000      1.155
+++ ehci.c      26 May 2014 11:19:31 -0000
@@ -1075,6 +1075,18 @@ ehci_activate(struct device *self, int a
                ehci_reset(sc);
 
                sc->sc_bus.use_polling--;
+
+               if (!TAILQ_EMPTY(&sc->sc_intrhead)) {
+                       printf("%s: intr head not empty\n",
+                           sc->sc_bus.bdev.dv_xname);
+                       return (EINVAL);
+               }
+               if (sc->sc_intrxfer != NULL) {
+                       printf("%s: root intr not null\n",
+                           sc->sc_bus.bdev.dv_xname);
+                       return (EINVAL);
+               }
+
                break;
        case DVACT_RESUME:
                sc->sc_bus.use_polling++;
Index: usbdivar.h
===================================================================
RCS file: /home/ncvs/src/sys/dev/usb/usbdivar.h,v
retrieving revision 1.58
diff -u -p -r1.58 usbdivar.h
--- usbdivar.h  29 Mar 2014 18:09:31 -0000      1.58
+++ usbdivar.h  26 May 2014 11:50:33 -0000
@@ -105,6 +105,7 @@ struct usbd_bus {
        char                    dying;
        int                     flags;
 #define USB_BUS_CONFIG_PENDING 0x01
+#define USB_BUS_DISCONNECTING  0x02
        struct device          *usbctl;
        struct usb_device_stats stats;
        int                     intr_context;

Reply via email to