On Wed, 5 May 2004, David Brownell wrote: > Alan Stern wrote: > > I'm about ready to give up on device reset during probe. > > Preventing it would need a per-interface state flag, FWIW; > easily set by usbcore during probe(). Somewhat parallel to > the flag I suggested be held during SET_CONFIGURATION...
Preventing it could be as simple as not writing drivers that do it. No extra flags needed. :-) > > Consider this. There are three different paths that can call a driver's > > probe(): > > > > 1. Newly attached device is configured by usb_new_device(). > > Both the device and its parent hub are locked. > > Though I've described some alternatives to that. One issue is > how to cope with configuration errors ... currently the solution > involves counting retries. Likely additional per-port state would > be needed to switch strategies there. That might be worth doing, > given the irregular reports of devices that just keep retrying > forever during enumeration. (I seem to remember reports from Olaf, > but think he wasn't the only one with such devices.) I just saw almost the same thing on Bugzilla (bug #2610, comment #13). We could do something like /bin/init; stop responding to connect-status-change events on a port if there have been too many of them in some period of time. > > 2. New configuration is installed through sysfs. The device is > > locked but its parent hub isn't. > > All codepaths should use compatible locking. For the _first_ time, > we're really starting to look at how the hub driver locks ... we've > gotten rid of several spurious locks (BKL, bus rwsem, address0_sem, > even usbfs ps->devem), so this is really the first time the air has > been clear enough to even see the hub-specific issues those others > have masked! > > By which I conclude: since that sysfs locking was designed to wend > its way through the previous thicket of locking (now much thinned), > and hasn't been around long enough that user mode code should care, > it can be revisited. For example, by calling locktree(). Sysfs is just part of the problem. The whole core has to work together. And I can't help thinking that locktree is overkill for almost all cases. Can you name any problems that it solves? > > 3. Newly registered driver is matched to all unbound USB interfaces. > > Neither the device nor its parent hub is locked. > > But that probably doesn't actually _need_ much locking, beyond > what the bus rwsem already provides. So the only issue there > is that it's different. That's a big enough issue, IMO. Have you figured out how to prevent suspend during probe if the device isn't locked? Retry the suspend in a polling loop until a flag indicates the device isn't being probed any more? How will you prevent probe of a currently-being-suspended device? > > The driver doesn't know (and shouldn't know!) which path has called it, > > Right, but there can be constraints derived from the path. One example > would be usbcore preventing SET_CONFIGURATION until the previous one > completes (hence no recursion in probe). That's not really a constraint on a driver's probe(), since it can't call usb_set_configuration() anyway. > > And now consider this: Every path calling probe() definitely has acquired > > the USB subsystem rwsem. Proper locking order requires that _no_ device > > locks are acquired while holding the rwsem! > > ... unless the calls from usbcore/hub don't hold such locks before > calling the probe(). I don't understand. Probe() is always called with the rwsem held -- that's done by the driver model core. If it tries to acquire hub->serialize, and at the same time devices.c is holding hub->serialize and trying to acquire the rwsem, you're in trouble. > You and Oliver have both disliked the use of dev->serialize in some > of the contexts it's now used. OK, that's fair, but changing that > would mean resolving those problems in other ways. Doable; not done. I've spouted a lot of proposals, none of which have been all that great. At this stage I would prefer to sit back and poke holes in other people's ideas. > Do you have a model for how DFU should trigger the reset? That's where > the "queue resets" idea could help, but another option would be to have > some special return frome probe() which means "please reset the device". > That return could be processed during enumeration (SET_CONFIGURATION) as > well as during usb_register_driver(). That actually sounds good. It would be a lot easier than queuing a request for a different thread. If we did that, most of the need for device resets during probe would go away. > > Maybe neither top-down nor bottom-up locking is workable. Maybe the best > > approach is to have a single bus-wide semaphore, and only allow one > > significant action to happen at a time. That would simplify a lot of > > problems. (Even the newly-registered driver problem, although that would > > require locking _all_ the buses.) > > Top-down can clearly (to me!) work, and would have clear advantages given > that userspace code can initiate those actions concurrently. Given that > many potential concurrent accesses to the USB tree, the notion of any > kind of bus-wide semaphore bothers me a lot unless it's only held for > very brief times. (That is any such bus-wide lock shouldn't be used for > synchronizing.) Well, most of the actions we're talking about are fairly brief. Handling a newly-attached device might not be, though. > I think we can just continue working through these problems one at a time. Maybe we can. It'll take some hard thought... Alan Stern ------------------------------------------------------- This SF.Net email is sponsored by Sleepycat Software Learn developer strategies Cisco, Motorola, Ericsson & Lucent use to deliver higher performing products faster, at low TCO. http://www.sleepycat.com/telcomwpreg.php?From=osdnemail3 _______________________________________________ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
