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

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

[Sean Wang]

arch_cpu_idle_enter() directly calls amu_scale_freq_tick() to update
arch_freq_scale when a CPU enters idle. This bypasses the sft_data
pointer check that topology_clear_scale_freq_source() relies on.

As a result, even after calling topology_clear_scale_freq_source()
with SCALE_FREQ_SOURCE_ARCH to disable AMU-based frequency scaling,
the arch_freq_scale value can still be modified by AMU counters when
the CPU goes idle through the arch_cpu_idle_enter() path.

Add topology_is_scale_freq_source() helper to check whether a specific
frequency scaling source is currently registered for a CPU. Use it
in arch_cpu_idle_enter() to verify that AMU is the active source
before calling amu_scale_freq_tick().

This ensures that topology_clear_scale_freq_source() properly
disables AMU updates in both the tick path (already handled by
topology_scale_freq_tick()) and the idle path.

Co-developed-by: Xuewen Yan <xuewen.yan@unisoc.com>
Signed-off-by: Xuewen Yan <xuewen.yan@unisoc.com>
Signed-off-by: Sean Wang <seanwang1@lenovo.com>
---
 arch/arm64/kernel/topology.c  |  3 ++-
 drivers/base/arch_topology.c  | 13 +++++++++++++
 include/linux/arch_topology.h |  1 +
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index b32f13358fbb..1dbd8f4c3178 100644
--- 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();
 }
 
diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index 8c5e47c28d9a..6049e77bbe1d 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -127,6 +127,19 @@ void topology_clear_scale_freq_source(enum scale_freq_source source,
 }
 EXPORT_SYMBOL_GPL(topology_clear_scale_freq_source);
 
+bool topology_is_scale_freq_source(enum scale_freq_source source, unsigned int cpu)
+{
+	struct scale_freq_data *sfd;
+	bool ret;
+
+	rcu_read_lock_sched();
+	sfd = rcu_dereference_sched(*per_cpu_ptr(&sft_data, cpu));
+	ret = (sfd && sfd->source == source);
+	rcu_read_unlock_sched();
+
+	return ret;
+}
+
 void topology_scale_freq_tick(void)
 {
 	struct scale_freq_data *sfd = rcu_dereference_sched(*this_cpu_ptr(&sft_data));
diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h
index ebd7f8935f96..fb7c1fe74808 100644
--- a/include/linux/arch_topology.h
+++ b/include/linux/arch_topology.h
@@ -48,6 +48,7 @@ struct scale_freq_data {
 void topology_scale_freq_tick(void);
 void topology_set_scale_freq_source(struct scale_freq_data *data, const struct cpumask *cpus);
 void topology_clear_scale_freq_source(enum scale_freq_source source, const struct cpumask *cpus);
+bool topology_is_scale_freq_source(enum scale_freq_source source, unsigned int cpu);
 
 DECLARE_PER_CPU(unsigned long, hw_pressure);
 
-- 
2.25.1

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

[Beata Michalska]

On Tue, Aug 11, 2026 at 03:28:30PM +0800, Sean Wang wrote:
> arch_cpu_idle_enter() directly calls amu_scale_freq_tick() to update
> arch_freq_scale when a CPU enters idle. This bypasses the sft_data
> pointer check that topology_clear_scale_freq_source() relies on.
> 
> As a result, even after calling topology_clear_scale_freq_source()
> with SCALE_FREQ_SOURCE_ARCH to disable AMU-based frequency scaling,
> the arch_freq_scale value can still be modified by AMU counters when
> the CPU goes idle through the arch_cpu_idle_enter() path.
> 
> Add topology_is_scale_freq_source() helper to check whether a specific
> frequency scaling source is currently registered for a CPU. Use it
> in arch_cpu_idle_enter() to verify that AMU is the active source
> before calling amu_scale_freq_tick().
> 
> This ensures that topology_clear_scale_freq_source() properly
> disables AMU updates in both the tick path (already handled by
> topology_scale_freq_tick()) and the idle path.
> 
> Co-developed-by: Xuewen Yan <xuewen.yan@unisoc.com>
> Signed-off-by: Xuewen Yan <xuewen.yan@unisoc.com>
> Signed-off-by: Sean Wang <seanwang1@lenovo.com>
> ---
>  arch/arm64/kernel/topology.c  |  3 ++-
>  drivers/base/arch_topology.c  | 13 +++++++++++++
>  include/linux/arch_topology.h |  1 +
>  3 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
> index b32f13358fbb..1dbd8f4c3178 100644
> --- 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'.

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 ?

---
BR
Beata
>  }
>  
> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> index 8c5e47c28d9a..6049e77bbe1d 100644
> --- a/drivers/base/arch_topology.c
> +++ b/drivers/base/arch_topology.c
> @@ -127,6 +127,19 @@ void topology_clear_scale_freq_source(enum scale_freq_source source,
>  }
>  EXPORT_SYMBOL_GPL(topology_clear_scale_freq_source);
>  
> +bool topology_is_scale_freq_source(enum scale_freq_source source, unsigned int cpu)
> +{
> +	struct scale_freq_data *sfd;
> +	bool ret;
> +
> +	rcu_read_lock_sched();
> +	sfd = rcu_dereference_sched(*per_cpu_ptr(&sft_data, cpu));
> +	ret = (sfd && sfd->source == source);
> +	rcu_read_unlock_sched();
> +
> +	return ret;
> +}
> +
>  void topology_scale_freq_tick(void)
>  {
>  	struct scale_freq_data *sfd = rcu_dereference_sched(*this_cpu_ptr(&sft_data));
> diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h
> index ebd7f8935f96..fb7c1fe74808 100644
> --- a/include/linux/arch_topology.h
> +++ b/include/linux/arch_topology.h
> @@ -48,6 +48,7 @@ struct scale_freq_data {
>  void topology_scale_freq_tick(void);
>  void topology_set_scale_freq_source(struct scale_freq_data *data, const struct cpumask *cpus);
>  void topology_clear_scale_freq_source(enum scale_freq_source source, const struct cpumask *cpus);
> +bool topology_is_scale_freq_source(enum scale_freq_source source, unsigned int cpu);
>  
>  DECLARE_PER_CPU(unsigned long, hw_pressure);
>  
> -- 
> 2.25.1
> 

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

[Sean Wang1]

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.

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

[Beata Michalska]

On Wed, Aug 12, 2026 at 07:34:52AM +0000, Sean Wang1 wrote:
> 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.
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.

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