On Thu, Feb 20, 2003, David Brownell <[EMAIL PROTECTED]> wrote: > > >>Having looked at the history of that misnamed API: > >> > >> - Originally provided in 2.2.10 or so, I think OHCI was the only > >> user. It was misnamed at that time, given that it was used > >> only to handshake "device is gone". > >> > >> - Sometime in the 2.3 series, someone made a change to make the > >> usage in usbcore match that misnaming: just free memory. But > >> whoever that was did not change that single implementation of > >> the method (ohci) to match the changed semantics. > >> > >> - So that 2.3.x change created a bug where none existed before. > >> > >> It would only appear with OHCI *and* with a device driver that > >> was buggy and didn't disconnect() or refcount correctly. > > > > > > I made that change a long time ago when I implemented the reference > > counting code (IIRC). I may have missed the change to OHCI, but that > > doesn't mean it's misnamed. > > We'll just have to disagree on that. It may not be misnamed > in terms of the approach you have in mind ... but it wasn't > a name that matched its original concept/design/behavior. > > That's caused quite a lot of trouble because the mismatch was > subtle enough to only show in combination with other bugs. > That's the sort of problem that's pretty typical when too > much gets pushed into a "memory management" bucket.
This is fundamentally the problem. Your vision of how stuff is supposed to work in the USB code is solely your vision and doesn't match what was intended when I wrote the code or what the majority of the code does. You can keep thinking whatever you want, but it ain't reality. Just because you think you want to, or can, change how the API works, doesn't make it so. 2.4 USB doesn't work like you think it does. > > Back to the patch I've attached. It's against 2.4.19-pre3 (I'll update > > to Greg's CVS tree) and hasn't had the level of testing necessary to > > call it good. I also want to recheck that I caught all of the necessary > > paths. > > Seems like the main change there is that after disconnect, EDs > get cleaned up in the IRQ handler ... which means that when > the IRQ handler doesn't fire, nothing gets cleaned up. That > means some power management situations will (unnecessarily) > leak a lot, as will clean-up after the host controller dies > (maybe because of UE or cardbus eject). It won't leak EDs on power management. The cleanup will just be deferred until power comes back on. Big deal. However, if the driver is unloaded or the device is removed, there will be a leak of outstanding EDs. This is an easy fix and one of the things I wanted to look deeper into. > Is that basically what you were trying to achieve? If you mean leaking memory, no. > > usb-ohci.c | 72 +++++++++++-------------------------------------------------- > > usb-ohci.h | 58 +++++++++++++++++++++++++++++++------------------ > > 2 files changed, 50 insertions(+), 80 deletions(-) > > > > FWIW, we were hitting the BUG() call in sohci_free_dev(). > > With what device driver? As I've said, EVERY time I've looked > at such problems, the device driver has been at fault. Sometimes > quite blatantly, sometimes less so. EVERY TIME. usbnet. It does already go to great lengths to ensure it's done with all of the references it takes to the device, but it's not the only source of references to the device. > I'm positive that if you fixed that device driver's bug(s), > you'd stop getting that BUG(). > > A less invasive "fix" would be to just make the BUG() be "return 0". > That'd just leak memory, without affecting those other cases. > And such a buggy device driver deserves to leak ... :) Umm, I don't know why you still have this warped view of how the reference counting in 2.4 is supposed to work, but it's perfectly legal to have references past when disconnect() is called. > > Interestingly enough, the comment before sohci_free_dev is correct, but > > the implementation doesn't follow those rules. > > Right, I added it as part of a "document current calling conventions" > pass. It's missing a comment about those conventions being broken. :-P Once again, it doesn't change because you think it works differently than it really does. 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
