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;