linux-kernel - Re: [PATCH v2] cpufreq: check only freq_table in __resolve

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: <YwOn8rkvC1uUlxpn@e126311.manchester.arm.com>
Date:   Mon, 22 Aug 2022 16:59:46 +0100
From:   Kajetan Puchalski <kajetan.puchalski@....com>
To:     Lukasz Luba <lukasz.luba@....com>
Cc:     linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org,
        viresh.kumar@...aro.org, rafael@...nel.org,
        kajetan.puchalski@....com
Subject: Re: [PATCH v2] cpufreq: check only freq_table in __resolve_freq()

On Tue, Aug 16, 2022 at 01:01:57PM +0100, Lukasz Luba wrote:
> There is no need to check if the cpufreq driver implements callback
> cpufreq_driver::target_index. The logic in the __resolve_freq uses
> the frequency table available in the policy. It doesn't matter if the
> driver provides 'target_index' or 'target' callback. It just has to
> populate the 'policy->freq_table'.
> 
> Thus, check only frequency table during the frequency resolving call.
> 
> Acked-by: Viresh Kumar <viresh.kumar@...aro.org>
> Signed-off-by: Lukasz Luba <lukasz.luba@....com>
> ---
> Changes:
> v2:
> - collected ACK from Viresh
> - corrected patch description (Viresh)

Just to stress how important this fix is, without this patch while
running tests on a Pixel 6 cpufreq was reporting frequencies that
were completely not aligned with the OPPs on the device and harming
performance in the process.

For instance, after applying the patch the average score in Geekbench 5
increased by over 130 (about 5%).

The average score in Speedometer 2 increased by about 6 (over 6%).

>  drivers/cpufreq/cpufreq.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 7820c4e74289..69b3d61852ac 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -532,7 +532,7 @@ static unsigned int __resolve_freq(struct cpufreq_policy *policy,
>  
>  	target_freq = clamp_val(target_freq, policy->min, policy->max);
>  
> -	if (!cpufreq_driver->target_index)
> +	if (!policy->freq_table)
>  		return target_freq;
>  
>  	idx = cpufreq_frequency_table_target(policy, target_freq, relation);
> -- 
> 2.17.1
>