Re: [PATCH v4 10/27] mm/sparse: allow for alternate vmemmap section init at boot

[Frank van der Linden <fvdl@google.com>]

On Thu, Feb 27, 2025 at 9:20 AM Johannes Weiner <hannes@cmpxchg.org> wrote:
>
> On Thu, Feb 27, 2025 at 08:47:18AM -0800, Frank van der Linden wrote:
> > On Wed, Feb 26, 2025 at 10:09 AM Johannes Weiner <hannes@cmpxchg.org> wrote:
> > >
> > > On Tue, Feb 18, 2025 at 06:16:38PM +0000, Frank van der Linden wrote:
> > > > @@ -489,6 +489,14 @@ config SPARSEMEM_VMEMMAP
> > > >         SPARSEMEM_VMEMMAP uses a virtually mapped memmap to optimise
> > > >         pfn_to_page and page_to_pfn operations.  This is the most
> > > >         efficient option when sufficient kernel resources are available.
> > > > +
> > > > +config ARCH_WANT_SPARSEMEM_VMEMMAP_PREINIT
> > > > +     bool
> > > > +
> > > > +config SPARSEMEM_VMEMMAP_PREINIT
> > > > +     bool "Early init of sparse memory virtual memmap"
> > > > +     depends on SPARSEMEM_VMEMMAP && ARCH_WANT_SPARSEMEM_VMEMMAP_PREINIT
> > > > +     default y
> > >
> > > oldconfig just prompted me on this, but it's not clear to me what it
> > > does. Not even after skimming the changelog of the patch to be honest.
> > >
> > > Can you please add a help text that explains the user-visible effects
> > > of the toggle, as well as guidance as to who might care to change it?
> >
> > Hi Johannes,
> >
> > Thanks for your comment. How's this:
>
> Thanks for the quick reply!
>
> > Enables subsystems to pre-initialize memmap in their own way,
> > allowing for memory savings during boot. The HugeTLB code uses
> > this to initialize memmap for bootmem allocated gigantic hugepages
> > in a way that is done by HUGETLB_PAGE_OPTIMIZE_VMEMMAP. This
> > means saving this memory right away, instead of allocating it
> > first and then freeing it later. Not allocating these pages
> > at all during boot allows for specifying a bigger number of
> > hugepages on the kernel commandline on larger systems.
>
> That makes sense.
>
> But if it's infra code for a hugetlb feature, it should either be
> something that HUGETLB_PAGE_OPTIMIZE_VMEMMAP pulls in automatically,
> or at least be a hugetlb-specific option that pulls it in.
>
> Keep in mind that not everybody enables HUGETLBFS. In fact, hugetlb is
> default N. It's moot to ask users whether they want to enable infra
> code for a feature they aren't using, and default to Y no less. You're
> regressing innocent bystanders doing this.

The main reason that I added a separate config was:

1) I could see other subsystems use this.
2) The number of section flags is limited, so I wanted to put the one
I added inside an option instead of always using it.

If especially 2) is not a concern or can be solved differently, I'll
be happy to remove the option. I don't particularly like having it,
but I didn't see a better way.

Let me think of a way to clean this up a little, and suggestions are
welcome, of course.

- Frank