linux-kernel - Re: [linux-pm] [GIT PULL] One more power management fix for 2.6.37

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 4 Nov 2010 10:50:57 -0400 (EDT)
From:	Alan Stern <stern@...land.harvard.edu>
To:	"Rafael J. Wysocki" <rjw@...k.pl>
cc:	Dominik Brodowski <linux@...inikbrodowski.net>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Linux-pm mailing list <linux-pm@...ts.linux-foundation.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [linux-pm] [GIT PULL] One more power management fix for 2.6.37

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/