Re: [PATCH v2 9/9] xen: introduce a Kconfig option to configure NUMA nodes number

[Jan Beulich <jbeulich@xxxxxxxx>]

On 14.07.2022 12:14, Wei Chen wrote:
> Hi Jan,
> 
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@xxxxxxxx>
>> Sent: 2022年7月12日 22:34
>> To: Wei Chen <Wei.Chen@xxxxxxx>
>> Cc: nd <nd@xxxxxxx>; Andrew Cooper <andrew.cooper3@xxxxxxxxxx>; George
>> Dunlap <george.dunlap@xxxxxxxxxx>; Julien Grall <julien@xxxxxxx>; Stefano
>> Stabellini <sstabellini@xxxxxxxxxx>; Wei Liu <wl@xxxxxxx>; Roger Pau Monné
>> <roger.pau@xxxxxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx
>> Subject: Re: [PATCH v2 9/9] xen: introduce a Kconfig option to configure
>> NUMA nodes number
>>
>> On 08.07.2022 16:54, Wei Chen wrote:
>>> --- a/xen/arch/Kconfig
>>> +++ b/xen/arch/Kconfig
>>> @@ -17,3 +17,14 @@ config NR_CPUS
>>>       For CPU cores which support Simultaneous Multi-Threading or
>> similar
>>>       technologies, this the number of logical threads which Xen will
>>>       support.
>>> +
>>> +config NR_NUMA_NODES
>>> +   int "Maximum number of NUMA nodes supported"
>>> +   range 1 255
>>> +   default "64"
>>> +   depends on NUMA
>>
>> Does 1 make sense? That's not going to be NUMA then, I would say.
>>
> 
> Ok, we need at least 2 nodes to be a real NUMA.
> 
>> Does the value being (perhaps far) larger than NR_CPUS make sense?
>>
> 
> Arm has 128 default NR_CPUS (except some platforms) and x86 has 256.
> So I am not very clear about your comments about far larger? As my
> Understanding, one node has 2 or 4 cores are very common in a NUMA
> System.

The defaults are fine. But does it make sense to have 255 nodes when
just 32 CPUs were selected? I'm afraid kconfig is going to get in the
way, but I think I'd like the upper bound to be min(NR_CPUS, 255).

>> Why does the range end at a not-power-of-2 value? (I was actually
>> going to suggest to have a shift value specified here, until
>> spotting that NODES_SHIFT isn't used anywhere else, and hence
>> you're rightfully deleting it.)
>>
> 
> I think we have discussed about the 255 in v1. Because Xen is using
> u8 as nodeid_t, so 255 may be a upper bound.

Indeed, but that's something you could have mentioned in the commit
message as justification. But you could also have opted to make the
upper bound 128. Let me ask you: Are you aware of systems with more
than a dozen or so nodes, that Xen can in principle run on?

> And if use a shift value, from a user perspective, I don't like it.
> It needs to be converted, not intuitive enough. It also limits my
> input range, even though my numerical values are reasonable. Yes,
> if a machine has 15 node, we can ask them to input 16, but why not
> let the users decide? instead of being forced to enter 16 by the program?

At least x86 Linux also wants this specified as a shift value, so
people may actually be (more) used to that. Plus non-pwer-of-2
values may yield more complex calculations when the compiler
generates code. Think of a two-dimensional distance table, for
example.

Jan