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

Re: [XEN RFC PATCH 27/40] xen/arm: build CPU NUMA node map while creating cpu_logical_map



Hi Wei,

On 11/08/2021 11:24, Wei Chen wrote:
Sometimes, CPU logical ID maybe different with physical CPU ID.
Xen is using CPU logial ID for runtime usage, so we should use
CPU logical ID to create map between NUMA node and CPU.

This commit message gives the impression that you are trying to fix a bug. However, what you are explaining is the reason why the code will use the logical ID rather than physical ID.

I think the commit message should explain what the patch is doing. You can then add an explanation why you are using the CPU logical ID. Something like "Note we storing the CPU logical ID because...".



Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
---
  xen/arch/arm/smpboot.c | 31 ++++++++++++++++++++++++++++++-
  1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
index aa78958c07..dd5a45bffc 100644
--- a/xen/arch/arm/smpboot.c
+++ b/xen/arch/arm/smpboot.c
@@ -121,7 +121,12 @@ static void __init dt_smp_init_cpus(void)
      {
          [0 ... NR_CPUS - 1] = MPIDR_INVALID
      };
+    static nodeid_t node_map[NR_CPUS] __initdata =
+    {
+        [0 ... NR_CPUS - 1] = NUMA_NO_NODE
+    };
      bool bootcpu_valid = false;
+    uint32_t nid = 0;
      int rc;
mpidr = boot_cpu_data.mpidr.bits & MPIDR_HWID_MASK;
@@ -172,6 +177,26 @@ static void __init dt_smp_init_cpus(void)
              continue;
          }
+#ifdef CONFIG_DEVICE_TREE_NUMA
+        /*
+         *  When CONFIG_DEVICE_TREE_NUMA is set, try to fetch numa infomation
+         * from CPU dts node, otherwise the nid is always 0.
+         */
+        if ( !dt_property_read_u32(cpu, "numa-node-id", &nid) )

You can avoid the #ifdef by writing:

if ( IS_ENABLED(CONFIG_DEVICE_TREE_NUMA) && ... )

However, I would using CONFIG_NUMA because this code is already DT specific. So we can shorten the name a bit.

+        {
+            printk(XENLOG_WARNING
+                "cpu[%d] dts path: %s: doesn't have numa infomation!\n",

s/information/information/

+                cpuidx, dt_node_full_name(cpu));
+            /*
+             * The the early stage of NUMA initialization, when Xen found any

s/The/During/?

+             * CPU dts node doesn't have numa-node-id info, the NUMA will be
+             * treated as off, all CPU will be set to a FAKE node 0. So if we
+             * get numa-node-id failed here, we should set nid to 0.
+             */
+            nid = 0;
+        }
+#endif
+
          /*
           * 8 MSBs must be set to 0 in the DT since the reg property
           * defines the MPIDR[23:0]
@@ -231,9 +256,12 @@ static void __init dt_smp_init_cpus(void)
          {
              printk("cpu%d init failed (hwid %"PRIregister"): %d\n", i, hwid, 
rc);
              tmp_map[i] = MPIDR_INVALID;
+            node_map[i] = NUMA_NO_NODE;
          }
-        else
+        else {
              tmp_map[i] = hwid;
+            node_map[i] = nid;
+        }
      }
if ( !bootcpu_valid )
@@ -249,6 +277,7 @@ static void __init dt_smp_init_cpus(void)
              continue;
          cpumask_set_cpu(i, &cpu_possible_map);
          cpu_logical_map(i) = tmp_map[i];
+        numa_set_node(i, node_map[i]);
      }
  }
   >

Cheers,

--
Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.