USB: change locking for device-level autosuspend
This patch (as1323) changes the locking requirements for usb_autosuspend_device(), usb_autoresume_device(), and usb_try_autosuspend_device(). This isn't a very important change; mainly it's meant to make the locking more uniform. The most tricky part of the patch involves changes to usbdev_open(). To avoid an ABBA locking problem, it was necessary to reduce the region protected by usbfs_mutex. Since that mutex now protects only against simultaneous open and remove, this posed no difficulty -- its scope was larger than necessary. And it turns out that usbfs_mutex is no longer needed in usbdev_release() at all. The list of usbfs "ps" structures is now protected by the device lock instead of by usbfs_mutex. Signed-off-by: Alan Stern <stern@rowland.harvard.edu> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
This commit is contained in:
parent
0f3dda9f7f
commit
62e299e61a
|
@ -654,19 +654,21 @@ static int usbdev_open(struct inode *inode, struct file *file)
|
||||||
int ret;
|
int ret;
|
||||||
|
|
||||||
lock_kernel();
|
lock_kernel();
|
||||||
/* Protect against simultaneous removal or release */
|
|
||||||
mutex_lock(&usbfs_mutex);
|
|
||||||
|
|
||||||
ret = -ENOMEM;
|
ret = -ENOMEM;
|
||||||
ps = kmalloc(sizeof(struct dev_state), GFP_KERNEL);
|
ps = kmalloc(sizeof(struct dev_state), GFP_KERNEL);
|
||||||
if (!ps)
|
if (!ps)
|
||||||
goto out;
|
goto out_free_ps;
|
||||||
|
|
||||||
ret = -ENODEV;
|
ret = -ENODEV;
|
||||||
|
|
||||||
|
/* Protect against simultaneous removal or release */
|
||||||
|
mutex_lock(&usbfs_mutex);
|
||||||
|
|
||||||
/* usbdev device-node */
|
/* usbdev device-node */
|
||||||
if (imajor(inode) == USB_DEVICE_MAJOR)
|
if (imajor(inode) == USB_DEVICE_MAJOR)
|
||||||
dev = usbdev_lookup_by_devt(inode->i_rdev);
|
dev = usbdev_lookup_by_devt(inode->i_rdev);
|
||||||
|
|
||||||
#ifdef CONFIG_USB_DEVICEFS
|
#ifdef CONFIG_USB_DEVICEFS
|
||||||
/* procfs file */
|
/* procfs file */
|
||||||
if (!dev) {
|
if (!dev) {
|
||||||
|
@ -678,13 +680,19 @@ static int usbdev_open(struct inode *inode, struct file *file)
|
||||||
dev = NULL;
|
dev = NULL;
|
||||||
}
|
}
|
||||||
#endif
|
#endif
|
||||||
if (!dev || dev->state == USB_STATE_NOTATTACHED)
|
mutex_unlock(&usbfs_mutex);
|
||||||
goto out;
|
|
||||||
|
if (!dev)
|
||||||
|
goto out_free_ps;
|
||||||
|
|
||||||
|
usb_lock_device(dev);
|
||||||
|
if (dev->state == USB_STATE_NOTATTACHED)
|
||||||
|
goto out_unlock_device;
|
||||||
|
|
||||||
ret = usb_autoresume_device(dev);
|
ret = usb_autoresume_device(dev);
|
||||||
if (ret)
|
if (ret)
|
||||||
goto out;
|
goto out_unlock_device;
|
||||||
|
|
||||||
ret = 0;
|
|
||||||
ps->dev = dev;
|
ps->dev = dev;
|
||||||
ps->file = file;
|
ps->file = file;
|
||||||
spin_lock_init(&ps->lock);
|
spin_lock_init(&ps->lock);
|
||||||
|
@ -702,14 +710,17 @@ static int usbdev_open(struct inode *inode, struct file *file)
|
||||||
smp_wmb();
|
smp_wmb();
|
||||||
list_add_tail(&ps->list, &dev->filelist);
|
list_add_tail(&ps->list, &dev->filelist);
|
||||||
file->private_data = ps;
|
file->private_data = ps;
|
||||||
|
usb_unlock_device(dev);
|
||||||
snoop(&dev->dev, "opened by process %d: %s\n", task_pid_nr(current),
|
snoop(&dev->dev, "opened by process %d: %s\n", task_pid_nr(current),
|
||||||
current->comm);
|
current->comm);
|
||||||
out:
|
unlock_kernel();
|
||||||
if (ret) {
|
return ret;
|
||||||
kfree(ps);
|
|
||||||
usb_put_dev(dev);
|
out_unlock_device:
|
||||||
}
|
usb_unlock_device(dev);
|
||||||
mutex_unlock(&usbfs_mutex);
|
usb_put_dev(dev);
|
||||||
|
out_free_ps:
|
||||||
|
kfree(ps);
|
||||||
unlock_kernel();
|
unlock_kernel();
|
||||||
return ret;
|
return ret;
|
||||||
}
|
}
|
||||||
|
@ -724,10 +735,7 @@ static int usbdev_release(struct inode *inode, struct file *file)
|
||||||
usb_lock_device(dev);
|
usb_lock_device(dev);
|
||||||
usb_hub_release_all_ports(dev, ps);
|
usb_hub_release_all_ports(dev, ps);
|
||||||
|
|
||||||
/* Protect against simultaneous open */
|
|
||||||
mutex_lock(&usbfs_mutex);
|
|
||||||
list_del_init(&ps->list);
|
list_del_init(&ps->list);
|
||||||
mutex_unlock(&usbfs_mutex);
|
|
||||||
|
|
||||||
for (ifnum = 0; ps->ifclaimed && ifnum < 8*sizeof(ps->ifclaimed);
|
for (ifnum = 0; ps->ifclaimed && ifnum < 8*sizeof(ps->ifclaimed);
|
||||||
ifnum++) {
|
ifnum++) {
|
||||||
|
|
|
@ -1478,8 +1478,7 @@ void usb_autoresume_work(struct work_struct *work)
|
||||||
* driver requires remote-wakeup capability during autosuspend but remote
|
* driver requires remote-wakeup capability during autosuspend but remote
|
||||||
* wakeup is disabled, the autosuspend will fail.
|
* wakeup is disabled, the autosuspend will fail.
|
||||||
*
|
*
|
||||||
* Often the caller will hold @udev's device lock, but this is not
|
* The caller must hold @udev's device lock.
|
||||||
* necessary.
|
|
||||||
*
|
*
|
||||||
* This routine can run only in process context.
|
* This routine can run only in process context.
|
||||||
*/
|
*/
|
||||||
|
@ -1503,6 +1502,8 @@ void usb_autosuspend_device(struct usb_device *udev)
|
||||||
* for an active interface is greater than 0, or autosuspend is not allowed
|
* for an active interface is greater than 0, or autosuspend is not allowed
|
||||||
* for any other reason, no autosuspend request will be queued.
|
* for any other reason, no autosuspend request will be queued.
|
||||||
*
|
*
|
||||||
|
* The caller must hold @udev's device lock.
|
||||||
|
*
|
||||||
* This routine can run only in process context.
|
* This routine can run only in process context.
|
||||||
*/
|
*/
|
||||||
void usb_try_autosuspend_device(struct usb_device *udev)
|
void usb_try_autosuspend_device(struct usb_device *udev)
|
||||||
|
@ -1526,8 +1527,7 @@ void usb_try_autosuspend_device(struct usb_device *udev)
|
||||||
* @udev's usage counter is incremented to prevent subsequent autosuspends.
|
* @udev's usage counter is incremented to prevent subsequent autosuspends.
|
||||||
* However if the autoresume fails then the usage counter is re-decremented.
|
* However if the autoresume fails then the usage counter is re-decremented.
|
||||||
*
|
*
|
||||||
* Often the caller will hold @udev's device lock, but this is not
|
* The caller must hold @udev's device lock.
|
||||||
* necessary (and attempting it might cause deadlock).
|
|
||||||
*
|
*
|
||||||
* This routine can run only in process context.
|
* This routine can run only in process context.
|
||||||
*/
|
*/
|
||||||
|
|
|
@ -352,6 +352,7 @@ set_autosuspend(struct device *dev, struct device_attribute *attr,
|
||||||
return -EINVAL;
|
return -EINVAL;
|
||||||
value *= HZ;
|
value *= HZ;
|
||||||
|
|
||||||
|
usb_lock_device(udev);
|
||||||
udev->autosuspend_delay = value;
|
udev->autosuspend_delay = value;
|
||||||
if (value >= 0)
|
if (value >= 0)
|
||||||
usb_try_autosuspend_device(udev);
|
usb_try_autosuspend_device(udev);
|
||||||
|
@ -359,6 +360,7 @@ set_autosuspend(struct device *dev, struct device_attribute *attr,
|
||||||
if (usb_autoresume_device(udev) == 0)
|
if (usb_autoresume_device(udev) == 0)
|
||||||
usb_autosuspend_device(udev);
|
usb_autosuspend_device(udev);
|
||||||
}
|
}
|
||||||
|
usb_unlock_device(udev);
|
||||||
return count;
|
return count;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
Reference in New Issue