Re: [PATCH 1/2] numa: Add simple generic NUMA emulation

[Tvrtko Ursulin <tvrtko.ursulin@igalia.com>]

Hi Greg,

Gentle reminder on the opens from this thread. Let me re-summarise the 
question below:

On 26/06/2024 12:47, Tvrtko Ursulin wrote:
> 
> Hi Greg,
> 
> On 26/06/2024 08:38, Greg Kroah-Hartman wrote:
>> On Tue, Jun 25, 2024 at 01:58:02PM +0100, Tvrtko Ursulin wrote:
>>> From: Maíra Canal <mcanal@igalia.com>
>>>
>>> Add some common code for splitting the memory into N emulated NUMA 
>>> memory
>>> nodes.
>>>
>>> Individual architecture can then enable selecting this option and use 
>>> the
>>> existing numa=fake=<N> kernel argument to enable it.
>>>
>>> Memory is always split into equally sized chunks.
>>>
>>> Signed-off-by: Maíra Canal <mcanal@igalia.com>
>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
>>> Co-developed-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>>> Cc: Will Deacon <will@kernel.org>
>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>> Cc: “Rafael J. Wysocki" <rafael@kernel.org>
>>> ---
>>>   drivers/base/Kconfig          |  7 ++++
>>>   drivers/base/Makefile         |  1 +
>>>   drivers/base/arch_numa.c      |  6 ++++
>>>   drivers/base/numa_emulation.c | 67 +++++++++++++++++++++++++++++++++++
>>>   drivers/base/numa_emulation.h | 21 +++++++++++
>>
>> Why not just properly describe the numa topology in your bootloader or
>> device tree and not need any such "fake" stuff at all?
>>
>> Also, you are now asking me to maintain these new files, not something
>> I'm comfortable doing at all sorry.
> 
> Mostly because ae3c107cd8be ("numa: Move numa implementation to common 
> code") and existing common code in drivers/base/arch_numa.c it appeared 
> it could be acceptable to add the simple NUMA emulation into the common 
> code too. Then building upon the same concept as on x86 where no need 
> for firmware changes is needed for experimenting with different 
> configurations.
> 
> Would folding into arch_numa.c so no new files are added address your 
> concern, or your main issue is the emulation in general?

Re-iterating and slightly re-formulating this question I see three options:

a)
Fold the new simple generic code into the existing arch_numa.c, 
addressing the "no new files" objection, if that was the main objection.

b)
Move completely into arch code - aka you don't want to see it under 
drivers/base at all, ever, regardless of how simple the new code is, or 
that common NUMA code is already there.

c)
Strong nack for either a) or b) - so "do it in the firmware" comment.

Trying to understand your position so we can progress this.

Thanks,

Tvrtko

> 
>  >> +    if (str_has_prefix(opt, "fake="))
>  >> +        return numa_emu_cmdline(opt + 5);
>  >
>  > You did not document this at all :(
> 
> That was indeed an oversight. Just need to "copy with edits" some stuff 
> from Documentation/arch/x86/x86_64/boot-options.rst.
> 
> Regards,
> 
> Tvrtko