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:

quoted

On 24/02/26 23:25, Marc Zyngier wrote:

quoted

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