lkml.org 
[lkml]   [2022]   [Nov]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] tools/cpupower: Choose first online CPU to display details
From
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

\
 
 \ /
  Last update: 2022-11-21 21:27    [W:0.030 / U:1.504 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site