On Fri, 30 Apr 2004, Pete Zaitcev wrote: > On Fri, 30 Apr 2004 23:09:31 -0400 (EDT) > Alan Stern <[EMAIL PROTECTED]> wrote: > > > It may be that we're making ->serialize do too much. For example, my > > recent change to devices.c uses it to prevent topology changes while > > traversing the tree, but that function could easily be served by something > > else, like a bus-specific topology semaphore. This would specifically be > > intended to protect the children[] arrays and nothing else. (Your > > flattened list, too, if that is adopted.) Locking rule: Always lock > > usbdev->serialize _before_ locking usbdev->bus->topology_sem. > > Alan is right here, this is how my attempts to reuse dev->serialize > floundered in 2.4 just today. The dev->serialize covers a lot and sits > in a very unfortunate path, so it's an exception. However my general > rule is to merge locks as much as possible, as a universal cure for > ordering problems. > > -- Pete
Bus topology changes occur pretty infrequently, so we could easily get by with a single global topology semaphore rather than bus-specific ones. In fact, that's part of what the USB subsystem rwsem already does. But that rwsem is involved in other things (like driver binding), and it is manipulated by the driver model core in ways that don't match our needs. I like the way this idea is leading. We can have a global topology semaphore protect all the children[] arrays and also the driver-model lists, since the two data structure views should always be consistent. Then the ->serialize locks won't be needed for tree traversal. Instead they can be used by the hub driver to ensure that only one thing happens on a hub at any time. When suspending or resetting a port, the rule should be to acquire the serialize lock on the child first, then on its parent hub. That matches the requirements of usb_reset_device(), and the new suspend code can easily be changed to comply. Whenever a thread executing in the hub driver acquires a hub's serialize lock, it should check that the hub device is still in USB_STATE_CONFIGURED. That will prevent problems with trying to manipulate suspended or disconnected hubs. The only loose end is usb_disconnect(). It can be made to work as follows: When disconnecting a hub first make sure the state is set to USB_STATE_DISCONNECTED, then acquire and release the serialize lock to synchronize with any other threads executing in the hub driver. Once that's done no more children can be added to that hub, so we can safely disconnect all the existing children, then acquire serialize again and unregister the hub itself. This seems like a workable approach that solves all the problems I've been able to think up so far. Comments, anybody? Alan Stern ------------------------------------------------------- This SF.Net email is sponsored by: Oracle 10g Get certified on the hottest thing ever to hit the market... Oracle 10g. Take an Oracle 10g class now, and we'll give you the exam FREE. http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click _______________________________________________ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
