linux-kernel - Re: [RFC][PATCH] thermal/idle_inject: Support 100% idle injection

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] [day] [month] [year] [list]

Date:   Wed, 21 Dec 2022 21:43:24 +0100
From:   Daniel Lezcano <daniel.lezcano@...aro.org>
To:     srinivas pandruvada <srinivas.pandruvada@...ux.intel.com>,
        rafael@...nel.org
Cc:     linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC][PATCH] thermal/idle_inject: Support 100% idle injection

On 21/12/2022 21:36, srinivas pandruvada wrote:
> On Wed, 2022-12-21 at 14:43 +0100, Daniel Lezcano wrote:
>>
>> Hi Srinivas,
>>
>>
>> On 09/12/2022 02:36, Srinivas Pandruvada wrote:
>>> The users of idle injection framework allow 100% idle injection.
>>> For
>>> example: thermal/cpuidle_cooling.c driver. When the ratio set to
>>> 100%,
>>> the runtime_duration becomes zero.
>>>
>>> In the function idle_inject_set_duration() in idle injection
>>> framework
>>> run_duration_us == 0 is silently ignored, without any error (it is
>>> a
>>> void function). So, the caller will assume that everything is fine
>>> and
>>> 100% idle is effective. But in reality the idle inject will be
>>> whatever
>>> set before.
>>
>> Good catch
>>
>>> There are two options:
>>> - The caller change their max state to 99% instead of 100% and
>>> document that 100% is not supported by idle inject framework
>>> - Support 100% idle support in idle inject framework
>>
>> Yes, from my POV a CPU being impossible to cool down for any reason
>> should end up by staying off.
>>
>>> Since there are other protections via RT throttling, this framework
>>> can
>>> allow 100% idle. The RT throttling will be activated at 95% idle by
>>> default. The caller disabling RT throttling and injecting 100%
>>> idle,
>>> should be aware that CPU can't be used at all.
>>
>> Would it make sense to write a trace in this case ?
> 
> There is one printk already:
> printk_deferred_once("sched: RT throttling activated\n")
> You mean we should add
> 
> trace_sched_* for this?


I meant the CPU is going 100% idle


>>> The idle inject timer is started for (run_duration_us +
>>> idle_duration_us)
>>> duration. Hence replace (run_duration_us && idle_duration_us) with
>>> (run_duration_us + idle_duration_us) in the function
>>> idle_inject_set_duration().
>>
>> Sounds good to me
>>
> I will submit a patch for this.
> 
> Thanks,
> Srinivas
> 
>>> Signed-off-by: Srinivas Pandruvada
>>> <srinivas.pandruvada@...ux.intel.com>
>>> ---
>>>    drivers/powercap/idle_inject.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/powercap/idle_inject.c
>>> b/drivers/powercap/idle_inject.c
>>> index f48e71501429..4a4fe60d2563 100644
>>> --- a/drivers/powercap/idle_inject.c
>>> +++ b/drivers/powercap/idle_inject.c
>>> @@ -184,7 +184,7 @@ void idle_inject_set_duration(struct
>>> idle_inject_device *ii_dev,
>>>                                unsigned int run_duration_us,
>>>                                unsigned int idle_duration_us)
>>>    {
>>> -       if (run_duration_us && idle_duration_us) {
>>> +       if (run_duration_us + idle_duration_us) {
>>>                  WRITE_ONCE(ii_dev->run_duration_us,
>>> run_duration_us);
>>>                  WRITE_ONCE(ii_dev->idle_duration_us,
>>> idle_duration_us);
>>>          }
>>
> 

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