linux-kernel - Re: [PATCH] tools/cpupower: Choose first online CPU to display details

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 21 Nov 2022 13:26:38 -0700
From:   Shuah Khan <skhan@...uxfoundation.org>
To:     Saket Kumar Bhaskar <skb99@...ux.vnet.ibm.com>,
        linux-kernel@...r.kernel.org
Cc:     trenn@...e.com, shuah@...nel.org, ray.huang@....com,
        linux-pm@...r.kernel.org, Shuah Khan <skhan@...uxfoundation.org>
Subject: Re: [PATCH] tools/cpupower: Choose first online CPU to display
 details

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@...ux.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