The code in hci_usb_unlink_urbs() calls usb_unlink_urb() to perform a synchronous unlink of each pending URB. It then moves each URB to the completed list, and then frees everything on the completed list. By doing this, the code implicitly assumes that when usb_unlink_urb() returns the URB will have completed and be ready to be deallocated. That's not always true.
(David, are you reading this? It's a perfect example of the sort of error the current API encourages.)
And last time this came up, I thought there was agreement that adding some new synchronous urb_begone() function was fine. I really think synchronous unlink should just vanish from the API; it's virtually never going to be used correctly.
The problem is that while there are two distinct API events here, usb_unlink_urb() quite properly guarantees only the first one:
- urb unlink, moving the URB off the hardware's queue
so it finishes early (canceling it)- urb completion, giving the URB back to the driver.
Clearly driver disconnect must synchronize on the latter. So when bluetooth uses an API that synchronizes on the former, naturally it's broken.
By the way, note the lifecycle issue here. If URBs could never be reused, sloppiness here could become more forgivable. But since they are reused, there are more object states.
That's very much like the "don't reuse struct device" rules that you're looking at in another context. That rule is a simplification, and one I can support because creating devices is rare ... but URBs can be submitted thousands of times per second by a single driver, so they really do need a lifecycle that includes re-use.
They didn't show up with the OHCI driver because that driver implements a disable_endpoint() method. The method waits until all URBS for a given endpoint are completely finished, thereby preventing the Bluetooth driver from deallocating the storage too early. UHCI doesn't yet implement disable_endpoint(), which accounts for the difference you see.
So a quick'n'dirty fix might be a uhci_disable_endpoint() call that sleeps for N>1 msec ... that assumes the UHCI driver's IRQ handler will be able to fire, and issue the completions.
It might be good to do that, just to help cover up the drivers that synchronize on the wrong thing. (Although as you noted, Bluetooth needs to be fixed regardless.)
- Dave
------------------------------------------------------- This SF.Net email is sponsored by: IBM Linux Tutorials Free Linux tutorial presented by Daniel Robbins, President and CEO of GenToo technologies. Learn everything from fundamentals to system administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click _______________________________________________ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
