[CC: list pared down]

On Sun, 28 Aug 2005, Daniel Ritz wrote:

> > I was going to fix things by passing the driver as an explicit argument to 
> > device_bind_driver and device_release_driver.  That way the subroutines 
> > can test the current value of dev->driver while holding the semaphore, 
> > avoiding a race.
> 
> an extra argument to device_bind_driver() is fine but i don't quite see the
> point for an extra argument in device_release_driver().

The reason is simple: When device_release_driver runs, the current 
driver may no longer be what the caller thinks it is!

> device_bind_driver() with a driver argument should check:
> - if it's already bound, return success if bound to the requested driver
> - if dev->driver is already set, but not bound, bind and return success

There's no point worrying about this case.  With a proper API it will
never exist.  For registered devices, dev->driver will _always_ be equal
to the bound driver (except at times like during probe, while dev->sem is
held).

> - if dev->driver not set, bind and return success
> 
> but look what USB does in usb_driver_claim_interface(). device_bind_driver()
> is only called if the device is already bound to the bus.

Of course.  We don't want to bind a driver to a device if the device
itself hasn't been registered yet.  Trying to do that would mess up sysfs.

> swapping these two lines in bus_add_device() would make that check go away, 
> no?
>                 device_attach(dev);
>                 klist_add_tail(&bus->klist_devices, &dev->knode_bus);

No it wouldn't.  The problem isn't that the code in bus_add_device needs
to be reordered; the problem is that we want to avoid calling
device_bind_driver before calling device_add.


> BTW i find that usb_driver_claim_interface() is strange anyway. i mean an usb
> driver is per inteface yet it sets the driver to the device.

??  What on earth are you talking about?

>  so if a usb device
> has two (or more) interfaces this will lead to troubles if different drivers
> for the different interfaces are required. (usbaudio for example binds the
> same driver for both, the input and the ouput interface. it would lead to
> double registration if the knode_bus check wouldn't be there). 
> so i think the .driver member should go back to struct usb_interface were it
> has been before (btw. the comment in usb.h still describes the .driver member
> even when it was removed). i think either abstract that interface thingy
> in the driver core or create a dummy driver for usb that binds to the device
> and does the handling of the driver per interface right. no?

Have I missed something here?  Since when has the .driver member been 
removed from usb_interface?


> > Calls to device_add with dev->driver already set can be handled by making
> > the initial driver an explicit argument to device_attach.  There's still
> > the possibility of a race (another driver manages to get there first), but
> > at least it won't cause an oops.
> 
> bus_add_device() also?

bus_add_device can continue to receive the initial driver in dev->driver, 
but it should store that value in a local variable and set dev->driver to 
NULL.  Then it can pass the local variable to device_attach.

>  and device_release_driver() should also check if the
> knode_driver is attached before messing with klist_remove().

The patch I sent in before (which you said you hadn't read yet) already
takes care of that.

> i think we need to make a list of requirements to API, locking, etc.
> that shows the needs of the different subsystems like USB, serio, pcmcia/CB,
> etc. and then make a race-free locking design from that. currently some
> subsystems work around limitations in the driver core...let's get rid of these
> workarounds (and to make hch happy kill the klists :).

My patch already gets rid of the driver's klist of devices.  The other two 
(the bus's klist of devices and the device's klist of children) seem 
harmless, since a node can never be removed from one klist and then added 
to another.

I know the requirements for USB but not for those other subsystems.  
Talking with their maintainers may help.  In some cases the interfaces can 
be simplified now that manual driver binding is available through sysfs.

Alan Stern



-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to