All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] arm64: topology: add source check in arch_cpu_idle_enter()
@ 2026-08-11  7:28 Sean Wang
  2026-08-11 10:02 ` Beata Michalska
  0 siblings, 1 reply; 2+ messages in thread
From: Sean Wang @ 2026-08-11  7:28 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Sudeep Holla, Greg Kroah-Hartman,
	rafael, Danilo Krummrich
  Cc: Lifeng Zheng, Beata Michalska, Xuewen Yan, Geert Uytterhoeven,
	Sumit Gupta, Yunhui Cui, linux-arm-kernel, linux-kernel,
	driver-core, 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


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v3] arm64: topology: add source check in arch_cpu_idle_enter()
  2026-08-11  7:28 [PATCH v3] arm64: topology: add source check in arch_cpu_idle_enter() Sean Wang
@ 2026-08-11 10:02 ` Beata Michalska
  0 siblings, 0 replies; 2+ messages in thread
From: Beata Michalska @ 2026-08-11 10:02 UTC (permalink / raw)
  To: Sean Wang
  Cc: Catalin Marinas, Will Deacon, Sudeep Holla, Greg Kroah-Hartman,
	rafael, Danilo Krummrich, Lifeng Zheng, Xuewen Yan,
	Geert Uytterhoeven, Sumit Gupta, Yunhui Cui, linux-arm-kernel,
	linux-kernel, driver-core

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
> 


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-08-11 10:03 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-11  7:28 [PATCH v3] arm64: topology: add source check in arch_cpu_idle_enter() Sean Wang
2026-08-11 10:02 ` Beata Michalska

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.