[PATCH v2] arm64: topology: Fix false warning in counters_read_on_cpu() for same-CPU reads

[PATCH v2] arm64: topology: Fix false warning in counters_read_on_cpu() for same-CPU reads

[Sumit Gupta]

The counters_read_on_cpu() function warns when called with IRQs
disabled to prevent deadlock in smp_call_function_single(). However,
this warning is spurious when reading counters on the current CPU,
since no IPI is needed for same CPU reads.

Commit 12eb8f4fff24 ("cpufreq: CPPC: Update FIE arch_freq_scale in
ticks for non-PCC regs") changed the CPPC Frequency Invariance Engine
to read AMU counters directly from the scheduler tick for non-PCC
register spaces (like FFH), instead of deferring to a kthread. This
means counters_read_on_cpu() is now called with IRQs disabled from
the tick handler, triggering the warning:

| WARNING: arch/arm64/kernel/topology.c:410 at counters_read_on_cpu
| ...
| Call trace:
|  counters_read_on_cpu+0x88/0xa8 (P)
|  cpc_read_ffh+0xdc/0x148
|  cpc_read+0x260/0x518
|  cppc_get_perf_ctrs+0xf0/0x398
|  __cppc_scale_freq_tick+0x4c/0x148 [cppc_cpufreq]
|  cppc_scale_freq_tick+0x44/0x88 [cppc_cpufreq]
|  topology_scale_freq_tick+0x34/0x58
|  sched_tick+0x58/0x300
|  update_process_times+0xcc/0x120
|  tick_nohz_handler+0xa8/0x260
|  __hrtimer_run_queues+0x154/0x360
|  hrtimer_interrupt+0xf4/0x2b0
|  arch_timer_handler_phys+0x4c/0x78
|  ....
|  CPPC Cpufreq:__cppc_scale_freq_tick: failed to read perf counters
|  ....

Fix this by calling the counter read function directly for same CPU
case, bypassing smp_call_function_single(). Use get_cpu() to disable
preemption, as the counter read functions call this_cpu_has_cap()
which requires a non-preemptible context.

Fixes: 997c021abc6e ("cpufreq: CPPC: Update FIE arch_freq_scale in ticks for non-PCC regs")
Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
Reviewed-by: Jie Zhan <zhanjie9@hisilicon.com>
---
v1 -> v2:
 - Rebased on v7.0-rc1
 - Updated Fixes tag to match upstream commit SHA
---
 arch/arm64/kernel/topology.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index 3fe1faab0362..c3e883e99aa0 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -400,12 +400,29 @@ static inline
 int counters_read_on_cpu(int cpu, smp_call_func_t func, u64 *val)
 {
 	/*
-	 * Abort call on counterless CPU or when interrupts are
-	 * disabled - can lead to deadlock in smp sync call.
+	 * Abort call on counterless CPU.
 	 */
 	if (!cpu_has_amu_feat(cpu))
 		return -EOPNOTSUPP;
 
+	/*
+	 * For same-CPU reads, call the function directly since no IPI
+	 * is needed and this is safe even with IRQs disabled.
+	 * Use get_cpu() to disable preemption as the counter read
+	 * functions call this_cpu_has_cap() which requires a
+	 * non-preemptible context.
+	 */
+	if (cpu == get_cpu()) {
+		func(val);
+		put_cpu();
+		return 0;
+	}
+	put_cpu();
+
+	/*
+	 * Reading from a remote CPU requires IRQs enabled to avoid
+	 * deadlock in smp_call_function_single().
+	 */
 	if (WARN_ON_ONCE(irqs_disabled()))
 		return -EPERM;
 
-- 
2.34.1

Re: [PATCH v2] arm64: topology: Fix false warning in counters_read_on_cpu() for same-CPU reads

[Marc Zyngier]

On Tue, 24 Feb 2026 09:27:14 +0000,
Sumit Gupta <sumitg@nvidia.com> wrote:
> 
> The counters_read_on_cpu() function warns when called with IRQs
> disabled to prevent deadlock in smp_call_function_single(). However,
> this warning is spurious when reading counters on the current CPU,
> since no IPI is needed for same CPU reads.
> 
> Commit 12eb8f4fff24 ("cpufreq: CPPC: Update FIE arch_freq_scale in
> ticks for non-PCC regs") changed the CPPC Frequency Invariance Engine
> to read AMU counters directly from the scheduler tick for non-PCC
> register spaces (like FFH), instead of deferring to a kthread. This
> means counters_read_on_cpu() is now called with IRQs disabled from
> the tick handler, triggering the warning:
> 
> | WARNING: arch/arm64/kernel/topology.c:410 at counters_read_on_cpu
> | ...
> | Call trace:
> |  counters_read_on_cpu+0x88/0xa8 (P)
> |  cpc_read_ffh+0xdc/0x148
> |  cpc_read+0x260/0x518
> |  cppc_get_perf_ctrs+0xf0/0x398
> |  __cppc_scale_freq_tick+0x4c/0x148 [cppc_cpufreq]
> |  cppc_scale_freq_tick+0x44/0x88 [cppc_cpufreq]
> |  topology_scale_freq_tick+0x34/0x58
> |  sched_tick+0x58/0x300
> |  update_process_times+0xcc/0x120
> |  tick_nohz_handler+0xa8/0x260
> |  __hrtimer_run_queues+0x154/0x360
> |  hrtimer_interrupt+0xf4/0x2b0
> |  arch_timer_handler_phys+0x4c/0x78
> |  ....
> |  CPPC Cpufreq:__cppc_scale_freq_tick: failed to read perf counters
> |  ....
> 
> Fix this by calling the counter read function directly for same CPU
> case, bypassing smp_call_function_single(). Use get_cpu() to disable
> preemption, as the counter read functions call this_cpu_has_cap()
> which requires a non-preemptible context.
> 
> Fixes: 997c021abc6e ("cpufreq: CPPC: Update FIE arch_freq_scale in ticks for non-PCC regs")
> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
> Reviewed-by: Jie Zhan <zhanjie9@hisilicon.com>
> ---
> v1 -> v2:
>  - Rebased on v7.0-rc1
>  - Updated Fixes tag to match upstream commit SHA
> ---
>  arch/arm64/kernel/topology.c | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
> index 3fe1faab0362..c3e883e99aa0 100644
> --- a/arch/arm64/kernel/topology.c
> +++ b/arch/arm64/kernel/topology.c
> @@ -400,12 +400,29 @@ static inline
>  int counters_read_on_cpu(int cpu, smp_call_func_t func, u64 *val)
>  {
>  	/*
> -	 * Abort call on counterless CPU or when interrupts are
> -	 * disabled - can lead to deadlock in smp sync call.
> +	 * Abort call on counterless CPU.
>  	 */
>  	if (!cpu_has_amu_feat(cpu))
>  		return -EOPNOTSUPP;
>  
> +	/*
> +	 * For same-CPU reads, call the function directly since no IPI
> +	 * is needed and this is safe even with IRQs disabled.
> +	 * Use get_cpu() to disable preemption as the counter read
> +	 * functions call this_cpu_has_cap() which requires a
> +	 * non-preemptible context.
> +	 */
> +	if (cpu == get_cpu()) {
> +		func(val);
> +		put_cpu();
> +		return 0;
> +	}
> +	put_cpu();

A slightly more elegant way to write this would be:

diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index 3fe1faab03620..83c7b346dc8cf 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -406,6 +406,13 @@ int counters_read_on_cpu(int cpu, smp_call_func_t func, u64 *val)
 	if (!cpu_has_amu_feat(cpu))
 		return -EOPNOTSUPP;
 
+	scoped_guard(preempt) {
+		if (cpu == raw_smp_processor_id()) {
+			func(val);
+			return 0;
+		}
+	}
+
 	if (WARN_ON_ONCE(irqs_disabled()))
 		return -EPERM;
 
But I'm more concerned by the overall pattern of doing these things in
random contexts. Going back to the original patch (997c021abc6e) that
states:

"However, this deferred update mechanism is unnecessary and introduces extra
 overhead for non-PCC register spaces (e.g. System Memory or FFH), where
 accessing the regs won't sleep and can be safely performed from the tick
 context."

Clearly, the AMU registers cannot be arbitrarily accessed without
blocking when accessed from one CPU to another, so either this
function is never called in a cross-cpu context (and the warning
should be removed), or the premises of this change are wrong.

Which one is it?

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH v2] arm64: topology: Fix false warning in counters_read_on_cpu() for same-CPU reads

[Sumit Gupta]

Hi Marc,


On 24/02/26 23:25, Marc Zyngier wrote:
> External email: Use caution opening links or attachments
>
>
> On Tue, 24 Feb 2026 09:27:14 +0000,
> Sumit Gupta <sumitg@nvidia.com> wrote:
>> The counters_read_on_cpu() function warns when called with IRQs
>> disabled to prevent deadlock in smp_call_function_single(). However,
>> this warning is spurious when reading counters on the current CPU,
>> since no IPI is needed for same CPU reads.
>>
>> Commit 12eb8f4fff24 ("cpufreq: CPPC: Update FIE arch_freq_scale in
>> ticks for non-PCC regs") changed the CPPC Frequency Invariance Engine
>> to read AMU counters directly from the scheduler tick for non-PCC
>> register spaces (like FFH), instead of deferring to a kthread. This
>> means counters_read_on_cpu() is now called with IRQs disabled from
>> the tick handler, triggering the warning:
>>
>> | WARNING: arch/arm64/kernel/topology.c:410 at counters_read_on_cpu
>> | ...
>> | Call trace:
>> |  counters_read_on_cpu+0x88/0xa8 (P)
>> |  cpc_read_ffh+0xdc/0x148
>> |  cpc_read+0x260/0x518
>> |  cppc_get_perf_ctrs+0xf0/0x398
>> |  __cppc_scale_freq_tick+0x4c/0x148 [cppc_cpufreq]
>> |  cppc_scale_freq_tick+0x44/0x88 [cppc_cpufreq]
>> |  topology_scale_freq_tick+0x34/0x58
>> |  sched_tick+0x58/0x300
>> |  update_process_times+0xcc/0x120
>> |  tick_nohz_handler+0xa8/0x260
>> |  __hrtimer_run_queues+0x154/0x360
>> |  hrtimer_interrupt+0xf4/0x2b0
>> |  arch_timer_handler_phys+0x4c/0x78
>> |  ....
>> |  CPPC Cpufreq:__cppc_scale_freq_tick: failed to read perf counters
>> |  ....
>>
>> Fix this by calling the counter read function directly for same CPU
>> case, bypassing smp_call_function_single(). Use get_cpu() to disable
>> preemption, as the counter read functions call this_cpu_has_cap()
>> which requires a non-preemptible context.
>>
>> Fixes: 997c021abc6e ("cpufreq: CPPC: Update FIE arch_freq_scale in ticks for non-PCC regs")
>> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
>> Reviewed-by: Jie Zhan <zhanjie9@hisilicon.com>
>> ---
>> v1 -> v2:
>>   - Rebased on v7.0-rc1
>>   - Updated Fixes tag to match upstream commit SHA
>> ---
>>   arch/arm64/kernel/topology.c | 21 +++++++++++++++++++--
>>   1 file changed, 19 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
>> index 3fe1faab0362..c3e883e99aa0 100644
>> --- a/arch/arm64/kernel/topology.c
>> +++ b/arch/arm64/kernel/topology.c
>> @@ -400,12 +400,29 @@ static inline
>>   int counters_read_on_cpu(int cpu, smp_call_func_t func, u64 *val)
>>   {
>>        /*
>> -      * Abort call on counterless CPU or when interrupts are
>> -      * disabled - can lead to deadlock in smp sync call.
>> +      * Abort call on counterless CPU.
>>         */
>>        if (!cpu_has_amu_feat(cpu))
>>                return -EOPNOTSUPP;
>>
>> +     /*
>> +      * For same-CPU reads, call the function directly since no IPI
>> +      * is needed and this is safe even with IRQs disabled.
>> +      * Use get_cpu() to disable preemption as the counter read
>> +      * functions call this_cpu_has_cap() which requires a
>> +      * non-preemptible context.
>> +      */
>> +     if (cpu == get_cpu()) {
>> +             func(val);
>> +             put_cpu();
>> +             return 0;
>> +     }
>> +     put_cpu();
> A slightly more elegant way to write this would be:
>
> diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
> index 3fe1faab03620..83c7b346dc8cf 100644
> --- a/arch/arm64/kernel/topology.c
> +++ b/arch/arm64/kernel/topology.c
> @@ -406,6 +406,13 @@ int counters_read_on_cpu(int cpu, smp_call_func_t func, u64 *val)
>          if (!cpu_has_amu_feat(cpu))
>                  return -EOPNOTSUPP;
>
> +       scoped_guard(preempt) {
> +               if (cpu == raw_smp_processor_id()) {
> +                       func(val);
> +                       return 0;
> +               }
> +       }
> +
>          if (WARN_ON_ONCE(irqs_disabled()))
>                  return -EPERM;

Thank you for the suggestion.
Will switched to this in v3.


> But I'm more concerned by the overall pattern of doing these things in
> random contexts. Going back to the original patch (997c021abc6e) that
> states:
>
> "However, this deferred update mechanism is unnecessary and introduces extra
>   overhead for non-PCC register spaces (e.g. System Memory or FFH), where
>   accessing the regs won't sleep and can be safely performed from the tick
>   context."
>
> Clearly, the AMU registers cannot be arbitrarily accessed without
> blocking when accessed from one CPU to another, so either this
> function is never called in a cross-cpu context (and the warning
> should be removed), or the premises of this change are wrong.
>
> Which one is it?
>
> Thanks,
>
>          M.

The function is also called in cross-CPU contexts, but only from
process context (get_rate, sysfs show) with IRQs enabled and
calling smp_call_function_single() is safe. In the tick path,
cppc_scale_freq_tick() uses per_cpu(cppc_freq_inv, smp_processor_id()).
So we always read the current CPU's counters and no cross-CPU access.

The premise of 997c021abc6e applies to same-CPU accesses only:
Reading the local CPU's AMU regs does not sleep and is safe from
tick context.
The irqs_disabled() WARN is still needed to guard against unsafe
cross-CPU calls. This also returns '-EPERM' unlike the WARN inside
smp_call_function_single(). So we fail instead of risking deadlock.

Thank you,
Sumit Gupta

Re: [PATCH v2] arm64: topology: Fix false warning in counters_read_on_cpu() for same-CPU reads

[Will Deacon]

On Wed, Feb 25, 2026 at 05:08:03PM +0530, Sumit Gupta wrote:
> On 24/02/26 23:25, Marc Zyngier wrote:
> > But I'm more concerned by the overall pattern of doing these things in
> > random contexts. Going back to the original patch (997c021abc6e) that
> > states:
> > 
> > "However, this deferred update mechanism is unnecessary and introduces extra
> >   overhead for non-PCC register spaces (e.g. System Memory or FFH), where
> >   accessing the regs won't sleep and can be safely performed from the tick
> >   context."
> > 
> > Clearly, the AMU registers cannot be arbitrarily accessed without
> > blocking when accessed from one CPU to another, so either this
> > function is never called in a cross-cpu context (and the warning
> > should be removed), or the premises of this change are wrong.
> > 
> > Which one is it?
> 
> The function is also called in cross-CPU contexts, but only from
> process context (get_rate, sysfs show) with IRQs enabled and
> calling smp_call_function_single() is safe. In the tick path,
> cppc_scale_freq_tick() uses per_cpu(cppc_freq_inv, smp_processor_id()).
> So we always read the current CPU's counters and no cross-CPU access.
> 
> The premise of 997c021abc6e applies to same-CPU accesses only:
> Reading the local CPU's AMU regs does not sleep and is safe from
> tick context.
> The irqs_disabled() WARN is still needed to guard against unsafe
> cross-CPU calls. This also returns '-EPERM' unlike the WARN inside
> smp_call_function_single(). So we fail instead of risking deadlock.

Hmm, so why isn't this structured such as:

	if (irqs_disabled()) {
		/* XXX: Explain the tick case */
		if (WARN_ON_ONCE(cpu != smp_processor_id()))
			return -EPERM;
		func(val);
	} else {
		smp_call_function_single(cpu, func, val, 1);
	}

	return 0;

That way, the irqs-enabled case remains the same and doesn't need to
mess with preemption.

Will

Re: [PATCH v2] arm64: topology: Fix false warning in counters_read_on_cpu() for same-CPU reads

[Sumit Gupta]

Hi Will,


On 26/02/26 03:46, Will Deacon wrote:
> External email: Use caution opening links or attachments
>
>
> On Wed, Feb 25, 2026 at 05:08:03PM +0530, Sumit Gupta wrote:
>> On 24/02/26 23:25, Marc Zyngier wrote:
>>> But I'm more concerned by the overall pattern of doing these things in
>>> random contexts. Going back to the original patch (997c021abc6e) that
>>> states:
>>>
>>> "However, this deferred update mechanism is unnecessary and introduces extra
>>>    overhead for non-PCC register spaces (e.g. System Memory or FFH), where
>>>    accessing the regs won't sleep and can be safely performed from the tick
>>>    context."
>>>
>>> Clearly, the AMU registers cannot be arbitrarily accessed without
>>> blocking when accessed from one CPU to another, so either this
>>> function is never called in a cross-cpu context (and the warning
>>> should be removed), or the premises of this change are wrong.
>>>
>>> Which one is it?
>> The function is also called in cross-CPU contexts, but only from
>> process context (get_rate, sysfs show) with IRQs enabled and
>> calling smp_call_function_single() is safe. In the tick path,
>> cppc_scale_freq_tick() uses per_cpu(cppc_freq_inv, smp_processor_id()).
>> So we always read the current CPU's counters and no cross-CPU access.
>>
>> The premise of 997c021abc6e applies to same-CPU accesses only:
>> Reading the local CPU's AMU regs does not sleep and is safe from
>> tick context.
>> The irqs_disabled() WARN is still needed to guard against unsafe
>> cross-CPU calls. This also returns '-EPERM' unlike the WARN inside
>> smp_call_function_single(). So we fail instead of risking deadlock.
> Hmm, so why isn't this structured such as:
>
>          if (irqs_disabled()) {
>                  /* XXX: Explain the tick case */
>                  if (WARN_ON_ONCE(cpu != smp_processor_id()))
>                          return -EPERM;
>                  func(val);
>          } else {
>                  smp_call_function_single(cpu, func, val, 1);
>          }
>
>          return 0;
>
> That way, the irqs-enabled case remains the same and doesn't need to
> mess with preemption.
>
> Will

I will do the change in v3 as suggested.


Thankyou
Sumit Gupta