linux-kernel - Re: [PATCH V6 2/3] cpuidle: play_idle: Specify play

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: <89b421d7-4207-9b6c-15d8-e17d395cfc98@linaro.org>
Date:   Wed, 13 Nov 2019 13:37:14 +0100
From:   Daniel Lezcano <daniel.lezcano@...aro.org>
To:     "Rafael J. Wysocki" <rjw@...ysocki.net>
Cc:     mathieu.poirier@...aro.org, mingo@...hat.com, peterz@...radead.org,
        linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org,
        ulf.hansson@...aro.org
Subject: Re: [PATCH V6 2/3] cpuidle: play_idle: Specify play_idle with an idle
 state

On 08/11/2019 02:20, Rafael J. Wysocki wrote:
> On Wednesday, October 30, 2019 8:51:40 AM CET Daniel Lezcano wrote:
>> Currently, the play_idle function does not allow to tell which idle
>> state we want to go. Improve this by passing the idle state as
>> parameter to the function.
>>
>> Export cpuidle_find_deepest_state() symbol as it is used from the
>> intel_powerclamp driver as a module.

[ ... ]

>>  /**
>> diff --git a/drivers/thermal/intel/intel_powerclamp.c b/drivers/thermal/intel/intel_powerclamp.c
>> index 53216dcbe173..b55786c169ae 100644
>> --- a/drivers/thermal/intel/intel_powerclamp.c
>> +++ b/drivers/thermal/intel/intel_powerclamp.c
>> @@ -29,6 +29,7 @@
>>  #include <linux/delay.h>
>>  #include <linux/kthread.h>
>>  #include <linux/cpu.h>
>> +#include <linux/cpuidle.h>
>>  #include <linux/thermal.h>
>>  #include <linux/slab.h>
>>  #include <linux/tick.h>
>> @@ -430,7 +431,8 @@ static void clamp_idle_injection_func(struct kthread_work *work)
>>  	if (should_skip)
>>  		goto balance;
>>  
>> -	play_idle(jiffies_to_usecs(w_data->duration_jiffies));
>> +	play_idle(jiffies_to_usecs(w_data->duration_jiffies),
>> +		  cpuidle_find_deepest_state());
> 
> I don't see a reason for changing the code here like this.
> 
> What you really need is to have a way to set a limit on the idle
> state exit latency for idle injection on ARM.
> 
> For that you can pass the exit latency limit to play_idle(), but then
> you need to change powerclamp to pass UNIT_MAX or similar which is
> ugly, or you can redefine cpuidle_use_deepest_state() to take the
> exit latency limit as the arg (with 0 meaning use_deepest_state == false).

I'm confused with the "... ugly, *or* ...". In any case we have to
specify a latency constraint to play_idle(), no?

> In the latter case, it would be quite straightforward to add an
> exit_latency argument to cpuidle_find_deepest_state() and note that
> find_deepest_state() takes a max_latency arg already, so that would be
> a trivial change (hint!).




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