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]
Date:	Sun, 18 Nov 2012 10:17:47 +0100
From:	Daniel Lezcano <daniel.lezcano@...aro.org>
To:	Francesco Lavra <francescolavra.fl@...il.com>
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

On 11/18/2012 09:40 AM, Francesco Lavra wrote:
> 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

Yep :)



-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ