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

Re: [Xen-devel] [RFC PATCH v2 09/25] ARM: NUMA: Add existing ARM numa code under CONFIG_NUMA

On 05/09/2017 08:14 AM, Vijay Kilari wrote:
On Mon, May 8, 2017 at 9:28 PM, Julien Grall <julien.grall@xxxxxxx> wrote:
Hi Vijay,

On 28/03/17 16:53, vijay.kilari@xxxxxxxxx wrote:

From: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxx>

Right now CONFIG_NUMA is not enabled for ARM and
existing code in asm-arm/numa.h is for !CONFIG_NUMA.
Hence put this code under #ifndef CONFIG_NUMA.

This help to make this changes work when CONFIG_NUMA
is not enabled.

But you always turn NUMA on by default (see patch #24) and there is no
possibility to turn off NUMA.

Yes at the end of the series we enable NUMA by default.
But the the intermittent patches of this patch series fails to compile.

So for helping this series, you add code that will get rotten???

I don't like this idea at all, we should avoid to add code in Xen that will not be used.

Also define NODES_SHIFT macro for ARM to value 2.
This limits number of NUMA nodes supported to 4.
There is not hard restrictions on this value set to 2.

Again, why only 2 when x86 is supporting 6?

Furthermore, this is not related to this patch itself and should be part of
separate patch.

Lastly, why don't you move that to a Kconfig allowing the user to configure
the number of Nodes?


Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@xxxxxxxxxx>
 xen/include/asm-arm/numa.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/xen/include/asm-arm/numa.h b/xen/include/asm-arm/numa.h
index 53f99af..924bfc0 100644
--- a/xen/include/asm-arm/numa.h
+++ b/xen/include/asm-arm/numa.h
@@ -3,6 +3,10 @@

 typedef uint8_t nodeid_t;

+/* Limit number of NUMA nodes supported to 4 */
+#define NODES_SHIFT 2

Why this is not covered by CONFIG_NUMA?

The below define is used in generic code irrespective of CONFIG_NUMA

#define MAX_NUMNODES    (1 << NODES_SHIFT)

As you may have noticed NODES_SHIFT currently does not exist on ARM and we are still able to compile the generic code. So why do you need to do it unconditionally?

If you look at the code, xen/numa.h will define NODES_SHIFT to 0 if it has not previously defined. So I still don't see any reason on what you are doing.


Julien Grall

Xen-devel mailing list



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