usb: no locking for reading descriptors in sysfs
Quting the relevant thread: > In fact, I suspect the locking added by the kernel 3.13 commit for > read_descriptors() is invalid because read_descriptors() performs no USB > activity; read_descriptors() just reads information from an allocated > memory structure. This structure is protected as the structure is > existing before and after the sysfs vfs descriptors entry is created or > destroyed. You're right. For some reason I thought that usb_deauthorize_device() would destroy the rawdescriptor structures (as mentioned in that commit's Changelog), but it doesn't. The locking in read_descriptors() is unnecessary. > The information is only written at the time of enumeration > and does not change. At least that is my understanding. > > It is noted that in our testing of kernel 3.8 on ARM, that sysfs > read_descriptors() was non-blocking because the kernel 3.13 comment was > not there. > > The pre-kernel 3.13 sysfs read_descriptors() seemed to work OK. > > Proposal: > ========= > > Remove the usb_lock_device(udev) and usb_unlock_device(udev) from > devices/usb/core/sysfs.c in read_descriptors() that was added by the > kernel 3.13 commit > "232275a USB: fix substandard locking for the sysfs files" > > Any comments to this proposal ? It seems okay to me. Please submit a patch. So this removes the locking making the point about -EINTR in the first path moot. Signed-off-by: Oliver Neukum <oneukum@suse.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
parent
7dd9cba5bb
commit
b4a90d04ac
|
@ -843,16 +843,13 @@ read_descriptors(struct file *filp, struct kobject *kobj,
|
|||
struct usb_device *udev = to_usb_device(dev);
|
||||
size_t nleft = count;
|
||||
size_t srclen, n;
|
||||
int cfgno, rc;
|
||||
int cfgno;
|
||||
void *src;
|
||||
|
||||
/* The binary attribute begins with the device descriptor.
|
||||
* Following that are the raw descriptor entries for all the
|
||||
* configurations (config plus subsidiary descriptors).
|
||||
*/
|
||||
rc = usb_lock_device_interruptible(udev);
|
||||
if (rc < 0)
|
||||
return -EINTR;
|
||||
for (cfgno = -1; cfgno < udev->descriptor.bNumConfigurations &&
|
||||
nleft > 0; ++cfgno) {
|
||||
if (cfgno < 0) {
|
||||
|
@ -873,7 +870,6 @@ read_descriptors(struct file *filp, struct kobject *kobj,
|
|||
off -= srclen;
|
||||
}
|
||||
}
|
||||
usb_unlock_device(udev);
|
||||
return count - nleft;
|
||||
}
|
||||
|
||||
|
|
Loading…
Reference in a new issue