Re: [linux-usb-devel] [PATCH 1/9] USB usbfs: take a reference to the usb device

[Duncan Sands]

On Saturday 17 April 2004 22:33, Oliver Neukum wrote:
> > riddled with races of this kind.  The simplest solution is to
> > systematically take ps->dev->serialize when entering the usbfs routines,
> > which is what my patches do.  This should be regarded as a first step: it
>
> What is the alternative?

The alternative is to start at the lower levels and work up (while here I propose
starting at the top levels and working down): trying to lock small regions in many
places.  I rejected this as too error prone (remember that usbfs is a bit of a mess).
Anyway, if done correctly the end result would be much the same as applying this
patch and doing step (2).

> > gives correctness, but at the cost of a probable performance hit.  In
> > later steps we can (1) turn dev->serialize into a rwsem
>
> Rwsems are _slower_ in the normal case of no contention.

Right, but remember that dev->serialize is per device, not per interface.  So if two
programs grab different interfaces of the same device using usbfs, or if multiple
threads in the same program beat on the same interface, then they could lose time
fighting for dev->serialize when in fact they could run in parallel.  Personally I doubt
it matters much, but since most of usbfs only requires read access to the data structures
protected by dev->serialize, it seems logical to use a rwsem.

> > (2) push the acquisition of dev->serialize down to the lower levels as
> > they are fixed up.
>
> Why?

Efficiency.  The main reason is that the copy to/from user calls are inside the locked region :)
As for the other places where the lock could be dropped, I guess measurement is required to
see if it gains anything.

Ciao,

Duncan.
-
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/