[Qemu-devel] [RFC 4/4] numa: check threads of the same core are on the same node

[Laurent Vivier <lvivier@redhat.com>]

A core cannot be split between two nodes.
To check if a thread of the same core has already been assigned to a node,
this patch reverses the numa topology checking order and exits if the
topology is not valid.

Update test/numa-test accordingly.

Fixes: 722387e78daf ("spapr: get numa node mapping from possible_cpus instead of numa_get_node_for_cpu()")
Cc: imammedo@redhat.com
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 hw/core/machine.c | 27 ++++++++++++++++++++++++---
 tests/numa-test.c |  4 ++--
 2 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index a2c29692b55e..c0a556b0dce7 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -602,6 +602,7 @@ void machine_set_cpu_numa_node(MachineState *machine,
     MachineClass *mc = MACHINE_GET_CLASS(machine);
     bool match = false;
     int i;
+    const CpuInstanceProperties *previous_props = NULL;
 
     if (!mc->possible_cpu_arch_ids) {
         error_setg(errp, "mapping of CPUs to NUMA node is not supported");
@@ -634,18 +635,38 @@ void machine_set_cpu_numa_node(MachineState *machine,
         }
 
         /* skip slots with explicit mismatch */
-        if (props->has_thread_id && props->thread_id != slot->props.thread_id) {
+        if (props->has_socket_id && props->socket_id != slot->props.socket_id) {
                 continue;
         }
 
-        if (props->has_core_id && props->core_id != slot->props.core_id) {
+        if (props->has_core_id) {
+            if (props->core_id != slot->props.core_id) {
                 continue;
+            }
+            if (slot->props.has_node_id) {
+                /* we have a node where our core is already assigned */
+                previous_props = &slot->props;
+            }
         }
 
-        if (props->has_socket_id && props->socket_id != slot->props.socket_id) {
+        if (props->has_thread_id && props->thread_id != slot->props.thread_id) {
                 continue;
         }
 
+        /* check current thread matches node of the thread of the same core */
+        if (previous_props && previous_props->has_node_id &&
+            previous_props->node_id != props->node_id) {
+            char *cpu_str = cpu_props_to_string(props);
+            char *node_str = cpu_props_to_string(previous_props);
+            error_setg(errp,  "Invalid node-id=%"PRIu64" of [%s]: core-id "
+                              "[%s] is already assigned to node-id %"PRIu64,
+                              props->node_id, cpu_str,
+                              node_str, previous_props->node_id);
+            g_free(cpu_str);
+            g_free(node_str);
+            return;
+        }
+
         /* reject assignment if slot is already assigned, for compatibility
          * of legacy cpu_index mapping with SPAPR core based mapping do not
          * error out if cpu thread and matched core have the same node-id */
diff --git a/tests/numa-test.c b/tests/numa-test.c
index 5280573fc992..a7c3c5b4dee8 100644
--- a/tests/numa-test.c
+++ b/tests/numa-test.c
@@ -112,7 +112,7 @@ static void pc_numa_cpu(const void *data)
         "-numa cpu,node-id=1,socket-id=0 "
         "-numa cpu,node-id=0,socket-id=1,core-id=0 "
         "-numa cpu,node-id=0,socket-id=1,core-id=1,thread-id=0 "
-        "-numa cpu,node-id=1,socket-id=1,core-id=1,thread-id=1");
+        "-numa cpu,node-id=0,socket-id=1,core-id=1,thread-id=1");
     qtest_start(cli);
     cpus = get_cpus(&resp);
     g_assert(cpus);
@@ -141,7 +141,7 @@ static void pc_numa_cpu(const void *data)
         } else if (socket == 1 && core == 1 && thread == 0) {
             g_assert_cmpint(node, ==, 0);
         } else if (socket == 1 && core == 1 && thread == 1) {
-            g_assert_cmpint(node, ==, 1);
+            g_assert_cmpint(node, ==, 0);
         } else {
             g_assert(false);
         }
-- 
2.20.1