tools/power/turbostat: Fix long time duration RAPL calculation
cover tools/power/turbostat: Fix long time duration RAPL calculation
A combination of patch set to fix the recently introduced long time duration RAPL calculation issue found on AMD CPUs.
Youling, Kurt, Bingsong, Artem, could you please confirm again if this patch set works for you on your platform?
Bas Nieuwenhuizen (1):
tools/power/turbostat: Fix turbostat for AMD Zen CPUs
Calvin Walton (1):
tools/power turbostat: Fix offset overflow issue in index converting
tools/power/x86/turbostat/turbostat.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-)
Signature · 1 line
2.25.1
PATCH 1/2 tools/power/turbostat: Fix turbostat for AMD Zen CPUs
From: Bas Nieuwenhuizen <[email protected]>
It was reported that on Zen+ system turbostat started exiting, which was tracked down to the MSR_PKG_ENERGY_STAT read failing because offset_to_idx wasn't returning a non-negative index.
This patch combined the modification from Bingsong Si and
Bas Nieuwenhuizen and addd the MSR to the index system as alternative for MSR_PKG_ENERGY_STATUS.
--- tools/power/x86/turbostat/turbostat.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
tools/power/x86/turbostat/turbostat.c +6 -2
@@ -297,7 +297,10 @@ int idx_to_offset(int idx)switch (idx) {case IDX_PKG_ENERGY : - offset =MSR_PKG_ENERGY_STATUS ; +if (do_rapl &RAPL_AMD_F17H ) + offset =MSR_PKG_ENERGY_STAT ; +else + offset =MSR_PKG_ENERGY_STATUS ;break ;case IDX_DRAM_ENERGY : offset =MSR_DRAM_ENERGY_STATUS ;
@@ -326,6 +329,7 @@ int offset_to_idx(int offset)switch (offset) {case MSR_PKG_ENERGY_STATUS : +case MSR_PKG_ENERGY_STAT : idx =IDX_PKG_ENERGY ;break ;case MSR_DRAM_ENERGY_STATUS :
@@ -353,7 +357,7 @@ int idx_valid(int idx) {switch (idx) {case IDX_PKG_ENERGY : -return do_rapl &RAPL_PKG ; +return do_rapl & (RAPL_PKG |RAPL_AMD_F17H );case IDX_DRAM_ENERGY :return do_rapl &RAPL_DRAM ;case IDX_PP0_ENERGY :
Signature · 1 line
2.25.1
PATCH 2/2 tools/power turbostat: Fix offset overflow issue in index converting
In reply to Chen YuFrom: Calvin Walton <[email protected]>
The idx_to_offset() function returns type int (32-bit signed), but MSR_PKG_ENERGY_STAT is u32 and would be interpreted as a negative number. The end result is that it hits the if (offset < 0) check in update_msr_sum() which prevents the timer callback from updating the stat in the background when long durations are used. The similar issue exists in offset_to_idx() and update_msr_sum(). Fix this issue by converting the 'int' to 'off_t' accordingly.
--- tools/power/x86/turbostat/turbostat.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
tools/power/x86/turbostat/turbostat.c +6 -5
@@ -291,9 +291,9 @@ struct msr_sum_array {/* The percpu MSR sum array.*/ struct msr_sum_array *per_cpu_msr_sum; -int idx_to_offset (int idx) +off_t idx_to_offset (int idx) { -int offset; +off_t offset;switch (idx) {case IDX_PKG_ENERGY :
@@ -323,7 +323,7 @@ int idx_to_offset(int idx)return offset; } -int offset_to_idx (int offset) +int offset_to_idx (off_t offset) {int idx;
@@ -3276,7 +3276,7 @@ static int update_msr_sum(struct thread_data *t, struct core_data *c, struct pkgfor (i =IDX_PKG_ENERGY ; i <IDX_COUNT ; i++) {unsigned long long msr_cur, msr_last; -int offset; +off_t offset;if (!idx_valid (i))continue ;
@@ -3285,7 +3285,8 @@ static int update_msr_sum(struct thread_data *t, struct core_data *c, struct pkgcontinue ; ret =get_msr (cpu, offset, &msr_cur);if (ret) { -fprintf (outf,"Can not update msr(0x%x)\n" , offset); +fprintf (outf,"Can not update msr(0x%llx)\n" , + (unsigned long long )offset);continue ; }
Signature · 1 line
2.25.1
Both patches look good to me. I tested both on AMD Epyc Rome. Iverified the missing output is fixed and used instrumentationto verify the sign-extension issues are resolved.
Please add:
Regards,
Terry
Quoted 15 lines · Chen Yu
A combination of patch set to fix the recently introduced long time duration RAPL calculation issue found on AMD CPUs.
Youling, Kurt, Bingsong, Artem, could you please confirm again if this patch set works for you on your platform?
Bas Nieuwenhuizen (1):
tools/power/turbostat: Fix turbostat for AMD Zen CPUsCalvin Walton (1):
tools/powerturbostat: Fix offset overflow issue in index convertingtools/power/x86/turbostat/turbostat.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-)