Messages in this thread | | | Date | Mon, 21 Nov 2022 13:26:38 -0700 | Subject | Re: [PATCH] tools/cpupower: Choose first online CPU to display details | From | Shuah Khan <> |
| |
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
| |