On Fri, Jan 04, 2002, David Brownell <[EMAIL PROTECTED]> wrote: > > Just to clarify, dec_and_test can return true when called from an > > interrupt context. The dec_and_test implementation doesn't care what > > context it is. > > Right, but under what circumstances should that code ever be called > in_interrupt() to get rid of the last reference to that device? > > I don't think there can be one. If there were, then the hub cleanup > would have happened already, and the USB system state would > have been inconsistent in between those intervals...
The only situation I could think of, that I mentioned in my previous email and is the paragraph below this, should be considered a bug. If you're doing an asynchronous unlink in driver->disconnect(), then you should be waiting for it to complete. > > In almost every case, usb_free_dev will be called in the khubd context. > > The only case I can think of where it would be called from an interrupt > > context (in the bug free case) would be a device disconnecting and the > > driver doing an asynchronous unlink (like for the uhci drivers). Then > > the reference would then hit 0 in the interrupt context. > > Hmm, I'd thought otherwise. When driver->disconnect() returns, then > why wouldn't it be a bug if there were requests still pending from > that driver? Yes, that's a bug in the driver. FWIW, every time I mentioned disconnect(), I was referring to bus->op->disconnect(), not driver->disconnect() > > > > That would indicate there is a reference > > > > counting bug somewhere. > > > > > > Yes, such bugs do crop up from time to time. OHCI reports > > > calls to the "free device" routine in_interrupt() as errors, it > > > might be helpful if the UHCIs did the same thing. > > Actually I lied -- the error is reported only when the call is > in_interrupt() AND there's still stuff to clean up. It'd be > more correct (and probably safe now, but not this time > last year) to drop the "in_interrupt()" constraint. Gotcha, that makes sense. In that case, that error is definately warranted and should be investigated. > > sohci_free_dev doesn't need to be so complex. > > Its analogue in the "ohci-hcd" driver (for 2.5) is simpler. > The "hcd" layer does some of the cleanup, and reports > the error if there's work still to be done -- regardless of > whether it's in_interrupt() or not. Cool, sounds like the right thing to do. > > I used to have code > > similar to that in uhci.c, but then realized it's not necessary since in > > a normal working setup, it can never be called when there are still > > URB's around. > > But the problem is that it _has_ been ... not all device drivers have > been free of refcounting bugs. > > > It does look like usb-ohci.c allocates some state per device on device > > connect and disconnect, so that code may need to remain, but all of the > > functions it calls should be interrupt safe (pci_pool_free, etc) > > They are, and that's not at all the issue. It's as I (correcting myself :) > stated above: when the device is freed but there are still URBs > active, some device driver has trashed the refcount. Yes, but we shouldn't be working around buggy drivers. I have no problem with the warning you mentioned, but let the system crash if the refcount is trashed or if there's a bug somewhere else. JE _______________________________________________ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
