On Fri, 8 Jan 2016, Dean Jenkins wrote: > Hi, > > Here is a summary of the kernel ticket that I created at > https://bugzilla.kernel.org/show_bug.cgi?id=110531 > > Before kernel 3.13, sysfs read_descriptors() was non-blocking and in > kernel 3.13 the following commit was added > "232275a USB: fix substandard locking for the sysfs files" > which added usb_lock_device(udev) which can cause blocking to > read_descriptors(). > > Blocking occurs when sysfs read_descriptors' udev == hub_event's hdev value. > > If USB comms have failed in hub_port_init() then the blocking can be up > to 30 seconds due to the 6 retry attempts at re-establishing USB comms > with a USB device. One could argue that 30 seconds is too long and hub_port_init() does too many retries. However, you don't seem to be making this argument. > This period of mutex blocking can be too long for applications as the > application's thread for reading sysfs hangs until > usb_unlock_device(hdev) is called in hub_event(). _Any_ period of blocking can be too long for applications, if the applications use a timeout that is too short. ... > Whilst the connection attempts are being attempted, hub_events() holds a > device lock referenced by hdev. However, sysfs read_descriptors() can be > blocked on this held lock when udev == hdev of hub_events(). When > read_descriptors() is blocked, this blocking can be up to 30 seconds. ... > It can be seen that sysfs read_descriptors() is accessing the root hub's > (I think) descriptor information but the USB communications issue is for > the USB device connected to the root hub. In other words, the > read_descriptors() blocking is not occurring on the device that is > malfunctioning. > > The root hub's descriptor information is hard-coded (I think) and > therefore read_descriptors() does not need locking to read this > information. This is because the sysfs vfs entry's lifetime is bracketed > by the root hub being present. What would happen if the hub in question hadn't been the root hub? As far as I can tell, you would still have exactly the same difficulty. So the fact that root hubs have hard-coded descriptors is not relevant to your problem. > 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. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
