Re: [PATCH 1/2] spapr: number of SMP sockets must be equal to NUMA nodes

[Daniel Henrique Barboza <danielhb413@gmail.com>]

On 3/25/21 5:56 AM, Cédric Le Goater wrote:
> On 3/25/21 3:10 AM, David Gibson wrote:
>> On Tue, Mar 23, 2021 at 02:21:33PM -0300, Daniel Henrique Barboza wrote:
>>>
>>>
>>> On 3/22/21 10:03 PM, David Gibson wrote:
>>>> On Fri, Mar 19, 2021 at 03:34:52PM -0300, Daniel Henrique Barboza wrote:
>>>>> Kernel commit 4bce545903fa ("powerpc/topology: Update
>>>>> topology_core_cpumask") cause a regression in the pseries machine when
>>>>> defining certain SMP topologies [1]. The reasoning behind the change is
>>>>> explained in kernel commit 4ca234a9cbd7 ("powerpc/smp: Stop updating
>>>>> cpu_core_mask"). In short, cpu_core_mask logic was causing troubles with
>>>>> large VMs with lots of CPUs and was changed by cpu_cpu_mask because, as
>>>>> far as the kernel understanding of SMP topologies goes, both masks are
>>>>> equivalent.
>>>>>
>>>>> Further discussions in the kernel mailing list [2] shown that the
>>>>> powerpc kernel always considered that the number of sockets were equal
>>>>> to the number of NUMA nodes. The claim is that it doesn't make sense,
>>>>> for Power hardware at least, 2+ sockets being in the same NUMA node. The
>>>>> immediate conclusion is that all SMP topologies the pseries machine were
>>>>> supplying to the kernel, with more than one socket in the same NUMA node
>>>>> as in [1], happened to be correctly represented in the kernel by
>>>>> accident during all these years.
>>>>>
>>>>> There's a case to be made for virtual topologies being detached from
>>>>> hardware constraints, allowing maximum flexibility to users. At the same
>>>>> time, this freedom can't result in unrealistic hardware representations
>>>>> being emulated. If the real hardware and the pseries kernel don't
>>>>> support multiple chips/sockets in the same NUMA node, neither should we.
>>>>>
>>>>> Starting in 6.0.0, all sockets must match an unique NUMA node in the
>>>>> pseries machine. qtest changes were made to adapt to this new
>>>>> condition.
>>>>
>>>> Oof.  I really don't like this idea.  It means a bunch of fiddly work
>>>> for users to match these up, for no real gain.  I'm also concerned
>>>> that this will require follow on changes in libvirt to not make this a
>>>> really cryptic and irritating point of failure.
>>>
>>> Haven't though about required Libvirt changes, although I can say that there
>>> will be some amount to be mande and it will probably annoy existing users
>>> (everyone that has a multiple socket per NUMA node topology).
>>>
>>> There is not much we can do from the QEMU layer aside from what I've proposed
>>> here. The other alternative is to keep interacting with the kernel folks to
>>> see if there is a way to keep our use case untouched.
>>
>> Right.  Well.. not necessarily untouched, but I'm hoping for more
>> replies from Cédric to my objections and mpe's.  Even with sockets
>> being a kinda meaningless concept in PAPR, I don't think tying it to
>> NUMA nodes makes sense.
> 
> I did a couple of replies in different email threads but maybe not
> to all. I felt it was going nowhere :/ Couple of thoughts,
> 
> Shouldn't we get rid of the socket concept, die also, under pseries
> since they don't exist under PAPR ? We only have numa nodes, cores,
> threads AFAICT.

I don't think we work with 'die'.

Getting rid of the 'socket' representation is sensible regarding PAPR,
but the effect for pseries will be similar to what this patch is already
doing: users could have multiple sockets in the same NUMA node, and then
they won't. Either because we got rid of the 'socket' representation or
because socket == NUMA node.

> 
> Should we diverged from PAPR and add extra DT properties "qemu,..." ?
> There are a couple of places where Linux checks for the underlying
> hypervisor already.
> 
>>> This also means that
>>> 'ibm,chip-id' will probably remain in use since it's the only place where
>>> we inform cores per socket information to the kernel.
>>
>> Well.. unless we can find some other sensible way to convey that
>> information.  I haven't given up hope for that yet.
> 
> Well, we could start by fixing the value in QEMU. It is broken today.


I'll look into it. It makes more sense to talk about keeping it when
it's working properly.



DHB


> 
> 
> This is all coming from some work we did last year to evaluate our HW
> (mostly for XIVE) on 2s, 4s, 16s systems on baremetal, KVM and PowerVM.
> We saw some real problems because Linux did not have a clear view of the
> topology. See the figures here :
> 
> http://patchwork.ozlabs.org/project/linuxppc-dev/patch/20210303174857.1760393-9-clg@kaod.org/
> 
> The node id is a key parameter for system resource management, memory
> allocation, interrupt affinity, etc. Linux scales much better if used
> correctly.
> 
> C.
>