linux-kernel - Re: [PATCH 2/3] arm64: topology: Use current freq in governor for idle cpus in cpuinfo_avg

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]

Message-ID: <6dff34d1-731e-4e18-a719-04da498b28ee@huawei.com>
Date: Sun, 16 Nov 2025 15:46:57 +0800
From: "yubowen (H)" <yubowen8@...wei.com>
To: Beata Michalska <beata.michalska@....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

在 2025/11/11 1:11, Beata Michalska 写道:
> 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
Hi Beata,

Thank you for pointing out the important distinction between hardware and
software views of current frequency. The motivation behind this patch is
not to blur these boundaries, but rather to avoid returning -EAGAIN when
all CPUs under a policy are idle — a relatively common scenario,
especially in idle or low-utilization systems.

I'm relatively new to x86 architecture, so I might be missing something,
but it appears that on x86, a fallback to software-derived frequency
(e.g., via scaling_cur_freq) is always in effect in arch_freq_get_on_cpu()
when the last hardware update is too stale. However, I couldn't locate the
config option that controls this behavior — could you please clarify which
option you were referring to?

I fully understand the concern about mixing hardware and software
frequency views. That said, when no CPU is active and hardware provides
no real-time feedback, there is no available hardware source for current
frequency. In such cases, falling back to the last known frequency
(as maintained by the cpufreq core) seems like a reasonable compromise
— it provides a stale-but-plausible value instead of an error, which
improves usability for monitoring tools and user-space interfaces.

That said, I'm open to better alternatives. If there's a more appropriate
way to handle this scenario — such as deferring to the governor,
introducing a policy-specific fallback, or adjusting the cpufreq core
logic — I’d be happy to revise the patch accordingly.

Thanks again for the reference thread [1], it's very helpful.

BR
Bowen Yu
>> 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