On Sat, 10 Jul 2004, David Brownell wrote: > Alan Stern wrote: > > Okay, in a separate thread on LKML Andrew Morton has said he doesn't > > really like my approach either. Then what should it be? > > It also conflicts with the locktree() model, since it doesn't > actually obey a parent-first locking policy. > > So there's no locking "this hub and below"; usb_lock_device() > will gladly grant most any unclaimed udev->serialize at all. > I'm not keen on trying to handle resumes initiated from within > trees that are partially suspended ... best to finish suspend > processing before starting other changes in that tree!
I don't understand these comments at all. How does usb_lock_device() violate a parent-first locking policy in any way that wasn't violated before? Your strictly from-the-very-top-on-down locktree() algorithm ought to work just as well when calling usb_lock_device() as when calling down(&udev->serialize). > > Clumsily use different routines for locking the first vs. the > > others in a nested set? (Or clumsily force drivers that want > > to lock more than one device to explicitly acquire and release > > the readlock?) > > The way you call everything "clumsy" makes me suspect you > really don't like these notions particularly much ... :) > But since your other option was a single global lock, these > look to be far more congenial directions. You're right that I don't like it much, but it certainly can be done. There are relatively few places that need to lock more than one device at a time, and they can all be fixed up. Most of them will be fairly simple to change; probably usb_disconnect() will be the worst case. > If you're really keen on hiding the rwsem, then there's no > way around the notion of different rules to get the first > lock and the subsequent ones -- short of reentrant locks, > which are REALLY worth avoiding. > > But I have no problem with that; it's what locktree does > already, after all. What it _needs_ to do in fact. My biggest stumbling block is what the API should be (and how to document it!). Should the rwsem be exposed? Should routines directly do down(&udev->serialize) for locking all devices after the first? Should there be a separate pair of routines to encapsulate the notion of nested locks? How should such a pair cope with the way locktree() doesn't nest its multiple locks? How can this be documented in a way that won't make people wonder why such a simple idea needs to be made so complex? > I started to look at making usb_lock_device() be locktree() > underneath. Issues: > > - Code now doing lock_device(hdev->child[i]) would change; > the top level would lock_device(), lower levels would > just down(&udev->serialize) and thus, at most, wait for > a previously-started task to complete. I only saw a > couple obvious instances to change. > > - The "device is gone" fault mode is handled differently. > Much of the code you touched was just failing there, > after getting the lock; locktree() doesn't bother even > grabbing the lock when it'd be meaningless. > > - That special lock_for_reset() logic needs more thought. > Maybe it just needs a trylock() version of locktree(). > > There were a couple other minor tweaks, but the big issues > were the "lock never fails" policy, and lock_for_reset(). I don't understand why this is needed. On all occasions when a device is locked, apart from suspend() and resume(), it's not necessary to lock the entire subtree below the device. Locktree() would be overkill. It's not clear why usb_lock_device() has to check whether the device is gone. If the device is gone, that fact will be detected later on anyway. Why disturb all the existing code that assumes device locking always succeeds? If lock_for_reset() isn't changed to the locktree() model, does it need to be changed at all? I don't see any reason why. The only case where it could matter would be if you had previously locked another device like the parent hub, but you were running outside of probe() and disconnect() -- I don't think that particular combination ever occurs. Alan Stern ------------------------------------------------------- This SF.Net email sponsored by Black Hat Briefings & Training. Attend Black Hat Briefings & Training, Las Vegas July 24-29 - digital self defense, top technical experts, no vendor pitches, unmatched networking opportunities. Visit www.blackhat.com _______________________________________________ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
