Hi Lianbo,
Good catch for the corner case where get_cpus_present() may be 0.
Looks good to me, and thank you!
Best regards,
Lucas
On Wed, Jan 15, 2025 at 10:42 PM lijiang <lijiang(a)redhat.com> wrote:
 Thank you for the fix, Lucas.
 On Sat, Jan 4, 2025 at 5:54 AM <devel-request(a)lists.crash-utility.osci.io>
 wrote:
> Date: Fri,  3 Jan 2025 16:52:18 -0500
> From: soakley(a)redhat.com
> Subject: [Crash-utility] [PATCH] Fix misleading CPU count in
>         display_sys_stats()
> To: devel(a)lists.crash-utility.osci.io
> Cc: Lucas Oakley <soakley(a)redhat.com>
> Message-ID: <20250103215218.712496-1-soakley(a)redhat.com>
> Content-Type: text/plain; charset="US-ASCII"; x-default=true
>
> From: Lucas Oakley <soakley(a)redhat.com>
>
> This simplication fixes the total CPU count being reported
> incorrectly in ppc64le and s390x systems when some number of
> CPUs have been offlined, as the kt->cpus value is adjusted.
> This adds the word "OFFLINE" to the 'sys' output for s390x
> and ppc64le, like exists for x86_64 and aarch64 when examining
> systems with offlined CPUs.
>
> Without patch:
>
>   KERNEL: /debug/4.18.0-477.10.1.el8_8.s390x/vmlinux
> DUMPFILE: /proc/kcore
>     CPUS: 1
>
> With patch:
>
>   KERNEL: /debug/4.18.0-477.10.1.el8_8.s390x/vmlinux
> DUMPFILE: /proc/kcore
>     CPUS: 2 [OFFLINE: 1]
>
> Signed-off-by: Lucas Oakley <soakley(a)redhat.com>
> ---
>  kernel.c | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/kernel.c b/kernel.c
> index 8c2e0ca..3e190f1 100644
> --- a/kernel.c
> +++ b/kernel.c
> @@ -5816,15 +5816,13 @@ display_sys_stats(void)
>                                 pc->kvmdump_mapfile);
>         }
>
> -       if (machine_type("PPC64"))
> -               fprintf(fp, "        CPUS: %d\n", get_cpus_to_display());
> -       else {
> -               fprintf(fp, "        CPUS: %d", kt->cpus);
> -               if (kt->cpus - get_cpus_to_display())
> -                       fprintf(fp, " [OFFLINE: %d]",
> -                               kt->cpus - get_cpus_to_display());
> -               fprintf(fp, "\n");
> -       }
> +        int number_cpus_to_display = get_cpus_to_display();
> +        int number_cpus_present = get_cpus_present();
> +        fprintf(fp, "        CPUS: %d", number_cpus_present);
> +        if (number_cpus_present != number_cpus_to_display)
> +                fprintf(fp, " [OFFLINE: %d]",
> +                    number_cpus_present - number_cpus_to_display);
> +        fprintf(fp, "\n");
>
>         if (ACTIVE())
>                 get_xtime(&kt->date);
>
 What do you think about the following changes?
 diff --git a/kernel.c b/kernel.c
 index 8c2e0ca50482..2f451cc6056b 100644
 --- a/kernel.c
 +++ b/kernel.c
 @@ -5816,15 +5816,16 @@ display_sys_stats(void)
                                 pc->kvmdump_mapfile);
         }
 -       if (machine_type("PPC64"))
 -               fprintf(fp, "        CPUS: %d\n", get_cpus_to_display());
 -       else {
 -               fprintf(fp, "        CPUS: %d", kt->cpus);
 -               if (kt->cpus - get_cpus_to_display())
 -                       fprintf(fp, " [OFFLINE: %d]",
 -                               kt->cpus - get_cpus_to_display());
 -               fprintf(fp, "\n");
 -       }
 +        int number_cpus_to_display = get_cpus_to_display();
 +        int number_cpus_present = get_cpus_present();
 +        if (!number_cpus_present)
 +            number_cpus_present = kt->cpus;
 +
 +        fprintf(fp, "        CPUS: %d", number_cpus_present);
 +        if (number_cpus_present > number_cpus_to_display)
 +                fprintf(fp, " [OFFLINE: %d]",
 +                    number_cpus_present - number_cpus_to_display);
 +        fprintf(fp, "\n");
         if (ACTIVE())
                 get_xtime(&kt->date);
 Thanks
 Lianbo
 --
> 2.47.1
>