diff mbox series

tools/power/turbostat: fix compatibility with older kernels

Message ID 20210127132444.981120-1-dedekind1@gmail.com
State New
Headers show
Series tools/power/turbostat: fix compatibility with older kernels | expand

Commit Message

Artem Bityutskiy Jan. 27, 2021, 1:24 p.m. UTC
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(-)

Comments

Doug Smythies Jan. 27, 2021, 8:15 p.m. UTC | #1
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
Borislav Petkov Jan. 28, 2021, 5:10 p.m. UTC | #2
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.

-- 
Regards/Gruss,
    Boris.

SUSE Software Solutions Germany GmbH, GF: Felix Imendörffer, HRB 36809, AG Nürnberg
diff mbox series

Patch

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)