On Wed, 25 Apr 2007, Oliver Neukum wrote: > > Yes, the problem is clear. Presumably you want to change things to go > > like this: > > > > usb_storage_control_thread(): > > usb_pm_lock(us->pusb_dev); > > mutex_lock(&us->dev_mutex); > > if (test_bit(US_FLIDX_DISCONNECTING, &us->flags)) ... > > __usb_autopm_get_interface(); > > usb_pm_unlock(us->pusb_dev); > > > > Right? I've been thinking about this exact same problem recently. It > > affect every USB driver, in principle. You might prefer the approach I > > thought up; it involves adding a new mutex to struct us_data like so: > > > > storage_suspend(): > > mutex_lock(&us->suspend_mutex); > > > > usb_stor_control_thread(): > > mutex_lock(&us->dev_mutex); > > if (test_bit(US_FLIDX_DISCONNECTING, &us->flags)) ... > > usb_autopm_get_interface(); > > mutex_lock(&us->suspend_mutex); > > > > Either way is awkward, but mine is a little cleaner. What do you think? > > If you want me to think I can do that. If both solutions we come up > are awkward, we are doing something wrong. It seems to me we should > reduce the number of locks, not add new locks. Can we get rid of the > special pm lock, use a general lock and export that to the drivers?
No, that's impossible. It's because the locking rules for the pm_mutex are opposite those for dev->sem: The order of acquisition of the dev->sem locks is _down_ the device tree, but the order of acquisition of the udev->pm_mutex locks is _up_ the device tree. However in this particular case we may indeed be able to simplify things. Just do this: usb_stor_control_thread(): usb_autopm_get_interface(); mutex_lock(&us->dev_mutex); if (test_bit(US_FLIDX_DISCONNECTING, &us->flags)) ... The only possible problem here is that the code does PM-related stuff at a time when we aren't certain the driver is still bound to the interface. So change the way storage_disconnect() works; make it wait until the control thread has exited. If we convert over to using kthread_stop() this will happen naturally. I'll work on a patch to do that conversion... Alan Stern ------------------------------------------------------------------------- This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ _______________________________________________ [email protected] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
