On 30/07/2019 10:44, Jan Beulich wrote:
On 29.07.2019 14:12, Andrew Cooper wrote:
There is no need to use runtime variable-length clearing when MAX_NUMNODES is
known to the compiler. Drop these functions and use the initialisers instead.
The only slight concern I have with this is that it further locks
down the maximum remaining to be a compile time constant. But this
is not an objection, just a remark.
The maximum number of nodes I'm aware of at all is 10, and we currently default to 64.
I don't think it is likely that we'll get to a point where a runtime nodesize is a realistic consideration that we would want to take.
@@ -67,7 +65,34 @@ typedef struct { DECLARE_BITMAP(bits, MAX_NUMNODES); } nodemask_t;
#define nodemask_bits(src) ((src)->bits)
-extern nodemask_t _unused_nodemask_arg_;
+#define NODEMASK_LAST_WORD BITMAP_LAST_WORD_MASK(MAX_NUMNODES)
+
+#define NODEMASK_NONE \
+((nodemask_t) {{ \
+ [0 ... BITS_TO_LONGS(MAX_NUMNODES) - 1] = 0 \
+}})
+
+#if MAX_NUMNODES <= BITS_PER_LONG
+
+#define NODEMASK_ALL ((nodemask_t) {{ NODEMASK_LAST_WORD }})
+#define NODEMASK_OF(node) ((nodemask_t) {{ 1UL << (node) }})
+
+#else /* MAX_NUMNODES > BITS_PER_LONG */
+
+#define NODEMASK_ALL \
+((nodemask_t) {{ \
+ [0 ... BITS_TO_LONGS(MAX_NUMNODES) - 2] = ~0UL, \
+ [BITS_TO_LONGS(MAX_NUMNODES) - 1] = NODEMASK_LAST_WORD \
+}})
+
+#define NODEMASK_OF(node) \
+({ \
+ nodemask_t m = NODES_NONE; \
+ m.bits[(node) / BITS_PER_LONG] = 1UL << ((node) % BITS_PER_LONG); \
I think you will want to avoid the double evaluation of "node"
here. With this taken care of
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
I'm afraid this is a bit more complicated after I spotted another opencoding of NODEMASK_OF().
diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
index 00b64c3322..24af9bc471 100644
--- a/xen/arch/arm/smpboot.c
+++ b/xen/arch/arm/smpboot.c
@@ -46,7 +46,7 @@ struct cpuinfo_arm cpu_data[NR_CPUS];
register_t __cpu_logical_map[NR_CPUS] = { [0 ... NR_CPUS-1] = MPIDR_INVALID };
/* Fake one node for now. See also include/asm-arm/numa.h */
-nodemask_t __read_mostly node_online_map = { { [0] = 1UL } };
+nodemask_t __read_mostly node_online_map = NODEMASK_OF(0);
/* Xen stack for bringing up the first CPU. */
static unsigned char __initdata cpu0_boot_stack[STACK_SIZE]
diff --git a/xen/arch/x86/numa.c b/xen/arch/x86/numa.c
index 7473f83b7b..9a55c013e5 100644
--- a/xen/arch/x86/numa.c
+++ b/xen/arch/x86/numa.c
@@ -47,7 +47,7 @@ nodeid_t apicid_to_node[MAX_LOCAL_APIC] = {
};
cpumask_t node_to_cpumask[MAX_NUMNODES] __read_mostly;
-nodemask_t __read_mostly node_online_map = { { [0] = 1UL } };
+nodemask_t __read_mostly node_online_map = NODEMASK_OF(0);
bool numa_off;
s8 acpi_numa = 0;
diff --git a/xen/include/xen/nodemask.h b/xen/include/xen/nodemask.h
index 9933fec5c4..c474dca3f0 100644
--- a/xen/include/xen/nodemask.h
+++ b/xen/include/xen/nodemask.h
@@ -86,11 +86,9 @@ typedef struct { DECLARE_BITMAP(bits, MAX_NUMNODES); } nodemask_t;
}})
#define NODEMASK_OF(node) \
-({ \
- nodemask_t m = NODES_NONE; \
- m.bits[(node) / BITS_PER_LONG] = 1UL << ((node) % BITS_PER_LONG); \
- m; \
-})
+((nodemask_t) {{ \
+ [(node) / BITS_PER_LONG] = 1UL << ((node) % BITS_PER_LONG) \
+}})
#endif /* MAX_NUMNODES */
and to be used as a static initialiser, NODEMASK_OF() needs to be an ICE and can't use ({}) .
I don't see a way to avoid expanding node twice, but given that its wrapper is in ALL_CAPS and obviously a macro.
Furthermore, experimenting with a deliberate attempt to provoke this, I got
numa.c: In function ‘numa_initmem_init’:
/local/xen.git/xen/include/xen/nodemask.h:90:10: error: nonconstant array index in initializer
[(node) / BITS_PER_LONG] = 1UL << ((node) % BITS_PER_LONG) \
^
numa.c:274:23: note: in expansion of macro ‘NODEMASK_OF’
node_online_map = NODEMASK_OF(foo++);
^~~~~~~~~~~
/local/xen.git/xen/include/xen/nodemask.h:90:10: note: (near initialization for ‘(anonymous).bits’)
[(node) / BITS_PER_LONG] = 1UL << ((node) % BITS_PER_LONG) \
^
numa.c:274:23: note: in expansion of macro ‘NODEMASK_OF’
node_online_map = NODEMASK_OF(foo++);
^~~~~~~~~~~
from GCC 6.3, which I think covers everything we need, and will prevent side effects from double expansion in practice.
~Andrew
