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

[Sumit Gupta <sumitg@nvidia.com>]

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