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 <liji...@redhat.com> wrote:
> Thank you for the fix, Lucas.
>
> On Sat, Jan 4, 2025 at 5:54 AM <devel-requ...@lists.crash-utility.osci.io>
> wrote:
>
>> Date: Fri, 3 Jan 2025 16:52:18 -0500
>> From: soak...@redhat.com
>> Subject: [Crash-utility] [PATCH] Fix misleading CPU count in
>> display_sys_stats()
>> To: devel@lists.crash-utility.osci.io
>> Cc: Lucas Oakley <soak...@redhat.com>
>> Message-ID: <20250103215218.712496-1-soak...@redhat.com>
>> Content-Type: text/plain; charset="US-ASCII"; x-default=true
>>
>> From: Lucas Oakley <soak...@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 <soak...@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
>>
>
--
Crash-utility mailing list -- devel@lists.crash-utility.osci.io
To unsubscribe send an email to devel-le...@lists.crash-utility.osci.io
https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/
Contribution Guidelines: https://github.com/crash-utility/crash/wiki
