Date: Wed, 13 Feb 2013 12:35:17 +0000 (UTC) From: Hans Petter Selasky <hselasky@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r246759 - in head/sys: dev/usb dev/usb/controller sys Message-ID: <201302131235.r1DCZHEp065195@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: hselasky Date: Wed Feb 13 12:35:17 2013 New Revision: 246759 URL: http://svnweb.freebsd.org/changeset/base/246759 Log: Resolve a LOR after r246616. Protect control requests using the USB device enumeration lock. Make sure all callers of usbd_enum_lock() check the return value. Remove the control transfer specific lock. Bump the FreeBSD version number, hence external USB modules may need to be recompiled due to a USB device structure change. MFC after: 1 week Modified: head/sys/dev/usb/controller/usb_controller.c head/sys/dev/usb/usb_dev.c head/sys/dev/usb/usb_dev.h head/sys/dev/usb/usb_device.c head/sys/dev/usb/usb_device.h head/sys/dev/usb/usb_handle_request.c head/sys/dev/usb/usb_hub.c head/sys/dev/usb/usb_request.c head/sys/sys/param.h Modified: head/sys/dev/usb/controller/usb_controller.c ============================================================================== --- head/sys/dev/usb/controller/usb_controller.c Wed Feb 13 10:18:26 2013 (r246758) +++ head/sys/dev/usb/controller/usb_controller.c Wed Feb 13 12:35:17 2013 (r246759) @@ -427,6 +427,7 @@ usb_bus_suspend(struct usb_proc_msg *pm) struct usb_bus *bus; struct usb_device *udev; usb_error_t err; + uint8_t do_unlock; bus = ((struct usb_bus_msg *)pm)->bus; udev = bus->devices[USB_ROOT_HUB_ADDR]; @@ -447,7 +448,7 @@ usb_bus_suspend(struct usb_proc_msg *pm) bus_generic_shutdown(bus->bdev); - usbd_enum_lock(udev); + do_unlock = usbd_enum_lock(udev); err = usbd_set_config_index(udev, USB_UNCONFIG_INDEX); if (err) @@ -464,7 +465,8 @@ usb_bus_suspend(struct usb_proc_msg *pm) if (bus->methods->set_hw_power_sleep != NULL) (bus->methods->set_hw_power_sleep) (bus, USB_HW_POWER_SUSPEND); - usbd_enum_unlock(udev); + if (do_unlock) + usbd_enum_unlock(udev); USB_BUS_LOCK(bus); } @@ -480,6 +482,7 @@ usb_bus_resume(struct usb_proc_msg *pm) struct usb_bus *bus; struct usb_device *udev; usb_error_t err; + uint8_t do_unlock; bus = ((struct usb_bus_msg *)pm)->bus; udev = bus->devices[USB_ROOT_HUB_ADDR]; @@ -489,7 +492,7 @@ usb_bus_resume(struct usb_proc_msg *pm) USB_BUS_UNLOCK(bus); - usbd_enum_lock(udev); + do_unlock = usbd_enum_lock(udev); #if 0 DEVMETHOD(usb_take_controller, NULL); /* dummy */ #endif @@ -523,7 +526,8 @@ usb_bus_resume(struct usb_proc_msg *pm) "attach root HUB\n"); } - usbd_enum_unlock(udev); + if (do_unlock) + usbd_enum_unlock(udev); USB_BUS_LOCK(bus); } @@ -539,6 +543,7 @@ usb_bus_shutdown(struct usb_proc_msg *pm struct usb_bus *bus; struct usb_device *udev; usb_error_t err; + uint8_t do_unlock; bus = ((struct usb_bus_msg *)pm)->bus; udev = bus->devices[USB_ROOT_HUB_ADDR]; @@ -550,7 +555,7 @@ usb_bus_shutdown(struct usb_proc_msg *pm bus_generic_shutdown(bus->bdev); - usbd_enum_lock(udev); + do_unlock = usbd_enum_lock(udev); err = usbd_set_config_index(udev, USB_UNCONFIG_INDEX); if (err) @@ -567,7 +572,8 @@ usb_bus_shutdown(struct usb_proc_msg *pm if (bus->methods->set_hw_power_sleep != NULL) (bus->methods->set_hw_power_sleep) (bus, USB_HW_POWER_SHUTDOWN); - usbd_enum_unlock(udev); + if (do_unlock) + usbd_enum_unlock(udev); USB_BUS_LOCK(bus); } Modified: head/sys/dev/usb/usb_dev.c ============================================================================== --- head/sys/dev/usb/usb_dev.c Wed Feb 13 10:18:26 2013 (r246758) +++ head/sys/dev/usb/usb_dev.c Wed Feb 13 12:35:17 2013 (r246759) @@ -218,10 +218,10 @@ usb_ref_device(struct usb_cdev_privdata mtx_unlock(&usb_ref_lock); /* - * We need to grab the sx-lock before grabbing the - * FIFO refs to avoid deadlock at detach! + * We need to grab the enumeration SX-lock before + * grabbing the FIFO refs to avoid deadlock at detach! */ - usbd_enum_lock(cpd->udev); + crd->do_unlock = usbd_enum_lock(cpd->udev); mtx_lock(&usb_ref_lock); @@ -282,9 +282,10 @@ usb_ref_device(struct usb_cdev_privdata return (0); error: - if (crd->is_uref) { + if (crd->do_unlock) usbd_enum_unlock(cpd->udev); + if (crd->is_uref) { if (--(cpd->udev->refcount) == 0) { cv_signal(&cpd->udev->ref_cv); } @@ -336,7 +337,7 @@ usb_unref_device(struct usb_cdev_privdat DPRINTFN(2, "cpd=%p is_uref=%d\n", cpd, crd->is_uref); - if (crd->is_uref) + if (crd->do_unlock) usbd_enum_unlock(cpd->udev); mtx_lock(&usb_ref_lock); Modified: head/sys/dev/usb/usb_dev.h ============================================================================== --- head/sys/dev/usb/usb_dev.h Wed Feb 13 10:18:26 2013 (r246758) +++ head/sys/dev/usb/usb_dev.h Wed Feb 13 12:35:17 2013 (r246759) @@ -84,6 +84,7 @@ struct usb_cdev_refdata { uint8_t is_write; /* location has write access */ uint8_t is_uref; /* USB refcount decr. needed */ uint8_t is_usbfs; /* USB-FS is active */ + uint8_t do_unlock; /* USB enum unlock needed */ }; struct usb_fs_privdata { Modified: head/sys/dev/usb/usb_device.c ============================================================================== --- head/sys/dev/usb/usb_device.c Wed Feb 13 10:18:26 2013 (r246758) +++ head/sys/dev/usb/usb_device.c Wed Feb 13 12:35:17 2013 (r246759) @@ -1546,9 +1546,6 @@ usb_alloc_device(device_t parent_dev, st return (NULL); } /* initialise our SX-lock */ - sx_init_flags(&udev->ctrl_sx, "USB device SX lock", SX_DUPOK); - - /* initialise our SX-lock */ sx_init_flags(&udev->enum_sx, "USB config SX lock", SX_DUPOK); sx_init_flags(&udev->sr_sx, "USB suspend and resume SX lock", SX_NOWITNESS); @@ -2119,7 +2116,6 @@ usb_free_device(struct usb_device *udev, &udev->cs_msg[0], &udev->cs_msg[1]); USB_BUS_UNLOCK(udev->bus); - sx_destroy(&udev->ctrl_sx); sx_destroy(&udev->enum_sx); sx_destroy(&udev->sr_sx); Modified: head/sys/dev/usb/usb_device.h ============================================================================== --- head/sys/dev/usb/usb_device.h Wed Feb 13 10:18:26 2013 (r246758) +++ head/sys/dev/usb/usb_device.h Wed Feb 13 12:35:17 2013 (r246759) @@ -181,7 +181,6 @@ union usb_device_scratch { struct usb_device { struct usb_clear_stall_msg cs_msg[2]; /* generic clear stall * messages */ - struct sx ctrl_sx; struct sx enum_sx; struct sx sr_sx; struct mtx device_mtx; Modified: head/sys/dev/usb/usb_handle_request.c ============================================================================== --- head/sys/dev/usb/usb_handle_request.c Wed Feb 13 10:18:26 2013 (r246758) +++ head/sys/dev/usb/usb_handle_request.c Wed Feb 13 12:35:17 2013 (r246759) @@ -149,6 +149,7 @@ usb_handle_set_config(struct usb_xfer *x { struct usb_device *udev = xfer->xroot->udev; usb_error_t err = 0; + uint8_t do_unlock; /* * We need to protect against other threads doing probe and @@ -156,7 +157,8 @@ usb_handle_set_config(struct usb_xfer *x */ USB_XFER_UNLOCK(xfer); - usbd_enum_lock(udev); + /* Prevent re-enumeration */ + do_unlock = usbd_enum_lock(udev); if (conf_no == USB_UNCONFIG_NO) { conf_no = USB_UNCONFIG_INDEX; @@ -179,7 +181,8 @@ usb_handle_set_config(struct usb_xfer *x goto done; } done: - usbd_enum_unlock(udev); + if (do_unlock) + usbd_enum_unlock(udev); USB_XFER_LOCK(xfer); return (err); } @@ -221,6 +224,7 @@ usb_handle_iface_request(struct usb_xfer int error; uint8_t iface_index; uint8_t temp_state; + uint8_t do_unlock; if ((req.bmRequestType & 0x1F) == UT_INTERFACE) { iface_index = req.wIndex[0]; /* unicast */ @@ -234,7 +238,8 @@ usb_handle_iface_request(struct usb_xfer */ USB_XFER_UNLOCK(xfer); - usbd_enum_lock(udev); + /* Prevent re-enumeration */ + do_unlock = usbd_enum_lock(udev); error = ENXIO; @@ -350,17 +355,20 @@ tr_repeat: goto tr_stalled; } tr_valid: - usbd_enum_unlock(udev); + if (do_unlock) + usbd_enum_unlock(udev); USB_XFER_LOCK(xfer); return (0); tr_short: - usbd_enum_unlock(udev); + if (do_unlock) + usbd_enum_unlock(udev); USB_XFER_LOCK(xfer); return (USB_ERR_SHORT_XFER); tr_stalled: - usbd_enum_unlock(udev); + if (do_unlock) + usbd_enum_unlock(udev); USB_XFER_LOCK(xfer); return (USB_ERR_STALLED); } Modified: head/sys/dev/usb/usb_hub.c ============================================================================== --- head/sys/dev/usb/usb_hub.c Wed Feb 13 10:18:26 2013 (r246758) +++ head/sys/dev/usb/usb_hub.c Wed Feb 13 12:35:17 2013 (r246759) @@ -243,7 +243,9 @@ uhub_explore_sub(struct uhub_softc *sc, /* check if device should be re-enumerated */ if (child->flags.usb_mode == USB_MODE_HOST) { - usbd_enum_lock(child); + uint8_t do_unlock; + + do_unlock = usbd_enum_lock(child); if (child->re_enumerate_wait) { err = usbd_set_config_index(child, USB_UNCONFIG_INDEX); @@ -262,7 +264,8 @@ uhub_explore_sub(struct uhub_softc *sc, child->re_enumerate_wait = 0; err = 0; } - usbd_enum_unlock(child); + if (do_unlock) + usbd_enum_unlock(child); } /* check if probe and attach should be done */ @@ -716,6 +719,7 @@ uhub_explore(struct usb_device *udev) usb_error_t err; uint8_t portno; uint8_t x; + uint8_t do_unlock; hub = udev->hub; sc = hub->hubsoftc; @@ -737,7 +741,7 @@ uhub_explore(struct usb_device *udev) * Make sure we don't race against user-space applications * like LibUSB: */ - usbd_enum_lock(udev); + do_unlock = usbd_enum_lock(udev); for (x = 0; x != hub->nports; x++) { up = hub->ports + x; @@ -817,7 +821,8 @@ uhub_explore(struct usb_device *udev) up->restartcnt = 0; } - usbd_enum_unlock(udev); + if (do_unlock) + usbd_enum_unlock(udev); /* initial status checked */ sc->sc_flags |= UHUB_FLAG_DID_EXPLORE; Modified: head/sys/dev/usb/usb_request.c ============================================================================== --- head/sys/dev/usb/usb_request.c Wed Feb 13 10:18:26 2013 (r246758) +++ head/sys/dev/usb/usb_request.c Wed Feb 13 12:35:17 2013 (r246759) @@ -389,9 +389,8 @@ usbd_get_hr_func(struct usb_device *udev * than 30 seconds is treated like a 30 second timeout. This USB stack * does not allow control requests without a timeout. * - * NOTE: This function is thread safe. All calls to - * "usbd_do_request_flags" will be serialised by the use of an - * internal "sx_lock". + * NOTE: This function is thread safe. All calls to "usbd_do_request_flags" + * will be serialized by the use of the USB device enumeration lock. * * Returns: * 0: Success @@ -415,7 +414,7 @@ usbd_do_request_flags(struct usb_device uint16_t length; uint16_t temp; uint16_t acttemp; - uint8_t enum_locked; + uint8_t do_unlock; if (timeout < 50) { /* timeout is too small */ @@ -427,8 +426,6 @@ usbd_do_request_flags(struct usb_device } length = UGETW(req->wLength); - enum_locked = usbd_enum_is_locked(udev); - DPRINTFN(5, "udev=%p bmRequestType=0x%02x bRequest=0x%02x " "wValue=0x%02x%02x wIndex=0x%02x%02x wLength=0x%02x%02x\n", udev, req->bmRequestType, req->bRequest, @@ -459,17 +456,16 @@ usbd_do_request_flags(struct usb_device } /* - * We need to allow suspend and resume at this point, else the - * control transfer will timeout if the device is suspended! + * Grab the USB device enumeration SX-lock serialization is + * achieved when multiple threads are involved: */ - if (enum_locked) - usbd_sr_unlock(udev); + do_unlock = usbd_enum_lock(udev); /* - * Grab the default sx-lock so that serialisation - * is achieved when multiple threads are involved: + * We need to allow suspend and resume at this point, else the + * control transfer will timeout if the device is suspended! */ - sx_xlock(&udev->ctrl_sx); + usbd_sr_unlock(udev); hr_func = usbd_get_hr_func(udev); @@ -713,10 +709,10 @@ usbd_do_request_flags(struct usb_device USB_XFER_UNLOCK(xfer); done: - sx_xunlock(&udev->ctrl_sx); + usbd_sr_lock(udev); - if (enum_locked) - usbd_sr_lock(udev); + if (do_unlock) + usbd_enum_unlock(udev); if ((mtx != NULL) && (mtx != &Giant)) mtx_lock(mtx); Modified: head/sys/sys/param.h ============================================================================== --- head/sys/sys/param.h Wed Feb 13 10:18:26 2013 (r246758) +++ head/sys/sys/param.h Wed Feb 13 12:35:17 2013 (r246759) @@ -58,7 +58,7 @@ * in the range 5 to 9. */ #undef __FreeBSD_version -#define __FreeBSD_version 1000027 /* Master, propagated to newvers */ +#define __FreeBSD_version 1000028 /* Master, propagated to newvers */ /* * __FreeBSD_kernel__ indicates that this system uses the kernel of FreeBSD,
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201302131235.r1DCZHEp065195>