RE: [External] Re: [PATCH v3] arm64: topology: add source check in arch_cpu_idle_enter()

On Tus, Aug 11, 2026 at 06:03PM, Beata Michalska wrote:

> > --- a/arch/arm64/kernel/topology.c
> > +++ b/arch/arm64/kernel/topology.c
> > @@ -175,7 +175,8 @@ void arch_cpu_idle_enter(void)
> >
> >  	/* Kick in AMU update but only if one has not happened already */
> >  	if (housekeeping_cpu(cpu, HK_TYPE_TICK) &&
> > -
> time_is_before_jiffies(per_cpu(cpu_amu_samples.last_scale_update, cpu)))
> > +
> time_is_before_jiffies(per_cpu(cpu_amu_samples.last_scale_update, cpu))
> &&
> > +	    topology_is_scale_freq_source(SCALE_FREQ_SOURCE_ARCH, cpu))
> >  		amu_scale_freq_tick();
> I'm not entirely convinced you gained a lot by that.
> It's one additional check per each enter_idle for case where AMUs are the
> source vs 2 additional check when it is not.
> Will try to figure out smth less 'invasive'.
> 

First, I think that the rcu_read_lock_sched()/unlock() in
topology_is_scale_freq_source() is unnecessary. arch_cpu_idle_enter()
is called from do_idle() after local_irq_disable() at
kernel/sched/idle.c:340, which satisfies the rcu_sched grace period
requirement. This means we can call rcu_dereference_sched() directly
without explicit RCU lock.

I have two options to propose:

Option A: Keep the helper, but drop the explicit RCU lock

    bool topology_is_scale_freq_source(enum scale_freq_source source,
                                       unsigned int cpu)
    {
        struct scale_freq_data *sfd;
        sfd = rcu_dereference_sched(*per_cpu_ptr(&sft_data, cpu));
        return sfd && sfd->source == source;
    }

Option B: Drop the helper entirely, check directly in arch_cpu_idle_enter()
If a generic exported helper feels too invasive, we can do the
check locally within arch_cpu_idle_enter() without touching
drivers/base/arch_topology.c at all:

    if (housekeeping_cpu(cpu, HK_TYPE_TICK) &&
        time_is_before_jiffies(per_cpu(cpu_amu_samples.last_scale_update, cpu))) {
        struct scale_freq_data *sfd;
        sfd = rcu_dereference_sched(*this_cpu_ptr(&sft_data));
        if (sfd && sfd->source == SCALE_FREQ_SOURCE_ARCH)
            amu_scale_freq_tick();
    }

This keeps the change entirely in arm64 code and avoids adding
a new exported symbol. Which approach would you prefer?

> Aside: I should have probably asked that earlier, but I am not sure I do fully
> understand the case we are trying to fix here.
> The topology_set_scale_freq_source prefers arch source to others. So if the
> AMUs were chosen to server as the source for the freq scale - I do not see
> why the sfd would be changed. That would require calling sequence clear-set
> to get a different source in place. I do understand the issue itself, though how
> did we end up there in the first place ?

The issue arises when topology_clear_scale_freq_source() is called
with SCALE_FREQ_SOURCE_ARCH to explicitly disable AMU-based frequency
scaling. This API is exported (EXPORT_SYMBOL_GPL), so it is designed
to be used by modules or subsystems that need to replace the frequency
invariance mechanism at runtime.
 
After clearing, the tick path (topology_scale_freq_tick()) correctly
skips the AMU update because sft_data is set to NULL. However, the
idle path (arch_cpu_idle_enter()) bypasses this check by calling
amu_scale_freq_tick() directly, so arch_freq_scale still gets
modified by AMU counters.
 
This creates an inconsistency: the tick path respects
topology_clear_scale_freq_source() but the idle path does not.
 
The goal of this patch is to make the idle path consistent with
the tick path, ensuring that topology_clear_scale_freq_source()
fully disables AMU updates across all paths.