On Thu, 4 Nov 2010, Rafael J. Wysocki wrote:
> OK, so I think we can relax the locking in dpm_[suspend/resume]_noirq() to
> avoid executing callbacks under dpm_list_mtx, like in the (untested) patch
> below.
>
> Alan, do you see any immediate problem with that?
>
> Rafael
>
> ---
> drivers/base/power/main.c | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> Index: linux-2.6/drivers/base/power/main.c
> ===================================================================
> --- linux-2.6.orig/drivers/base/power/main.c
> +++ linux-2.6/drivers/base/power/main.c
> @@ -480,15 +480,23 @@ void dpm_resume_noirq(pm_message_t state
>
> mutex_lock(&dpm_list_mtx);
> transition_started = false;
> - list_for_each_entry(dev, &dpm_list, power.entry)
> + list_for_each_entry(dev, &dpm_list, power.entry) {
> + get_device(dev);
> if (dev->power.status > DPM_OFF) {
> int error;
>
> dev->power.status = DPM_OFF;
> +
> + mutex_unlock(&dpm_list_mtx);
> +
> error = device_resume_noirq(dev, state);
> +
> + mutex_lock(&dpm_list_mtx);
> if (error)
> pm_dev_err(dev, state, " early", error);
> }
> + put_device(dev);
> + }
> mutex_unlock(&dpm_list_mtx);
> dpm_show_time(starttime, state, "early");
> resume_device_irqs();
> @@ -796,12 +804,19 @@ int dpm_suspend_noirq(pm_message_t state
> suspend_device_irqs();
> mutex_lock(&dpm_list_mtx);
> list_for_each_entry_reverse(dev, &dpm_list, power.entry) {
> + get_device(dev);
> + mutex_unlock(&dpm_list_mtx);
> +
> error = device_suspend_noirq(dev, state);
> +
> + mutex_lock(&dpm_list_mtx);
> if (error) {
> pm_dev_err(dev, state, " late", error);
> + put_device(dev);
> break;
> }
> dev->power.status = DPM_OFF_IRQ;
> + put_device(dev);
> }
> mutex_unlock(&dpm_list_mtx);
> if (error)
This won't work if the callback tries to unregister the device, but of
course the old code wouldn't work in that case either. We should
document this requirement. It means that your get_device and
put_device calls aren't needed.
Aside from that I don't see any problems. In principle there shouldn't
be any other processes running at this time that would want to access
either the device or the dpm_list. Maybe this means the socket locking
isn't needed in the pcmcia late-suspend and early-resume routines,
which would be a simpler solution.
Alan Stern
_______________________________________________
linux-pm mailing list
linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/linux-pm
