Hello. On 19-01-2013 21:54, Alan Stern wrote:
Furthermore, the locking in usbfs doesn't look right. A process should be able to submit as many URBs as it wants, of whatever type, at any time.
You mean USBDEVFS_SUBMITURB ioctl()? That's indeed an issue with 2.4 patch (and mine, of course) -- it still accounts this ioctl() as exclusive and so only permits one instance of URB. I should have documented it internally as a side effect of the v1 patch but I'm not really familiar with usbfs, and so documented only what was fixed by Pete back in 2006 (USBDEVFS_BULK).
I also meant USBDEVFS_BULK and USBDEVFS_CONTROL.
But those are synchronous, aren't they? I don't see a problem with them.
Now that I look more closely at the patch, I wonder why it adds a lock in usbdev_do_ioctl? Won't that lock still be held when proc_bulk is called and tries to acquire its own lock? Won't that cause a deadlock?
Yes, I'm now seeing that I did a fatal mistake in my haste by not merging in the third patch which divided ioctl's into exclusive and non-exclusice groups. Stupid me, the v2/v3 patches are totally useless then.
Also, why add a lock to usb_dump_desc in devices.c? None of the routines in that file try to communicate with the device.
That was a part of the original patch. I haven't gone into detail about what these functions are doing.
> The locking should be by file handle, not by "read" vs. "write".
You mean we still don't allow more than one URB per file handle? That would probably require a lock in the 'struct dev_state'...
No, this is what I mean: There's no point preventing usbfs from sending bulk URBs while usb-storage is using the device. usbfs already prevents that. What you want to do is prevent usbfs from sending control URBs at the wrong time. But when usb-storage isn't using the device, there's no reason to restrict how many URBs usbfs can send.
OK, fair enough.
Therefore you probably should use an rwsem. Have usb-storage lock it for writing, and have usbfs lock it for reading in proc_control and proc_submiturb (with matching unlocks at the right places).
Thanks for your suggestion.
Alan Stern
WBR, Sergei -- 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
