mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v5] arm64: topology: fix arch_freq_get_on_cpu() overflow above 4.19 GHz
@ 2026-09-17 18:20 Oleg Keri
  2026-09-21 13:50 ` Dietmar Eggemann
  2026-09-22 16:14 ` Will Deacon
  0 siblings, 2 replies; 4+ messages in thread
From: Oleg Keri @ 2026-09-17 18:20 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Mark Rutland, Ingo Molnar,
	Peter Zijlstra, Juri Lelli, Vincent Guittot, Dietmar Eggemann,
	Steven Rostedt, Ben Segall, Mel Gorman, Valentin Schneider,
	K Prateek Nayak, Prasanna Kumar T S M, Sumit Gupta,
	Beata Michalska
  Cc: linux-arm-kernel, linux-kernel, Ananthu C V, linux-pm

arch_freq_get_on_cpu() computes the product of the frequency scale and
the reference frequency as a u64, but assigns it to an unsigned int
before shifting it back down:

	freq = scale * arch_scale_freq_ref(cpu);
	freq >>= SCHED_CAPACITY_SHIFT;

The product is truncated to 32 bits before the shift, so the result
wraps once arch_scale_freq_ref() exceeds 2^32 / SCHED_CAPACITY_SCALE,
i.e. 4194304 kHz.

On a Snapdragon X2 Elite (Glymur) laptop, whose boost OPP is 4723200
kHz, cpuinfo_avg_freq reports 524283 kHz instead of ~4723200 kHz while
the CPU demonstrably runs at the boost frequency: a fixed workload
completes in 1.72 s at the 4723200 kHz OPP versus 2.01 s at 4032000
kHz, matching the 1.171 frequency ratio.

Compute it with cap_scale(), turned into a static inline taking u64 and
moved to <linux/sched/topology.h> so it is usable outside kernel/sched.

Fixes: 16d1e27475f6 ("arm64: Provide an AMU-based version of arch_freq_get_on_cpu")
Signed-off-by: Oleg Keri <okerixx@gmail.com>
---
Changes in v5:
- cap_scale() becomes a static inline taking u64 arguments instead of
  a macro, as Dietmar proposed and Peter agreed, after Peter pointed out
  that the macro relies on one operand being u64.  It now lives in
  <linux/sched/topology.h>, where SCHED_CAPACITY_SHIFT is visible,
  rather than <linux/topology.h>.
- Patch 2/2 of v4 is dropped: it duplicated Ananthu C V's series [1],
  which I tested instead.  This fix is needed with that series, since
  it puts the reference above 4194304 kHz from boot.
- v4: https://lore.kernel.org/all/20260917125112.2283-1-okerixx@gmail.com/

[1] https://lore.kernel.org/all/20260908-schedutil-boost-frequency-handling-v2-0-25312a713699@oss.qualcomm.com/

 arch/arm64/kernel/topology.c   | 6 ++----
 include/linux/sched/topology.h | 5 +++++
 kernel/sched/sched.h           | 2 --
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index d28438f8b83f..39dd7f8575cd 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -19,6 +19,7 @@
 #include <linux/init.h>
 #include <linux/percpu.h>
 #include <linux/sched/isolation.h>
+#include <linux/sched/topology.h>
 #include <linux/xarray.h>
 
 #include <asm/cpu.h>
@@ -186,7 +187,6 @@ int arch_freq_get_on_cpu(int cpu)
 	struct amu_cntr_sample *amu_sample;
 	unsigned int start_cpu = cpu;
 	unsigned long last_update;
-	unsigned int freq = 0;
 	u64 scale;
 
 	if (!amu_fie_cpu_supported(cpu) || !arch_scale_freq_ref(cpu))
@@ -245,9 +245,7 @@ int arch_freq_get_on_cpu(int cpu)
 	 * (see amu_scale_freq_tick for details)
 	 */
 	scale = arch_scale_freq_capacity(cpu);
-	freq = scale * arch_scale_freq_ref(cpu);
-	freq >>= SCHED_CAPACITY_SHIFT;
-	return freq;
+	return cap_scale(arch_scale_freq_ref(cpu), scale);
 }
 
 static void amu_fie_setup(const struct cpumask *cpus)
diff --git a/include/linux/sched/topology.h b/include/linux/sched/topology.h
index b5d9d7c2b8ad..922b2f015e89 100644
--- a/include/linux/sched/topology.h
+++ b/include/linux/sched/topology.h
@@ -234,6 +234,11 @@ static inline void rebuild_sched_domains_energy(void)
 }
 #endif
 
+static inline u64 cap_scale(u64 value, u64 scale)
+{
+	return value * scale >> SCHED_CAPACITY_SHIFT;
+}
+
 #ifndef arch_scale_cpu_capacity
 /**
  * arch_scale_cpu_capacity - get the capacity scale factor of a given CPU.
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 6c3ad70e58b8..45796fccdc84 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -236,8 +236,6 @@ static inline int task_has_dl_policy(struct task_struct *p)
 	return dl_policy(p->policy);
 }
 
-#define cap_scale(v, s)		((v)*(s) >> SCHED_CAPACITY_SHIFT)
-
 static inline void update_avg(u64 *avg, u64 sample)
 {
 	s64 diff = sample - *avg;

base-commit: 0d9d0dbf2fddcff5859d623e90ca73c4054276e1
-- 
2.55.0


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

* Re: [PATCH v5] arm64: topology: fix arch_freq_get_on_cpu() overflow above 4.19 GHz
  2026-09-17 18:20 [PATCH v5] arm64: topology: fix arch_freq_get_on_cpu() overflow above 4.19 GHz Oleg Keri
@ 2026-09-21 13:50 ` Dietmar Eggemann
  2026-09-22 16:14 ` Will Deacon
  1 sibling, 0 replies; 4+ messages in thread
From: Dietmar Eggemann @ 2026-09-21 13:50 UTC (permalink / raw)
  To: Oleg Keri, Catalin Marinas, Will Deacon, Mark Rutland,
	Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot,
	Steven Rostedt, Ben Segall, Mel Gorman, Valentin Schneider,
	K Prateek Nayak, Prasanna Kumar T S M, Sumit Gupta,
	Beata Michalska
  Cc: linux-arm-kernel, linux-kernel, Ananthu C V, linux-pm

On 17.09.26 20:20, Oleg Keri wrote:
> arch_freq_get_on_cpu() computes the product of the frequency scale and
> the reference frequency as a u64, but assigns it to an unsigned int
> before shifting it back down:
> 
> 	freq = scale * arch_scale_freq_ref(cpu);
> 	freq >>= SCHED_CAPACITY_SHIFT;
> 
> The product is truncated to 32 bits before the shift, so the result
> wraps once arch_scale_freq_ref() exceeds 2^32 / SCHED_CAPACITY_SCALE,
> i.e. 4194304 kHz.
> 
> On a Snapdragon X2 Elite (Glymur) laptop, whose boost OPP is 4723200
> kHz, cpuinfo_avg_freq reports 524283 kHz instead of ~4723200 kHz while
> the CPU demonstrably runs at the boost frequency: a fixed workload
> completes in 1.72 s at the 4723200 kHz OPP versus 2.01 s at 4032000
> kHz, matching the 1.171 frequency ratio.
> 
> Compute it with cap_scale(), turned into a static inline taking u64 and
> moved to <linux/sched/topology.h> so it is usable outside kernel/sched.
> 
> Fixes: 16d1e27475f6 ("arm64: Provide an AMU-based version of arch_freq_get_on_cpu")
> Signed-off-by: Oleg Keri <okerixx@gmail.com>

Reviewed-by: Dietmar Eggemann <dietmar.eggemann@arm.com>

> ---
> Changes in v5:
> - cap_scale() becomes a static inline taking u64 arguments instead of
>   a macro, as Dietmar proposed and Peter agreed, after Peter pointed out
>   that the macro relies on one operand being u64.  It now lives in
>   <linux/sched/topology.h>, where SCHED_CAPACITY_SHIFT is visible,
>   rather than <linux/topology.h>.
> - Patch 2/2 of v4 is dropped: it duplicated Ananthu C V's series [1],
>   which I tested instead.  This fix is needed with that series, since
>   it puts the reference above 4194304 kHz from boot.
> - v4: https://lore.kernel.org/all/20260917125112.2283-1-okerixx@gmail.com/
> 
> [1] https://lore.kernel.org/all/20260908-schedutil-boost-frequency-handling-v2-0-25312a713699@oss.qualcomm.com/
> 
>  arch/arm64/kernel/topology.c   | 6 ++----
>  include/linux/sched/topology.h | 5 +++++
>  kernel/sched/sched.h           | 2 --
>  3 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
> index d28438f8b83f..39dd7f8575cd 100644
> --- a/arch/arm64/kernel/topology.c
> +++ b/arch/arm64/kernel/topology.c
> @@ -19,6 +19,7 @@
>  #include <linux/init.h>
>  #include <linux/percpu.h>
>  #include <linux/sched/isolation.h>
> +#include <linux/sched/topology.h>
>  #include <linux/xarray.h>
>  
>  #include <asm/cpu.h>
> @@ -186,7 +187,6 @@ int arch_freq_get_on_cpu(int cpu)
>  	struct amu_cntr_sample *amu_sample;
>  	unsigned int start_cpu = cpu;
>  	unsigned long last_update;
> -	unsigned int freq = 0;
>  	u64 scale;
>  
>  	if (!amu_fie_cpu_supported(cpu) || !arch_scale_freq_ref(cpu))
> @@ -245,9 +245,7 @@ int arch_freq_get_on_cpu(int cpu)
>  	 * (see amu_scale_freq_tick for details)
>  	 */
>  	scale = arch_scale_freq_capacity(cpu);
> -	freq = scale * arch_scale_freq_ref(cpu);
> -	freq >>= SCHED_CAPACITY_SHIFT;
> -	return freq;
> +	return cap_scale(arch_scale_freq_ref(cpu), scale);
>  }
>  
>  static void amu_fie_setup(const struct cpumask *cpus)
> diff --git a/include/linux/sched/topology.h b/include/linux/sched/topology.h
> index b5d9d7c2b8ad..922b2f015e89 100644
> --- a/include/linux/sched/topology.h
> +++ b/include/linux/sched/topology.h
> @@ -234,6 +234,11 @@ static inline void rebuild_sched_domains_energy(void)
>  }
>  #endif
>  
> +static inline u64 cap_scale(u64 value, u64 scale)
> +{
> +	return value * scale >> SCHED_CAPACITY_SHIFT;
> +}
> +
>  #ifndef arch_scale_cpu_capacity
>  /**
>   * arch_scale_cpu_capacity - get the capacity scale factor of a given CPU.
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index 6c3ad70e58b8..45796fccdc84 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -236,8 +236,6 @@ static inline int task_has_dl_policy(struct task_struct *p)
>  	return dl_policy(p->policy);
>  }
>  
> -#define cap_scale(v, s)		((v)*(s) >> SCHED_CAPACITY_SHIFT)
> -
>  static inline void update_avg(u64 *avg, u64 sample)
>  {
>  	s64 diff = sample - *avg;
> 
> base-commit: 0d9d0dbf2fddcff5859d623e90ca73c4054276e1




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

* Re: [PATCH v5] arm64: topology: fix arch_freq_get_on_cpu() overflow above 4.19 GHz
  2026-09-17 18:20 [PATCH v5] arm64: topology: fix arch_freq_get_on_cpu() overflow above 4.19 GHz Oleg Keri
  2026-09-21 13:50 ` Dietmar Eggemann
@ 2026-09-22 16:14 ` Will Deacon
  2026-09-23  7:35   ` Oleg Keri
  1 sibling, 1 reply; 4+ messages in thread
From: Will Deacon @ 2026-09-22 16:14 UTC (permalink / raw)
  To: Oleg Keri
  Cc: Catalin Marinas, Mark Rutland, Ingo Molnar, Peter Zijlstra,
	Juri Lelli, Vincent Guittot, Dietmar Eggemann, Steven Rostedt,
	Ben Segall, Mel Gorman, Valentin Schneider, K Prateek Nayak,
	Prasanna Kumar T S M, Sumit Gupta, Beata Michalska,
	linux-arm-kernel, linux-kernel, Ananthu C V, linux-pm

On Thu, Sep 17, 2026 at 08:20:59PM +0200, Oleg Keri wrote:
> arch_freq_get_on_cpu() computes the product of the frequency scale and
> the reference frequency as a u64, but assigns it to an unsigned int
> before shifting it back down:
> 
> 	freq = scale * arch_scale_freq_ref(cpu);
> 	freq >>= SCHED_CAPACITY_SHIFT;
> 
> The product is truncated to 32 bits before the shift, so the result
> wraps once arch_scale_freq_ref() exceeds 2^32 / SCHED_CAPACITY_SCALE,
> i.e. 4194304 kHz.
> 
> On a Snapdragon X2 Elite (Glymur) laptop, whose boost OPP is 4723200
> kHz, cpuinfo_avg_freq reports 524283 kHz instead of ~4723200 kHz while
> the CPU demonstrably runs at the boost frequency: a fixed workload
> completes in 1.72 s at the 4723200 kHz OPP versus 2.01 s at 4032000
> kHz, matching the 1.171 frequency ratio.
> 
> Compute it with cap_scale(), turned into a static inline taking u64 and
> moved to <linux/sched/topology.h> so it is usable outside kernel/sched.
> 
> Fixes: 16d1e27475f6 ("arm64: Provide an AMU-based version of arch_freq_get_on_cpu")
> Signed-off-by: Oleg Keri <okerixx@gmail.com>
> ---
> Changes in v5:
> - cap_scale() becomes a static inline taking u64 arguments instead of
>   a macro, as Dietmar proposed and Peter agreed, after Peter pointed out
>   that the macro relies on one operand being u64.  It now lives in
>   <linux/sched/topology.h>, where SCHED_CAPACITY_SHIFT is visible,
>   rather than <linux/topology.h>.
> - Patch 2/2 of v4 is dropped: it duplicated Ananthu C V's series [1],
>   which I tested instead.  This fix is needed with that series, since
>   it puts the reference above 4194304 kHz from boot.
> - v4: https://lore.kernel.org/all/20260917125112.2283-1-okerixx@gmail.com/
> 
> [1] https://lore.kernel.org/all/20260908-schedutil-boost-frequency-handling-v2-0-25312a713699@oss.qualcomm.com/
> 
>  arch/arm64/kernel/topology.c   | 6 ++----
>  include/linux/sched/topology.h | 5 +++++
>  kernel/sched/sched.h           | 2 --
>  3 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
> index d28438f8b83f..39dd7f8575cd 100644
> --- a/arch/arm64/kernel/topology.c
> +++ b/arch/arm64/kernel/topology.c
> @@ -19,6 +19,7 @@
>  #include <linux/init.h>
>  #include <linux/percpu.h>
>  #include <linux/sched/isolation.h>
> +#include <linux/sched/topology.h>
>  #include <linux/xarray.h>
>  
>  #include <asm/cpu.h>
> @@ -186,7 +187,6 @@ int arch_freq_get_on_cpu(int cpu)
>  	struct amu_cntr_sample *amu_sample;
>  	unsigned int start_cpu = cpu;
>  	unsigned long last_update;
> -	unsigned int freq = 0;
>  	u64 scale;
>  
>  	if (!amu_fie_cpu_supported(cpu) || !arch_scale_freq_ref(cpu))
> @@ -245,9 +245,7 @@ int arch_freq_get_on_cpu(int cpu)
>  	 * (see amu_scale_freq_tick for details)
>  	 */
>  	scale = arch_scale_freq_capacity(cpu);
> -	freq = scale * arch_scale_freq_ref(cpu);
> -	freq >>= SCHED_CAPACITY_SHIFT;
> -	return freq;
> +	return cap_scale(arch_scale_freq_ref(cpu), scale);
>  }
>  
>  static void amu_fie_setup(const struct cpumask *cpus)

The arm64 part looks fine to me, so for that:

Acked-by: Will Deacon <will@kernel.org>

However...

> diff --git a/include/linux/sched/topology.h b/include/linux/sched/topology.h
> index b5d9d7c2b8ad..922b2f015e89 100644
> --- a/include/linux/sched/topology.h
> +++ b/include/linux/sched/topology.h
> @@ -234,6 +234,11 @@ static inline void rebuild_sched_domains_energy(void)
>  }
>  #endif
>  
> +static inline u64 cap_scale(u64 value, u64 scale)
> +{
> +	return value * scale >> SCHED_CAPACITY_SHIFT;
> +}

... does this introduce unnecessary 64-bit arithmetic for 32-bit
architectures that currently pass 'unsigned long' to the existing macro?

Will

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

* Re: [PATCH v5] arm64: topology: fix arch_freq_get_on_cpu() overflow above 4.19 GHz
  2026-09-22 16:14 ` Will Deacon
@ 2026-09-23  7:35   ` Oleg Keri
  0 siblings, 0 replies; 4+ messages in thread
From: Oleg Keri @ 2026-09-23  7:35 UTC (permalink / raw)
  To: Will Deacon
  Cc: Catalin Marinas, Mark Rutland, Ingo Molnar, Peter Zijlstra,
	Juri Lelli, Vincent Guittot, Dietmar Eggemann, Steven Rostedt,
	Ben Segall, Mel Gorman, Valentin Schneider, K Prateek Nayak,
	Prasanna Kumar T S M, Sumit Gupta, Beata Michalska,
	linux-arm-kernel, linux-kernel, Ananthu C V, linux-pm

Hi Will,

On Tue, Sep 22, 2026, Will Deacon wrote:
> ... does this introduce unnecessary 64-bit arithmetic for 32-bit
> architectures that currently pass 'unsigned long' to the existing macro?

Good catch. The truncation happens in the unsigned int temporary in
arch_freq_get_on_cpu(), so v6 will drop that temporary and leave
kernel/sched alone.

Thanks,
Oleg

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

end of thread, other threads:[~2026-09-23  7:35 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-17 18:20 [PATCH v5] arm64: topology: fix arch_freq_get_on_cpu() overflow above 4.19 GHz Oleg Keri
2026-09-21 13:50 ` Dietmar Eggemann
2026-09-22 16:14 ` Will Deacon
2026-09-23  7:35   ` Oleg Keri

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®