| Message ID | 20210127132444.981120-1-dedekind1@gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Len Brown |
| Headers | show |
| Series | tools/power/turbostat: fix compatibility with older kernels | expand |
On Wed, Jan 27, 2021 at 03:24:44PM +0200, Artem Bityutskiy wrote: > From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com> > > Commit > > 6d6501d912a9 tools/power/turbostat: Read energy_perf_bias from sysfs > > added a useful EPB print by reading EPB from sysfs's 'energy_perf_bias' file. > However, older kernels, which do not necessarily have that sysfs file (e.g., > Centos 7's stock kernel does not have it). As a result, turbostat fails with > older kernels. > > This patch fixes the problem by ignoring the sysfs file read errors. > > Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com> > --- > tools/power/x86/turbostat/turbostat.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c > index 389ea5209a83..12e014f2c24b 100644 > --- a/tools/power/x86/turbostat/turbostat.c > +++ b/tools/power/x86/turbostat/turbostat.c > @@ -1839,7 +1839,9 @@ int get_epb(int cpu) > > sprintf(path, "/sys/devices/system/cpu/cpu%d/power/energy_perf_bias", cpu); > > - fp = fopen_or_die(path, "r"); > + fp = fopen(path, "r"); > + if (!fp) > + return -1; > > ret = fscanf(fp, "%d", &epb); > if (ret != 1) So I was under the impression that things in tools/ are tied to the kernel version they're shipped with. Which means, you should probably get the one from the centos kernel. However, if this is supposed to work on older kernels too, then 6d6501d912a9 ("tools/power/turbostat: Read energy_perf_bias from sysfs") would need to be extended to first test the sysfs file's existence and if not there, fall back to the MSR reading...
On 2021.01.27 Borislav Petkov wrote: > On Wed, Jan 27, 2021 at 03:24:44PM +0200, Artem Bityutskiy wrote: > > From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com> > > > > Commit > > > > 6d6501d912a9 tools/power/turbostat: Read energy_perf_bias from sysfs > > > > added a useful EPB print by reading EPB from sysfs's 'energy_perf_bias' file. > > However, older kernels, which do not necessarily have that sysfs file (e.g., > > Centos 7's stock kernel does not have it). As a result, turbostat fails with > > older kernels. > > > > This patch fixes the problem by ignoring the sysfs file read errors. > > > > Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com> > > --- > > tools/power/x86/turbostat/turbostat.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c > > index 389ea5209a83..12e014f2c24b 100644 > > --- a/tools/power/x86/turbostat/turbostat.c > > +++ b/tools/power/x86/turbostat/turbostat.c > > @@ -1839,7 +1839,9 @@ int get_epb(int cpu) > > > > sprintf(path, "/sys/devices/system/cpu/cpu%d/power/energy_perf_bias", cpu); > > > > - fp = fopen_or_die(path, "r"); > > + fp = fopen(path, "r"); > > + if (!fp) > > + return -1; > > > > ret = fscanf(fp, "%d", &epb); > > if (ret != 1) > > So I was under the impression that things in tools/ are tied to the > kernel version they're shipped with. Which means, you should probably > get the one from the centos kernel. > > However, if this is supposed to work on older kernels too, then It is supposed to work with old kernels. Here is a quote from Len from [1] (2019.09.05) : > The latest turbostat and x86_energy_perf_policy utilities > in the upstream kernel tree should always be backward > compatible with all old kernels. If that is EVER not the > case, I want to know about it. > > Yes, I know that some distros ship old versions of these > utilities built out of their matching kernel tree snapshots. > Yes, applying upstream fixes to .stable for such distros is a good thing. > > However, the better solution for these particular utilities, is that > they simply always use upstream utilities -- even with old kernels. > > When somebody reports a problem and I need them to run these tools, > 100% of the time, I start by sending them the latest upstream version > to replace the old version shipped by the distro. Which is also what I do. I was also trying, so far without success, to get a distro to relax its stringent, unnecessary, turbostat dependency checking [2]. > > 6d6501d912a9 ("tools/power/turbostat: Read energy_perf_bias from sysfs") > > would need to be extended to first test the sysfs file's existence and > if not there, fall back to the MSR reading... Yes agree, the information is still required. [1] https://marc.info/?l=linux-pm&m=156770359620861&w=2 [2] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1844201
On Wed, Jan 27, 2021 at 12:15:10PM -0800, Doug Smythies wrote: > It is supposed to work with old kernels. Here is a quote from Len from > [1] (2019.09.05) : Yeah, reportedly, this is the general strategy with tools in tools/. > Which is also what I do. I was also trying, so far without success, to > get a distro to relax its stringent, unnecessary, turbostat dependency > checking [2]. I see. > Yes agree, the information is still required. Yeah, lemme do a proper patch tomorrow. Thx.
On Wed, Jan 27, 2021 at 09:33:46PM +0100, Borislav Petkov wrote:
> Yeah, lemme do a proper patch tomorrow.
Artem, how's that?
---
diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index 389ea5209a83..a7c4f0772e53 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -1834,12 +1834,15 @@ int get_mp(int cpu, struct msr_counter *mp, unsigned long long *counterp)
int get_epb(int cpu)
{
char path[128 + PATH_BYTES];
+ unsigned long long msr;
int ret, epb = -1;
FILE *fp;
sprintf(path, "/sys/devices/system/cpu/cpu%d/power/energy_perf_bias", cpu);
- fp = fopen_or_die(path, "r");
+ fp = fopen(path, "r");
+ if (!fp)
+ goto msr_fallback;
ret = fscanf(fp, "%d", &epb);
if (ret != 1)
@@ -1848,6 +1851,11 @@ int get_epb(int cpu)
fclose(fp);
return epb;
+
+msr_fallback:
+ get_msr(cpu, MSR_IA32_ENERGY_PERF_BIAS, &msr);
+
+ return msr & 0xf;
}
void get_apic_id(struct thread_data *t)
---
Thx.
diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c index 389ea5209a83..12e014f2c24b 100644 --- a/tools/power/x86/turbostat/turbostat.c +++ b/tools/power/x86/turbostat/turbostat.c @@ -1839,7 +1839,9 @@ int get_epb(int cpu) sprintf(path, "/sys/devices/system/cpu/cpu%d/power/energy_perf_bias", cpu); - fp = fopen_or_die(path, "r"); + fp = fopen(path, "r"); + if (!fp) + return -1; ret = fscanf(fp, "%d", &epb); if (ret != 1)