[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aRIc20ErWTQr4dRa@arm.com>
Date: Mon, 10 Nov 2025 18:11:55 +0100
From: Beata Michalska <beata.michalska@....com>
To: Bowen Yu <yubowen8@...wei.com>
Cc: linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
catalin.marinas@....com, will@...nel.org, ptsm@...ux.microsoft.com,
linuxarm@...wei.com, jonathan.cameron@...wei.com,
zhanjie9@...ilicon.com, prime.zeng@...ilicon.com,
wanghuiqiang@...wei.com, xuwei5@...wei.com, zhenglifeng1@...wei.com,
zhangpengjie2@...wei.com
Subject: Re: [PATCH 2/3] arm64: topology: Use current freq in governor for
idle cpus in cpuinfo_avg_freq
On Tue, Nov 04, 2025 at 03:55:43PM +0800, Bowen Yu wrote:
> The current cpuinfo_avg_freq interface returns an error when all CPUs
> under a policy are idle, which is relatively common. To address this, it
> is better to use the current frequency stored in the governor. This
> implementation is also used on x86 architecture.
Well, the very reason of having this knob was not to mix hardware and software
view of so called 'current frequency'. Note that for x86 there is a dedicated
config option that keeps the behaviour you have mentioned for backward
compatibility.
cpuinfo_avg_freq -> average freq as seen by hw
cpuinfo_cur_freq -> closes one can get to current frequency - hw feedback
scaling_cur_freq -> requested freq, so smth the software believes should be the
current freq (often might not be)
The following thread might be useful -> [1]
---
[1] https://lore.kernel.org/all/20240603114811.oio3uemniib5uaa2@vireshk-i7/
---
BR
Beata
>
> Since the current frequency in the governor is the last known frequency,
> it should be more user-friendly.
>
> This patch also removes redundant !housekeeping_cpu() check since it is
> inherently done when checking jiffies.
>
> Original output when all cpus under a policy are idle:
> [root@...alhost home]# cat /sys/devices/system/cpu/cpufreq/policy0/
> cpuinfo_avg_freq
> cat: /sys/devices/system/cpu/cpufreq/policy0/cpuinfo_avg_freq: Resource
> temporarily unavailable
>
> Output after changes:
> [root@...alhost home]# cat /sys/devices/system/cpu/cpufreq/policy0
> /cpuinfo_avg_freq
> 1200000
>
> Signed-off-by: Bowen Yu <yubowen8@...wei.com>
> ---
> arch/arm64/kernel/topology.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
> index c0dbc27289ea..f1370a4a4df9 100644
> --- a/arch/arm64/kernel/topology.c
> +++ b/arch/arm64/kernel/topology.c
> @@ -333,14 +333,13 @@ int arch_freq_get_on_cpu(int cpu)
> if (!idle_cpu(ref_cpu))
> break;
> }
> +
> + if (ref_cpu >= nr_cpu_ids) {
> + cpufreq_cpu_put(policy);
> + return cpufreq_quick_get(start_cpu);
> + }
>
> cpufreq_cpu_put(policy);
> -
> - if (ref_cpu >= nr_cpu_ids)
> - /* No alternative to pull info from */
> - return -EAGAIN;
> -
> - cpu = ref_cpu;
> } else {
> break;
> }
> --
> 2.33.0
Powered by blists - more mailing lists