Messages in this thread | | | From | Duncan Sands <> | Subject | Re: [linux-usb-devel] Re: [OOPS, usbcore, releaseintf] 2.6.0-test10-mm1 | Date | Tue, 9 Dec 2003 21:36:21 +0100 |
| |
On Tuesday 09 December 2003 16:55, Alan Stern wrote: > On Tue, 9 Dec 2003, Duncan Sands wrote: > > > You may simply have to release the lock because calling > > > usb_set_configuration and then reacquire it afterwards. > > > > Right, I did this in my patch along with the other changes, but in fact > > it could be fixed separately. > > Doesn't this approach work? I don't see anything wrong with it. (Read > "before" rather than "because" above -- my fingers don't always do what my > mind wants them to do.)
You mean, drop ps->devsem, take dev->serialize, check for disconnect, proceed if not disconnected, do some stuff (traverse the configuration for example), drop dev->serialize, take ps->devsem, check for disconnect, proceed if not disconnected? Well yes, but doing this all over the place would only make the whole driver more complicated and more fragile.
> > Well, you could just ensure you have a reference to the usb_device, and > > change usb_set_configuration and friends so that they don't Oops if the > > device has been disconnected. This should be done anyway by the way - > > surely all core routines should behave themselves (eg: by failing with > > an error code) when called with a not-yet-freed struct usb_device? > > Yes, that's the correct way to handle it. > > > > I mean it won't cause an oops, although it might provide an invalid > > > result. It's not _required_ by the API (maybe it should be). > > > > It will cause an Oops - actconfig may be NULL. This is the case after > > disconnect for example, and also momentarily the case doing configuration > > changes. > > Sorry -- what I _really_ meant to say was that usb_ifnum_to_if needs to be > rewritten to add a test for actconfig == NULL. Once that's done properly, > calling it without holding the lock won't oops even though it also might > not give you the right answer. Minor point; nobody would want to do that. > > > The disconnect routine is only called if you have claimed an interface. > > If usbfs is looking for an interface to claim (and hasn't yet claimed > > one), then disconnect will not be called. There is code in inode.c that > > informs usbfs when the device has been disconnected, but now that > > disconnect is per-interface, that is not good enough. > > What about the call to usbfs_remove_device that's in usb_disconnect?
That's the code in inode.c that I mentioned.
Ciao,
Duncan. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |