RE: [PATCH] tools/power/turbostat: fix compatibility with older kernels

["Doug Smythies" <dsmythies@telus.net>]

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