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,
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "[email protected]"
