Reading port status doesn't require a lock ... responding to changes, or initiating them, should.
That's an excellent point, something I had not ever considered. It almost solves the entire problem!
There's just one or two loose ends to consider, like what happens if a device is unplugged during a reset. We don't want hub_port_init() in the reset thread deleting the usb_device structure; that should be left up to khubd.
Right. You're going over the reset stuff anyway, so I'm sure you can come up with a solution.
We _can't_ use hubdev->serialize to lock ports. (Fortunately we don't have to, and using the child's lock is just as simple.) The reason is clear: Sometimes a thread will need to lock a port while already holding the child device's lock. If that meant locking the hub, it would involve acquiring locks in the wrong order.
Those "sometimes" cases are the responding/initiating changes cases, right?
How were you going to solve the register_driver() -> probe() path?
hub_port_init() is called in two different contexts: initializing a new device and resetting an existing one. Resetting is easy; the caller will already hold the appropriate lock. In the initialization path the hub driver will acquire the readlock in the usual way, by calling usb_lock_device() before calling usb_new_device(). That's the correct thing to do, since there might be a new driver registering at that same time -- we wouldn't want it to probe the new device until the initialization procedure is finished.
OK.
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).
No, usb_probe_interface() doesn't need to lock anything. The locks will already have been acquired, either by usb_register() or by the caller of usb_set_configuration(). (There's another obscure pathway to consider, USBDEVFS_CONNECT -- it should be treated the same as usb_register().)
Maybe you're thinking that once you hold the writelock you still need to hold dev->serialize? That's not necessary. Holding the writelock will prevent everyone else from locking _any_ device.
I was thinking you still wanted to ensure that probe() was always called in the same kind of context ... including dev->serialize.
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.
Yes, that would be the first patch. That, and removing unnecessary locking of the subsystem rwsem. The state spinlock can come next. When you think about all the details involved, there's quite a lot that needs to be changed.
Yep, more than one patch. That spinlock is probably easiest though!
- 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
