On Sat, 1 May 2004, David Brownell wrote: > Alan Stern wrote: > > > 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. > > Which "topology" do you mean though ... the physical (hubs, devices) > or the logical (interfaces in current configurations)? Or were you > thinking of combining those? (In which case the bus rwsem would > seem to partly address the problem.)
Physical. This would be only for struct usb_device, not for struct usb_interface, because only usb_device has a children[] array. It might not be necessary to do this if we used the driver model links exclusively and did away with ->children[], but since it's there it needs to be protected. And it's becoming clear that ->serialize is the wrong way to do so. > > 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. > > That's what the current suspend code does ... the notion of > acquiring locks going _up_ the physical tree bothers me, but > so far it's sufficient. I beg to differ... In your most recently posted patch, usb_suspend_device() acquires hubdev->serialize _before_ hub_port_suspend() acquires dev->serialize, not _after_. The business of going up the tree is a little odd, yes, but it's limited to a couple of cases where we go from a device only to its parent hub. We never go all the way up to the root (unless of course the parent happens to be the root hub). > > 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. > > Or config 0 hubs. Right. > > 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. > > I keep wanting a "mark this whole tree disconnected" stage in that > process ... I left it out because it wasn't relevant to my point. But yes, the _very_ first step of disconnecting a hub should be to change its state and the state of all its children recursively to USB_STATE_DISCONNECTED. This should be done without waiting to acquire any ->serialize locks (which means that usbdev->state will need to be protected by a global spinlock). Then everything else I described can take place. 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
