Re: [PATCH v2 2/2] mm/page_alloc: refactor build_node_zonelist() out of build_zonelists()

From: Gregory Price

Date: Sun Sep 20 2026 - 23:30:19 EST


On Sun, Sep 20, 2026 at 10:49:05PM -0400, Zi Yan wrote:
> On Fri Sep 11, 2026 at 11:04 PM EDT, Gregory Price wrote:
> > +static void build_node_zonelist(pg_data_t *pgdat, const nodemask_t *candidates,
> > + int zlidx)
> > {
> > - static int node_order[MAX_NUMNODES];
> > - int node, nr_nodes = 0;
> > + struct zoneref *zonerefs = pgdat->node_zonelists[zlidx]._zonerefs;
>
> Why does build_node_zonelist() need to have a new zlidx instead of using
> ZONELIST_FALLBACK like build_zonelists_in_node_order() did?
>

The intent is to build new zonelist over a set of candidate nodes, and
it's also just clearer: build ZONELIST_FALLBACK from N_MEMORY.

With this we get:

build_node_zonelists(pgdat, &node_states[N_MEMORY_COMMON], ZONELIST_FALLBACK);
build_node_zonelists(pgdat, &node_states[N_MEMORY], ZONELIST_PRIVATE);
#ifdef PAGEALLOC_KTEST
nodemask_andnot(&private_only, &node_states[N_MEMORY],
&node_states[N_MEMORY_COMMON]
build_node_zonelists(pgdat, &private_only, ZONELIST_KTEST);
#endif


> > +static void build_zonelists(pg_data_t *pgdat)
> > +{
> > + build_node_zonelist(pgdat, &node_states[N_MEMORY], ZONELIST_FALLBACK);
> > + build_thisnode_zonelists(pgdat);
>
> If build_thisnode_zonelists() means ZONELIST_NOFALLBACK, why
> cannot build_node_zonelist() imply ZONELIST_FALLBACK?
>

thisnode actually means ZONELIST_X+1 as opposed to ZONELIST_NOFALLBACK.

Since folks are adamant about not allowing another GFP flag for zonelist
selection (beyond GFP_THISNODE), the result of this is that all future
zonelist additions must carry a FALLBACK + NOFALLBACK variant.

The question you actually want to ask is why build_thisnode_zonelists()
even exists - it should be part of build_node_zonelist()

I can probably follow up this series by just folding eveything into

/* build zlidx and zlidx+1 (nofallback) */
build_node_zonelists(pgdat, candidates, zlidx);

And add a BUILD_ON_BUG/ASSERT that forces any CONFIG_NUMA to have
balanced zonelist additions.

But I don't think it's strictly necessary for any of this, and we're
just shuffling code from one place to another. Probably I can just add
that improvement when we add the next zonelist. In the meantime - this
makes it easier to add new zonelists as-is (and just makes the code more
readable).

~Gregory