| Message ID | 20240308204957.2007212-1-justin.ernst@hpe.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Len Brown |
| Headers | show |
| Series | tools/power/turbostat: Fix uncore frequency file string | expand |
On Freitag, 8. März 2024 21:49:57 CET Justin Ernst wrote: > Running turbostat on a 16 socket HPE Scale-up Compute 3200 (SapphireRapids) > fails with: turbostat: > /sys/devices/system/cpu/intel_uncore_frequency/package_010_die_00/current_f > req_khz: open failed: No such file or directory > > We observe the sysfs uncore frequency directories named: > ... > package_09_die_00/ > package_10_die_00/ > package_11_die_00/ > ... > package_15_die_00/ > > The culprit is an incorrect sprintf format string "package_0%d_die_0%d" used > with each instance of reading uncore frequency files. > uncore-frequency-common.c creates the sysfs directory with the format > "package_%02d_die_%02d". Once the package value reaches double digits, the > formats diverge. Yep, currently in: drivers/platform/x86/intel/uncore-frequency/uncore-frequency-common.c line 238. Looks like the proper bugfix. > Change each instance of "package_0%d_die_0%d" to "package_%02d_die_%02d". > > Signed-off-by: Justin Ernst <justin.ernst@hpe.com> Reviewed-by: Thomas Renninger <trenn@suse.de> Thomas
Thanks for the patch, Justin, Looks like the probe part of this was already fixed in my git tree, so I lopped off that hunk and kept your 1st hunk. Let me know if it works, or if I screwed it up. latest is in this tree: https://git.kernel.org/pub/scm/linux/kernel/git/lenb/linux.git/ thanks, -Len On Fri, Mar 8, 2024 at 3:50 PM Justin Ernst <justin.ernst@hpe.com> wrote: > > Running turbostat on a 16 socket HPE Scale-up Compute 3200 (SapphireRapids) fails with: > turbostat: /sys/devices/system/cpu/intel_uncore_frequency/package_010_die_00/current_freq_khz: open failed: No such file or directory > > We observe the sysfs uncore frequency directories named: > ... > package_09_die_00/ > package_10_die_00/ > package_11_die_00/ > ... > package_15_die_00/ > > The culprit is an incorrect sprintf format string "package_0%d_die_0%d" used > with each instance of reading uncore frequency files. uncore-frequency-common.c > creates the sysfs directory with the format "package_%02d_die_%02d". Once the > package value reaches double digits, the formats diverge. > > Change each instance of "package_0%d_die_0%d" to "package_%02d_die_%02d". > > Signed-off-by: Justin Ernst <justin.ernst@hpe.com> > --- > tools/power/x86/turbostat/turbostat.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c > index 7a334377f92b..2a15a23cb726 100644 > --- a/tools/power/x86/turbostat/turbostat.c > +++ b/tools/power/x86/turbostat/turbostat.c > @@ -2599,7 +2599,7 @@ unsigned long long get_uncore_mhz(int package, int die) > { > char path[128]; > > - sprintf(path, "/sys/devices/system/cpu/intel_uncore_frequency/package_0%d_die_0%d/current_freq_khz", package, > + sprintf(path, "/sys/devices/system/cpu/intel_uncore_frequency/package_%02d_die_%02d/current_freq_khz", package, > die); > > return (snapshot_sysfs_counter(path) / 1000); > @@ -4589,20 +4589,20 @@ static void probe_intel_uncore_frequency(void) > for (j = 0; j < topo.num_die; ++j) { > int k, l; > > - sprintf(path, "/sys/devices/system/cpu/intel_uncore_frequency/package_0%d_die_0%d/min_freq_khz", > + sprintf(path, "/sys/devices/system/cpu/intel_uncore_frequency/package_%02d_die_%02d/min_freq_khz", > i, j); > k = read_sysfs_int(path); > - sprintf(path, "/sys/devices/system/cpu/intel_uncore_frequency/package_0%d_die_0%d/max_freq_khz", > + sprintf(path, "/sys/devices/system/cpu/intel_uncore_frequency/package_%02d_die_%02d/max_freq_khz", > i, j); > l = read_sysfs_int(path); > fprintf(outf, "Uncore Frequency pkg%d die%d: %d - %d MHz ", i, j, k / 1000, l / 1000); > > sprintf(path, > - "/sys/devices/system/cpu/intel_uncore_frequency/package_0%d_die_0%d/initial_min_freq_khz", > + "/sys/devices/system/cpu/intel_uncore_frequency/package_%02d_die_%02d/initial_min_freq_khz", > i, j); > k = read_sysfs_int(path); > sprintf(path, > - "/sys/devices/system/cpu/intel_uncore_frequency/package_0%d_die_0%d/initial_max_freq_khz", > + "/sys/devices/system/cpu/intel_uncore_frequency/package_%02d_die_%02d/initial_max_freq_khz", > i, j); > l = read_sysfs_int(path); > fprintf(outf, "(%d - %d MHz)\n", k / 1000, l / 1000); > -- > 2.26.2 >
> From: Len Brown <lenb@kernel.org> > Sent: Tuesday, April 2, 2024 12:46 PM > > Thanks for the patch, Justin, > > Looks like the probe part of this was already fixed in my git tree, so > I lopped off that hunk and kept your 1st hunk. > > Let me know if it works, or if I screwed it up. It works! I tested on a 16-socket system with "..., package_10_die_00/, package_11_die_00/, etc" directories present. Thanks for the link to your latest tree. It made it very easy to build and test your patch. Cheers, -Justin > > latest is in this tree: > https://git.kernel.org/pub/scm/linux/kernel/git/lenb/linux.git/ > > thanks, > -Len > > On Fri, Mar 8, 2024 at 3:50 PM Justin Ernst <justin.ernst@hpe.com> wrote: > > > > Running turbostat on a 16 socket HPE Scale-up Compute 3200 (SapphireRapids) fails with: > > turbostat: /sys/devices/system/cpu/intel_uncore_frequency/package_010_die_00/current_freq_khz: open > failed: No such file or directory > > > > We observe the sysfs uncore frequency directories named: > > ... > > package_09_die_00/ > > package_10_die_00/ > > package_11_die_00/ > > ... > > package_15_die_00/ > > > > The culprit is an incorrect sprintf format string "package_0%d_die_0%d" used > > with each instance of reading uncore frequency files. uncore-frequency-common.c > > creates the sysfs directory with the format "package_%02d_die_%02d". Once the > > package value reaches double digits, the formats diverge. > > > > Change each instance of "package_0%d_die_0%d" to "package_%02d_die_%02d". > > > > Signed-off-by: Justin Ernst <justin.ernst@hpe.com> > > --- > > tools/power/x86/turbostat/turbostat.c | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c > > index 7a334377f92b..2a15a23cb726 100644 > > --- a/tools/power/x86/turbostat/turbostat.c > > +++ b/tools/power/x86/turbostat/turbostat.c > > @@ -2599,7 +2599,7 @@ unsigned long long get_uncore_mhz(int package, int die) > > { > > char path[128]; > > > > - sprintf(path, > "/sys/devices/system/cpu/intel_uncore_frequency/package_0%d_die_0%d/current_freq_khz", package, > > + sprintf(path, > "/sys/devices/system/cpu/intel_uncore_frequency/package_%02d_die_%02d/current_freq_khz", package, > > die); > > > > return (snapshot_sysfs_counter(path) / 1000); > > @@ -4589,20 +4589,20 @@ static void probe_intel_uncore_frequency(void) > > for (j = 0; j < topo.num_die; ++j) { > > int k, l; > > > > - sprintf(path, > "/sys/devices/system/cpu/intel_uncore_frequency/package_0%d_die_0%d/min_freq_khz", > > + sprintf(path, > "/sys/devices/system/cpu/intel_uncore_frequency/package_%02d_die_%02d/min_freq_khz", > > i, j); > > k = read_sysfs_int(path); > > - sprintf(path, > "/sys/devices/system/cpu/intel_uncore_frequency/package_0%d_die_0%d/max_freq_khz", > > + sprintf(path, > "/sys/devices/system/cpu/intel_uncore_frequency/package_%02d_die_%02d/max_freq_khz", > > i, j); > > l = read_sysfs_int(path); > > fprintf(outf, "Uncore Frequency pkg%d die%d: %d - %d MHz ", i, j, k / 1000, > l / 1000); > > > > sprintf(path, > > - > "/sys/devices/system/cpu/intel_uncore_frequency/package_0%d_die_0%d/initial_min_freq_khz", > > + > "/sys/devices/system/cpu/intel_uncore_frequency/package_%02d_die_%02d/initial_min_freq_khz", > > i, j); > > k = read_sysfs_int(path); > > sprintf(path, > > - > "/sys/devices/system/cpu/intel_uncore_frequency/package_0%d_die_0%d/initial_max_freq_khz", > > + > "/sys/devices/system/cpu/intel_uncore_frequency/package_%02d_die_%02d/initial_max_freq_khz", > > i, j); > > l = read_sysfs_int(path); > > fprintf(outf, "(%d - %d MHz)\n", k / 1000, l / 1000); > > -- > > 2.26.2 > > > > > -- > Len Brown, Intel
diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c index 7a334377f92b..2a15a23cb726 100644 --- a/tools/power/x86/turbostat/turbostat.c +++ b/tools/power/x86/turbostat/turbostat.c @@ -2599,7 +2599,7 @@ unsigned long long get_uncore_mhz(int package, int die) { char path[128]; - sprintf(path, "/sys/devices/system/cpu/intel_uncore_frequency/package_0%d_die_0%d/current_freq_khz", package, + sprintf(path, "/sys/devices/system/cpu/intel_uncore_frequency/package_%02d_die_%02d/current_freq_khz", package, die); return (snapshot_sysfs_counter(path) / 1000); @@ -4589,20 +4589,20 @@ static void probe_intel_uncore_frequency(void) for (j = 0; j < topo.num_die; ++j) { int k, l; - sprintf(path, "/sys/devices/system/cpu/intel_uncore_frequency/package_0%d_die_0%d/min_freq_khz", + sprintf(path, "/sys/devices/system/cpu/intel_uncore_frequency/package_%02d_die_%02d/min_freq_khz", i, j); k = read_sysfs_int(path); - sprintf(path, "/sys/devices/system/cpu/intel_uncore_frequency/package_0%d_die_0%d/max_freq_khz", + sprintf(path, "/sys/devices/system/cpu/intel_uncore_frequency/package_%02d_die_%02d/max_freq_khz", i, j); l = read_sysfs_int(path); fprintf(outf, "Uncore Frequency pkg%d die%d: %d - %d MHz ", i, j, k / 1000, l / 1000); sprintf(path, - "/sys/devices/system/cpu/intel_uncore_frequency/package_0%d_die_0%d/initial_min_freq_khz", + "/sys/devices/system/cpu/intel_uncore_frequency/package_%02d_die_%02d/initial_min_freq_khz", i, j); k = read_sysfs_int(path); sprintf(path, - "/sys/devices/system/cpu/intel_uncore_frequency/package_0%d_die_0%d/initial_max_freq_khz", + "/sys/devices/system/cpu/intel_uncore_frequency/package_%02d_die_%02d/initial_max_freq_khz", i, j); l = read_sysfs_int(path); fprintf(outf, "(%d - %d MHz)\n", k / 1000, l / 1000);
Running turbostat on a 16 socket HPE Scale-up Compute 3200 (SapphireRapids) fails with: turbostat: /sys/devices/system/cpu/intel_uncore_frequency/package_010_die_00/current_freq_khz: open failed: No such file or directory We observe the sysfs uncore frequency directories named: ... package_09_die_00/ package_10_die_00/ package_11_die_00/ ... package_15_die_00/ The culprit is an incorrect sprintf format string "package_0%d_die_0%d" used with each instance of reading uncore frequency files. uncore-frequency-common.c creates the sysfs directory with the format "package_%02d_die_%02d". Once the package value reaches double digits, the formats diverge. Change each instance of "package_0%d_die_0%d" to "package_%02d_die_%02d". Signed-off-by: Justin Ernst <justin.ernst@hpe.com> --- tools/power/x86/turbostat/turbostat.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)