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

[Alan Stern]

On Mon, 8 Dec 2003, Duncan Sands wrote:

> > > You can't keep the ps->devsem lock and use ps->dev->serialize, because it
> > > leads to deadlock.
> >
> > How so?  Remember that I am almost totally unfamiliar with the details of
> > the usbfs code.  Are you saying there are places where the driver holds
> > one lock and needs to acquire the other and vice versa?
> 
> Yes.  ps->devsem is used to protect against disconnection: all top level
> routines take it (as a read lock), and in driver_disconnect it is taken as a
> write lock.  Top level routines call lower level routines which sometimes
> need to take dev->serialize (and do already in several places).
> 
> Thus: ps->devsem taken, then dev->serialize.
> 
> However, dev->serialize is taken by the USB core before calling
> driver_disconnect.
> 
> Thus: dev->serialize taken, then ps->devsem.

This is a tricky situation, no doubt about it.

Your situation is a little different from the usual one because ps->devsem
locks the whole device, not just a single interface.  It should still be
able to work.  But maybe you're right; since ps->devsem locks the same
thing as ps->dev->serialize, maybe it's not needed.  By the way, when
usbfs takes ownership of a device, does it bind to the device's
interfaces?

> Right.  And why should (for example) dev->serialize not be held when it
> calls usb_set_configuration? - because usb_set_configuration takes
> dev->serialize.  This is one of the places I mentioned above where
> deadlock can occur right now.

You may simply have to release the lock because calling 
usb_set_configuration and then reacquire it afterwards.

That leads to the question of how to assure that the device doesn't go 
away before usb_set_configuration is called.  Perhaps 
usb_set_configuration and usb_unbind_interface should be changed to 
require the caller to hold the serialize lock.


> > If you call usb_ifnum_to_if() you ought to hold the serialize lock;
> > otherwise the configuration might change out from under you.  But it's not
> > necessary.  Likewise for usb_epnum_to_ep_desc if you're looking up an
> > endpoint that isn't part of an interface you have bound.
> 
> Why isn't it necessary?  As far as I can see it is vital.

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).

Actually, there's another sense in which it's not necessary.  Since
changing configurations first involves unbinding the existing drivers, if
you hold a driver-private lock that will block your disconnect routine
then you can safely call usb_ifnum_to_if even without holding
dev->serialize.

> > There's some sort of misunderstanding here.  It's not fatal to do
> > usb_put_dev() after disconnect, provided you called usb_get_dev() earlier.
> > I'm not sure what the cause was of the oops you were getting, but it
> > wasn't that.
> 
> It was AFAICS, though of course it shouldn't be.

I didn't note the reason for the oops.  Was it a segmentation violation?  
The usb_device memory isn't deallocated until the reference count goes to 
0.  Maybe something was doing an extra usb_put_dev.

Alan Stern

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/