(1) If both subsys.rwsem and dev->serialize are taken, then subsys.rwsem must be taken first.
Yes.
Erm, no. As I pointed out the other day, it's a locking hierarchy. It must always go the other way:
- dev->serialize first. That controls the existence of the devices representing each interface ... which are different depending on device configuration. (Example, one config might have three interfaces, another one might just have one.)
Needs to be grabbed during normal enumeration paths and by paths that can re-enumerate -- reset_device(). The critical step is setting the configuration.
- subsys.rwsem is grabbed automatically by the driver model core when it probes to see which driver will bind to the per-interface devices, or unbinds them (disconnect callbacks).
It needs to be manually grabbed during claim() and release() style driver binding ... usbfs was doing this wrong, and I recently posted a patch that should fix it (same thread as the other binding fixups).
If you get the lock sequence wrong you'll eventually deadlock with something that's using the correct lock sequence. Like khubd -- wedging much of USB.
(2) dev->serialize atomizes changes to the struct usb_device.
Why then is dev->serialize not taken in usb_reset_device (except in a dud code path)?
It IS taken in usb_reset_device(), and that "dud" path is broken because that needs to be done in some other task context. (Because the reset must fail, and yet the device is still there -- it needs to re-enumerate, which that thread can't do.)
That "physical" reset_device doesn't grab it, since its caller is guaranteed to already have the lock.
Also, why isn't dev->serialize enough to protect against probe() during usb_reset_device? After all, won't dev->serialize be held during the probe calls (I didn't check this and I'm in need of coffee - I hope I'm on the right planet...)
Why? See above about lock hierarchy. When devices are configured -- during enumeration, or eventually during the re-enumeration on that "dud" codepath) -- devices are created for each interface.
In the current code definitely not. You must make sure that the configuration is still available. Guarding against probe() during reset is not enough. AFAIK David is currently rewriting this.
I have some unfinished code, but it's got to wait utnil those two "fix driver binding" patches get merged: the one for usbcore (everyone seemed OK with that one), and the other for usbfs (no feedback yet).
There are entirely too many locks involved in all this, and many of them will cause problems the minute any very "interesting" things start getting done ... which is why locking cleanups need to be done first.
- Dave
Regards Oliver
------------------------------------------------------- This SF.net email is sponsored by: IBM Linux Tutorials. Become an expert in LINUX or just sharpen your skills. Sign up for IBM's Free Linux Tutorials. Learn everything from the bash shell to sys admin. Click now! http://ads.osdn.com/?ad_id=1278&alloc_id=3371&op=click _______________________________________________ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
