linux-kernel - Re: [PATCH v3] cpufreq/cppc: Remove the desired

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: <dd536c45-cf2a-4b00-b1cc-c23e3157a4b8@arm.com>
Date: Wed, 12 Jun 2024 11:08:54 +0200
From: Pierre Gondois <pierre.gondois@....com>
To: Riwen Lu <luriwen@...mail.com>
Cc: linux-pm@...r.kernel.org, viresh.kumar@...aro.org, rafael@...nel.org,
 linux-kernel@...r.kernel.org, ionela.voinescu@....com,
 beata.michalska@....com, hotran@....com, Riwen Lu <luriwen@...inos.cn>
Subject: Re: [PATCH v3] cpufreq/cppc: Remove the desired_perf compare when set
 target

Hello Riwen,

This function seems to be the only cpufreq function saving and comparing the
requested frequency with the last requested frequency. This seems to be more the
task of the cpufreq framework than the cpufreq driver.

So FYIW, the patch looks good to me.

On 5/30/24 13:08, Riwen Lu wrote:
> From: Riwen Lu <luriwen@...inos.cn>
> 
> There is a case that desired_perf is exactly the same with the old perf,
> but the actual current freq is not.
> 
> This happened in S3 while the cpufreq governor is set to powersave.
> During cpufreq resume process, the booting CPU's new_freq obtained via
> .get() is the highest frequency, while the policy->cur and
> cpu->perf_ctrls.desired_perf are in the lowest level(powersave
> governor). Causing the warning: "CPU frequency out of sync:", and set
> policy->cur to new_freq.

(new paragraph)

Then the governor->limits() calls
> cppc_cpufreq_set_target() to configures the CPU frequency and returns
> directly because the desired_perf converted from target_freq is the
> same with cpu->perf_ctrls.desired_perf and both are the lowest_perf.

(new paragraph)

> Since target_freq and policy->cur have been compared in
> __cpufreq_driver_target(), there's no need to compare desired_perf
> and cpu->perf_ctrls.desired_perf again in cppc_cpufreq_set_target()
> to ensure that the CPU frequency is properly configured.

NIT:
Would it be possible to make distinct paragraphs ?

> 
> Signed-off-by: Riwen Lu <luriwen@...inos.cn>
> 
> ---
> v1 -> v2:
>   - Update commit message and email.
> v2 -> v3:
>   - Update patch subject and commit message.
>   - Remove the desired_perf compare logic.
> ---
>   drivers/cpufreq/cppc_cpufreq.c | 3 ---
>   1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> index 15f1d41920a3..337cece61ab5 100644
> --- a/drivers/cpufreq/cppc_cpufreq.c
> +++ b/drivers/cpufreq/cppc_cpufreq.c
> @@ -295,9 +295,6 @@ static int cppc_cpufreq_set_target(struct cpufreq_policy *policy,
>   	int ret = 0;
>   
>   	desired_perf = cppc_khz_to_perf(&cpu_data->perf_caps, target_freq);
> -	/* Return if it is exactly the same perf */
> -	if (desired_perf == cpu_data->perf_ctrls.desired_perf)
> -		return ret;
>   
>   	cpu_data->perf_ctrls.desired_perf = desired_perf;
>   	freqs.old = policy->cur;