(8): Reading port status messages and marking the child device as USB_STATE_NOTATTACHED when a connect change occurs
And issuing disconnect() notifications ... that's three separate things, and I think only the third one (notifications) should require holding the device/port's lock.
The third part was already present on my list as (1). Clearly only that third part requires holding the device's lock. But the first part (reading port status messages) requires holding the port's lock -- so if the port's lock _is_ the device's lock we end up with a possibility for blockage.
Reading port status doesn't require a lock ... responding to changes, or initiating them, should. It still doesn't seem to me that any lock other than hub->serialize should be needed for that task.
No, sorry about the proliferation of uninformative numeric labels. In simpler terms: If we use the child device's ->serialize also to lock the hub port, then khubd's attempts to read the port status could be delayed by things like child device config changes -- things that could just as easily proceed in parallel with reading the port status. (That's because config changes have to lock the child device whereas port status messages merely have to lock the port.)
But if we use hub->serialize to lock ports, they WILL go in parallel, and without adding more locks.
In short, maybe it's not a good idea to use the child device's lock as a port lock. What do you think? (Remember that it was you who asked earlier why I wanted to distinguish between the two halves, hub port vs. device, of a single link! "Why separate entities", you said.)
That was more of a "persuade me those are the _right_ entities" kind of question, though it took a while to lead that way ... :)
Using a lock on the hub -- vs each of its ports -- would seem equally correct, at the cost of some potential concurrency. And it's a LOT simpler!
... you want to make sure that every device below the hub gets suspended and _stays_ suspended until you're done. The "stays suspended" part could be violated by resume() if resume() didn't use locktree. ...
Apart from that, there doesn't appear to be any need for locktree. Although other operations can affect entire subtrees, they don't do so in ways that require extended periods of protection.
That's a good way to put it. Reset and disconnect are basically obliterating the subtree before they start, and the only issue with disconnect is how to clean it ASAP. But we really don't want to have to have trees that don't know if they're suspending or resuming!
How were you going to solve the register_driver() -> probe() path?
That was described in my earlier message:
http://marc.theaimsgroup.com/?l=linux-usb-devel&m=108387954820032&w=2
The basic idea is to use an overall rwsem in addition to all the
->serialize locks. To lock a single device you would first get
read-access to the rwsem. When registering or deregistering a driver you
first get write-access to the rwsem, thereby effectively locking _all_ the
usb_devices.
I was overlooking that part of the message ... but still, what I was wondering is where the locks would be gotten. How would the hub_port_init() paths (enumeration, reset) get that readlock?
As you said, usb_register() gets the writelock, but shouldn't usb_probe_interface() then be grabbing dev->serialize if it's going to be held on all probe() paths? That'd change the early enumeration locking a bunch, and maybe fault cleanup (since dev->serialize no longer suffices).
Good. So apart from the questions above, it looks like we've found a solution.
Other than removing khubd_sem, sounds like what's in my BK tree now.
You don't have the new rwsem, and many of the details are different. For
example, we would never need to explicitly use the USB subsystem rwsem
since binding events will be protected by the device lock. Also we have to add a spinlock to protect udev->state, so that the NOTATTACHED marking can be done safely.
The spinlock has been a separate issue for a while. I think the first patch would be to make that rwsem work.
Right now, the only question I'm still concerned about is how to handle the port locking. Use the child device's ->serialize, or have a separate per-port bitmask for each hub?
Or just use hubdev->serialize? (Instead of khubd_sem.)
- Dave
------------------------------------------------------- This SF.Net email is sponsored by: SourceForge.net Broadband Sign-up now for SourceForge Broadband and get the fastest 6.0/768 connection for only $19.95/mo for the first 3 months! http://ads.osdn.com/?ad_id=2562&alloc_id=6184&op=click _______________________________________________ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
