[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