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

[Gregory Price <gourry@gourry.net>]

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