linux-kernel - Re: [RFC] cpuidle - remove the power

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <50A89F08.7020307@gmail.com>
Date:	Sun, 18 Nov 2012 09:40:40 +0100
From:	Francesco Lavra <francescolavra.fl@...il.com>
To:	Daniel Lezcano <daniel.lezcano@...aro.org>
CC:	linux-pm@...r.kernel.org, khilman@...com,
	deepthi@...ux.vnet.ibm.com, g.trinabh@...il.com,
	linaro-dev@...ts.linaro.org, len.brown@...el.com,
	linux-kernel@...r.kernel.org, rjw@...k.pl, jwerner@...omium.org,
	akpm@...ux-foundation.org, snanda@...omium.org
Subject: Re: [RFC] cpuidle - remove the power_specified field in the driver

Hi,

On 11/12/2012 09:26 PM, Daniel Lezcano wrote:
> This patch follows the discussion about reinitializing the power usage
> when a C-state is added/removed.
> 
>  https://lkml.org/lkml/2012/10/16/518
> 
> We realized the power usage field is never filled and when it is
> filled for tegra, the power_specified flag is not set making all these
> values to be resetted when the driver is initialized with the set_power_state
> function.
> 
> Julius and I feel this is over-engineered and the power_specified
> flag could be simply removed and continue assuming the states are
> backward sorted.
> 
> The menu governor select function is simplified as the power is ordered.
> Actually the condition is always true with the current code.
> 
> The cpuidle_play_dead function is also simplified by doing a reverse lookup
> on the array.
> 
> The set_power_states function is removed as it does no make sense anymore.
> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@...aro.org>
> ---
>  drivers/cpuidle/cpuidle.c        |   17 ++++-------------
>  drivers/cpuidle/driver.c         |   25 -------------------------
>  drivers/cpuidle/governors/menu.c |    8 ++------
>  include/linux/cpuidle.h          |    2 +-
>  4 files changed, 7 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index 711dd83..f983262 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -69,24 +69,15 @@ int cpuidle_play_dead(void)
>  {
>  	struct cpuidle_device *dev = __this_cpu_read(cpuidle_devices);
>  	struct cpuidle_driver *drv = cpuidle_get_cpu_driver(dev);
> -	int i, dead_state = -1;
> -	int power_usage = -1;
> +	int i;
>  
>  	if (!drv)
>  		return -ENODEV;
>  
>  	/* Find lowest-power state that supports long-term idle */
> -	for (i = CPUIDLE_DRIVER_STATE_START; i < drv->state_count; i++) {
> -		struct cpuidle_state *s = &drv->states[i];
> -
> -		if (s->power_usage < power_usage && s->enter_dead) {
> -			power_usage = s->power_usage;
> -			dead_state = i;
> -		}
> -	}
> -
> -	if (dead_state != -1)
> -		return drv->states[dead_state].enter_dead(dev, dead_state);
> +	for (i = drv->state_count; i >= CPUIDLE_DRIVER_STATE_START; i--)
> +		if (drv->states[i].play_dead)

I guess you meant drv->states[i].enter_dead

> +			return drv->states[i].enter_dead(dev, i);

--
Francesco
--
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/