On Wed, Feb 19, 2003 at 05:44:54PM -0500, Johannes Erdfelt wrote: > 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.
Hm, uhci-hcd oopses if this happens :) I could reproduce this very easily without that urb change but with the usb-serial changes. > Actually, it seems to me that that unlinking after disconnect would need > to be a requirement for random disconnects to work. I thought that too. > > > 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. uhci-hcd deadlocks if this happens :) The traceback had the uhci driver waiting for a timeout somewhere, I don't remember where, sorry. And what about urbs without timeouts (like bulk reads)? > > > >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. Good, so the existing code works fine for that. > > > 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. I'm teanding to agree. > 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. What happens if the driver doesn't unlink the urb on disconnect, but only does so a long time after disconnect happens? I image the existing urb times out, right? > > > >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. Excactly. > 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. Or just prevent unloding unless all reference counts are at zero :) Unfortunatly modules like the keyboard driver don't fit that model well... > > 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. Sure, but even when the HCD "owns" the urb, it can callback into the driver, and the driver better still be in memory while the urb is alive. > Look at struct sk_buff for instance, there is no reference count on it. Not yet :) thanks, greg k-h ------------------------------------------------------- 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
