[Crash-utility] Re: [PATCH] Fix misleading CPU count in display_sys_stats()

[Lucas Oakley <soakley@xxxxxxxxxx>]

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@xxxxxxxxxx> wrote:

Thank you for the fix, Lucas.

On Sat, Jan 4, 2025 at 5:54 AM <devel-request@xxxxxxxxxxxxxxxxxxxxxxxxxxx> wrote:

Date: Fri,  3 Jan 2025 16:52:18 -0500
From: soakley@xxxxxxxxxx
Subject: [PATCH] Fix misleading CPU count in
        display_sys_stats()
To: devel@xxxxxxxxxxxxxxxxxxxxxxxxxxx
Cc: Lucas Oakley <soakley@xxxxxxxxxx>
Message-ID: <20250103215218.712496-1-soakley@xxxxxxxxxx>
Content-Type: text/plain; charset="US-ASCII"; x-default=true

From: Lucas Oakley <soakley@xxxxxxxxxx>

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@xxxxxxxxxx>
---
 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@xxxxxxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxxxxxx
https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/
Contribution Guidelines: https://github.com/crash-utility/crash/wiki