On Tue, Feb 18, 2003, David Brownell <[EMAIL PROTECTED]> wrote: > >>>In 2.4, uhci.c holds a reference to the device until after it's done > >>>calling the completion handler. > >>> > >>>Now in 2.5, the hcd layer doesn't hold that reference anymore it seems. > >>> > >>>That should be fixed. > > Seems like a UHCI-specific expectation, so it's UHCI that should > be adding the extra reference count. (Rather than breaking the > other HCDs, which just rely on the URB having a valid device ptr.)
If they make that assumption too, then they should be using reference counting too. > >>Hm, well in my patch, I'm making the assumption that after deallocate() > >>the device is really gone, is that true? > > For the drivers that use bus->op->allocate(), yes. Not UHCI though. UHCI doesn't really care since it doesn't track any state per device. > >>What does the hcd use to tell if the dev is really present or not? > >>That's what I'm after. > > I don't know how UHCI deduces that, since it doesn't use the > explicit notification. I think the idea is that it shouldn't > be caring ... although the evidence is that it does. Nope, it doesn't care. UHCI doesn't care if the device physically exists or not, nor should it. It just queues stuff up. > > I don't think setting dev->bus to NULL is the right way, simply because > > the bandwidth is not available to the rest of the system until all of > > the URBs have been fully removed. To handle the bandwidth correctly, we > > need that pointer to the end. > > I think it's the best way in the current system to flag a device that's > gone. The whole of usbcore already checks for null pointers, and it's > a classic solution. DEVICE_GONE in <linux/device.h> enum device_state > doesn't seem to help, and needs a more solid backup anyway. But with reference counting, NULL pointers aren't really needed. Just keep the proper reference count. 2.4 used NULL pointers to denote things like the URB is done which is kinda wacky. > FWIW both EHCI and OHCI track bandwidth at the level of the QH. That's > the resource, matching an active device endpoint, that's more appropriate > for such roles than URBs (since each QH can have many URBs, using the > same bandwidth reservation in sequence). > > > > Also, deallocate() needs to be called when there are no more URBs for > > the OHCI driver to work (atleast in 2.4, I'm assuming the 2.5 driver has > > similar requirements) > > It needs to be called when the device is gone, in a context that > can sleep. "No more URBs" is an expectation that I think we should > think about changing ... "hcd_free_dev" expects that, but it's worth > changing that to be a bit more driver-friendly. Nope. deallocate() needs to be called when all references to the device are gone and that can happen in any context. I'm really surprised that usb-ohci.c still gets this wrong in 2.4, even after our "discussion" months ago. I have a patch for this BTW. I still need to test it with Isoc devices before I submit it. 2.5 should be no different. If the hcd layer makes assumption otherwise, it needs to be fixed. > The requirements for a clean driver shutdown (rmmod or device disconnect) > have been discussed here many times. > > (a) all pending URBs get completed/canceled; > (b) no new URBs get submitted; > (c) driver forgets about that device. > > In 2.4 all that was the responsibility of disconnect() methods, and when > (not "if"!) they had bugs, Bad Stuff happened. Yes, bugs cause bad stuff to happen. > We can offload some that from device drivers by a patch like > Greg's, addressing (b); and making deallocate() nuke all pending > URBs, addressing (a). Thereby preventing driver bugs from making > certain types of chaos, and simplifying some disconnect() logic. Absolutely not. deallocate() has no responsibility to nuke any URBs. That is the drivers responsibility. Bugs will always cause indeterminate behaviour. You will never be able to protect against all of them. This is the exact same reference counting discussion we had months ago. I suggest you go back and reread it again. JE ------------------------------------------------------- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf _______________________________________________ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
