[RFC PATCH] perf/x86/rapl: Fix a bug caused by overflow timer being calculated using a 200W Max TDP

[RFC PATCH] perf/x86/rapl: Fix a bug caused by overflow timer being calculated using a 200W Max TDP

[Dhruv Gupta]

The RAPL energy status counters are 32 bits wide, so the PMU uses an
hrtimer to fold them into the 64-bit event count before they can wrap.
rapl_event_update() sign-extends the 32-bit difference between two
reads, so the counter must advance by less than half of its range
between updates. A larger advance is treated as a negative delta and
the event count goes backwards by 2^32 counter units.

The timer interval is the time it takes to use half the counter range
at a reference power of 200W. Any package that sustains more than 200W
between two expirations therefore loses energy. This applies to every
energy unit, since the interval scales with it: with 2^-16 J units the
interval is 163.84s and at most 32768 J can be accounted per interval,
and with 2^-14 J units it is 655.36s with the same 200W limit. Many
current server processors draw more than that under load.

On an AMD EPYC 9654 (max default TDP 360W) running stress-ng on all
CPUs, the package draws about 346W, or about 56 kJ per 163.84s
interval. Over an 1100s run, "perf stat -a -e power/energy-pkg/"
reported 4294882289.81 Joules, i.e. -85006 J, while reading the same
event every second with "perf stat -I 1000" gave 373745.7 J. The
difference is 458752 J, exactly seven wraps of the 65536 J counter.

This patch raises the reference power to 1000W. With 2^-16 J units the
interval becomes 32.768s, and with 2^-14 J units 131.072s. The cost is
one MSR read per active event per package per interval.

With this change, the same workload run for 300s reports 104062.71 J
with plain "perf stat" and 104062.7 J with "perf stat -I 1000".

Additionally this patch drops the hw-unit check and the 1ms fallback. The
unit is a 5-bit MSR field, so it is always 0-31 and the branch was
always taken. Also the timer is sized from rapl_core_hw_unit instead of
rapl_pkg_hw_unit[0]. Both are filled using the same power-unit MSR field
and are identical.

Signed-off-by: Dhruv Gupta <dhruv.gupta@amd.com>
---
 arch/x86/events/rapl.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/arch/x86/events/rapl.c b/arch/x86/events/rapl.c
index 8ed03c32f560..43ac9e1c98b4 100644
--- a/arch/x86/events/rapl.c
+++ b/arch/x86/events/rapl.c
@@ -106,6 +106,7 @@ static const char *const rapl_core_domain_name __initconst = "core";
  */
 #define RAPL_EVENT_MASK	0xFFULL
 #define RAPL_CNTR_WIDTH 32
+#define RAPL_TIMER_REF_WATTS 1000
 
 #define RAPL_EVENT_ATTR_STR(_name, v, str)					\
 static struct perf_pmu_events_attr event_attr_##v = {				\
@@ -639,16 +640,12 @@ static int rapl_check_hw_unit(void)
 
 	/*
 	 * Calculate the timer rate:
-	 * Use reference of 200W for scaling the timeout to avoid counter
-	 * overflows. 200W = 200 Joules/sec
-	 * Divide interval by 2 to avoid lockstep (2 * 100)
-	 * if hw unit is 32, then we use 2 ms 1/200/2
+	 * Use reference of 1000W for scaling the timeout to avoid counter
+	 * overflows. 1000W = 1000 Joules/sec
+	 * Divide interval by 2 to avoid lockstep (2 * RAPL_TIMER_REF_WATTS)
 	 */
-	rapl_timer_ms = 2;
-	if (rapl_pkg_hw_unit[0] < 32) {
-		rapl_timer_ms = (1000 / (2 * 100));
-		rapl_timer_ms *= (1ULL << (32 - rapl_pkg_hw_unit[0] - 1));
-	}
+	rapl_timer_ms = (1000ULL << (32 - rapl_core_hw_unit)) /
+		(2 * RAPL_TIMER_REF_WATTS);
 	return 0;
 }
 
-- 
2.43.0

Re: [RFC PATCH] perf/x86/rapl: Fix a bug caused by overflow timer being calculated using a 200W Max TDP

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] perf/x86/rapl: 64-bit division in rapl_check_hw_unit() breaks
  32-bit builds
--

commit 64815f3ce3036f1ab83ef82a1cc4dd1237aac3f9
Author: Dhruv Gupta <dhruv.gupta@amd.com>

perf/x86/rapl: Fix a bug caused by overflow timer being calculated using a 200W Max TDP

This commit raises the reference power used to compute the RAPL energy counter
overflow timer interval from 200W to 1000W and simplifies the interval
calculation in rapl_check_hw_unit().

> diff --git a/arch/x86/events/rapl.c b/arch/x86/events/rapl.c
> index 8ed03c32f5605..43ac9e1c98b45 100644
> --- a/arch/x86/events/rapl.c
> +++ b/arch/x86/events/rapl.c
> @@ -106,6 +106,7 @@ static const char *const rapl_core_domain_name __initconst = "core";
>   */
>  #define RAPL_EVENT_MASK	0xFFULL
>  #define RAPL_CNTR_WIDTH 32
> +#define RAPL_TIMER_REF_WATTS 1000
>  
>  #define RAPL_EVENT_ATTR_STR(_name, v, str)					\
>  static struct perf_pmu_events_attr event_attr_##v = {				\
> @@ -639,16 +640,12 @@ static int rapl_check_hw_unit(void)
>  
>  	/*
>  	 * Calculate the timer rate:
> -	 * Use reference of 200W for scaling the timeout to avoid counter
> -	 * overflows. 200W = 200 Joules/sec
> -	 * Divide interval by 2 to avoid lockstep (2 * 100)
> -	 * if hw unit is 32, then we use 2 ms 1/200/2
> +	 * Use reference of 1000W for scaling the timeout to avoid counter
> +	 * overflows. 1000W = 1000 Joules/sec
> +	 * Divide interval by 2 to avoid lockstep (2 * RAPL_TIMER_REF_WATTS)
>  	 */
> -	rapl_timer_ms = 2;
> -	if (rapl_pkg_hw_unit[0] < 32) {
> -		rapl_timer_ms = (1000 / (2 * 100));
> -		rapl_timer_ms *= (1ULL << (32 - rapl_pkg_hw_unit[0] - 1));
> -	}
> +	rapl_timer_ms = (1000ULL << (32 - rapl_core_hw_unit)) /
> +		(2 * RAPL_TIMER_REF_WATTS);

[Severity: High]
Does this 64-bit division break 32-bit x86 builds?  Dividing the runtime
64-bit value (1000ULL << (32 - rapl_core_hw_unit)) by 2000 with the plain
/ operator emits a call to __udivdi3 on CONFIG_X86_32, causing a link
failure when CONFIG_PERF_EVENTS_INTEL_RAPL is enabled.  Using div_u64()
would avoid this.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111151050.329724-1-dhruv.gupta@amd.com?part=1

Re: [RFC PATCH] perf/x86/rapl: Fix a bug caused by overflow timer being calculated using a 200W Max TDP

[Borislav Petkov]

On Wed, Nov 11, 2026 at 03:10:50PM +0000, Dhruv Gupta wrote:
> The RAPL energy status counters are 32 bits wide, so the PMU uses an
> hrtimer to fold them into the 64-bit event count before they can wrap.
> rapl_event_update() sign-extends the 32-bit difference between two
> reads, so the counter must advance by less than half of its range
> between updates. A larger advance is treated as a negative delta and
> the event count goes backwards by 2^32 counter units.
> 
> The timer interval is the time it takes to use half the counter range
> at a reference power of 200W. Any package that sustains more than 200W
> between two expirations therefore loses energy. This applies to every
> energy unit, since the interval scales with it: with 2^-16 J units the
> interval is 163.84s and at most 32768 J can be accounted per interval,
> and with 2^-14 J units it is 655.36s with the same 200W limit. Many
> current server processors draw more than that under load.
> 
> On an AMD EPYC 9654 (max default TDP 360W) running stress-ng on all
> CPUs, the package draws about 346W, or about 56 kJ per 163.84s
> interval. Over an 1100s run, "perf stat -a -e power/energy-pkg/"
> reported 4294882289.81 Joules, i.e. -85006 J, while reading the same
> event every second with "perf stat -I 1000" gave 373745.7 J. The
> difference is 458752 J, exactly seven wraps of the 65536 J counter.
> 
> This patch raises the reference power to 1000W. With 2^-16 J units the
> interval becomes 32.768s, and with 2^-14 J units 131.072s. The cost is
> one MSR read per active event per package per interval.
> 
> With this change, the same workload run for 300s reports 104062.71 J
> with plain "perf stat" and 104062.7 J with "perf stat -I 1000".
> 
> Additionally this patch drops the hw-unit check and the 1ms fallback. The

Avoid having "This patch" or "This commit" in the commit message. It is
tautologically useless.

Also, do

$ git grep 'This patch' Documentation/process

for more details.

Please read:

https://docs.kernel.org/process/maintainer-tip.html

before submitting tip patches.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette