On Fri, 29 Apr 2005, Roman Kagan wrote: > On Tue, Apr 26, 2005 at 11:57:34AM -0400, Alan Stern wrote: > > On Tue, 26 Apr 2005, Roman Kagan wrote: > > > > > On Mon, Apr 25, 2005 at 04:01:07PM -0400, Alan Stern wrote: > > > > I think usb_driver_claim_interface is correct as it stands. It was a > > > > mistake to leave out from usb_driver_release_interface originally the > > > > line > > > > setting iface->condition to USB_INTERFACE_UNBINDING. > > > > > > But it would be asymmetric then. > > > > In what sense? You mean because we don't also protect against a driver > > calling usb_driver_claim_interface from within that interface's probe? > > We don't need to check for that, do we? > > Well, I'm not sure, but we do it now (from 2.6.12-rc2):
Yes, we do. Okay, are you still concerned about the new code being asymmetric somehow? > > There are other reasons for keeping usb_interface.condition. For one > > thing, the driver-model data doesn't tell when a bind or unbind is in > > progress. That information is used elsewhere in usbcore. > > AFAICT this "elsewhere" is only usb_lock_device_for_reset(). That's right. > And it > looks like all its users (scsi reset in storage/scsiglue.c and > image/microtek.c) don't really care if it is transitioning or already in > an established bound state. They care if it is in the UNBINDING transition. This is because of the potential for a strange deadlock between disconnect() and the SCSI layer: If the error handler is running then scsi_remove_host won't return until it's done, and the error handler might very well be waiting to lock the device for a USB reset. (Also usb-storage used to care if it was in the BINDING transition, but it doesn't care about that case any more.) > > Here is a fix for driver_detach(). It's a little ugly because it avoids > > using the klist iterator, but there's no way around it -- the iterator > > simply can't be made to work here. Not only does it prevent > > device_release_driver() from calling klist_remove(), but also it prevents > > doing get_device() while holding the klist spinlock. > > I was about to cook up something very much like this :) Does the patch look good? If you like it, I'll submit it to Greg. I don't know what has happened to Pat Mochel -- he hasn't posted anything to the public mailing lists since April 7. > > With this patch in place, the code in usb_driver_release_interface() > > doesn't really need to be changed. > > Well, except for the inversion of the test, as given in my original > post. Yes. :-) > > However I think it should be. As it > > is now, it relies on internal knowledge of how the driver-model core > > works, > > 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? Can you think of a better way? 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
