Commit 11c763cc authored by Alan Stern's avatar Alan Stern Committed by Greg Kroah-Hartman

[PATCH] USB: Updated USB device locking

This patch reintroduces the USB device locking code we tried out
earlier.  As before, it solves the problem of effectively locking all
the devices while drivers are registered and unregistered by introducing
an rwsem.  Unlike the earlier attempt, this version does not ever try to
acquire a lock re-entrantly.  I trust that will eliminate the races and
hang-ups you observed with the earlier version.  There are also copious
comments explaining exactly how things should work.

The patch interacts slightly with the locktree() code introduced by
David for suspend/resume support.  It doesn't change the functionality
at all; it just updates the routine to follow the new locking rules.
Signed-off-by: default avatarAlan Stern <stern@rowland.harvard.edu>
Signed-off-by: default avatarGreg Kroah-Hartman <greg@kroah.com>
parent fa4b40f4
...@@ -451,7 +451,7 @@ static char *usb_dump_string(char *start, char *end, const struct usb_device *de ...@@ -451,7 +451,7 @@ static char *usb_dump_string(char *start, char *end, const struct usb_device *de
* nbytes - the maximum number of bytes to write * nbytes - the maximum number of bytes to write
* skip_bytes - the number of bytes to skip before writing anything * skip_bytes - the number of bytes to skip before writing anything
* file_offset - the offset into the devices file on completion * file_offset - the offset into the devices file on completion
* The caller must own the usbdev->serialize semaphore. * The caller must own the device lock.
*/ */
static ssize_t usb_device_dump(char __user **buffer, size_t *nbytes, loff_t *skip_bytes, loff_t *file_offset, static ssize_t usb_device_dump(char __user **buffer, size_t *nbytes, loff_t *skip_bytes, loff_t *file_offset,
struct usb_device *usbdev, struct usb_bus *bus, int level, int index, int count) struct usb_device *usbdev, struct usb_bus *bus, int level, int index, int count)
...@@ -586,9 +586,9 @@ static ssize_t usb_device_read(struct file *file, char __user *buf, size_t nbyte ...@@ -586,9 +586,9 @@ static ssize_t usb_device_read(struct file *file, char __user *buf, size_t nbyte
/* recurse through all children of the root hub */ /* recurse through all children of the root hub */
if (!bus->root_hub) if (!bus->root_hub)
continue; continue;
down(&bus->root_hub->serialize); usb_lock_device(bus->root_hub);
ret = usb_device_dump(&buf, &nbytes, &skip_bytes, ppos, bus->root_hub, bus, 0, 0, 0); ret = usb_device_dump(&buf, &nbytes, &skip_bytes, ppos, bus->root_hub, bus, 0, 0, 0);
up(&bus->root_hub->serialize); usb_unlock_device(bus->root_hub);
if (ret < 0) { if (ret < 0) {
up(&usb_bus_list_lock); up(&usb_bus_list_lock);
return ret; return ret;
......
...@@ -113,7 +113,7 @@ static ssize_t usbdev_read(struct file *file, char __user *buf, size_t nbytes, l ...@@ -113,7 +113,7 @@ static ssize_t usbdev_read(struct file *file, char __user *buf, size_t nbytes, l
int i; int i;
pos = *ppos; pos = *ppos;
down(&dev->serialize); usb_lock_device(dev);
if (!connected(dev)) { if (!connected(dev)) {
ret = -ENODEV; ret = -ENODEV;
goto err; goto err;
...@@ -175,7 +175,7 @@ static ssize_t usbdev_read(struct file *file, char __user *buf, size_t nbytes, l ...@@ -175,7 +175,7 @@ static ssize_t usbdev_read(struct file *file, char __user *buf, size_t nbytes, l
} }
err: err:
up(&dev->serialize); usb_unlock_device(dev);
return ret; return ret;
} }
...@@ -517,7 +517,7 @@ static int usbdev_release(struct inode *inode, struct file *file) ...@@ -517,7 +517,7 @@ static int usbdev_release(struct inode *inode, struct file *file)
struct usb_device *dev = ps->dev; struct usb_device *dev = ps->dev;
unsigned int ifnum; unsigned int ifnum;
down(&dev->serialize); usb_lock_device(dev);
list_del_init(&ps->list); list_del_init(&ps->list);
if (connected(dev)) { if (connected(dev)) {
...@@ -526,7 +526,7 @@ static int usbdev_release(struct inode *inode, struct file *file) ...@@ -526,7 +526,7 @@ static int usbdev_release(struct inode *inode, struct file *file)
releaseintf(ps, ifnum); releaseintf(ps, ifnum);
destroy_all_async(ps); destroy_all_async(ps);
} }
up(&dev->serialize); usb_unlock_device(dev);
usb_put_dev(dev); usb_put_dev(dev);
ps->dev = NULL; ps->dev = NULL;
kfree(ps); kfree(ps);
...@@ -558,10 +558,10 @@ static int proc_control(struct dev_state *ps, void __user *arg) ...@@ -558,10 +558,10 @@ static int proc_control(struct dev_state *ps, void __user *arg)
snoop(&dev->dev, "control read: bRequest=%02x bRrequestType=%02x wValue=%04x wIndex=%04x\n", snoop(&dev->dev, "control read: bRequest=%02x bRrequestType=%02x wValue=%04x wIndex=%04x\n",
ctrl.bRequest, ctrl.bRequestType, ctrl.wValue, ctrl.wIndex); ctrl.bRequest, ctrl.bRequestType, ctrl.wValue, ctrl.wIndex);
up(&dev->serialize); usb_unlock_device(dev);
i = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0), ctrl.bRequest, ctrl.bRequestType, i = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0), ctrl.bRequest, ctrl.bRequestType,
ctrl.wValue, ctrl.wIndex, tbuf, ctrl.wLength, tmo); ctrl.wValue, ctrl.wIndex, tbuf, ctrl.wLength, tmo);
down(&dev->serialize); usb_lock_device(dev);
if ((i > 0) && ctrl.wLength) { if ((i > 0) && ctrl.wLength) {
if (usbfs_snoop) { if (usbfs_snoop) {
dev_info(&dev->dev, "control read: data "); dev_info(&dev->dev, "control read: data ");
...@@ -589,10 +589,10 @@ static int proc_control(struct dev_state *ps, void __user *arg) ...@@ -589,10 +589,10 @@ static int proc_control(struct dev_state *ps, void __user *arg)
printk ("%02x ", (unsigned char)(tbuf)[j]); printk ("%02x ", (unsigned char)(tbuf)[j]);
printk("\n"); printk("\n");
} }
up(&dev->serialize); usb_unlock_device(dev);
i = usb_control_msg(dev, usb_sndctrlpipe(dev, 0), ctrl.bRequest, ctrl.bRequestType, i = usb_control_msg(dev, usb_sndctrlpipe(dev, 0), ctrl.bRequest, ctrl.bRequestType,
ctrl.wValue, ctrl.wIndex, tbuf, ctrl.wLength, tmo); ctrl.wValue, ctrl.wIndex, tbuf, ctrl.wLength, tmo);
down(&dev->serialize); usb_lock_device(dev);
} }
free_page((unsigned long)tbuf); free_page((unsigned long)tbuf);
if (i<0) { if (i<0) {
...@@ -636,9 +636,9 @@ static int proc_bulk(struct dev_state *ps, void __user *arg) ...@@ -636,9 +636,9 @@ static int proc_bulk(struct dev_state *ps, void __user *arg)
kfree(tbuf); kfree(tbuf);
return -EINVAL; return -EINVAL;
} }
up(&dev->serialize); usb_unlock_device(dev);
i = usb_bulk_msg(dev, pipe, tbuf, len1, &len2, tmo); i = usb_bulk_msg(dev, pipe, tbuf, len1, &len2, tmo);
down(&dev->serialize); usb_lock_device(dev);
if (!i && len2) { if (!i && len2) {
if (copy_to_user(bulk.data, tbuf, len2)) { if (copy_to_user(bulk.data, tbuf, len2)) {
kfree(tbuf); kfree(tbuf);
...@@ -652,9 +652,9 @@ static int proc_bulk(struct dev_state *ps, void __user *arg) ...@@ -652,9 +652,9 @@ static int proc_bulk(struct dev_state *ps, void __user *arg)
return -EFAULT; return -EFAULT;
} }
} }
up(&dev->serialize); usb_unlock_device(dev);
i = usb_bulk_msg(dev, pipe, tbuf, len1, &len2, tmo); i = usb_bulk_msg(dev, pipe, tbuf, len1, &len2, tmo);
down(&dev->serialize); usb_lock_device(dev);
} }
kfree(tbuf); kfree(tbuf);
if (i < 0) { if (i < 0) {
...@@ -1025,9 +1025,9 @@ static int proc_reapurb(struct dev_state *ps, void __user *arg) ...@@ -1025,9 +1025,9 @@ static int proc_reapurb(struct dev_state *ps, void __user *arg)
break; break;
if (signal_pending(current)) if (signal_pending(current))
break; break;
up(&dev->serialize); usb_unlock_device(dev);
schedule(); schedule();
down(&dev->serialize); usb_lock_device(dev);
} }
remove_wait_queue(&ps->wait, &wait); remove_wait_queue(&ps->wait, &wait);
set_current_state(TASK_RUNNING); set_current_state(TASK_RUNNING);
...@@ -1150,7 +1150,11 @@ static int proc_ioctl (struct dev_state *ps, void __user *arg) ...@@ -1150,7 +1150,11 @@ static int proc_ioctl (struct dev_state *ps, void __user *arg)
/* let kernel drivers try to (re)bind to the interface */ /* let kernel drivers try to (re)bind to the interface */
case USBDEVFS_CONNECT: case USBDEVFS_CONNECT:
usb_unlock_device(ps->dev);
usb_lock_all_devices();
bus_rescan_devices(intf->dev.bus); bus_rescan_devices(intf->dev.bus);
usb_unlock_all_devices();
usb_lock_device(ps->dev);
break; break;
/* talk directly to the interface's driver */ /* talk directly to the interface's driver */
...@@ -1193,9 +1197,9 @@ static int usbdev_ioctl(struct inode *inode, struct file *file, unsigned int cmd ...@@ -1193,9 +1197,9 @@ static int usbdev_ioctl(struct inode *inode, struct file *file, unsigned int cmd
if (!(file->f_mode & FMODE_WRITE)) if (!(file->f_mode & FMODE_WRITE))
return -EPERM; return -EPERM;
down(&dev->serialize); usb_lock_device(dev);
if (!connected(dev)) { if (!connected(dev)) {
up(&dev->serialize); usb_unlock_device(dev);
return -ENODEV; return -ENODEV;
} }
...@@ -1295,7 +1299,7 @@ static int usbdev_ioctl(struct inode *inode, struct file *file, unsigned int cmd ...@@ -1295,7 +1299,7 @@ static int usbdev_ioctl(struct inode *inode, struct file *file, unsigned int cmd
ret = proc_ioctl(ps, p); ret = proc_ioctl(ps, p);
break; break;
} }
up(&dev->serialize); usb_unlock_device(dev);
if (ret >= 0) if (ret >= 0)
inode->i_atime = CURRENT_TIME; inode->i_atime = CURRENT_TIME;
return ret; return ret;
......
...@@ -797,9 +797,9 @@ int usb_register_root_hub (struct usb_device *usb_dev, struct device *parent_dev ...@@ -797,9 +797,9 @@ int usb_register_root_hub (struct usb_device *usb_dev, struct device *parent_dev
return (retval < 0) ? retval : -EMSGSIZE; return (retval < 0) ? retval : -EMSGSIZE;
} }
down (&usb_dev->serialize); usb_lock_device (usb_dev);
retval = usb_new_device (usb_dev); retval = usb_new_device (usb_dev);
up (&usb_dev->serialize); usb_unlock_device (usb_dev);
if (retval) { if (retval) {
usb_dev->bus->root_hub = NULL; usb_dev->bus->root_hub = NULL;
dev_err (parent_dev, "can't register root hub for %s, %d\n", dev_err (parent_dev, "can't register root hub for %s, %d\n",
......
...@@ -36,7 +36,9 @@ ...@@ -36,7 +36,9 @@
#include "hcd.h" #include "hcd.h"
#include "hub.h" #include "hub.h"
/* Protect struct usb_device state and children members */ /* Protect struct usb_device->state and ->children members
* Note: Both are also protected by ->serialize, except that ->state can
* change to USB_STATE_NOTATTACHED even when the semaphore isn't held. */
static spinlock_t device_state_lock = SPIN_LOCK_UNLOCKED; static spinlock_t device_state_lock = SPIN_LOCK_UNLOCKED;
/* khubd's worklist and its lock */ /* khubd's worklist and its lock */
...@@ -857,17 +859,6 @@ static void hub_start_disconnect(struct usb_device *hdev) ...@@ -857,17 +859,6 @@ static void hub_start_disconnect(struct usb_device *hdev)
} }
static void recursively_mark_NOTATTACHED(struct usb_device *udev)
{
int i;
for (i = 0; i < udev->maxchild; ++i) {
if (udev->children[i])
recursively_mark_NOTATTACHED(udev->children[i]);
}
udev->state = USB_STATE_NOTATTACHED;
}
/* grab device/port lock, returning index of that port (zero based). /* grab device/port lock, returning index of that port (zero based).
* protects the upstream link used by this device from concurrent * protects the upstream link used by this device from concurrent
* tree operations like suspend, resume, reset, and disconnect, which * tree operations like suspend, resume, reset, and disconnect, which
...@@ -884,21 +875,16 @@ static int locktree(struct usb_device *udev) ...@@ -884,21 +875,16 @@ static int locktree(struct usb_device *udev)
/* root hub is always the first lock in the series */ /* root hub is always the first lock in the series */
hdev = udev->parent; hdev = udev->parent;
if (!hdev) { if (!hdev) {
down(&udev->serialize); usb_lock_device(udev);
return 0; return 0;
} }
/* on the path from root to us, lock everything from /* on the path from root to us, lock everything from
* top down, dropping parent locks when not needed * top down, dropping parent locks when not needed
*
* NOTE: if disconnect were to ignore the locking, we'd need
* to get extra refcounts to everything since hdev->children
* and udev->parent could be invalidated while we work...
*/ */
t = locktree(hdev); t = locktree(hdev);
if (t < 0) if (t < 0)
return t; return t;
spin_lock_irq(&device_state_lock);
for (t = 0; t < hdev->maxchild; t++) { for (t = 0; t < hdev->maxchild; t++) {
if (hdev->children[t] == udev) { if (hdev->children[t] == udev) {
/* everything is fail-fast once disconnect /* everything is fail-fast once disconnect
...@@ -910,33 +896,45 @@ static int locktree(struct usb_device *udev) ...@@ -910,33 +896,45 @@ static int locktree(struct usb_device *udev)
/* when everyone grabs locks top->bottom, /* when everyone grabs locks top->bottom,
* non-overlapping work may be concurrent * non-overlapping work may be concurrent
*/ */
spin_unlock_irq(&device_state_lock);
down(&udev->serialize); down(&udev->serialize);
up(&hdev->serialize); up(&hdev->serialize);
return t; return t;
} }
} }
spin_unlock_irq(&device_state_lock); usb_unlock_device(hdev);
up(&hdev->serialize);
return -ENODEV; return -ENODEV;
} }
static void recursively_mark_NOTATTACHED(struct usb_device *udev)
{
int i;
for (i = 0; i < udev->maxchild; ++i) {
if (udev->children[i])
recursively_mark_NOTATTACHED(udev->children[i]);
}
udev->state = USB_STATE_NOTATTACHED;
}
/** /**
* usb_set_device_state - change a device's current state (usbcore, hcds) * usb_set_device_state - change a device's current state (usbcore, hcds)
* @udev: pointer to device whose state should be changed * @udev: pointer to device whose state should be changed
* @new_state: new state value to be stored * @new_state: new state value to be stored
* *
* udev->state is _not_ protected by the device lock. This * udev->state is _not_ fully protected by the device lock. Although
* most transitions are made only while holding the lock, the state can
* can change to USB_STATE_NOTATTACHED at almost any time. This
* is so that devices can be marked as disconnected as soon as possible, * is so that devices can be marked as disconnected as soon as possible,
* without having to wait for the semaphore to be released. Instead, * without having to wait for any semaphores to be released. As a result,
* changes to the state must be protected by the device_state_lock spinlock. * all changes to any device's state must be protected by the
* device_state_lock spinlock.
* *
* Once a device has been added to the device tree, all changes to its state * Once a device has been added to the device tree, all changes to its state
* should be made using this routine. The state should _not_ be set directly. * should be made using this routine. The state should _not_ be set directly.
* *
* If udev->state is already USB_STATE_NOTATTACHED then no change is made. * If udev->state is already USB_STATE_NOTATTACHED then no change is made.
* Otherwise udev->state is set to new_state, and if new_state is * Otherwise udev->state is set to new_state, and if new_state is
* USB_STATE_NOTATTACHED then all of udev's descendant's states are also set * USB_STATE_NOTATTACHED then all of udev's descendants' states are also set
* to USB_STATE_NOTATTACHED. * to USB_STATE_NOTATTACHED.
*/ */
void usb_set_device_state(struct usb_device *udev, void usb_set_device_state(struct usb_device *udev,
...@@ -987,11 +985,12 @@ static void release_address(struct usb_device *udev) ...@@ -987,11 +985,12 @@ static void release_address(struct usb_device *udev)
/** /**
* usb_disconnect - disconnect a device (usbcore-internal) * usb_disconnect - disconnect a device (usbcore-internal)
* @pdev: pointer to device being disconnected, into a locked hub * @pdev: pointer to device being disconnected
* Context: !in_interrupt () * Context: !in_interrupt ()
* *
* Something got disconnected. Get rid of it, and all of its children. * Something got disconnected. Get rid of it and all of its children.
* If *pdev is a normal device then the parent hub should be locked. *
* If *pdev is a normal device then the parent hub must already be locked.
* If *pdev is a root hub then this routine will acquire the * If *pdev is a root hub then this routine will acquire the
* usb_bus_list_lock on behalf of the caller. * usb_bus_list_lock on behalf of the caller.
* *
...@@ -1017,9 +1016,11 @@ void usb_disconnect(struct usb_device **pdev) ...@@ -1017,9 +1016,11 @@ void usb_disconnect(struct usb_device **pdev)
usb_set_device_state(udev, USB_STATE_NOTATTACHED); usb_set_device_state(udev, USB_STATE_NOTATTACHED);
/* lock the bus list on behalf of HCDs unregistering their root hubs */ /* lock the bus list on behalf of HCDs unregistering their root hubs */
if (!udev->parent) if (!udev->parent) {
down(&usb_bus_list_lock); down(&usb_bus_list_lock);
down(&udev->serialize); usb_lock_device(udev);
} else
down(&udev->serialize);
dev_info (&udev->dev, "USB disconnect, address %d\n", udev->devnum); dev_info (&udev->dev, "USB disconnect, address %d\n", udev->devnum);
...@@ -1044,14 +1045,16 @@ void usb_disconnect(struct usb_device **pdev) ...@@ -1044,14 +1045,16 @@ void usb_disconnect(struct usb_device **pdev)
usbfs_remove_device(udev); usbfs_remove_device(udev);
usb_remove_sysfs_dev_files(udev); usb_remove_sysfs_dev_files(udev);
/* Avoid races with recursively_mark_NOTATTACHED() and locktree() */ /* Avoid races with recursively_mark_NOTATTACHED() */
spin_lock_irq(&device_state_lock); spin_lock_irq(&device_state_lock);
*pdev = NULL; *pdev = NULL;
spin_unlock_irq(&device_state_lock); spin_unlock_irq(&device_state_lock);
up(&udev->serialize); if (!udev->parent) {
if (!udev->parent) usb_unlock_device(udev);
up(&usb_bus_list_lock); up(&usb_bus_list_lock);
} else
up(&udev->serialize);
device_unregister(&udev->dev); device_unregister(&udev->dev);
} }
...@@ -1516,16 +1519,14 @@ static int __usb_suspend_device (struct usb_device *udev, int port, u32 state) ...@@ -1516,16 +1519,14 @@ static int __usb_suspend_device (struct usb_device *udev, int port, u32 state)
{ {
int status; int status;
/* caller owns the udev device lock */
if (port < 0) if (port < 0)
return port; return port;
/* NOTE: udev->serialize released on all real returns! */
if (state <= udev->dev.power.power_state if (state <= udev->dev.power.power_state
|| state < PM_SUSPEND_MEM || state < PM_SUSPEND_MEM
|| udev->state == USB_STATE_SUSPENDED || udev->state == USB_STATE_SUSPENDED
|| udev->state == USB_STATE_NOTATTACHED) { || udev->state == USB_STATE_NOTATTACHED) {
up(&udev->serialize);
return 0; return 0;
} }
...@@ -1605,7 +1606,6 @@ static int __usb_suspend_device (struct usb_device *udev, int port, u32 state) ...@@ -1605,7 +1606,6 @@ static int __usb_suspend_device (struct usb_device *udev, int port, u32 state)
if (status == 0) if (status == 0)
udev->dev.power.power_state = state; udev->dev.power.power_state = state;
up(&udev->serialize);
return status; return status;
} }
...@@ -1629,7 +1629,15 @@ static int __usb_suspend_device (struct usb_device *udev, int port, u32 state) ...@@ -1629,7 +1629,15 @@ static int __usb_suspend_device (struct usb_device *udev, int port, u32 state)
*/ */
int usb_suspend_device(struct usb_device *udev, u32 state) int usb_suspend_device(struct usb_device *udev, u32 state)
{ {
return __usb_suspend_device(udev, locktree(udev), state); int port, status;
port = locktree(udev);
if (port < 0)
return port;
status = __usb_suspend_device(udev, port, state);
usb_unlock_device(udev);
return status;
} }
/* /*
...@@ -1642,7 +1650,7 @@ static int finish_port_resume(struct usb_device *udev) ...@@ -1642,7 +1650,7 @@ static int finish_port_resume(struct usb_device *udev)
int status; int status;
u16 devstatus; u16 devstatus;
/* caller owns udev->serialize */ /* caller owns the udev device lock */
dev_dbg(&udev->dev, "usb resume\n"); dev_dbg(&udev->dev, "usb resume\n");
udev->dev.power.power_state = PM_SUSPEND_ON; udev->dev.power.power_state = PM_SUSPEND_ON;
...@@ -1822,10 +1830,12 @@ int usb_resume_device(struct usb_device *udev) ...@@ -1822,10 +1830,12 @@ int usb_resume_device(struct usb_device *udev)
status); status);
} }
up(&udev->serialize); usb_unlock_device(udev);
/* rebind drivers that had no suspend() */ /* rebind drivers that had no suspend() */
usb_lock_all_devices();
bus_rescan_devices(&usb_bus_type); bus_rescan_devices(&usb_bus_type);
usb_unlock_all_devices();
return status; return status;
} }
...@@ -1867,6 +1877,7 @@ static int hub_suspend(struct usb_interface *intf, u32 state) ...@@ -1867,6 +1877,7 @@ static int hub_suspend(struct usb_interface *intf, u32 state)
continue; continue;
down(&udev->serialize); down(&udev->serialize);
status = __usb_suspend_device(udev, port, state); status = __usb_suspend_device(udev, port, state);
up(&udev->serialize);
if (status < 0) if (status < 0)
dev_dbg(&intf->dev, "suspend port %d --> %d\n", dev_dbg(&intf->dev, "suspend port %d --> %d\n",
port, status); port, status);
...@@ -2595,7 +2606,7 @@ static void hub_events(void) ...@@ -2595,7 +2606,7 @@ static void hub_events(void)
} }
loop: loop:
up(&hdev->serialize); usb_unlock_device(hdev);
usb_put_dev(hdev); usb_put_dev(hdev);
} /* end while (1) */ } /* end while (1) */
......
...@@ -1132,6 +1132,8 @@ int usb_set_interface(struct usb_device *dev, int interface, int alternate) ...@@ -1132,6 +1132,8 @@ int usb_set_interface(struct usb_device *dev, int interface, int alternate)
* use usb_set_interface() on the interfaces it claims. Resetting the whole * use usb_set_interface() on the interfaces it claims. Resetting the whole
* configuration would affect other drivers' interfaces. * configuration would affect other drivers' interfaces.
* *
* The caller must own the device lock.
*
* Returns zero on success, else a negative error code. * Returns zero on success, else a negative error code.
*/ */
int usb_reset_configuration(struct usb_device *dev) int usb_reset_configuration(struct usb_device *dev)
...@@ -1142,9 +1144,9 @@ int usb_reset_configuration(struct usb_device *dev) ...@@ -1142,9 +1144,9 @@ int usb_reset_configuration(struct usb_device *dev)
if (dev->state == USB_STATE_SUSPENDED) if (dev->state == USB_STATE_SUSPENDED)
return -EHOSTUNREACH; return -EHOSTUNREACH;
/* caller must own dev->serialize (config won't change) /* caller must have locked the device and must own
* and the usb bus readlock (so driver bindings are stable); * the usb bus readlock (so driver bindings are stable);
* so calls during probe() are fine * calls during probe() are fine
*/ */
for (i = 1; i < 16; ++i) { for (i = 1; i < 16; ++i) {
...@@ -1199,7 +1201,7 @@ static void release_interface(struct device *dev) ...@@ -1199,7 +1201,7 @@ static void release_interface(struct device *dev)
* usb_set_configuration - Makes a particular device setting be current * usb_set_configuration - Makes a particular device setting be current
* @dev: the device whose configuration is being updated * @dev: the device whose configuration is being updated
* @configuration: the configuration being chosen. * @configuration: the configuration being chosen.
* Context: !in_interrupt(), caller holds dev->serialize * Context: !in_interrupt(), caller owns the device lock
* *
* This is used to enable non-default device modes. Not all devices * This is used to enable non-default device modes. Not all devices
* use this kind of configurability; many devices only have one * use this kind of configurability; many devices only have one
...@@ -1220,8 +1222,8 @@ static void release_interface(struct device *dev) ...@@ -1220,8 +1222,8 @@ static void release_interface(struct device *dev)
* usb_set_interface(). * usb_set_interface().
* *
* This call is synchronous. The calling context must be able to sleep, * This call is synchronous. The calling context must be able to sleep,
* and must not hold the driver model lock for USB; usb device driver * must own the device lock, and must not hold the driver model's USB
* probe() methods may not use this routine. * bus rwsem; usb device driver probe() methods cannot use this routine.
* *
* Returns zero on success, or else the status code returned by the * Returns zero on success, or else the status code returned by the
* underlying call that failed. On succesful completion, each interface * underlying call that failed. On succesful completion, each interface
...@@ -1236,8 +1238,6 @@ int usb_set_configuration(struct usb_device *dev, int configuration) ...@@ -1236,8 +1238,6 @@ int usb_set_configuration(struct usb_device *dev, int configuration)
struct usb_interface **new_interfaces = NULL; struct usb_interface **new_interfaces = NULL;
int n, nintf; int n, nintf;
/* dev->serialize guards all config changes */
for (i = 0; i < dev->descriptor.bNumConfigurations; i++) { for (i = 0; i < dev->descriptor.bNumConfigurations; i++) {
if (dev->config[i].desc.bConfigurationValue == configuration) { if (dev->config[i].desc.bConfigurationValue == configuration) {
cp = &dev->config[i]; cp = &dev->config[i];
......
...@@ -55,9 +55,9 @@ set_bConfigurationValue (struct device *dev, const char *buf, size_t count) ...@@ -55,9 +55,9 @@ set_bConfigurationValue (struct device *dev, const char *buf, size_t count)
if (sscanf (buf, "%u", &config) != 1 || config > 255) if (sscanf (buf, "%u", &config) != 1 || config > 255)
return -EINVAL; return -EINVAL;
down(&udev->serialize); usb_lock_device(udev);
value = usb_set_configuration (udev, config); value = usb_set_configuration (udev, config);
up(&udev->serialize); usb_unlock_device(udev);
return (value < 0) ? value : count; return (value < 0) ? value : count;
} }
......
...@@ -39,6 +39,7 @@ ...@@ -39,6 +39,7 @@
#include <linux/spinlock.h> #include <linux/spinlock.h>
#include <linux/errno.h> #include <linux/errno.h>
#include <linux/smp_lock.h> #include <linux/smp_lock.h>
#include <linux/rwsem.h>
#include <linux/usb.h> #include <linux/usb.h>
#include <asm/io.h> #include <asm/io.h>
...@@ -62,6 +63,8 @@ const char *usbcore_name = "usbcore"; ...@@ -62,6 +63,8 @@ const char *usbcore_name = "usbcore";
int nousb; /* Disable USB when built into kernel image */ int nousb; /* Disable USB when built into kernel image */
/* Not honored on modular build */ /* Not honored on modular build */
static DECLARE_RWSEM(usb_all_devices_rwsem);
static int generic_probe (struct device *dev) static int generic_probe (struct device *dev)
{ {
...@@ -151,7 +154,9 @@ int usb_register(struct usb_driver *new_driver) ...@@ -151,7 +154,9 @@ int usb_register(struct usb_driver *new_driver)
new_driver->driver.probe = usb_probe_interface; new_driver->driver.probe = usb_probe_interface;
new_driver->driver.remove = usb_unbind_interface; new_driver->driver.remove = usb_unbind_interface;
usb_lock_all_devices();
retval = driver_register(&new_driver->driver); retval = driver_register(&new_driver->driver);
usb_unlock_all_devices();
if (!retval) { if (!retval) {
pr_info("%s: registered new driver %s\n", pr_info("%s: registered new driver %s\n",
...@@ -180,7 +185,9 @@ void usb_deregister(struct usb_driver *driver) ...@@ -180,7 +185,9 @@ void usb_deregister(struct usb_driver *driver)
{ {
pr_info("%s: deregistering driver %s\n", usbcore_name, driver->name); pr_info("%s: deregistering driver %s\n", usbcore_name, driver->name);
usb_lock_all_devices();
driver_unregister (&driver->driver); driver_unregister (&driver->driver);
usb_unlock_all_devices();
usbfs_update_special(); usbfs_update_special();
} }
...@@ -202,7 +209,7 @@ void usb_deregister(struct usb_driver *driver) ...@@ -202,7 +209,7 @@ void usb_deregister(struct usb_driver *driver)
* alternate settings available for this interfaces. * alternate settings available for this interfaces.
* *
* Don't call this function unless you are bound to one of the interfaces * Don't call this function unless you are bound to one of the interfaces
* on this device or you own the dev->serialize semaphore! * on this device or you have locked the device!
*/ */
struct usb_interface *usb_ifnum_to_if(struct usb_device *dev, unsigned ifnum) struct usb_interface *usb_ifnum_to_if(struct usb_device *dev, unsigned ifnum)
{ {
...@@ -235,7 +242,7 @@ struct usb_interface *usb_ifnum_to_if(struct usb_device *dev, unsigned ifnum) ...@@ -235,7 +242,7 @@ struct usb_interface *usb_ifnum_to_if(struct usb_device *dev, unsigned ifnum)
* drivers avoid such mistakes. * drivers avoid such mistakes.
* *
* Don't call this function unless you are bound to the intf interface * Don't call this function unless you are bound to the intf interface
* or you own the device's ->serialize semaphore! * or you have locked the device!
*/ */
struct usb_host_interface *usb_altnum_to_altsetting(struct usb_interface *intf, struct usb_host_interface *usb_altnum_to_altsetting(struct usb_interface *intf,
unsigned int altnum) unsigned int altnum)
...@@ -303,11 +310,12 @@ usb_epnum_to_ep_desc(struct usb_device *dev, unsigned epnum) ...@@ -303,11 +310,12 @@ usb_epnum_to_ep_desc(struct usb_device *dev, unsigned epnum)
* way to bind to an interface is to return the private data from * way to bind to an interface is to return the private data from
* the driver's probe() method. * the driver's probe() method.
* *
* Callers must own the driver model's usb bus writelock. So driver * Callers must own the device lock and the driver model's usb_bus_type.subsys
* probe() entries don't need extra locking, but other call contexts * writelock. So driver probe() entries don't need extra locking,
* may need to explicitly claim that lock. * but other call contexts may need to explicitly claim those locks.
*/ */
int usb_driver_claim_interface(struct usb_driver *driver, struct usb_interface *iface, void* priv) int usb_driver_claim_interface(struct usb_driver *driver,
struct usb_interface *iface, void* priv)
{ {
struct device *dev = &iface->dev; struct device *dev = &iface->dev;
...@@ -336,8 +344,8 @@ int usb_driver_claim_interface(struct usb_driver *driver, struct usb_interface * ...@@ -336,8 +344,8 @@ int usb_driver_claim_interface(struct usb_driver *driver, struct usb_interface *
* also causes the driver disconnect() method to be called. * also causes the driver disconnect() method to be called.
* *
* This call is synchronous, and may not be used in an interrupt context. * This call is synchronous, and may not be used in an interrupt context.
* Callers must own the usb_device serialize semaphore and the driver model's * Callers must own the device lock and the driver model's usb_bus_type.subsys
* usb bus writelock. So driver disconnect() entries don't need extra locking, * writelock. So driver disconnect() entries don't need extra locking,
* but other call contexts may need to explicitly claim those locks. * but other call contexts may need to explicitly claim those locks.
*/ */
void usb_driver_release_interface(struct usb_driver *driver, void usb_driver_release_interface(struct usb_driver *driver,
...@@ -830,6 +838,112 @@ void usb_put_intf(struct usb_interface *intf) ...@@ -830,6 +838,112 @@ void usb_put_intf(struct usb_interface *intf)
put_device(&intf->dev); put_device(&intf->dev);
} }
/* USB device locking
*
* Although locking USB devices should be straightforward, it is
* complicated by the way the driver-model core works. When a new USB
* driver is registered or unregistered, the core will automatically
* probe or disconnect all matching interfaces on all USB devices while
* holding the USB subsystem writelock. There's no good way for us to
* tell which devices will be used or to lock them beforehand; our only
* option is to effectively lock all the USB devices.
*
* We do that by using a private rw-semaphore, usb_all_devices_rwsem.
* When locking an individual device you must first acquire the rwsem's
* readlock. When a driver is registered or unregistered the writelock
* must be held. These actions are encapsulated in the subroutines
* below, so all a driver needs to do is call usb_lock_device() and
* usb_unlock_device().
*
* Complications arise when several devices are to be locked at the same
* time. Only hub-aware drivers that are part of usbcore ever have to
* do this; nobody else needs to worry about it. The problem is that
* usb_lock_device() must not be called to lock a second device since it
* would acquire the rwsem's readlock reentrantly, leading to deadlock if
* another thread was waiting for the writelock. The solution is simple:
*
* When locking more than one device, call usb_lock_device()
* to lock the first one. Lock the others by calling
* down(&udev->serialize) directly.
*
* When unlocking multiple devices, use up(&udev->serialize)
* to unlock all but the last one. Unlock the last one by
* calling usb_unlock_device().
*
* When locking both a device and its parent, always lock the
* the parent first.
*/
/**
* usb_lock_device - acquire the lock for a usb device structure
* @udev: device that's being locked
*
* Use this routine when you don't hold any other device locks;
* to acquire nested inner locks call down(&udev->serialize) directly.
* This is necessary for proper interaction with usb_lock_all_devices().
*/
void usb_lock_device(struct usb_device *udev)
{
down_read(&usb_all_devices_rwsem);
down(&udev->serialize);
}
/**
* usb_trylock_device - attempt to acquire the lock for a usb device structure
* @udev: device that's being locked
*
* Don't use this routine if you already hold a device lock;
* use down_trylock(&udev->serialize) instead.
* This is necessary for proper interaction with usb_lock_all_devices().
*
* Returns 1 if successful, 0 if contention.
*/
int usb_trylock_device(struct usb_device *udev)
{
if (!down_read_trylock(&usb_all_devices_rwsem))
return 0;
if (down_trylock(&udev->serialize)) {
up_read(&usb_all_devices_rwsem);
return 0;
}
return 1;
}
/**
* usb_unlock_device - release the lock for a usb device structure
* @udev: device that's being unlocked
*
* Use this routine when releasing the only device lock you hold;
* to release inner nested locks call up(&udev->serialize) directly.
* This is necessary for proper interaction with usb_lock_all_devices().
*/
void usb_unlock_device(struct usb_device *udev)
{
up(&udev->serialize);
up_read(&usb_all_devices_rwsem);
}
/**
* usb_lock_all_devices - acquire the lock for all usb device structures
*
* This is necessary when registering a new driver or probing a bus,
* since the driver-model core may try to use any usb_device.
*/
void usb_lock_all_devices(void)
{
down_write(&usb_all_devices_rwsem);
}
/**
* usb_unlock_all_devices - release the lock for all usb device structures
*/
void usb_unlock_all_devices(void)
{
up_write(&usb_all_devices_rwsem);
}
static struct usb_device *match_device(struct usb_device *dev, static struct usb_device *match_device(struct usb_device *dev,
u16 vendor_id, u16 product_id) u16 vendor_id, u16 product_id)
{ {
...@@ -851,8 +965,10 @@ static struct usb_device *match_device(struct usb_device *dev, ...@@ -851,8 +965,10 @@ static struct usb_device *match_device(struct usb_device *dev,
/* look through all of the children of this device */ /* look through all of the children of this device */
for (child = 0; child < dev->maxchild; ++child) { for (child = 0; child < dev->maxchild; ++child) {
if (dev->children[child]) { if (dev->children[child]) {
down(&dev->children[child]->serialize);
ret_dev = match_device(dev->children[child], ret_dev = match_device(dev->children[child],
vendor_id, product_id); vendor_id, product_id);
up(&dev->children[child]->serialize);
if (ret_dev) if (ret_dev)
goto exit; goto exit;
} }
...@@ -887,7 +1003,9 @@ struct usb_device *usb_find_device(u16 vendor_id, u16 product_id) ...@@ -887,7 +1003,9 @@ struct usb_device *usb_find_device(u16 vendor_id, u16 product_id)
bus = container_of(buslist, struct usb_bus, bus_list); bus = container_of(buslist, struct usb_bus, bus_list);
if (!bus->root_hub) if (!bus->root_hub)
continue; continue;
usb_lock_device(bus->root_hub);
dev = match_device(bus->root_hub, vendor_id, product_id); dev = match_device(bus->root_hub, vendor_id, product_id);
usb_unlock_device(bus->root_hub);
if (dev) if (dev)
goto exit; goto exit;
} }
...@@ -1373,6 +1491,10 @@ EXPORT_SYMBOL(usb_put_dev); ...@@ -1373,6 +1491,10 @@ EXPORT_SYMBOL(usb_put_dev);
EXPORT_SYMBOL(usb_get_dev); EXPORT_SYMBOL(usb_get_dev);
EXPORT_SYMBOL(usb_hub_tt_clear_buffer); EXPORT_SYMBOL(usb_hub_tt_clear_buffer);
EXPORT_SYMBOL(usb_lock_device);
EXPORT_SYMBOL(usb_trylock_device);
EXPORT_SYMBOL(usb_unlock_device);
EXPORT_SYMBOL(usb_driver_claim_interface); EXPORT_SYMBOL(usb_driver_claim_interface);
EXPORT_SYMBOL(usb_driver_release_interface); EXPORT_SYMBOL(usb_driver_release_interface);
EXPORT_SYMBOL(usb_match_id); EXPORT_SYMBOL(usb_match_id);
......
...@@ -22,6 +22,8 @@ extern int usb_get_device_descriptor(struct usb_device *dev, ...@@ -22,6 +22,8 @@ extern int usb_get_device_descriptor(struct usb_device *dev,
unsigned int size); unsigned int size);
extern int usb_set_configuration(struct usb_device *dev, int configuration); extern int usb_set_configuration(struct usb_device *dev, int configuration);
extern void usb_lock_all_devices(void);
extern void usb_unlock_all_devices(void);
/* for labeling diagnostics */ /* for labeling diagnostics */
extern const char *usbcore_name; extern const char *usbcore_name;
......
...@@ -81,7 +81,7 @@ static int ehci_hub_suspend (struct usb_hcd *hcd) ...@@ -81,7 +81,7 @@ static int ehci_hub_suspend (struct usb_hcd *hcd)
} }
/* caller owns root->serialize, and should reset/reinit on error */ /* caller has locked the root hub, and should reset/reinit on error */
static int ehci_hub_resume (struct usb_hcd *hcd) static int ehci_hub_resume (struct usb_hcd *hcd)
{ {
struct ehci_hcd *ehci = hcd_to_ehci (hcd); struct ehci_hcd *ehci = hcd_to_ehci (hcd);
......
...@@ -138,7 +138,7 @@ static inline struct ed *find_head (struct ed *ed) ...@@ -138,7 +138,7 @@ static inline struct ed *find_head (struct ed *ed)
return ed; return ed;
} }
/* caller owns root->serialize */ /* caller has locked the root hub */
static int ohci_hub_resume (struct usb_hcd *hcd) static int ohci_hub_resume (struct usb_hcd *hcd)
{ {
struct ohci_hcd *ohci = hcd_to_ohci (hcd); struct ohci_hcd *ohci = hcd_to_ohci (hcd);
...@@ -282,9 +282,9 @@ static void ohci_rh_resume (void *_hcd) ...@@ -282,9 +282,9 @@ static void ohci_rh_resume (void *_hcd)
{ {
struct usb_hcd *hcd = _hcd; struct usb_hcd *hcd = _hcd;
down (&hcd->self.root_hub->serialize); usb_lock_device (hcd->self.root_hub);
(void) ohci_hub_resume (hcd); (void) ohci_hub_resume (hcd);
up (&hcd->self.root_hub->serialize); usb_unlock_device (hcd->self.root_hub);
} }
#else #else
...@@ -362,12 +362,12 @@ ohci_hub_status_data (struct usb_hcd *hcd, char *buf) ...@@ -362,12 +362,12 @@ ohci_hub_status_data (struct usb_hcd *hcd, char *buf)
&& ((OHCI_CTRL_HCFS | OHCI_SCHED_ENABLES) && ((OHCI_CTRL_HCFS | OHCI_SCHED_ENABLES)
& ohci->hc_control) & ohci->hc_control)
== OHCI_USB_OPER == OHCI_USB_OPER
&& down_trylock (&hcd->self.root_hub->serialize) == 0 && usb_trylock_device (hcd->self.root_hub)
) { ) {
ohci_vdbg (ohci, "autosuspend\n"); ohci_vdbg (ohci, "autosuspend\n");
(void) ohci_hub_suspend (&ohci->hcd); (void) ohci_hub_suspend (&ohci->hcd);
ohci->hcd.state = USB_STATE_RUNNING; ohci->hcd.state = USB_STATE_RUNNING;
up (&hcd->self.root_hub->serialize); usb_unlock_device (hcd->self.root_hub);
} }
#endif #endif
......
...@@ -121,9 +121,9 @@ static int ohci_pci_suspend (struct usb_hcd *hcd, u32 state) ...@@ -121,9 +121,9 @@ static int ohci_pci_suspend (struct usb_hcd *hcd, u32 state)
#ifdef CONFIG_USB_SUSPEND #ifdef CONFIG_USB_SUSPEND
(void) usb_suspend_device (hcd->self.root_hub, state); (void) usb_suspend_device (hcd->self.root_hub, state);
#else #else
down (&hcd->self.root_hub->serialize); usb_lock_device (hcd->self.root_hub);
(void) ohci_hub_suspend (hcd); (void) ohci_hub_suspend (hcd);
up (&hcd->self.root_hub->serialize); usb_unlock_device (hcd->self.root_hub);
#endif #endif
/* let things settle down a bit */ /* let things settle down a bit */
...@@ -169,9 +169,9 @@ static int ohci_pci_resume (struct usb_hcd *hcd) ...@@ -169,9 +169,9 @@ static int ohci_pci_resume (struct usb_hcd *hcd)
/* get extra cleanup even if remote wakeup isn't in use */ /* get extra cleanup even if remote wakeup isn't in use */
retval = usb_resume_device (hcd->self.root_hub); retval = usb_resume_device (hcd->self.root_hub);
#else #else
down (&hcd->self.root_hub->serialize); usb_lock_device (hcd->self.root_hub);
retval = ohci_hub_resume (hcd); retval = ohci_hub_resume (hcd);
up (&hcd->self.root_hub->serialize); usb_unlock_device (hcd->self.root_hub);
#endif #endif
if (retval == 0) { if (retval == 0) {
......
...@@ -281,6 +281,14 @@ struct usb_bus { ...@@ -281,6 +281,14 @@ struct usb_bus {
struct usb_tt; struct usb_tt;
/*
* struct usb_device - kernel's representation of a USB device
*
* FIXME: Write the kerneldoc!
*
* Usbcore drivers should not set usbdev->state directly. Instead use
* usb_set_device_state().
*/
struct usb_device { struct usb_device {
int devnum; /* Address on USB bus */ int devnum; /* Address on USB bus */
char devpath [16]; /* Use in messages: /port/port/... */ char devpath [16]; /* Use in messages: /port/port/... */
...@@ -331,6 +339,10 @@ struct usb_device { ...@@ -331,6 +339,10 @@ struct usb_device {
extern struct usb_device *usb_get_dev(struct usb_device *dev); extern struct usb_device *usb_get_dev(struct usb_device *dev);
extern void usb_put_dev(struct usb_device *dev); extern void usb_put_dev(struct usb_device *dev);
extern void usb_lock_device(struct usb_device *udev);
extern int usb_trylock_device(struct usb_device *udev);
extern void usb_unlock_device(struct usb_device *udev);
/* mostly for devices emulating SCSI over USB */ /* mostly for devices emulating SCSI over USB */
extern int usb_reset_device(struct usb_device *dev); extern int usb_reset_device(struct usb_device *dev);
extern int __usb_reset_device(struct usb_device *dev); extern int __usb_reset_device(struct usb_device *dev);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment