Get thread and socket information in virsh nodeinfo.

The current code for "nodeinfo" is pretty naive
about socket and thread information.  To determine the
sockets, it just takes the number of cpus and divides
by the number of cores.  For the thread count, it always
sets it to 1.  With more recent Intel machines, however,
hyperthreading is again an option, meaning that these
heuristics no longer work and give bogus numbers.  This
patch goes through /sys to get the additional
information so we properly report it.

Note that I had to edit the tests not to report on
socket and thread counts, since these are determined
dynamically now.

v2: As pointed out by Eric Blake, gnulib provides
    count-one-bits (which is LGPLv2+).  Use it instead
    of a hand-coded popcnt.

Signed-off-by: Chris Lalancette <clalance@redhat.com>

diff --git a/bootstrap.conf b/bootstrap.conf
index 58ef2abd04e641e545f8fbd79738e89a3f084138..157092f9cb649d42bb70b0726a30470cb4fa69e6 100644 (file)
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -25,6 +25,7 @@ c-ctype
 canonicalize-lgpl
 close
 connect
+count-one-bits
 dirname-lgpl
 getaddrinfo
 gethostname

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 4648be1f7d8213299aa0ea2e9b56988d953eb502..2dab5b29c87dbc680916e3cd0cf736d5197f5ef5 100644 (file)
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -28,6 +28,7 @@
 #include <stdlib.h>
 #include <stdint.h>
 #include <errno.h>
+#include <dirent.h>
 
 #if HAVE_NUMACTL
 # define NUMA_VERSION1_COMPATIBILITY 1
@@ -45,6 +46,7 @@
 #include "util.h"
 #include "logging.h"
 #include "virterror_internal.h"
+#include "count-one-bits.h"
 
 
 #define VIR_FROM_THIS VIR_FROM_NONE
@@ -55,22 +57,109 @@
 
 #ifdef __linux__
 #define CPUINFO_PATH "/proc/cpuinfo"
+#define CPU_SYS_PATH "/sys/devices/system/cpu"
 
-/* NB, these are not static as we need to call them from testsuite */
+/* NB, this is not static as we need to call it from the testsuite */
 int linuxNodeInfoCPUPopulate(virConnectPtr conn, FILE *cpuinfo,
                              virNodeInfoPtr nodeinfo);
 
-int linuxNodeInfoCPUPopulate(virConnectPtr conn, FILE *cpuinfo, virNodeInfoPtr nodeinfo) {
+static unsigned long count_thread_siblings(int cpu)
+{
+    unsigned long ret = 0;
+    char *path = NULL;
+    FILE *pathfp = NULL;
+    char str[1024];
+    int i;
+
+    if (virAsprintf(&path, "%s/cpu%d/topology/thread_siblings", CPU_SYS_PATH,
+                    cpu) < 0) {
+        virReportOOMError();
+        return 0;
+    }
+
+    pathfp = fopen(path, "r");
+    if (pathfp == NULL) {
+        virReportSystemError(errno, _("cannot open %s"), path);
+        VIR_FREE(path);
+        return 0;
+    }
+
+    if (fgets(str, sizeof(str), pathfp) == NULL) {
+        virReportSystemError(errno, _("cannot read from %s"), path);
+        goto cleanup;
+    }
+
+    i = 0;
+    while (str[i] != '\0') {
+        if (str[i] != '\n' && str[i] != ',')
+            ret += count_one_bits(str[i] - '0');
+        i++;
+    }
+
+cleanup:
+    fclose(pathfp);
+    VIR_FREE(path);
+
+    return ret;
+}
+
+static int parse_socket(int cpu)
+{
+    char *path = NULL;
+    FILE *pathfp;
+    char socket_str[1024];
+    char *tmp;
+    int socket;
+
+    if (virAsprintf(&path, "%s/cpu%d/topology/physical_package_id",
+                    CPU_SYS_PATH, cpu) < 0) {
+        virReportOOMError();
+        return -1;
+    }
+
+    pathfp = fopen(path, "r");
+    if (pathfp == NULL) {
+        virReportSystemError(errno, _("cannot open %s"), path);
+        goto cleanup;
+    }
+
+    if (fgets(socket_str, sizeof(socket_str), pathfp) == NULL) {
+        virReportSystemError(errno, _("cannot read from %s"), path);
+        goto cleanup;
+    }
+    if (virStrToLong_i(socket_str, &tmp, 10, &socket) < 0) {
+        nodeReportError(NULL, VIR_ERR_INTERNAL_ERROR,
+                        _("could not convert '%s' to an integer"),
+                        socket_str);
+        goto cleanup;
+    }
+
+cleanup:
+    fclose(pathfp);
+    VIR_FREE(path);
+
+    return socket;
+}
+
+int linuxNodeInfoCPUPopulate(virConnectPtr conn, FILE *cpuinfo,
+                             virNodeInfoPtr nodeinfo)
+{
     char line[1024];
+    DIR *cpudir = NULL;
+    struct dirent *cpudirent = NULL;
+    int cpu;
+    unsigned long cur_threads;
+    int socket;
+    unsigned long long socket_mask = 0;
 
     nodeinfo->cpus = 0;
     nodeinfo->mhz = 0;
-    nodeinfo->nodes = nodeinfo->sockets = nodeinfo->cores = nodeinfo->threads = 1;
+    nodeinfo->nodes = nodeinfo->cores = 1;
 
     /* NB: It is impossible to fill our nodes, since cpuinfo
      * has not knowledge of NUMA nodes */
 
-    /* XXX hyperthreads */
+    /* NOTE: hyperthreads are ignored here; they are parsed out of /sys */
     while (fgets(line, sizeof(line), cpuinfo) != NULL) {
         char *buf = line;
         if (STRPREFIX(buf, "processor")) { /* aka a single logical CPU */
@@ -122,12 +211,38 @@ int linuxNodeInfoCPUPopulate(virConnectPtr conn, FILE *cpuinfo, virNodeInfoPtr n
         return -1;
     }
 
-    /*
-     * Can't reliably count sockets from proc metadata, so
-     * infer it based on total CPUs vs cores.
-     * XXX hyperthreads
+    /* OK, we've parsed what we can out of /proc/cpuinfo.  Get the socket
+     * and thread information from /sys
      */
-    nodeinfo->sockets = nodeinfo->cpus / nodeinfo->cores;
+    cpudir = opendir(CPU_SYS_PATH);
+    if (cpudir == NULL) {
+        virReportSystemError(errno, _("cannot opendir %s"), CPU_SYS_PATH);
+        return -1;
+    }
+    while ((cpudirent = readdir(cpudir))) {
+        if (sscanf(cpudirent->d_name, "cpu%d", &cpu) != 1)
+            continue;
+
+        socket = parse_socket(cpu);
+        if (socket < 0) {
+            closedir(cpudir);
+            return -1;
+        }
+        if (!(socket_mask & (1 << socket))) {
+            socket_mask |= (1 << socket);
+            nodeinfo->sockets++;
+        }
+
+        cur_threads = count_thread_siblings(cpu);
+        if (cur_threads == 0) {
+            closedir(cpudir);
+            return -1;
+        }
+        if (cur_threads > nodeinfo->threads)
+            nodeinfo->threads = cur_threads;
+    }
+
+    closedir(cpudir);
 
     return 0;
 }

diff --git a/tests/nodeinfodata/linux-nodeinfo-1.txt b/tests/nodeinfodata/linux-nodeinfo-1.txt
index e52e20ab72fe9cec44472cba38bddbdb393608d7..09e29464b2cc7b541340faffa2b6cad79c24042c 100644 (file)
--- a/tests/nodeinfodata/linux-nodeinfo-1.txt
+++ b/tests/nodeinfodata/linux-nodeinfo-1.txt
@@ -1 +1 @@
-CPUs: 2, MHz: 2800, Nodes: 1, Sockets: 1, Cores: 2, Threads: 1
+CPUs: 2, MHz: 2800, Nodes: 1, Cores: 2

diff --git a/tests/nodeinfodata/linux-nodeinfo-2.txt b/tests/nodeinfodata/linux-nodeinfo-2.txt
index 12e819ba7d9b99600c1a59b19b439eec5ec07e6b..e4eea943d2e7450951d0398f5abc16a17b8bfec7 100644 (file)
--- a/tests/nodeinfodata/linux-nodeinfo-2.txt
+++ b/tests/nodeinfodata/linux-nodeinfo-2.txt
@@ -1 +1 @@
-CPUs: 2, MHz: 2211, Nodes: 1, Sockets: 1, Cores: 2, Threads: 1
+CPUs: 2, MHz: 2211, Nodes: 1, Cores: 2

diff --git a/tests/nodeinfodata/linux-nodeinfo-3.txt b/tests/nodeinfodata/linux-nodeinfo-3.txt
index d285781507b665b84ad5c10ca5d63a5733788de6..17d4d8e213ee712641fad641e84d6e2a7f5c2d0f 100644 (file)
--- a/tests/nodeinfodata/linux-nodeinfo-3.txt
+++ b/tests/nodeinfodata/linux-nodeinfo-3.txt
@@ -1 +1 @@
-CPUs: 4, MHz: 1595, Nodes: 1, Sockets: 2, Cores: 2, Threads: 1
+CPUs: 4, MHz: 1595, Nodes: 1, Cores: 2

diff --git a/tests/nodeinfodata/linux-nodeinfo-4.txt b/tests/nodeinfodata/linux-nodeinfo-4.txt
index 991d4f93e08bad485ddff947b339fde911ff8f1b..5a5c919ae03d1e6405752eaea9f41b467fecc291 100644 (file)
--- a/tests/nodeinfodata/linux-nodeinfo-4.txt
+++ b/tests/nodeinfodata/linux-nodeinfo-4.txt
@@ -1 +1 @@
-CPUs: 4, MHz: 1000, Nodes: 1, Sockets: 1, Cores: 4, Threads: 1
+CPUs: 4, MHz: 1000, Nodes: 1, Cores: 4

diff --git a/tests/nodeinfodata/linux-nodeinfo-5.txt b/tests/nodeinfodata/linux-nodeinfo-5.txt
index dce7ada3e44943490ecfba81f3b29dbf0f1f21f0..54abb5d39d5f37f1a16db91b071ddb763c106902 100644 (file)
--- a/tests/nodeinfodata/linux-nodeinfo-5.txt
+++ b/tests/nodeinfodata/linux-nodeinfo-5.txt
@@ -1 +1 @@
-CPUs: 4, MHz: 2814, Nodes: 1, Sockets: 2, Cores: 2, Threads: 1
+CPUs: 4, MHz: 2814, Nodes: 1, Cores: 2

diff --git a/tests/nodeinfodata/linux-nodeinfo-6.txt b/tests/nodeinfodata/linux-nodeinfo-6.txt
index 75cdaa97222cb96d59589b02fdaa5b2968130d40..f89e35ee6ed274240140d0213ea3506ed6115dea 100644 (file)
--- a/tests/nodeinfodata/linux-nodeinfo-6.txt
+++ b/tests/nodeinfodata/linux-nodeinfo-6.txt
@@ -1 +1 @@
-CPUs: 4, MHz: 1000, Nodes: 1, Sockets: 2, Cores: 2, Threads: 1
+CPUs: 4, MHz: 1000, Nodes: 1, Cores: 2

diff --git a/tests/nodeinfotest.c b/tests/nodeinfotest.c
index 4cb248acfbbc11a96f37bb5901562b9d76ed8fb1..b3b91ad3b8287e550a5a1a9613f213d7fea285f9 100644 (file)
--- a/tests/nodeinfotest.c
+++ b/tests/nodeinfotest.c
@@ -47,9 +47,8 @@ static int linuxTestCompareFiles(const char *cpuinfofile, const char *outputfile
     fclose(cpuinfo);
 
     snprintf(actualData, MAX_FILE,
-             "CPUs: %u, MHz: %u, Nodes: %u, Sockets: %u, Cores: %u, Threads: %u\n",
-             nodeinfo.cpus, nodeinfo.mhz, nodeinfo.nodes, nodeinfo.sockets,
-             nodeinfo.cores, nodeinfo.threads);
+             "CPUs: %u, MHz: %u, Nodes: %u, Cores: %u\n",
+             nodeinfo.cpus, nodeinfo.mhz, nodeinfo.nodes, nodeinfo.cores);
 
     if (STRNEQ(actualData, expectData)) {
         if (getenv("DEBUG_TESTS")) {