linux-kernel - Re: [PATCH 3/3] thermal: cpu_cooling: Migrate to using the EM framework

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:   Fri, 29 Mar 2019 18:17:58 +0100
From:   Daniel Lezcano <daniel.lezcano@...aro.org>
To:     Quentin Perret <quentin.perret@....com>
Cc:     edubezval@...il.com, rui.zhang@...el.com, javi.merino@...nel.org,
        viresh.kumar@...aro.org, amit.kachhap@...il.com, rjw@...ysocki.net,
        will.deacon@....com, catalin.marinas@....com,
        dietmar.eggemann@....com, ionela.voinescu@....com,
        linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 3/3] thermal: cpu_cooling: Migrate to using the EM
 framework

On 29/03/2019 10:16, Quentin Perret wrote:
> Hi Daniel,
> 
> On Thursday 28 Mar 2019 at 21:23:35 (+0100), Daniel Lezcano wrote:
>>>  /**
>>>   * struct time_in_idle - Idle time stats
>>>   * @time: previous reading of the absolute time that this cpu was idle
>>> @@ -82,7 +70,7 @@ struct time_in_idle {
>>>   *	frequency.
>>>   * @max_level: maximum cooling level. One less than total number of valid
>>>   *	cpufreq frequencies.
>>> - * @freq_table: Freq table in descending order of frequencies
>>> + * @em: Reference on the Energy Model of the device
>>>   * @cdev: thermal_cooling_device pointer to keep track of the
>>>   *	registered cooling device.
>>>   * @policy: cpufreq policy.
>>> @@ -98,7 +86,7 @@ struct cpufreq_cooling_device {
>>>  	unsigned int cpufreq_state;
>>>  	unsigned int clipped_freq;
>>>  	unsigned int max_level;
>>> -	struct freq_table *freq_table;	/* In descending order */
>>> +	struct em_perf_domain *em;
>>
>> Why do you need to add this field? it will be accessible via policy->em, no?
> 
> You mean via the CPUFreq policy ? Then no, the EM isn't attached to the
> CPUFreq policy. And we can't attach it directly to the CPUFreq policy
> since in *theory* it is not required to map 1:1 to CPUFreq policies
> (even though that _is_ true for all existing platforms). That's one of
> the things this patch checks in that em_is_sane() function below.
> 
> FWIW, the idea of the design is, the EM framework is 'independent' and
> it's up to the client subsystems (scheduler, IPA) to check if it actually
> works for them. In the case of the scheduler, for example, we can't use
> an EM that's too complex because that would cause too much overhead, so
> we don't start EAS if that's not the case. See:
> 
>   https://elixir.bootlin.com/linux/latest/source/kernel/sched/topology.c#L367
> 
> In the case of IPA, we need to do something similar. We can't use an EM
> that doesn't map 1:1 to CPUFreq policies, so we bail out if that's not
> true, etc, ... This isn't supposed to trigger any time soon, but it's
> good to have a check just to be on the safe side I think.

Ok, makes sense. Thanks for the clarification.



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