> > 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. > > Actually, simply replacing ps->devsem with ps->dev->serialize > > cannot lead to any new deadlocks, it makes deadlocks that could > > occasionally happen always happen (such deadlocks exist right now in > > usbfs). Some of the current deadlocks can be eliminated without giving > > up ps->devsem, but not all. So the question is: must ps->dev->serialize > > be used? > > It must be held when you call usb_reset_configuration(). It must _not_ be > held when you call usb_set_configuration(). For usb_reset_device() right > now you must not hold it, although that may change in the future. For > usb_unbind_interface() you must not hold it. There's a note that > usb_driver_claim_interface() grabs the BKL for some reason having to do > with usbfs -- no doubt when usbfs is fixed that won't be needed and the > caller will be required to hold dev->serialize instead. 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. > 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. > > > Are they any reasons for not keeping ps->devsem? Since usbfs generally > > > acts as a driver and drivers generally don't have to concern themselves > > > with usbdev->serialize (the core handles it for them), shouldn't usbfs > > > also be able to ignore ps->dev->serialize? > > > > No, because it needs to do operations on interfaces it hasn't claimed > > (such as looking them up and claiming them). This is why it needs to > > protect itself, at least momentarily, against configurations shifting > > under it. This can be done by using the BKL more. However it can be > > done more simply using ps->dev->serialize (in fact it is simpler than > > what is there now). > > That agrees with my assessment. It ought to be possible to remove these > references to the BKL in favor of ps->dev->serialize. Yes, and that is what my patch does. And due to the above problem with deadlock it replaces ps->devsem with ps->dev->serialize everywhere. > > By the way, if it is somehow fatal to do usb_put_dev after disconnect, > > what is the point of referencing counting at all? You might as well > > free up the usb_device structure immediately after disconnect, since > > there is sure to be a reference before disconnect, and (apparently) > > there had better not be a reference after disconnect... > > 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. Ciao, Duncan. ------------------------------------------------------- This SF.net email is sponsored by: IBM Linux Tutorials. Become an expert in LINUX or just sharpen your skills. Sign up for IBM's Free Linux Tutorials. Learn everything from the bash shell to sys admin. Click now! http://ads.osdn.com/?ad_id=1278&alloc_id=3371&op=click _______________________________________________ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
