tools/cpupower: Choose first online CPU to display details
Commit Message
The default output of cpupower info utils shows unexpected output
when CPU 0 is disabled.
Considering a case where CPU 0 is disabled, output of cpupower idle-info:
Before change:
cpupower idle-info
CPUidle driver: pseries_idle
CPUidle governor: menu
analyzing CPU 0:
*is offline
After change:
./cpupower idle-info
CPUidle driver: pseries_idle
CPUidle governor: menu
analyzing CPU 0:
*is offline
analyzing CPU 1:
Number of idle states: 2
Available idle states: snooze Shared Cede
snooze:
Flags/Description: snooze
Latency: 0
Usage: 919
Duration: 68227
Shared Cede:
Flags/Description: Shared Cede
Latency: 10
Usage: 99324
Duration: 67871518243
If -c option is not passed, CPU 0 was chosen as the default chosen CPU to
display details. However when CPU 0 is offline, it results in showing
unexpected output. This commit chooses the first online CPU
instead of CPU 0, hence keeping the output more relevant in all cases.
Somewhat similar logic to set all CPUs in default case is present in
cpufreq-set.c, cpuidle-set.c, cpupower-set.c. But here we have added
logic to stop at first online CPU.
Signed-off-by: Saket Kumar Bhaskar <skb99@linux.vnet.ibm.com>
---
tools/power/cpupower/utils/cpufreq-info.c | 15 ++++++++++++---
tools/power/cpupower/utils/cpuidle-info.c | 15 ++++++++++++---
tools/power/cpupower/utils/cpupower-info.c | 16 +++++++++++++---
3 files changed, 37 insertions(+), 9 deletions(-)
Comments
On 11/20/22 22:35, Saket Kumar Bhaskar wrote:
> The default output of cpupower info utils shows unexpected output
> when CPU 0 is disabled.
>
> Considering a case where CPU 0 is disabled, output of cpupower idle-info:
>
> Before change:
> cpupower idle-info
> CPUidle driver: pseries_idle
> CPUidle governor: menu
> analyzing CPU 0:
> *is offline
>
> After change:
> ./cpupower idle-info
> CPUidle driver: pseries_idle
> CPUidle governor: menu
> analyzing CPU 0:
> *is offline
> analyzing CPU 1:
>
> Number of idle states: 2
> Available idle states: snooze Shared Cede
> snooze:
> Flags/Description: snooze
> Latency: 0
> Usage: 919
> Duration: 68227
> Shared Cede:
> Flags/Description: Shared Cede
> Latency: 10
> Usage: 99324
> Duration: 67871518243
>
> If -c option is not passed, CPU 0 was chosen as the default chosen CPU to
> display details. However when CPU 0 is offline, it results in showing
> unexpected output. This commit chooses the first online CPU
> instead of CPU 0, hence keeping the output more relevant in all cases.
>
> Somewhat similar logic to set all CPUs in default case is present in
> cpufreq-set.c, cpuidle-set.c, cpupower-set.c. But here we have added
> logic to stop at first online CPU.
>
> Signed-off-by: Saket Kumar Bhaskar <skb99@linux.vnet.ibm.com>
> ---
> tools/power/cpupower/utils/cpufreq-info.c | 15 ++++++++++++---
> tools/power/cpupower/utils/cpuidle-info.c | 15 ++++++++++++---
> tools/power/cpupower/utils/cpupower-info.c | 16 +++++++++++++---
> 3 files changed, 37 insertions(+), 9 deletions(-)
>
> diff --git a/tools/power/cpupower/utils/cpufreq-info.c b/tools/power/cpupower/utils/cpufreq-info.c
> index 0646f615fe2d..4001a5934494 100644
> --- a/tools/power/cpupower/utils/cpufreq-info.c
> +++ b/tools/power/cpupower/utils/cpufreq-info.c
> @@ -508,6 +508,7 @@ int cmd_freq_info(int argc, char **argv)
> unsigned int cpu = 0;
> unsigned int human = 0;
> int output_param = 0;
> + bool is_default = false;
>
> do {
> ret = getopt_long(argc, argv, "oefwldpgrasmybnc", info_opts,
> @@ -572,9 +573,11 @@ int cmd_freq_info(int argc, char **argv)
>
> ret = 0;
>
> - /* Default is: show output of CPU 0 only */
> - if (bitmask_isallclear(cpus_chosen))
> - bitmask_setbit(cpus_chosen, 0);
Why can't we just use base_cpu here instead of 0 and it will
all work without any more code - you can update the comment
to say default is base_cpu info.
thanks,
-- Shuah
@@ -508,6 +508,7 @@ int cmd_freq_info(int argc, char **argv)
unsigned int cpu = 0;
unsigned int human = 0;
int output_param = 0;
+ bool is_default = false;
do {
ret = getopt_long(argc, argv, "oefwldpgrasmybnc", info_opts,
@@ -572,9 +573,11 @@ int cmd_freq_info(int argc, char **argv)
ret = 0;
- /* Default is: show output of CPU 0 only */
- if (bitmask_isallclear(cpus_chosen))
- bitmask_setbit(cpus_chosen, 0);
+ /* Default is: set all CPUs */
+ if (bitmask_isallclear(cpus_chosen)) {
+ bitmask_setall(cpus_chosen);
+ is_default = true;
+ }
switch (output_param) {
case -1:
@@ -646,6 +649,12 @@ int cmd_freq_info(int argc, char **argv)
}
if (ret)
return ret;
+
+ if (is_default) {
+ bitmask_clearall(cpus_chosen);
+ bitmask_setbit(cpus_chosen, cpu);
+ break;
+ }
}
return ret;
}
@@ -139,6 +139,7 @@ int cmd_idle_info(int argc, char **argv)
extern int optind, opterr, optopt;
int ret = 0, cont = 1, output_param = 0, verbose = 1;
unsigned int cpu = 0;
+ bool is_default = false;
do {
ret = getopt_long(argc, argv, "os", info_opts, NULL);
@@ -176,9 +177,11 @@ int cmd_idle_info(int argc, char **argv)
cpuidle_exit(EXIT_FAILURE);
}
- /* Default is: show output of CPU 0 only */
- if (bitmask_isallclear(cpus_chosen))
- bitmask_setbit(cpus_chosen, 0);
+ /* Default is: set all CPUs */
+ if (bitmask_isallclear(cpus_chosen)) {
+ bitmask_setall(cpus_chosen);
+ is_default = true;
+ }
if (output_param == 0)
cpuidle_general_output();
@@ -208,6 +211,12 @@ int cmd_idle_info(int argc, char **argv)
break;
}
printf("\n");
+
+ if (is_default) {
+ bitmask_clearall(cpus_chosen);
+ bitmask_setbit(cpus_chosen, cpu);
+ break;
+ }
}
return EXIT_SUCCESS;
}
@@ -40,6 +40,7 @@ int cmd_info(int argc, char **argv)
int params;
} params = {};
int ret = 0;
+ bool is_default = false;
ret = uname(&uts);
if (!ret && (!strcmp(uts.machine, "ppc64le") ||
@@ -67,9 +68,12 @@ int cmd_info(int argc, char **argv)
if (!params.params)
params.params = 0x7;
- /* Default is: show output of CPU 0 only */
- if (bitmask_isallclear(cpus_chosen))
- bitmask_setbit(cpus_chosen, 0);
+ /* Default is: set all CPUs */
+ if (bitmask_isallclear(cpus_chosen)) {
+ bitmask_setall(cpus_chosen);
+ is_default = true;
+ }
+
/* Add more per cpu options here */
if (!params.perf_bias)
@@ -109,6 +113,12 @@ int cmd_info(int argc, char **argv)
} else
printf(_("perf-bias: %d\n"), ret);
}
+
+ if (is_default) {
+ bitmask_clearall(cpus_chosen);
+ bitmask_setbit(cpus_chosen, cpu);
+ break;
+ }
}
return 0;
}