> > > > (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 ... :) 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. > 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. > The following patch should do it. Yes, that looked right. - Dave _______________________________________________ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
