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

Re: [XEN RFC PATCH 13/40] xen/arm: introduce numa_set_node for Arm

On 25/08/2021 13:07, Wei Chen wrote:
Hi Julien,

Hi Wei,

-----Original Message-----
From: Julien Grall <julien@xxxxxxx>
Sent: 2021年8月25日 18:37
To: Wei Chen <Wei.Chen@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx;
sstabellini@xxxxxxxxxx; jbeulich@xxxxxxxx
Cc: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
Subject: Re: [XEN RFC PATCH 13/40] xen/arm: introduce numa_set_node for

Hi Wei,

On 11/08/2021 11:23, Wei Chen wrote:
This API is used to set one CPU to a NUMA node. If the system
configure NUMA off or system initialize NUMA failed, the
online NUMA node would set to only node#0. This will be done
in following patches. When NUMA turn off or init failed,
node_online_map will be cleared and set node#0 online. So we
use node_online_map to prevent to set a CPU to an offline node.

IHMO numa_set_node() should behave exactly the same way on x86 and Arm
because this is going to be used by the common code.

  From the commit message, I don't quite understand why the check is
necessary on Arm but not on x86. Can you clarify it?

Yes, in patch#27, in smpboot.c, dt_smp_init_cpus function.
We will parse CPU numa-node-id from dtb CPU node. If we get
a valid node ID for one CPU, we will invoke numa_set_node to
create CPU-NODE map. But in our testing, we found when NUMA
init failed, numa_set_node still can set CPU to a offline
or invalid NODE. So we're using node_online_map to prevent
this behavior. Otherwise we have to check node_online_map
everywhere before we call numa_set_node.

What do you mean by invalid NODE? Is it 0xFF (NUMA_NO_NODE)?

x86 actually is doing the same way, but it handles node_online_map
check out of numa_set_node:


I think numa_set_node() will want to be implemented in common code.

See my above comment. If x86 is ok, I think yes, we can do it
in common code.

... on x86, this check is performed outside of numa_set_node() for one caller whereas on Arm you are adding it in numa_set_node().

For example, numa_set_node() can be called with NUMA_NO_NODE. On x86, we would set cpu_to_node[] to that value. However, if I am not mistaken, on Arm we would set the value to 0.

This will change the behavior of users to cpu_to_node() later on (such as XEN_SYSCTL_cputopoinfo).

NUMA is not something architecture specific, so I dont't think the implementation should differ here.

In this case, I think numa_set_node() shouldn't check if the node is valid. Instead, the caller should take care of it if it is important.


Julien Grall



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