diff mbox series

[v3] cpupower: Add better frequency-info failure messaging

Message ID 20181217190400.11501-1-prarit@redhat.com (mailing list archive)
State Changes Requested
Delegated to: Shuah Khan
Headers show
Series [v3] cpupower: Add better frequency-info failure messaging | expand

Commit Message

Prarit Bhargava Dec. 17, 2018, 7:04 p.m. UTC
Tested with:

Performance Per Watt (OS)  (ie, kernel)

  current policy: frequency should be within 800 MHz and 3.00 GHz.
                  The governor "performance" may decide which speed to use
                  within this range.
  current CPU frequency: 2.03 GHz (asserted by call to kernel)

Performance Per Watt (DAPC) (ie, niether)

  current policy: frequency should be within 800 MHz and 3.00 GHz.
                  The governor "performance" may decide which speed to use
                  within this range.
  current CPU frequency: Unable to call hardware
  current CPU frequency: Unable to call kernel

Performance (ie, HW)

  current policy: frequency should be within 800 MHz and 3.00 GHz.
                  The governor "performance" may decide which speed to use
                  within this range.
  current CPU frequency: 2.33 GHz (asserted by call to kernel)

----8<----

If a Dell system has "OS DBPM(Performance Per Watt OS)" set in BIOS,
cpupower frequency-info outputs

  current CPU frequency: Unable to call hardware
  current CPU frequency: 2.50 GHz (asserted by call to kernel)

to indicate that cpupower cannot read the frequency from hardware and
is using the kernel's cpu frequency estimate.  This output is confusing
to end users who wonder why the hardware is not responding.

Update the cpupower frequency-info output message to only output the
error if both the hardware and kernel calls fail.

v2: Update logic and output separate error lines on failure.
v3: (trenn) keep printk's separate so that translations continue to work

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: Thomas Renninger <trenn@suse.com>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Stafford Horne <shorne@gmail.com>
---
 tools/power/cpupower/utils/cpufreq-info.c | 39 +++++++++++++++--------
 1 file changed, 25 insertions(+), 14 deletions(-)

Comments

Shuah Dec. 17, 2018, 9:42 p.m. UTC | #1
Hi Prarit,

On 12/17/18 12:04 PM, Prarit Bhargava wrote:
> Tested with:
> 
> Performance Per Watt (OS)  (ie, kernel)
> 
>    current policy: frequency should be within 800 MHz and 3.00 GHz.
>                    The governor "performance" may decide which speed to use
>                    within this range.
>    current CPU frequency: 2.03 GHz (asserted by call to kernel)
> 
> Performance Per Watt (DAPC) (ie, niether)
> 
>    current policy: frequency should be within 800 MHz and 3.00 GHz.
>                    The governor "performance" may decide which speed to use
>                    within this range.
>    current CPU frequency: Unable to call hardware
>    current CPU frequency: Unable to call kernel
> 
> Performance (ie, HW)
> 
>    current policy: frequency should be within 800 MHz and 3.00 GHz.
>                    The governor "performance" may decide which speed to use
>                    within this range.
>    current CPU frequency: 2.33 GHz (asserted by call to kernel)
> 
> ----8<----
> 
> If a Dell system has "OS DBPM(Performance Per Watt OS)" set in BIOS,
> cpupower frequency-info outputs
> 
>    current CPU frequency: Unable to call hardware
>    current CPU frequency: 2.50 GHz (asserted by call to kernel)
> 
> to indicate that cpupower cannot read the frequency from hardware and
> is using the kernel's cpu frequency estimate.  This output is confusing
> to end users who wonder why the hardware is not responding.
> 
> Update the cpupower frequency-info output message to only output the
> error if both the hardware and kernel calls fail.

What is being fixed and test information are in out of order?
Can you fix the change log

thanks,
-- Shuah
diff mbox series

Patch

diff --git a/tools/power/cpupower/utils/cpufreq-info.c b/tools/power/cpupower/utils/cpufreq-info.c
index c3f39d5128ee..d78f3ac49853 100644
--- a/tools/power/cpupower/utils/cpufreq-info.c
+++ b/tools/power/cpupower/utils/cpufreq-info.c
@@ -250,17 +250,22 @@  static int get_boost_mode(unsigned int cpu)
 
 /* --freq / -f */
 
-static int get_freq_kernel(unsigned int cpu, unsigned int human)
+static int get_freq_kernel(unsigned int cpu, unsigned int human, int fail_msg)
 {
 	unsigned long freq = cpufreq_get_freq_kernel(cpu);
-	printf(_("  current CPU frequency: "));
+
 	if (!freq) {
-		printf(_(" Unable to call to kernel\n"));
+		if (fail_msg) {
+			/* keep these separate so translations still work */
+			printf(_("  current CPU frequency: "));
+			printf(_("Unable to call to kernel\n"));
+		}
 		return -EINVAL;
 	}
-	if (human) {
+	printf(_("  current CPU frequency: "));
+	if (human)
 		print_speed(freq);
-	} else
+	else
 		printf("%lu", freq);
 	printf(_(" (asserted by call to kernel)\n"));
 	return 0;
@@ -269,17 +274,19 @@  static int get_freq_kernel(unsigned int cpu, unsigned int human)
 
 /* --hwfreq / -w */
 
-static int get_freq_hardware(unsigned int cpu, unsigned int human)
+static int get_freq_hardware(unsigned int cpu, unsigned int human, int fail_msg)
 {
 	unsigned long freq = cpufreq_get_freq_hardware(cpu);
-	printf(_("  current CPU frequency: "));
+
 	if (!freq) {
-		printf("Unable to call hardware\n");
+		if (fail_msg)
+			printf(_("  current CPU frequency: Unable to call hardware\n"));
 		return -EINVAL;
 	}
-	if (human) {
+	printf(_("  current CPU frequency: "));
+	if (human)
 		print_speed(freq);
-	} else
+	else
 		printf("%lu", freq);
 	printf(_(" (asserted by call to hardware)\n"));
 	return 0;
@@ -479,8 +486,12 @@  static void debug_output_one(unsigned int cpu)
 
 	get_available_governors(cpu);
 	get_policy(cpu);
-	if (get_freq_hardware(cpu, 1) < 0)
-		get_freq_kernel(cpu, 1);
+	if (get_freq_hardware(cpu, 1, 0)) {
+		if (get_freq_kernel(cpu, 1, 0)) {
+			printf(_("  current CPU frequency: Unable to call hardware\n"));
+			printf(_("  current CPU frequency: Unable to call kernel\n"));
+		}
+	}
 	get_boost_mode(cpu);
 }
 
@@ -631,10 +642,10 @@  int cmd_freq_info(int argc, char **argv)
 			ret = get_hardware_limits(cpu, human);
 			break;
 		case 'w':
-			ret = get_freq_hardware(cpu, human);
+			ret = get_freq_hardware(cpu, human, 1);
 			break;
 		case 'f':
-			ret = get_freq_kernel(cpu, human);
+			ret = get_freq_kernel(cpu, human, 1);
 			break;
 		case 's':
 			ret = get_freq_stats(cpu, human);