Re: [PATCH] PM: Acquire device locks on suspend

On Sunday, 6 of January 2008, Alan Stern wrote:

On Sun, 6 Jan 2008, Rafael J. Wysocki wrote:

quoted

quoted

No -- the whole idea here is to print an error message in the system
log if a driver's resume method tries to call device_del().  Deadlock 
is unavoidable in this case, but at least we'll know which driver is 
guilty.

Still, if we do that, we won't need to acquire dev->sem in device_pm_remove()
any more.

There's a window in lock_all_devices() when dpm_list_mtx isn't held.  
We don't want device_pm_remove() taking an already-locked device off 
the dpm_locked list at that time.  So we do need to acquire dev->sem in 
device_pm_remove().

Not if pm_sleep_rwsem is held by device_del(), since in that case we won't
reach lock_all_devices() (device_add() calls device_pm_remove() under
pm_sleep_rwsem already).

quoted

Apart from this, by acqiring pm_sleep_rwsem for reading in
device_del() we can prevent a suspend from starting while the device is being
removed.

Consider, for example, the scenario possible with the $subject patch:
- device_del() starts and notices pm_sleep_rwsem unlocked, so the warning is
  not printed
- it proceeds and everything before device_pm_remove() succeeds
- now, device_suspend() is called and locks dev->sem
- device_del() calls device_pm_remove() and blocks on that with the device
  partialy removed
I think we should prevent this from happening.

I don't see anything wrong with it.  All that will happen is that the 
removal will start before the suspend and finish after the resume.

In that case, we'll attempt to call the device's .suspend() and .resume()
routines, but we shouldn't do that, IMHO.

Rafael