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 <[email protected]>
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
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
