Am Sonntag, 21. Oktober 2001 23:45 schrieb David Brownell:
> > > > > (b) usb_find_interface_driver() uses the dev->serialize semaphore
> > > > > for exclusion, but usbdevfs uses the BKL instead.
> > > >
> > > > This is not all, it uses driver->serialize() as well.
> > >
> > > Ah so it does. But still, usbdevfs uses neither,
> > > and that's clearly wrong: three locks to cover
> > > one data item ... wrong, wrong, wrong. (BKL,
> > > dev->serialize, and driver->serialize).
> >
> > Yes, locking there is incorrect.
> > The BKL in usbdevfs is needed at least to prevent concurrent claimings of
> > the same interface. Taking a semaphore there should probably allow it to
> > be removed. Two locks are perhaps a little awkward by not in themselves a
> > bug.
>
> Such a superfluous lock (driver->serialize) is a bug
> waiting to happen, even if it's not currently active ... :)
Perhaps. On the other hand it's one additional lock of limited scope against
dozens of possibly racy drivers :-)
> Re driver->serialize:
> > > Which isn't a good API usage policy. Which drivers
> > > do that? I scanned a bunch, didn't see any expecting
> > > that to be needed. But I did see a bunch that do static
> > > init or use module_init() for such stuff.
> >
> > Have a close look at the probe() of printer.c.
>
> ... a driver which has slowly been getting repaired; it's
> not exactly had a maintainer, has long been troublesome.
OK. Possibly a subconcious choice :->
> > I chose to check this driver at random and saw a potential race in the
> > first driver I looked at. Relaxing locking in a stable kernel is really
> > not a good idea.
>
> It's not like I said "driver->serialize should be removed", for that
> very reason. However, I will say that any driver that thinks it's OK
> to initialize global state in probe() not module_init() should be fixed.
> (THEN it'd be OK to remove driver->serialize ... :) That's why
> I wanted to know what drivers do that! The half dozen I looked
> at didn't have that kind of bug, so it's not that widespread an issue.
Well, it's not only initialisation. Most drivers maintain a table or list of
attached devices they drive. There are drivers which don't manipulate this
data structures atomically, eg scanner.c. They all have to have locking added.
I'd strongly advocate caution. At least this is IMO 2.5 stuff.
Regards
Oliver
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
