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