On Wed, Feb 19, 2003, Greg KH <[EMAIL PROTECTED]> wrote: > On Wed, Feb 19, 2003 at 02:21:11PM -0800, David Brownell wrote: > > Greg KH wrote: > > > > > >Hm, maybe we should mark the device gone at the beginning of this call, > > >or right at the time we realize it's gone in the hub driver. That would > > >keep drivers from submitting urbs from within their disconnect > > >functions. But it would also prevent them from canceling them... Ah, > > > > Only because you put that check in the unlink path ... I think it'd > > be better to mark earlier, and allow cleanup at any time (not check). > > No, some of the hcds die a horrible death if you try to call unlink on a > urb after the device is gone :(
They shouldn't. It should be perfectly valid to do that. Actually, it seems to me that that unlinking after disconnect would need to be a requirement for random disconnects to work. > > In the gadget code I posted, I've made a point of making sure that > > when the host goes away, or an endpoint gets disabled, the cleanup > > paths continue to work ... only methods like enabling endpoints or > > submitting requests get blocked. So things start to quiesce right > > away, and shutdown is guaranteed to work in one pass, there's no > > need to patch it up later. > > > > No reason the host side can't use the same approach. > > Sure, fixing up the host controllers would be fine. I wouldn't mind > that, and then removing the check on unlink. But I don't think it's a > requirement as things seem to work the way they currently are (although > I'm waiting for Oliver to find a race somewhere :) It is racy and should be fixed. I actually don't have a problem with submitting a request for a device that's gone. It'll timeout. Big deal. If the driver doesn't like the timeout, then it can stop submitting requests after the device has disconnected. > > >but the hcd drivers should have already canceled them, right? Hm, but > > > > Nope, the only way an hcd can know the device is gone is by > > calling the misnamed bus->op->deallocate() routine. And > > because of the mess stemming from that misnaming, that code > > path doesn't even try to do that any more. > > Huh? Due to a change I just made? I don't see that. It seems to be > that way for a while. And Johannes just stated that uhci doesn't even > implement disconnect() so there's gotta be some way it determines that a > device isn't there anymore. Or it probably just doesn't care anymore, > which seems to be a valid thing to do. UHCI doesn't care since there is no per device state that is tracked. The other HCDs do have some per device state and use the functionality. Think of UHCI as the exception. The deallocate() callback is exactly what is needed. When all of the references are gone, let the HCD free any private state for that device. > > We'd need a new bus->op hook to let them do that. I think > > that'd be a good thing to do at some point ... and it'd be > > practical to have "hcd.c" handle the logic, without touching > > any of the hardware-specific code (ehci, ohci, uhci). > > I don't know, Johannes, I know you don't want to do that in uhci for > some reason, right? Because it's superfluous. The HCDs shouldn't be unlinking any URBs (I don't know if that was brought up, but has been brought up in the past). The driver that submitted the URB should be the only code that unlinks them. > > >then we don't know when the callback would have happened to be able to > > >unload the driver safely... > > > > I don't see why that would matter in that routine; > > that seems like a different discussion. > > No, you're saying that if a driver's disconnect() function has > completed, it is now safe to unload, right? Well if the hcd driver > hasn't cancelled the urb on device disconnect, then the callback can > still happen, in the driver module. Which could have just been > unloaded, oops :) disconnect() and module unload are really different paths. As far as I'm concerned, there are no requirements for the module to be done with a device after disconnect() is called. The reference counting handles all of the correctness for that. The only problem with it is when you unload the module. The reference counting won't handle that. You need to clean up everything outstanding immediately, or wait for everything to be cleaned up by some other means. > I'm still thinking through the urb module reference count stuff, and am > not entirely convinced that things are currently safe without it, but > other things have been distracting me... The URB reference counting may not be necessary. There's only 2 possible pieces of code that can use it: the driver and the HCD. As long as the rules about who owns the URB when and where are clearly defined (and implemented), then reference counting may not be necessary. Look at struct sk_buff for instance, there is no reference count on it. JE ------------------------------------------------------- This SF.net email is sponsored by: SlickEdit Inc. Develop an edge. The most comprehensive and flexible code editor you can use. Code faster. C/C++, C#, Java, HTML, XML, many more. FREE 30-Day Trial. www.slickedit.com/sourceforge _______________________________________________ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
