[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCHv2 6/7] capabilities: Add additional data to the NUMA topology info



On 01/23/13 11:04, Daniel P. Berrange wrote:
On Tue, Jan 22, 2013 at 10:30:25PM +0100, Peter Krempa wrote:
This patch adds data gathering to the NUMA gathering files and adds
support for outputting the data. The test driver and xend driver need to
be adapted to fill sensible data to the structure.
---
  src/conf/capabilities.c | 31 ++++++++++++++++++++++-----
  src/nodeinfo.c          | 57 ++++++++++++++++++++++++++++++++++++++++++++++---
  src/test/test_driver.c  |  3 +++
  src/xen/xend_internal.c |  1 +
  4 files changed, 84 insertions(+), 8 deletions(-)

diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 14d3498..c9036f4 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -686,27 +686,47 @@ virCapabilitiesDefaultGuestEmulator(virCapsPtr caps,
      return NULL;
  }

-static void
+static int
  virCapabilitiesFormatNUMATopology(virBufferPtr xml,
                                    size_t ncells,
                                    virCapsHostNUMACellPtr *cells)
  {
      int i;
      int j;
+    char *siblings;

      virBufferAddLit(xml, "    <topology>\n");
      virBufferAsprintf(xml, "      <cells num='%zu'>\n", ncells);
      for (i = 0; i < ncells; i++) {
          virBufferAsprintf(xml, "        <cell id='%d'>\n", cells[i]->num);
          virBufferAsprintf(xml, "          <cpus num='%d'>\n", cells[i]->ncpus);
-        for (j = 0; j < cells[i]->ncpus; j++)
-            virBufferAsprintf(xml, "            <cpu id='%d'/>\n",
+        for (j = 0; j < cells[i]->ncpus; j++) {
+            virBufferAsprintf(xml, "            <cpu id='%d'",
                                cells[i]->cpus[j].id);
+
+            if (cells[i]->cpus[j].siblings) {
+                if (!(siblings = virBitmapFormat(cells[i]->cpus[j].siblings))) {
+                    virReportOOMError();
+                    return -1;
+                }
+
+                virBufferAsprintf(xml,
+                                  " socket_id='%d' core_id='%d' siblings='%s'",
+                                  cells[i]->cpus[j].socket_id,
+                                  cells[i]->cpus[j].core_id,
+                                  siblings);
+                VIR_FREE(siblings);
+            }
+            virBufferAddLit(xml, "/>\n");
+        }
+
          virBufferAddLit(xml, "          </cpus>\n");
          virBufferAddLit(xml, "        </cell>\n");
      }
      virBufferAddLit(xml, "      </cells>\n");
      virBufferAddLit(xml, "    </topology>\n");
+
+    return 0;
  }

  /**
@@ -782,9 +802,10 @@ virCapabilitiesFormatXML(virCapsPtr caps)
          virBufferAddLit(&xml, "    </migration_features>\n");
      }

-    if (caps->host.nnumaCell)
+    if (caps->host.nnumaCell &&
          virCapabilitiesFormatNUMATopology(&xml, caps->host.nnumaCell,
-                                          caps->host.numaCell);
+                                          caps->host.numaCell) < 0)
+        return NULL;

      for (i = 0; i < caps->host.nsecModels; i++) {
          virBufferAddLit(&xml, "    <secmodel>\n");
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 5febcfb..dc6771a 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -79,9 +79,11 @@ freebsdNodeGetCPUCount(void)
  #ifdef __linux__
  # define CPUINFO_PATH "/proc/cpuinfo"
  # define SYSFS_SYSTEM_PATH "/sys/devices/system"
+# define SYSFS_CPU_PATH SYSFS_SYSTEM_PATH"/cpu"
  # define PROCSTAT_PATH "/proc/stat"
  # define MEMINFO_PATH "/proc/meminfo"
  # define SYSFS_MEMORY_SHARED_PATH "/sys/kernel/mm/ksm"
+# define SYSFS_THREAD_SIBLINGS_LIST_LENGHT_MAX 1024

  # define LINUX_NB_CPU_STATS 4
  # define LINUX_NB_MEMORY_STATS_ALL 4
@@ -1473,6 +1475,52 @@ cleanup:
  # define MASK_CPU_ISSET(mask, cpu) \
    (((mask)[((cpu) / n_bits(*(mask)))] >> ((cpu) % n_bits(*(mask)))) & 1)

+static virBitmapPtr
+virNodeGetSiblingsList(const char *dir, int cpu_id)
+{
+    char *path = NULL;
+    char *buf = NULL;
+    virBitmapPtr ret = NULL;
+
+    if (virAsprintf(&path, "%s/cpu%u/topology/thread_siblings_list",
+                    dir, cpu_id) < 0) {
+        virReportOOMError();
+        goto cleanup;
+    }
+
+    if (virFileReadAll(path, SYSFS_THREAD_SIBLINGS_LIST_LENGHT_MAX, &buf) < 0)
+        goto cleanup;
+
+    if (virBitmapParse(buf, ',', &ret, NUMA_MAX_N_CPUS) < 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("Failed to parse thread siblings"));
+        goto cleanup;
+    }
+
+cleanup:
+    VIR_FREE(buf);
+    VIR_FREE(path);
+    return ret;
+}
+
+static int
+virNodeCapsFillCPUInfo(int cpu_id, virCapsHostNUMACellCPUPtr cpu)
+{
+    cpu->id = cpu_id;
+    cpu->socket_id = virNodeGetCpuValue(SYSFS_CPU_PATH, cpu_id,
+                                        "topology/physical_package_id", -1);
+    cpu->core_id = virNodeGetCpuValue(SYSFS_CPU_PATH, cpu_id,
+                                      "topology/core_id", -1);
+
+    if (cpu->socket_id == -1 || cpu->core_id == -1)
+        return 0;
+
+    if (!(cpu->siblings = virNodeGetSiblingsList(SYSFS_CPU_PATH, cpu_id)))
+        return -1;
+
+    return 0;
+}
+
  int
  nodeCapsInitNUMA(virCapsPtr caps)
  {
@@ -1516,9 +1564,12 @@ nodeCapsInitNUMA(virCapsPtr caps)
          if (VIR_ALLOC_N(cpus, ncpus) < 0)
              goto cleanup;

-        for (ncpus = 0, i = 0 ; i < max_n_cpus ; i++)
-            if (MASK_CPU_ISSET(mask, i))
-                cpus[ncpus++].id = i;
+        for (ncpus = 0, i = 0 ; i < max_n_cpus ; i++) {
+            if (MASK_CPU_ISSET(mask, i)) {
+                if (virNodeCapsFillCPUInfo(i, cpus + ncpus++) < 0)
+                    goto cleanup;
+            }
+        }

          if (virCapabilitiesAddHostNUMACell(caps, n, ncpus, cpus) < 0)
              goto cleanup;
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 6909fa4..038f627 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -559,6 +559,9 @@ static int testOpenDefault(virConnectPtr conn) {
      }
      for (u = 0 ; u < 16 ; u++) {
          privconn->cells[u % 2].cpus[(u / 2)].id = u;
+        privconn->cells[u % 2].cpus[(u / 2)].socket_id = -1;
+        privconn->cells[u % 2].cpus[(u / 2)].core_id = -1;
+        privconn->cells[u % 2].cpus[(u / 2)].siblings = NULL;
      }

This is wrong because these fields are unsigned int.

Hm, yeah, I forgot to update this. Anyways, the data doesn't pose problem here as the output isn't enhanced until siblings is non-NULL.

This code gets fixed in 7/7.


diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index 57d8325..434f558 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -1161,6 +1161,7 @@ sexpr_to_xend_topology(const struct sexpr *root,
              ignore_value(virBitmapGetBit(cpuset, cpu, &used));
              if (used) {
                  cpuInfo[n].id = cpu;
+                cpuInfo[n].siblings = NULL;

As mentioned before, this should be initializing based on the nodeinfo.
Here you've allowed socket_id + core_id to all initialize to 0 which
is wrong.

Also here, the siblings are NULL so the new output isn't used at all. I added the condition so that the new code could be avoided until I prepare means to test the XEN support.


Daniel



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]