tools/power turbostat: Warn on bad ACPI LPIT data
diff mbox series

Message ID 20180925125926.10404-1-prarit@redhat.com
State Accepted, archived
Delegated to: Len Brown
Headers show
Series
  • tools/power turbostat: Warn on bad ACPI LPIT data
Related show

Commit Message

Prarit Bhargava Sept. 25, 2018, 12:59 p.m. UTC
On some systems /sys/devices/system/cpu/cpuidle/low_power_idle_cpu_residency_us
or /sys/devices/system/cpu/cpuidle/low_power_idle_system_residency_us
return a file error because of bad ACPI LPIT data from a misconfigured BIOS.
turbostat interprets this failure as a fatal error and outputs

	turbostat: CPU LPI: No data available

If the ACPI LPIT sysfs files return an error output a warning instead of
a fatal error, disable the ACPI LPIT evaluation code, and continue.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: Len Brown <lenb@kernel.org>
---
 tools/power/x86/turbostat/turbostat.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

Comments

Prarit Bhargava Oct. 8, 2018, 3:44 p.m. UTC | #1
Len?

P.

On 09/25/2018 08:59 AM, Prarit Bhargava wrote:
> On some systems /sys/devices/system/cpu/cpuidle/low_power_idle_cpu_residency_us
> or /sys/devices/system/cpu/cpuidle/low_power_idle_system_residency_us
> return a file error because of bad ACPI LPIT data from a misconfigured BIOS.
> turbostat interprets this failure as a fatal error and outputs
> 
> 	turbostat: CPU LPI: No data available
> 
> If the ACPI LPIT sysfs files return an error output a warning instead of
> a fatal error, disable the ACPI LPIT evaluation code, and continue.
> 
> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
> Cc: Len Brown <lenb@kernel.org>
> ---
>  tools/power/x86/turbostat/turbostat.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
> index 980bd9d20646..6b81decede49 100644
> --- a/tools/power/x86/turbostat/turbostat.c
> +++ b/tools/power/x86/turbostat/turbostat.c
> @@ -2831,8 +2831,11 @@ int snapshot_cpu_lpi_us(void)
>  	fp = fopen_or_die("/sys/devices/system/cpu/cpuidle/low_power_idle_cpu_residency_us", "r");
>  
>  	retval = fscanf(fp, "%lld", &cpuidle_cur_cpu_lpi_us);
> -	if (retval != 1)
> -		err(1, "CPU LPI");
> +	if (retval != 1) {
> +		fprintf(stderr, "Disabling Low Power Idle CPU output\n");
> +		BIC_NOT_PRESENT(BIC_CPU_LPI);
> +		return -1;
> +	}
>  
>  	fclose(fp);
>  
> @@ -2854,9 +2857,11 @@ int snapshot_sys_lpi_us(void)
>  	fp = fopen_or_die("/sys/devices/system/cpu/cpuidle/low_power_idle_system_residency_us", "r");
>  
>  	retval = fscanf(fp, "%lld", &cpuidle_cur_sys_lpi_us);
> -	if (retval != 1)
> -		err(1, "SYS LPI");
> -
> +	if (retval != 1) {
> +		fprintf(stderr, "Disabling Low Power Idle System output\n");
> +		BIC_NOT_PRESENT(BIC_SYS_LPI);
> +		return -1;
> +	}
>  	fclose(fp);
>  
>  	return 0;
>
Len Brown March 21, 2019, 3:14 a.m. UTC | #2
Interesting.  What is returned when this fails?

While I've applied your patch (thanks!), I wonder if the kernel should
really not be exporting these attributes if they don't work...

-Len

On Mon, Oct 8, 2018 at 11:44 AM Prarit Bhargava <prarit@redhat.com> wrote:
>
> Len?
>
> P.
>
> On 09/25/2018 08:59 AM, Prarit Bhargava wrote:
> > On some systems /sys/devices/system/cpu/cpuidle/low_power_idle_cpu_residency_us
> > or /sys/devices/system/cpu/cpuidle/low_power_idle_system_residency_us
> > return a file error because of bad ACPI LPIT data from a misconfigured BIOS.
> > turbostat interprets this failure as a fatal error and outputs
> >
> >       turbostat: CPU LPI: No data available
> >
> > If the ACPI LPIT sysfs files return an error output a warning instead of
> > a fatal error, disable the ACPI LPIT evaluation code, and continue.
> >
> > Signed-off-by: Prarit Bhargava <prarit@redhat.com>
> > Cc: Len Brown <lenb@kernel.org>
> > ---
> >  tools/power/x86/turbostat/turbostat.c | 15 ++++++++++-----
> >  1 file changed, 10 insertions(+), 5 deletions(-)
> >
> > diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
> > index 980bd9d20646..6b81decede49 100644
> > --- a/tools/power/x86/turbostat/turbostat.c
> > +++ b/tools/power/x86/turbostat/turbostat.c
> > @@ -2831,8 +2831,11 @@ int snapshot_cpu_lpi_us(void)
> >       fp = fopen_or_die("/sys/devices/system/cpu/cpuidle/low_power_idle_cpu_residency_us", "r");
> >
> >       retval = fscanf(fp, "%lld", &cpuidle_cur_cpu_lpi_us);
> > -     if (retval != 1)
> > -             err(1, "CPU LPI");
> > +     if (retval != 1) {
> > +             fprintf(stderr, "Disabling Low Power Idle CPU output\n");
> > +             BIC_NOT_PRESENT(BIC_CPU_LPI);
> > +             return -1;
> > +     }
> >
> >       fclose(fp);
> >
> > @@ -2854,9 +2857,11 @@ int snapshot_sys_lpi_us(void)
> >       fp = fopen_or_die("/sys/devices/system/cpu/cpuidle/low_power_idle_system_residency_us", "r");
> >
> >       retval = fscanf(fp, "%lld", &cpuidle_cur_sys_lpi_us);
> > -     if (retval != 1)
> > -             err(1, "SYS LPI");
> > -
> > +     if (retval != 1) {
> > +             fprintf(stderr, "Disabling Low Power Idle System output\n");
> > +             BIC_NOT_PRESENT(BIC_SYS_LPI);
> > +             return -1;
> > +     }
> >       fclose(fp);
> >
> >       return 0;
> >
Prarit Bhargava March 21, 2019, 12:04 p.m. UTC | #3
On 3/20/19 11:14 PM, Len Brown wrote:
> Interesting.  What is returned when this fails?
> 

ISTR I got an error that just said "CPU LPI: no data available".  I'll have to
dig back through my notes to find the bug that was filed.

> While I've applied your patch (thanks!), I wonder if the kernel should
> really not be exporting these attributes if they don't work...
> 

I dunno if it is up to the kernel to determine the validity of the contents of a
table?  That could be a slippery slope.

Also when I pointed out to the customer that this is an issue with their ACPI
tables they did respond back that they agreed and they required a fix.

P.

> -Len
> 
> On Mon, Oct 8, 2018 at 11:44 AM Prarit Bhargava <prarit@redhat.com> wrote:
>>
>> Len?
>>
>> P.
>>
>> On 09/25/2018 08:59 AM, Prarit Bhargava wrote:
>>> On some systems /sys/devices/system/cpu/cpuidle/low_power_idle_cpu_residency_us
>>> or /sys/devices/system/cpu/cpuidle/low_power_idle_system_residency_us
>>> return a file error because of bad ACPI LPIT data from a misconfigured BIOS.
>>> turbostat interprets this failure as a fatal error and outputs
>>>
>>>       turbostat: CPU LPI: No data available
>>>
>>> If the ACPI LPIT sysfs files return an error output a warning instead of
>>> a fatal error, disable the ACPI LPIT evaluation code, and continue.
>>>
>>> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
>>> Cc: Len Brown <lenb@kernel.org>
>>> ---
>>>  tools/power/x86/turbostat/turbostat.c | 15 ++++++++++-----
>>>  1 file changed, 10 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
>>> index 980bd9d20646..6b81decede49 100644
>>> --- a/tools/power/x86/turbostat/turbostat.c
>>> +++ b/tools/power/x86/turbostat/turbostat.c
>>> @@ -2831,8 +2831,11 @@ int snapshot_cpu_lpi_us(void)
>>>       fp = fopen_or_die("/sys/devices/system/cpu/cpuidle/low_power_idle_cpu_residency_us", "r");
>>>
>>>       retval = fscanf(fp, "%lld", &cpuidle_cur_cpu_lpi_us);
>>> -     if (retval != 1)
>>> -             err(1, "CPU LPI");
>>> +     if (retval != 1) {
>>> +             fprintf(stderr, "Disabling Low Power Idle CPU output\n");
>>> +             BIC_NOT_PRESENT(BIC_CPU_LPI);
>>> +             return -1;
>>> +     }
>>>
>>>       fclose(fp);
>>>
>>> @@ -2854,9 +2857,11 @@ int snapshot_sys_lpi_us(void)
>>>       fp = fopen_or_die("/sys/devices/system/cpu/cpuidle/low_power_idle_system_residency_us", "r");
>>>
>>>       retval = fscanf(fp, "%lld", &cpuidle_cur_sys_lpi_us);
>>> -     if (retval != 1)
>>> -             err(1, "SYS LPI");
>>> -
>>> +     if (retval != 1) {
>>> +             fprintf(stderr, "Disabling Low Power Idle System output\n");
>>> +             BIC_NOT_PRESENT(BIC_SYS_LPI);
>>> +             return -1;
>>> +     }
>>>       fclose(fp);
>>>
>>>       return 0;
>>>
> 
> 
>

Patch
diff mbox series

diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index 980bd9d20646..6b81decede49 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -2831,8 +2831,11 @@  int snapshot_cpu_lpi_us(void)
 	fp = fopen_or_die("/sys/devices/system/cpu/cpuidle/low_power_idle_cpu_residency_us", "r");
 
 	retval = fscanf(fp, "%lld", &cpuidle_cur_cpu_lpi_us);
-	if (retval != 1)
-		err(1, "CPU LPI");
+	if (retval != 1) {
+		fprintf(stderr, "Disabling Low Power Idle CPU output\n");
+		BIC_NOT_PRESENT(BIC_CPU_LPI);
+		return -1;
+	}
 
 	fclose(fp);
 
@@ -2854,9 +2857,11 @@  int snapshot_sys_lpi_us(void)
 	fp = fopen_or_die("/sys/devices/system/cpu/cpuidle/low_power_idle_system_residency_us", "r");
 
 	retval = fscanf(fp, "%lld", &cpuidle_cur_sys_lpi_us);
-	if (retval != 1)
-		err(1, "SYS LPI");
-
+	if (retval != 1) {
+		fprintf(stderr, "Disabling Low Power Idle System output\n");
+		BIC_NOT_PRESENT(BIC_SYS_LPI);
+		return -1;
+	}
 	fclose(fp);
 
 	return 0;