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

[Tvrtko Ursulin]

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@xxxxxxxxxx>
> > >
> > > 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@xxxxxxxxxx>
> > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxx>
> > > Co-developed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxx>
> > > Cc: Catalin Marinas <catalin.marinas@xxxxxxx>
> > > Cc: Will Deacon <will@xxxxxxxxxx>
> > > Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> > > Cc: “Rafael J. Wysocki" <rafael@xxxxxxxxxx>
> > > ---
> > >   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