Re: [linux-usb-devel] Re: [OOPS, usbcore, releaseintf] 2.6.0-test10-mm1

[Duncan Sands <baldrick@free.fr>]

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.