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

On Wed, Aug 12, 2026 at 07:34:52AM +0000, Sean Wang1 wrote:

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

quoted

quoted

--- 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)))

quoted

+

time_is_before_jiffies(per_cpu(cpu_amu_samples.last_scale_update, cpu))
&&

quoted

+	    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.

In this particular case RCU locking is not required, though you are exposing
an API that might be used in other curcumstances, so the least we could do is
document that.

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?

I do not mind this additional helper. Besides, not my place to either mind it
or not.
What I do mind is doing the check in the arch idle enter path. I would rather
see some notification triggered when the source gets changed so that
the previous sfd code can do some state transition that would avoid us having
to run the check in the first place.
Still pondering on that one.
Preferably I would drop that 'tick' call from there completely, but apparently
this was needed on some platforms to make AMU readings more reliable.

quoted

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.
 

So this is the bit I was missing: external module that does the switch
willingly giving up on arch provided freq scale source.
The rest is clear. Thanks.

---
BR
Beata

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.