On Wed, 4 May 2005, Roman Kagan wrote: > > Does the patch look good? If you like it, I'll submit it to Greg. > > Yes it looks reasonable. At the moment I can't test the problematic > multi-interface USB device case where that bug revealed itself, which > was observed by Duncan Sands; I'm attaching a simple test case we've > been using to trigger the problem, so you may want to try it out before > submitting. To do that, you need to load the test driver giving vendor and > product IDs of your multi-interface device as module parameters, and > then rmmod the driver while the device is still plugged in: it used to > spin forever (BTW it still does with 2.6.12-rc3, should I resend my > patch fixing it for non-klist case?)
Thanks for that test case; although I know the patch works okay under normal usage (single-interface devices), until now I didn't have any way to try a multi-interface device. > > > Agreed. IMHO if the driver model strives to take care of everything on > > > caller's behalf, it also has to gracefully handle recursive calls of one > > > of its exported interfaces, device_release_driver(). > > > > A simple way to do it would be for __device_release_driver to set > > dev->driver to NULL _before_ calling dev->remove instead of after. But > > would this be safe? > > That was also the first idea which came to my mind, but I didn't (and > still don't) feel like auditinig tons of code on whether it's safe... > > > Can you think of a better way? > > It may make sense to add some convention (e.g. explicit flag) to > indicate the state of the device wrt driver binding (bound, unbound, > transitioning from one to the other), and use it everywhere > consistently. It'd be also useful to export it in sysfs. But that's a > big piece work... That's essentially what I did for USB (without the sysfs part). It won't be all that hard to move it to the driver core. Whether it _should_ be moved is a harder question... Here's an idea I had recently to improve the klist library; tell me what you think. As a space optimization, instead of storing a struct completion in every klist_node, just put a wait_queue_header in struct klist. Under normal usage it's very rare to remove more than one klist_node from a klist at any time, so false wakeups won't be a problem. On Tue, 3 May 2005, Greg KH wrote: > At first glance, this looks sane. Did you test this code out? And if > so, care to resend with a Signed-off-by: line? Sorry, that should have been made more clear. The code was posted just as an RFC, not as a real submission. I haven't yet tested the problem case, but I will. Alan Stern ------------------------------------------------------- This SF.Net email is sponsored by: NEC IT Guy Games. Get your fingers limbered up and give it your best shot. 4 great events, 4 opportunities to win big! Highest score wins.NEC IT Guy Games. Play to win an NEC 61 plasma display. Visit http://www.necitguy.com/?r=20 _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
