Skip to content

Commit b312d88

Browse files
zhang-ruilenb
authored andcommitted
tools/power turbostat: Allow Zero return value for some RAPL registers
turbostat aborted with below messages on a dual-package system, turbostat: turbostat.c:3744: rapl_counter_accumulate: Assertion `dst->unit == src->unit' failed. Aborted This is because 1. the MSR_DRAM_PERF_STATUS returns Zero for one package, and non-Zero for another package 2. probe_msr() treats Zero return value as a failure so this feature is enabled on one package, and disabled for another package. 3. turbostat aborts because the feature is invalid on some package Unlike the RAPL energy counter registers, MSR_DRAM_PERF_STATUS can return Zero value, and this should not be treated as a failure. Fix the problem by allowing Zero return value for RAPL registers other than the energy counters. Fixes: 7c6fee2 ("tools/power turbostat: Check for non-zero value when MSR probing") Reported-by: Artem Bityutskiy <artem.bityutskiy@intel.com> Signed-off-by: Zhang Rui <rui.zhang@intel.com> Signed-off-by: Len Brown <len.brown@intel.com>
1 parent 1c7c738 commit b312d88

File tree

1 file changed

+18
-9
lines changed

1 file changed

+18
-9
lines changed

tools/power/x86/turbostat/turbostat.c

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2211,7 +2211,7 @@ int get_msr(int cpu, off_t offset, unsigned long long *msr)
22112211
return 0;
22122212
}
22132213

2214-
int probe_msr(int cpu, off_t offset)
2214+
int probe_rapl_msr(int cpu, off_t offset, int index)
22152215
{
22162216
ssize_t retval;
22172217
unsigned long long value;
@@ -2220,13 +2220,22 @@ int probe_msr(int cpu, off_t offset)
22202220

22212221
retval = pread(get_msr_fd(cpu), &value, sizeof(value), offset);
22222222

2223-
/*
2224-
* Expect MSRs to accumulate some non-zero value since the system was powered on.
2225-
* Treat zero as a read failure.
2226-
*/
2227-
if (retval != sizeof(value) || value == 0)
2223+
/* if the read failed, the probe fails */
2224+
if (retval != sizeof(value))
22282225
return 1;
22292226

2227+
/* If an Energy Status Counter MSR returns 0, the probe fails */
2228+
switch (index) {
2229+
case RAPL_RCI_INDEX_ENERGY_PKG:
2230+
case RAPL_RCI_INDEX_ENERGY_CORES:
2231+
case RAPL_RCI_INDEX_DRAM:
2232+
case RAPL_RCI_INDEX_GFX:
2233+
case RAPL_RCI_INDEX_ENERGY_PLATFORM:
2234+
if (value == 0)
2235+
return 1;
2236+
}
2237+
2238+
/* PKG,DRAM_PERF_STATUS MSRs, can return any value */
22302239
return 0;
22312240
}
22322241

@@ -7907,7 +7916,7 @@ void rapl_perf_init(void)
79077916
rci->flags[cai->rci_index] = cai->flags;
79087917

79097918
/* Use MSR for this counter */
7910-
} else if (!no_msr && cai->msr && probe_msr(cpu, cai->msr) == 0) {
7919+
} else if (!no_msr && cai->msr && probe_rapl_msr(cpu, cai->msr, cai->rci_index) == 0) {
79117920
rci->source[cai->rci_index] = COUNTER_SOURCE_MSR;
79127921
rci->msr[cai->rci_index] = cai->msr;
79137922
rci->msr_mask[cai->rci_index] = cai->msr_mask;
@@ -8045,7 +8054,7 @@ void msr_perf_init_(void)
80458054
cai->present = true;
80468055

80478056
/* User MSR for this counter */
8048-
} else if (!no_msr && cai->msr && probe_msr(cpu, cai->msr) == 0) {
8057+
} else if (!no_msr && cai->msr && probe_rapl_msr(cpu, cai->msr, cai->rci_index) == 0) {
80498058
cci->source[cai->rci_index] = COUNTER_SOURCE_MSR;
80508059
cci->msr[cai->rci_index] = cai->msr;
80518060
cci->msr_mask[cai->rci_index] = cai->msr_mask;
@@ -8159,7 +8168,7 @@ void cstate_perf_init_(bool soft_c1)
81598168

81608169
/* User MSR for this counter */
81618170
} else if (!no_msr && cai->msr && pkg_cstate_limit >= cai->pkg_cstate_limit
8162-
&& probe_msr(cpu, cai->msr) == 0) {
8171+
&& probe_rapl_msr(cpu, cai->msr, cai->rci_index) == 0) {
81638172
cci->source[cai->rci_index] = COUNTER_SOURCE_MSR;
81648173
cci->msr[cai->rci_index] = cai->msr;
81658174
}

0 commit comments

Comments
 (0)