On Thu, 29 Apr 2004, David Brownell wrote:
This is the kind of thing I meant earlier, when I mentioned that the basic policy for usbdev->serialize would be to own it when manipulating children. ...
I interpreted "children" to mean just usbdev->children[], not ports. If you allow it to include the ports as well, you rule out the possibility of manipulating different ports simultaneously. I admit that doesn't come up very often, but do you really want to prevent it entirely?
If finer grained locking turns out to be necessary, it could be added later. Are you sure all hubs can support that sort of concurrent manipulation? I'm not.
Plus, those ports are the ONLY way to access the child devices (in usbdev->children[]) ... for me, it makes no sense to lock the one and not the other.
Furthermore, you completely rule out the possibility of doing device resets. Since the caller of usb_reset_device() already owns the device's lock, the routine _can't_ acquire the parent hub's lock -- that would mean getting them in the wrong order.
Those locking issues do need care yes, but similar lock order problems have been solved before. I'd start by failing if down_trylock() fails, and then see what issues come up ...
What if the hub is suspended? No doubt you can come up with other questions in addition.
Disconnecting a suspended hub wouldn't be different from any other kind of disconnect. Drop power to the port, clean up software state. It'll have no children by then, so nothing else to do -- right?
I meant, what if we try to suspend a hub that has activity occurring on some of its ports? A new connection, for example.
If HUB-A is suspending HUB-B, and a new connection comes on HUB-A, no problem ... it's going to wait a long time to debounce anyway, and the suspend takes hardly any time.
If the new connection comes in on HUB-B, then I'd sure hope that remote wakeup was turned on. In which case there'd surely be a notification ... that's a wakeup event for hubs, nothing should be lost.
I'm more concerned with things like who should suspend()/resume() the interfaces on devices, and children of hubs. I could live with the answer being usbcore in both cases. That'd be almost trivial for interfaces ... but having five layers of external hub suggests to me that maybe khubd should have a list_head in usb_device, so that it can work with pre-flattened trees!
"Who does suspend/resume" will probably end up involving other parts of
the kernel. And it's not clear to me why you prefer a flattened list over
Yes, that's why I'll be interested in more comments on the suspend patch I sent. (Yes, it takes a while to grok for me too.)
a tree. The costs of recursion aren't all that bad, and with a list you would have a hard time maintaining the proper set of locks as you went along.
I said "maybe". In part because that's the model used by the existing PM code: a global list of all devices. Maybe USB would be better served by having similar lists ... maybe USB-specific PM code should handle all such stuff, when PM suspends a controller, instead of expecting the PM glue to do it (today, it obviously doesn't).
The main issue is recursion. I don't mind it, but then I don't know for sure that we would meet the 4K stack limit using recusion. The question doesn't exactly come up with a pre-flattened list.
- Dave
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
