[PATCH v4 1/8] mm/page_alloc: reset can_retry_reserves when the slowpath restarts

[PATCH v4 1/8] mm/page_alloc: reset can_retry_reserves when the slowpath restarts

[Gregory Price (Meta)]

Commit 2c4c3e29897d ("mm/page_alloc: simplify __alloc_pages_slowpath()
flow") retries an allocation once with reserve_flags when the caller
becomes eligible for reserves, guarded by can_retry_reserves.  The flag
is set at its declaration and never reset.

A restart after a cpuset update or a zonelist rebuild drops
reserve_flags and comes back with can_retry_reserves already false, so a
caller that cannot reclaim, such as GFP_NOWAIT | __GFP_MEMALLOC, fails
without trying the reserves again.

Reset can_retry_reserves at restart.  Only a cpuset or zonelist change
restarts the slowpath, so the retry still runs at most once per change.

With kprobes failing every attempt without reserves and the first with
them, and a memory block onlined during the walk, the allocation
returned NULL in 3 of 3 runs, and a page in 3 of 3 with this change.

Fixes: 2c4c3e29897d ("mm/page_alloc: simplify __alloc_pages_slowpath() flow")
Cc: stable@vger.kernel.org
Signed-off-by: Gregory Price (Meta) <gourry@gourry.net>
---
 mm/page_alloc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 2daf358269ae..78569ac6f2d9 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -4783,7 +4783,7 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
 	unsigned int zonelist_iter_cookie;
 	int reserve_flags;
 	bool compact_first = false;
-	bool can_retry_reserves = true;
+	bool can_retry_reserves;
 	unsigned long alloc_start_time = jiffies;
 
 	/*
@@ -4817,6 +4817,7 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
 	no_progress_loops = 0;
 	compact_result = COMPACT_SKIPPED;
 	compact_priority = DEF_COMPACT_PRIORITY;
+	can_retry_reserves = true;
 	cpuset_mems_cookie = read_mems_allowed_begin();
 	zonelist_iter_cookie = zonelist_iter_begin();
 
-- 
2.56.0

Re: [PATCH v4 1/8] mm/page_alloc: reset can_retry_reserves when the slowpath restarts

[Vlastimil Babka (SUSE)]

On 10/28/26 22:19, Gregory Price (Meta) wrote:
> Commit 2c4c3e29897d ("mm/page_alloc: simplify __alloc_pages_slowpath()
> flow") retries an allocation once with reserve_flags when the caller
> becomes eligible for reserves, guarded by can_retry_reserves.  The flag
> is set at its declaration and never reset.
> 
> A restart after a cpuset update or a zonelist rebuild drops
> reserve_flags

I don't follow, nothing "drops reserve_flags"? after the restart,
there will still be a get_page_from_freelist() without them followed by
get_page_from_freelist() with them, right?

> and comes back with can_retry_reserves already false, so a
> caller that cannot reclaim, such as GFP_NOWAIT | __GFP_MEMALLOC, fails
> without trying the reserves again.

The "goto retry" triggered by can_retry_reserves actually drops nodemask and
recalculates preferred zones. Perhaps "can_retry_reserves" wasn't the best name.

> Reset can_retry_reserves at restart.  Only a cpuset or zonelist change
> restarts the slowpath, so the retry still runs at most once per change.
> 
> With kprobes failing every attempt without reserves and the first with
> them, and a memory block onlined during the walk, the allocation
> returned NULL in 3 of 3 runs, and a page in 3 of 3 with this change.

So IIUC the allocation was racing with a memory online in a way that it
would fail if the online happened fully after the allocation. So this just
turns part of the race window into the allocation's favor?

> Fixes: 2c4c3e29897d ("mm/page_alloc: simplify __alloc_pages_slowpath() flow")
> Cc: stable@vger.kernel.org
> Signed-off-by: Gregory Price (Meta) <gourry@gourry.net>

I mean it's not wrong and if patch 5 depends on it (unless it also only
fixes a racy window in the same way I think is happening here) then so be
it. But the commit log would need correcting.

> ---
>  mm/page_alloc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 2daf358269ae..78569ac6f2d9 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -4783,7 +4783,7 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
>  	unsigned int zonelist_iter_cookie;
>  	int reserve_flags;
>  	bool compact_first = false;
> -	bool can_retry_reserves = true;
> +	bool can_retry_reserves;
>  	unsigned long alloc_start_time = jiffies;
>  
>  	/*
> @@ -4817,6 +4817,7 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
>  	no_progress_loops = 0;
>  	compact_result = COMPACT_SKIPPED;
>  	compact_priority = DEF_COMPACT_PRIORITY;
> +	can_retry_reserves = true;
>  	cpuset_mems_cookie = read_mems_allowed_begin();
>  	zonelist_iter_cookie = zonelist_iter_begin();
>  

Re: [PATCH v4 1/8] mm/page_alloc: reset can_retry_reserves when the slowpath restarts

[Gregory Price (Meta)]

On 2026-10-29 14:37 +0100, Vlastimil Babka (SUSE) wrote:
> On 10/28/26 22:19, Gregory Price (Meta) wrote:
> > Commit 2c4c3e29897d ("mm/page_alloc: simplify __alloc_pages_slowpath()
> > flow") retries an allocation once with reserve_flags when the caller
> > becomes eligible for reserves, guarded by can_retry_reserves.  The flag
> > is set at its declaration and never reset.
> > 
> > A restart after a cpuset update or a zonelist rebuild drops
> > reserve_flags
> 
> I don't follow, nothing "drops reserve_flags"? after the restart,
> there will still be a get_page_from_freelist() without them followed by
> get_page_from_freelist() with them, right?
> 

"Drops" is the wrong word, sorry.

After a restart the reserve bits are recomputed, but can_retry_reserves
is already false.  As a result no-reclaim/PF_MEMALLOC callers don't
get a reserve-retry attempt (we just exit w/ nopage).

Reclaim callers are fine.

This problem exists regardless of the rest of the series.

But you're right, I will rework the commit message.


> > Reset can_retry_reserves at restart.  Only a cpuset or zonelist change
> > restarts the slowpath, so the retry still runs at most once per change.
> > 
> > With kprobes failing every attempt without reserves and the first with
> > them, and a memory block onlined during the walk, the allocation
> > returned NULL in 3 of 3 runs, and a page in 3 of 3 with this change.
> 
> So IIUC the allocation was racing with a memory online in a way that it
> would fail if the online happened fully after the allocation. So this just
> turns part of the race window into the allocation's favor?
>
> I mean it's not wrong and if patch 5 depends on it (unless it also only
> fixes a racy window in the same way I think is happening here) then so be
> it. But the commit log would need correcting.
>

Not quite - without the online it would have succeeded.  The zonelist
rebuild rewrites the lists in-place, so a concurrent walk can miss valid
zones or pick the wrong zone.  The rest of the series detects more of
these restart scenarios.

Those restarts then exacerbate the issue this patch resolves.  For
example, patch 5 would free a reserve allocation that succeeded - and
then fail because can_retry_reserves is already false.

-- 
~Gregory

[PATCH v4 2/8] mm/page_alloc: detect zonelist rebuilds from memory hot-add

[Gregory Price (Meta)]

The zonelist_update_seq retry was limited to CONFIG_MEMORY_HOTREMOVE,
presumably because only hot-remove takes zones out of a zonelist.

However, build_zonerefs_node() rewrites the zonelists in place, so
onlining the first block of a zone shifts the entries that follow it
in every zonelist containing that zone.  This can cause a spurious
allocation failure for a __GFP_THISNODE caller.

For a node with only ZONE_NORMAL, onlining its first ZONE_MOVABLE block
rewrites the node's ZONELIST_NOFALLBACK list as:

  [NORMAL, NULL] -> [MOVABLE, NULL] -> [MOVABLE, NORMAL, NULL]

A concurrent GFP_KERNEL | __GFP_THISNODE allocation can observe the
intermediate list, skip ZONE_MOVABLE, and find no eligible zone. With
CONFIG_MEMORY_HOTREMOVE=n, check_retry_zonelist() returns 0 and the
allocation fails even though the node has free ZONE_NORMAL memory.

Gate the sequence checks on CONFIG_MEMORY_HOTPLUG, which covers every
configuration that rebuilds zonelists after boot.

Fixes: 3d36424b3b58 ("mm/page_alloc: fix race condition between build_all_zonelists and page allocation")
Cc: stable@vger.kernel.org
Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
Signed-off-by: Gregory Price (Meta) <gourry@gourry.net>
---
 mm/page_alloc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 78569ac6f2d9..52533992ea16 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -4404,7 +4404,7 @@ static DEFINE_SEQLOCK(zonelist_update_seq);
 
 static unsigned int zonelist_iter_begin(void)
 {
-	if (IS_ENABLED(CONFIG_MEMORY_HOTREMOVE))
+	if (IS_ENABLED(CONFIG_MEMORY_HOTPLUG))
 		return read_seqbegin(&zonelist_update_seq);
 
 	return 0;
@@ -4412,7 +4412,7 @@ static unsigned int zonelist_iter_begin(void)
 
 static unsigned int check_retry_zonelist(unsigned int seq)
 {
-	if (IS_ENABLED(CONFIG_MEMORY_HOTREMOVE))
+	if (IS_ENABLED(CONFIG_MEMORY_HOTPLUG))
 		return read_seqretry(&zonelist_update_seq, seq);
 
 	return seq;
-- 
2.56.0

[PATCH v4 3/8] mm/page_alloc: snapshot the preferred zone against shrink rebuilds

[Gregory Price (Meta)]

get_page_from_freelist() and alloc_pages_bulk_noprof() reread
ac->preferred_zoneref->zone at every use. Offlining a zone's last block
rebuilds the zonelists in place, and the shrunk list's NULL terminator
can land on the preferred slot mid-walk:

  CPU 0                             CPU 1
  get_page_from_freelist(GFP_KERNEL | __GFP_THISNODE)
    zone = NORMAL (slot 1)
                                    offline_pages()
                                      build_all_zonelists()
                                        [MOVABLE, NORMAL, NULL]
                                        -> [NORMAL, NULL]
    rmqueue(zonelist_zone(slot 1), zone)
      zone_statistics(NULL, zone)

  BUG: kernel NULL pointer dereference, address: 0000000000000060
  RIP: 0010:get_page_from_freelist+0x6a5/0x15f0
  Call Trace:
   __alloc_frozen_pages_noprof+0x125/0x330

Read the slot once. Zones are never freed, so a stale zone is safe, and
a NULL slot fails the same way as walking off the end.

Fixes: 11cd8638c37f ("mm, page_alloc: remove stop_machine from build_all_zonelists")
Cc: stable@vger.kernel.org
Signed-off-by: Gregory Price (Meta) <gourry@gourry.net>
---
 mm/page_alloc.c | 31 +++++++++++++++++++++++--------
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 52533992ea16..24106c881ec2 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -3794,12 +3794,21 @@ get_page_from_freelist(gfp_t gfp_mask, unsigned int order, int alloc_flags,
 {
 	struct zoneref *z;
 	struct zone *zone;
+	struct zone *preferred_zone;
 	struct pglist_data *last_pgdat = NULL;
 	bool last_pgdat_dirty_ok = false;
 	bool no_fallback;
 	bool skip_kswapd_nodes = nr_online_nodes > 1;
 	bool skipped_kswapd_nodes = false;
 
+	/*
+	 * A concurrent zonelist rebuild can NULL or replace this slot. Zones
+	 * are never freed, so reading it once keeps a usable zone.
+	 */
+	preferred_zone = READ_ONCE(ac->preferred_zoneref->zone);
+	if (!preferred_zone)
+		return NULL;
+
 retry:
 	/*
 	 * Scan zonelist, looking for a zone with enough free.
@@ -3846,7 +3855,7 @@ get_page_from_freelist(gfp_t gfp_mask, unsigned int order, int alloc_flags,
 		}
 
 		if (no_fallback && !defrag_mode && nr_online_nodes > 1 &&
-		    zone != zonelist_zone(ac->preferred_zoneref)) {
+		    zone != preferred_zone) {
 			int local_nid;
 
 			/*
@@ -3854,7 +3863,7 @@ get_page_from_freelist(gfp_t gfp_mask, unsigned int order, int alloc_flags,
 			 * fragmenting fallbacks. Locality is more important
 			 * than fragmentation avoidance.
 			 */
-			local_nid = zonelist_node_idx(ac->preferred_zoneref);
+			local_nid = zone_to_nid(preferred_zone);
 			if (zone_to_nid(zone) != local_nid) {
 				alloc_flags &= ~ALLOC_NOFRAGMENT;
 				goto retry;
@@ -3916,7 +3925,7 @@ get_page_from_freelist(gfp_t gfp_mask, unsigned int order, int alloc_flags,
 				goto try_this_zone;
 
 			if (!node_reclaim_enabled() ||
-			    !zone_allows_reclaim(zonelist_zone(ac->preferred_zoneref), zone))
+			    !zone_allows_reclaim(preferred_zone, zone))
 				continue;
 
 			if (!node_reclaim(zone->zone_pgdat, gfp_mask, order))
@@ -3929,7 +3938,7 @@ get_page_from_freelist(gfp_t gfp_mask, unsigned int order, int alloc_flags,
 		}
 
 try_this_zone:
-		page = rmqueue(zonelist_zone(ac->preferred_zoneref), zone, order,
+		page = rmqueue(preferred_zone, zone, order,
 				gfp_mask, alloc_flags, ac->migratetype);
 		if (page) {
 			prep_new_page(page, order, gfp_mask, alloc_flags);
@@ -5184,6 +5193,7 @@ unsigned long alloc_pages_bulk_noprof(gfp_t gfp, int preferred_nid,
 {
 	struct page *page;
 	struct zone *zone;
+	struct zone *preferred_zone;
 	struct zoneref *z;
 	struct per_cpu_pages *pcp;
 	struct list_head *pcp_list;
@@ -5232,6 +5242,11 @@ unsigned long alloc_pages_bulk_noprof(gfp_t gfp, int preferred_nid,
 	if (!prepare_alloc_pages(gfp, 0, preferred_nid, nodemask, &ac, &gfp, &alloc_flags))
 		goto out;
 
+	/* Read once; see get_page_from_freelist(). */
+	preferred_zone = READ_ONCE(ac.preferred_zoneref->zone);
+	if (!preferred_zone)
+		goto failed;
+
 	/* Find an allowed local zone that meets the low watermark. */
 	z = ac.preferred_zoneref;
 	for_next_zone_zonelist_nodemask(zone, z, ac.highest_zoneidx, ac.nodemask) {
@@ -5242,8 +5257,8 @@ unsigned long alloc_pages_bulk_noprof(gfp_t gfp, int preferred_nid,
 			continue;
 		}
 
-		if (nr_online_nodes > 1 && zone != zonelist_zone(ac.preferred_zoneref) &&
-		    zone_to_nid(zone) != zonelist_node_idx(ac.preferred_zoneref)) {
+		if (nr_online_nodes > 1 && zone != preferred_zone &&
+		    zone_to_nid(zone) != zone_to_nid(preferred_zone)) {
 			goto failed;
 		}
 
@@ -5251,7 +5266,7 @@ unsigned long alloc_pages_bulk_noprof(gfp_t gfp, int preferred_nid,
 retry_this_zone:
 		mark = wmark_pages(zone, alloc_flags & ALLOC_WMARK_MASK) + nr_pages - nr_populated;
 		if (zone_watermark_fast(zone, 0,  mark,
-				zonelist_zone_idx(ac.preferred_zoneref),
+				zone_idx(preferred_zone),
 				alloc_flags, gfp)) {
 			break;
 		}
@@ -5310,7 +5325,7 @@ unsigned long alloc_pages_bulk_noprof(gfp_t gfp, int preferred_nid,
 	pcp_spin_unlock(pcp);
 
 	__count_zid_vm_events(PGALLOC, zone_idx(zone), nr_account);
-	zone_statistics(zonelist_zone(ac.preferred_zoneref), zone, nr_account);
+	zone_statistics(preferred_zone, zone, nr_account);
 
 out:
 	return nr_populated;
-- 
2.56.0

Re: [PATCH v4 3/8] mm/page_alloc: snapshot the preferred zone against shrink rebuilds

[Vlastimil Babka (SUSE)]

On 10/28/26 22:19, Gregory Price (Meta) wrote:
> get_page_from_freelist() and alloc_pages_bulk_noprof() reread
> ac->preferred_zoneref->zone at every use. Offlining a zone's last block
> rebuilds the zonelists in place, and the shrunk list's NULL terminator
> can land on the preferred slot mid-walk:
> 
>   CPU 0                             CPU 1
>   get_page_from_freelist(GFP_KERNEL | __GFP_THISNODE)
>     zone = NORMAL (slot 1)
>                                     offline_pages()
>                                       build_all_zonelists()
>                                         [MOVABLE, NORMAL, NULL]
>                                         -> [NORMAL, NULL]
>     rmqueue(zonelist_zone(slot 1), zone)
>       zone_statistics(NULL, zone)
> 
>   BUG: kernel NULL pointer dereference, address: 0000000000000060
>   RIP: 0010:get_page_from_freelist+0x6a5/0x15f0
>   Call Trace:
>    __alloc_frozen_pages_noprof+0x125/0x330
> 
> Read the slot once. Zones are never freed, so a stale zone is safe, and
> a NULL slot fails the same way as walking off the end.
> 
> Fixes: 11cd8638c37f ("mm, page_alloc: remove stop_machine from build_all_zonelists")
> Cc: stable@vger.kernel.org
> Signed-off-by: Gregory Price (Meta) <gourry@gourry.net>

Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>

[PATCH v4 4/8] mm/mmzone: read each zoneref slot once when filtering by nodemask

[Gregory Price (Meta)]

__next_zones_zonelist() tests a slot's zone for NULL, then reads the
slot again to look up the zone's node. Offlining a zone's last block
rebuilds the zonelists in place, so the second read can return the
terminator:

  CPU 0                             CPU 1
  __next_zones_zonelist(nodemask)
    zonelist_zone(slot 1) = NORMAL
                                    offline_pages()
                                      build_all_zonelists()
                                        [MOVABLE, NORMAL, NULL]
                                        -> [NORMAL, NULL]
    zref_in_nodemask(slot 1)
      zone_to_nid(NULL)

Current GCC and clang merge the two reads, but nothing requires it.
With mm/mmzone.o built with -fno-inline -fno-ipa-sra, holding a
nodemask allocation between the reads while the node's last movable
block goes offline oopses every time:

  BUG: kernel NULL pointer dereference, address: 0000000000000060
  RIP: 0010:zone_to_nid+0x0/0x10
  Call Trace:
   zref_in_nodemask+0x9/0x20
   __next_zones_zonelist+0x23/0x70
   prepare_alloc_pages.constprop.0+0x16c/0x190
   __alloc_frozen_pages_noprof+0xaf/0x330

Read the slot once and test that zone. A NULL zone still ends the walk.

Fixes: 11cd8638c37f ("mm, page_alloc: remove stop_machine from build_all_zonelists")
Cc: stable@vger.kernel.org
Signed-off-by: Gregory Price (Meta) <gourry@gourry.net>
---
 mm/mmzone.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/mm/mmzone.c b/mm/mmzone.c
index 9cc9ef588580..009f1dd75dde 100644
--- a/mm/mmzone.c
+++ b/mm/mmzone.c
@@ -43,13 +43,14 @@ struct zone *next_zone(struct zone *zone)
 	return zone;
 }
 
-static inline int zref_in_nodemask(struct zoneref *zref,
-				   const nodemask_t *nodes)
+/* A NULL zone terminates the zonelist, so it ends the search. */
+static inline bool zone_in_nodemask(const struct zone *zone,
+				    const nodemask_t *nodes)
 {
 #ifdef CONFIG_NUMA
-	return node_isset(zonelist_node_idx(zref), *nodes);
+	return !zone || node_isset(zone_to_nid(zone), *nodes);
 #else
-	return 1;
+	return true;
 #endif /* CONFIG_NUMA */
 }
 
@@ -67,7 +68,7 @@ struct zoneref *__next_zones_zonelist(struct zoneref *z,
 			z++;
 	else
 		while (zonelist_zone_idx(z) > highest_zoneidx ||
-				(zonelist_zone(z) && !zref_in_nodemask(z, nodes)))
+		       !zone_in_nodemask(READ_ONCE(z->zone), nodes))
 			z++;
 
 	return z;
-- 
2.56.0

Re: [PATCH v4 4/8] mm/mmzone: read each zoneref slot once when filtering by nodemask

[Vlastimil Babka (SUSE)]

On 10/28/26 22:19, Gregory Price (Meta) wrote:
> __next_zones_zonelist() tests a slot's zone for NULL, then reads the
> slot again to look up the zone's node. Offlining a zone's last block
> rebuilds the zonelists in place, so the second read can return the
> terminator:
> 
>   CPU 0                             CPU 1
>   __next_zones_zonelist(nodemask)
>     zonelist_zone(slot 1) = NORMAL
>                                     offline_pages()
>                                       build_all_zonelists()
>                                         [MOVABLE, NORMAL, NULL]
>                                         -> [NORMAL, NULL]
>     zref_in_nodemask(slot 1)
>       zone_to_nid(NULL)
> 
> Current GCC and clang merge the two reads, but nothing requires it.
> With mm/mmzone.o built with -fno-inline -fno-ipa-sra, holding a
> nodemask allocation between the reads while the node's last movable
> block goes offline oopses every time:
> 
>   BUG: kernel NULL pointer dereference, address: 0000000000000060
>   RIP: 0010:zone_to_nid+0x0/0x10
>   Call Trace:
>    zref_in_nodemask+0x9/0x20
>    __next_zones_zonelist+0x23/0x70
>    prepare_alloc_pages.constprop.0+0x16c/0x190
>    __alloc_frozen_pages_noprof+0xaf/0x330
> 
> Read the slot once and test that zone. A NULL zone still ends the walk.
> 
> Fixes: 11cd8638c37f ("mm, page_alloc: remove stop_machine from build_all_zonelists")
> Cc: stable@vger.kernel.org
> Signed-off-by: Gregory Price (Meta) <gourry@gourry.net>

Oh, zonelist_node_idx() reads nid from the zone, it's not part of zoneref as
I thought. So it's the same after this patch, just with READ_ONCE().

Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>

> ---
>  mm/mmzone.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/mm/mmzone.c b/mm/mmzone.c
> index 9cc9ef588580..009f1dd75dde 100644
> --- a/mm/mmzone.c
> +++ b/mm/mmzone.c
> @@ -43,13 +43,14 @@ struct zone *next_zone(struct zone *zone)
>  	return zone;
>  }
>  
> -static inline int zref_in_nodemask(struct zoneref *zref,
> -				   const nodemask_t *nodes)
> +/* A NULL zone terminates the zonelist, so it ends the search. */
> +static inline bool zone_in_nodemask(const struct zone *zone,
> +				    const nodemask_t *nodes)
>  {
>  #ifdef CONFIG_NUMA
> -	return node_isset(zonelist_node_idx(zref), *nodes);
> +	return !zone || node_isset(zone_to_nid(zone), *nodes);
>  #else
> -	return 1;
> +	return true;
>  #endif /* CONFIG_NUMA */
>  }
>  
> @@ -67,7 +68,7 @@ struct zoneref *__next_zones_zonelist(struct zoneref *z,
>  			z++;
>  	else
>  		while (zonelist_zone_idx(z) > highest_zoneidx ||
> -				(zonelist_zone(z) && !zref_in_nodemask(z, nodes)))
> +		       !zone_in_nodemask(READ_ONCE(z->zone), nodes))
>  			z++;
>  
>  	return z;

[PATCH v4 5/8] mm/page_alloc: retry allocations that raced with a zonelist rebuild

[Gregory Price (Meta)]

Onlining the first block of a zone rebuilds every zonelist in place,
shifting the slots after the new zone.  The iterator approves a slot,
then the allocator loads the zone from it, so a GFP_KERNEL request can
land in ZONE_MOVABLE and a nodemask request on a node outside its mask:

  CPU 0                             CPU 1
  prepare_alloc_pages()
    first_zones_zonelist(nodemask {1})
      slot 0 (NORMAL1) approved
                                    online_pages()
                                      build_all_zonelists()
                                        [NORMAL1, NORMAL2, ...]
                                        -> [MOVABLE1, NORMAL1, ...]
  get_page_from_freelist()
    zone = zonelist_zone(slot 0)    /* MOVABLE1 */
    allocates from MOVABLE1

zonelist_update_seq detects the rebuild, but only the slowpath's failure
exits check it.

Check it whenever an allocation succeeds too, and if it changed, free
the page and retry, as the memcg charge failure path does.  Eligible
pages are dropped as well: zonelists only change when memory comes or
goes, and a retry is simpler than rechecking the page.

Freeing the page does not undo a block the allocation claimed.  A
GFP_KERNEL request that lands in a fresh ZONE_MOVABLE converts a free
block to MIGRATE_UNMOVABLE, which stays until the zone's movable free
list runs out and a movable allocation takes it back.

The fast and bulk paths read the seqlock's raw seqcount without waiting:
an ALLOC_NOLOCK caller can be an NMI that interrupted the writer, and on
PREEMPT_RT the seqlock accessor takes the spinlock.  A sample taken
mid-write costs a retry, or NULL under ALLOC_NOLOCK.
check_retry_zonelist() returns 0 when compiled out, so a backport
without the hot-add change stays harmless.

Fixes: 11cd8638c37f ("mm, page_alloc: remove stop_machine from build_all_zonelists")
Cc: stable@vger.kernel.org
Signed-off-by: Gregory Price (Meta) <gourry@gourry.net>
---
 mm/page_alloc.c | 34 +++++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 24106c881ec2..33aab9deaaa1 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -4419,12 +4419,25 @@ static unsigned int zonelist_iter_begin(void)
 	return 0;
 }
 
+/*
+ * Never wait for the writer, which an ALLOC_NOLOCK caller in NMI may have
+ * interrupted. Read the raw seqcount: on PREEMPT_RT the seqlock accessor takes
+ * the spinlock while a write is in progress.
+ */
+static unsigned int zonelist_iter_sample(void)
+{
+	if (IS_ENABLED(CONFIG_MEMORY_HOTPLUG))
+		return raw_seqcount_begin(&zonelist_update_seq.seqcount.seqcount);
+
+	return 0;
+}
+
 static unsigned int check_retry_zonelist(unsigned int seq)
 {
 	if (IS_ENABLED(CONFIG_MEMORY_HOTPLUG))
 		return read_seqretry(&zonelist_update_seq, seq);
 
-	return seq;
+	return 0;
 }
 
 /* Perform direct synchronous page reclaim */
@@ -5115,6 +5128,12 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
 	warn_alloc(gfp_mask, ac->nodemask,
 			"page allocation failure: order:%u", order);
 got_pg:
+	/* A rebuild during the walk may have handed out an ineligible zone. */
+	if (page && check_retry_zonelist(zonelist_iter_cookie)) {
+		__free_frozen_pages(page, order, FPI_NONE);
+		page = NULL;
+		goto restart;
+	}
 	return page;
 }
 
@@ -5199,6 +5218,7 @@ unsigned long alloc_pages_bulk_noprof(gfp_t gfp, int preferred_nid,
 	struct list_head *pcp_list;
 	struct alloc_context ac;
 	unsigned int alloc_flags = ALLOC_WMARK_LOW;
+	unsigned int zonelist_iter_cookie;
 	int nr_populated = 0, nr_account = 0;
 
 	/*
@@ -5239,6 +5259,7 @@ unsigned long alloc_pages_bulk_noprof(gfp_t gfp, int preferred_nid,
 	/* May set ALLOC_NOFRAGMENT, fragmentation will return 1 page. */
 	gfp &= gfp_allowed_mask;
 	gfp = current_gfp_context(gfp);
+	zonelist_iter_cookie = zonelist_iter_sample();
 	if (!prepare_alloc_pages(gfp, 0, preferred_nid, nodemask, &ac, &gfp, &alloc_flags))
 		goto out;
 
@@ -5288,6 +5309,10 @@ unsigned long alloc_pages_bulk_noprof(gfp_t gfp, int preferred_nid,
 	if (unlikely(!zone))
 		goto failed;
 
+	/* The zone was chosen from a zonelist that a rebuild may have changed. */
+	if (unlikely(check_retry_zonelist(zonelist_iter_cookie)))
+		goto failed;
+
 	/* spin_trylock may fail due to a parallel drain or IRQ reentrancy. */
 	pcp = pcp_spin_trylock(zone->per_cpu_pageset);
 	if (!pcp)
@@ -5428,6 +5453,7 @@ struct page *__alloc_frozen_pages_noprof(gfp_t gfp, unsigned int order,
 		.alloc_flags = alloc_flags,
 	};
 	unsigned int fastpath_alloc_flags = alloc_flags;
+	unsigned int zonelist_iter_cookie;
 
 	/* Other flags could be supported later if needed. */
 	if (WARN_ON(alloc_flags & ~(ALLOC_NOLOCK | ALLOC_NO_CODETAG)))
@@ -5457,6 +5483,7 @@ struct page *__alloc_frozen_pages_noprof(gfp_t gfp, unsigned int order,
 	 */
 	gfp = current_gfp_context(gfp);
 	alloc_gfp = gfp;
+	zonelist_iter_cookie = zonelist_iter_sample();
 	if (!prepare_alloc_pages(gfp, order, preferred_nid, nodemask, &ac,
 			&alloc_gfp, &fastpath_alloc_flags))
 		return NULL;
@@ -5473,6 +5500,11 @@ struct page *__alloc_frozen_pages_noprof(gfp_t gfp, unsigned int order,
 
 	/* First allocation attempt (or, for nolock, only attempt) */
 	page = get_page_from_freelist(alloc_gfp, order, fastpath_alloc_flags, &ac);
+	if (likely(page) && unlikely(check_retry_zonelist(zonelist_iter_cookie))) {
+		__free_frozen_pages(page, order,
+				    alloc_flags & ALLOC_NOLOCK ? FPI_NOLOCK : 0);
+		page = NULL;
+	}
 	if (likely(page) || (alloc_flags & ALLOC_NOLOCK))
 		goto out;
 
-- 
2.56.0

Re: [PATCH v4 5/8] mm/page_alloc: retry allocations that raced with a zonelist rebuild

[Vlastimil Babka (SUSE)]

On 10/28/26 22:19, Gregory Price (Meta) wrote:
> Onlining the first block of a zone rebuilds every zonelist in place,
> shifting the slots after the new zone.  The iterator approves a slot,
> then the allocator loads the zone from it, so a GFP_KERNEL request can
> land in ZONE_MOVABLE and a nodemask request on a node outside its mask:
> 
>   CPU 0                             CPU 1
>   prepare_alloc_pages()
>     first_zones_zonelist(nodemask {1})
>       slot 0 (NORMAL1) approved
>                                     online_pages()
>                                       build_all_zonelists()
>                                         [NORMAL1, NORMAL2, ...]
>                                         -> [MOVABLE1, NORMAL1, ...]
>   get_page_from_freelist()
>     zone = zonelist_zone(slot 0)    /* MOVABLE1 */
>     allocates from MOVABLE1
> 
> zonelist_update_seq detects the rebuild, but only the slowpath's failure
> exits check it.
> 
> Check it whenever an allocation succeeds too, and if it changed, free
> the page and retry, as the memcg charge failure path does.  Eligible
> pages are dropped as well: zonelists only change when memory comes or
> goes, and a retry is simpler than rechecking the page.
> 
> Freeing the page does not undo a block the allocation claimed.  A
> GFP_KERNEL request that lands in a fresh ZONE_MOVABLE converts a free
> block to MIGRATE_UNMOVABLE, which stays until the zone's movable free
> list runs out and a movable allocation takes it back.

Yeah that's fine.

> The fast and bulk paths read the seqlock's raw seqcount without waiting:
> an ALLOC_NOLOCK caller can be an NMI that interrupted the writer, and on
> PREEMPT_RT the seqlock accessor takes the spinlock.  A sample taken
> mid-write costs a retry, or NULL under ALLOC_NOLOCK.
> check_retry_zonelist() returns 0 when compiled out, so a backport
> without the hot-add change stays harmless.

Hmm wasn't even aware there's waiting possible otherwise for seqcounts.

> Fixes: 11cd8638c37f ("mm, page_alloc: remove stop_machine from build_all_zonelists")
> Cc: stable@vger.kernel.org
> Signed-off-by: Gregory Price (Meta) <gourry@gourry.net>

Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>

> ---
>  mm/page_alloc.c | 34 +++++++++++++++++++++++++++++++++-
>  1 file changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 24106c881ec2..33aab9deaaa1 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -4419,12 +4419,25 @@ static unsigned int zonelist_iter_begin(void)
>  	return 0;
>  }
>  
> +/*
> + * Never wait for the writer, which an ALLOC_NOLOCK caller in NMI may have
> + * interrupted. Read the raw seqcount: on PREEMPT_RT the seqlock accessor takes
> + * the spinlock while a write is in progress.
> + */
> +static unsigned int zonelist_iter_sample(void)
> +{
> +	if (IS_ENABLED(CONFIG_MEMORY_HOTPLUG))
> +		return raw_seqcount_begin(&zonelist_update_seq.seqcount.seqcount);
> +
> +	return 0;
> +}
> +
>  static unsigned int check_retry_zonelist(unsigned int seq)
>  {
>  	if (IS_ENABLED(CONFIG_MEMORY_HOTPLUG))
>  		return read_seqretry(&zonelist_update_seq, seq);
>  
> -	return seq;
> +	return 0;
>  }
>  
>  /* Perform direct synchronous page reclaim */
> @@ -5115,6 +5128,12 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
>  	warn_alloc(gfp_mask, ac->nodemask,
>  			"page allocation failure: order:%u", order);
>  got_pg:
> +	/* A rebuild during the walk may have handed out an ineligible zone. */
> +	if (page && check_retry_zonelist(zonelist_iter_cookie)) {
> +		__free_frozen_pages(page, order, FPI_NONE);
> +		page = NULL;
> +		goto restart;
> +	}
>  	return page;
>  }
>  
> @@ -5199,6 +5218,7 @@ unsigned long alloc_pages_bulk_noprof(gfp_t gfp, int preferred_nid,
>  	struct list_head *pcp_list;
>  	struct alloc_context ac;
>  	unsigned int alloc_flags = ALLOC_WMARK_LOW;
> +	unsigned int zonelist_iter_cookie;
>  	int nr_populated = 0, nr_account = 0;
>  
>  	/*
> @@ -5239,6 +5259,7 @@ unsigned long alloc_pages_bulk_noprof(gfp_t gfp, int preferred_nid,
>  	/* May set ALLOC_NOFRAGMENT, fragmentation will return 1 page. */
>  	gfp &= gfp_allowed_mask;
>  	gfp = current_gfp_context(gfp);
> +	zonelist_iter_cookie = zonelist_iter_sample();
>  	if (!prepare_alloc_pages(gfp, 0, preferred_nid, nodemask, &ac, &gfp, &alloc_flags))
>  		goto out;
>  
> @@ -5288,6 +5309,10 @@ unsigned long alloc_pages_bulk_noprof(gfp_t gfp, int preferred_nid,
>  	if (unlikely(!zone))
>  		goto failed;
>  
> +	/* The zone was chosen from a zonelist that a rebuild may have changed. */
> +	if (unlikely(check_retry_zonelist(zonelist_iter_cookie)))
> +		goto failed;
> +
>  	/* spin_trylock may fail due to a parallel drain or IRQ reentrancy. */
>  	pcp = pcp_spin_trylock(zone->per_cpu_pageset);
>  	if (!pcp)
> @@ -5428,6 +5453,7 @@ struct page *__alloc_frozen_pages_noprof(gfp_t gfp, unsigned int order,
>  		.alloc_flags = alloc_flags,
>  	};
>  	unsigned int fastpath_alloc_flags = alloc_flags;
> +	unsigned int zonelist_iter_cookie;
>  
>  	/* Other flags could be supported later if needed. */
>  	if (WARN_ON(alloc_flags & ~(ALLOC_NOLOCK | ALLOC_NO_CODETAG)))
> @@ -5457,6 +5483,7 @@ struct page *__alloc_frozen_pages_noprof(gfp_t gfp, unsigned int order,
>  	 */
>  	gfp = current_gfp_context(gfp);
>  	alloc_gfp = gfp;
> +	zonelist_iter_cookie = zonelist_iter_sample();
>  	if (!prepare_alloc_pages(gfp, order, preferred_nid, nodemask, &ac,
>  			&alloc_gfp, &fastpath_alloc_flags))
>  		return NULL;
> @@ -5473,6 +5500,11 @@ struct page *__alloc_frozen_pages_noprof(gfp_t gfp, unsigned int order,
>  
>  	/* First allocation attempt (or, for nolock, only attempt) */
>  	page = get_page_from_freelist(alloc_gfp, order, fastpath_alloc_flags, &ac);
> +	if (likely(page) && unlikely(check_retry_zonelist(zonelist_iter_cookie))) {
> +		__free_frozen_pages(page, order,
> +				    alloc_flags & ALLOC_NOLOCK ? FPI_NOLOCK : 0);
> +		page = NULL;
> +	}
>  	if (likely(page) || (alloc_flags & ALLOC_NOLOCK))
>  		goto out;
>  

[PATCH v4 6/8] mm/hugetlb: retry the pool walk when the zonelist is rebuilt

[Gregory Price (Meta)]

Onlining the first block of a zone rebuilds every zonelist in place,
shifting the slots after the new zone.  dequeue_hugetlb_folio_nodemask()
approves a slot by its node, then loads the zone from it, so a hugetlb
fault bound to one node can take a pool page from another:

  CPU 0                             CPU 1
  dequeue_hugetlb_folio_nodemask(nodemask {2})
    slot 3 (NORMAL2) approved
                                    online_pages()
                                      build_all_zonelists()
                                        [.., NORMAL1, NORMAL2]
                                        -> [.., MOVABLE1, NORMAL1, ...]
    zone = zonelist_zone(slot 3)    /* NORMAL1 */
    dequeue_hugetlb_folio_node_exact(h, 1)

Check zonelist_update_seq before using each zone, and on the way out
with nothing found, and restart the walk if it changed.  Checking before
the dequeue leaves nothing to put back.  Reset the last-tried node on
restart, so it is not skipped.

With kprobes holding the walk across the online, a fault bound to node 2
took a node 1 page in 3 of 3 runs, and a node 2 page in 3 of 3 with this
change.

Fixes: 11cd8638c37f ("mm, page_alloc: remove stop_machine from build_all_zonelists")
Cc: stable@vger.kernel.org
Signed-off-by: Gregory Price (Meta) <gourry@gourry.net>
---
 mm/hugetlb.c    | 13 ++++++++++---
 mm/page_alloc.c |  4 ++--
 mm/page_alloc.h |  3 +++
 3 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index b37d7d0db7fc..8edf323baf0f 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -1293,11 +1293,11 @@ static struct folio *dequeue_hugetlb_folio_node_exact(struct hstate *h,
 static struct folio *dequeue_hugetlb_folio_nodemask(struct hstate *h, gfp_t gfp_mask,
 							int nid, nodemask_t *nmask)
 {
-	unsigned int cpuset_mems_cookie;
+	unsigned int cpuset_mems_cookie, zonelist_cookie;
 	struct zonelist *zonelist;
 	struct zone *zone;
 	struct zoneref *z;
-	int node = NUMA_NO_NODE;
+	int node;
 
 	/* 'nid' should not be NUMA_NO_NODE. Try to catch any misuse of it and rectifiy. */
 	if (nid == NUMA_NO_NODE)
@@ -1306,10 +1306,16 @@ static struct folio *dequeue_hugetlb_folio_nodemask(struct hstate *h, gfp_t gfp_
 	zonelist = node_zonelist(nid, gfp_mask);
 
 retry_cpuset:
+	node = NUMA_NO_NODE;
 	cpuset_mems_cookie = read_mems_allowed_begin();
+	zonelist_cookie = zonelist_iter_begin();
 	for_each_zone_zonelist_nodemask(zone, z, zonelist, gfp_zone(gfp_mask), nmask) {
 		struct folio *folio;
 
+		/* A rebuild may have shifted a node outside @nmask into this slot. */
+		if (unlikely(check_retry_zonelist(zonelist_cookie)))
+			goto retry_cpuset;
+
 		if (!cpuset_zone_allowed(zone, gfp_mask))
 			continue;
 		/*
@@ -1324,7 +1330,8 @@ static struct folio *dequeue_hugetlb_folio_nodemask(struct hstate *h, gfp_t gfp_
 		if (folio)
 			return folio;
 	}
-	if (unlikely(read_mems_allowed_retry(cpuset_mems_cookie)))
+	if (unlikely(read_mems_allowed_retry(cpuset_mems_cookie) ||
+		     check_retry_zonelist(zonelist_cookie)))
 		goto retry_cpuset;
 
 	return NULL;
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 33aab9deaaa1..70830689048a 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -4411,7 +4411,7 @@ EXPORT_SYMBOL_GPL(fs_reclaim_release);
  */
 static DEFINE_SEQLOCK(zonelist_update_seq);
 
-static unsigned int zonelist_iter_begin(void)
+unsigned int zonelist_iter_begin(void)
 {
 	if (IS_ENABLED(CONFIG_MEMORY_HOTPLUG))
 		return read_seqbegin(&zonelist_update_seq);
@@ -4432,7 +4432,7 @@ static unsigned int zonelist_iter_sample(void)
 	return 0;
 }
 
-static unsigned int check_retry_zonelist(unsigned int seq)
+unsigned int check_retry_zonelist(unsigned int seq)
 {
 	if (IS_ENABLED(CONFIG_MEMORY_HOTPLUG))
 		return read_seqretry(&zonelist_update_seq, seq);
diff --git a/mm/page_alloc.h b/mm/page_alloc.h
index 2b1017a91928..53df6b8a0c00 100644
--- a/mm/page_alloc.h
+++ b/mm/page_alloc.h
@@ -265,6 +265,9 @@ extern void zone_pcp_disable(struct zone *zone);
 extern void zone_pcp_enable(struct zone *zone);
 extern void zone_pcp_init(struct zone *zone);
 
+unsigned int zonelist_iter_begin(void);
+unsigned int check_retry_zonelist(unsigned int seq);
+
 enum fallback_result {
 	/* Found suitable migratetype, *mt_out is valid. */
 	FALLBACK_FOUND,
-- 
2.56.0

Re: [PATCH v4 6/8] mm/hugetlb: retry the pool walk when the zonelist is rebuilt

[Vlastimil Babka (SUSE)]

On 10/28/26 22:19, Gregory Price (Meta) wrote:
> Onlining the first block of a zone rebuilds every zonelist in place,
> shifting the slots after the new zone.  dequeue_hugetlb_folio_nodemask()
> approves a slot by its node, then loads the zone from it, so a hugetlb
> fault bound to one node can take a pool page from another:
> 
>   CPU 0                             CPU 1
>   dequeue_hugetlb_folio_nodemask(nodemask {2})
>     slot 3 (NORMAL2) approved
>                                     online_pages()
>                                       build_all_zonelists()
>                                         [.., NORMAL1, NORMAL2]
>                                         -> [.., MOVABLE1, NORMAL1, ...]
>     zone = zonelist_zone(slot 3)    /* NORMAL1 */
>     dequeue_hugetlb_folio_node_exact(h, 1)
> 
> Check zonelist_update_seq before using each zone, and on the way out
> with nothing found, and restart the walk if it changed.  Checking before
> the dequeue leaves nothing to put back.  Reset the last-tried node on
> restart, so it is not skipped.
> 
> With kprobes holding the walk across the online, a fault bound to node 2
> took a node 1 page in 3 of 3 runs, and a node 2 page in 3 of 3 with this
> change.
> 
> Fixes: 11cd8638c37f ("mm, page_alloc: remove stop_machine from build_all_zonelists")
> Cc: stable@vger.kernel.org
> Signed-off-by: Gregory Price (Meta) <gourry@gourry.net>

LGTM, but...

> ---
>  mm/hugetlb.c    | 13 ++++++++++---
>  mm/page_alloc.c |  4 ++--
>  mm/page_alloc.h |  3 +++
>  3 files changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index b37d7d0db7fc..8edf323baf0f 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -1293,11 +1293,11 @@ static struct folio *dequeue_hugetlb_folio_node_exact(struct hstate *h,
>  static struct folio *dequeue_hugetlb_folio_nodemask(struct hstate *h, gfp_t gfp_mask,
>  							int nid, nodemask_t *nmask)
>  {
> -	unsigned int cpuset_mems_cookie;
> +	unsigned int cpuset_mems_cookie, zonelist_cookie;
>  	struct zonelist *zonelist;
>  	struct zone *zone;
>  	struct zoneref *z;
> -	int node = NUMA_NO_NODE;
> +	int node;
>  
>  	/* 'nid' should not be NUMA_NO_NODE. Try to catch any misuse of it and rectifiy. */
>  	if (nid == NUMA_NO_NODE)
> @@ -1306,10 +1306,16 @@ static struct folio *dequeue_hugetlb_folio_nodemask(struct hstate *h, gfp_t gfp_
>  	zonelist = node_zonelist(nid, gfp_mask);
>  
>  retry_cpuset:
> +	node = NUMA_NO_NODE;
>  	cpuset_mems_cookie = read_mems_allowed_begin();
> +	zonelist_cookie = zonelist_iter_begin();
>  	for_each_zone_zonelist_nodemask(zone, z, zonelist, gfp_zone(gfp_mask), nmask) {
>  		struct folio *folio;
>  
> +		/* A rebuild may have shifted a node outside @nmask into this slot. */
> +		if (unlikely(check_retry_zonelist(zonelist_cookie)))
> +			goto retry_cpuset;
> +
>  		if (!cpuset_zone_allowed(zone, gfp_mask))
>  			continue;
>  		/*
> @@ -1324,7 +1330,8 @@ static struct folio *dequeue_hugetlb_folio_nodemask(struct hstate *h, gfp_t gfp_
>  		if (folio)
>  			return folio;
>  	}
> -	if (unlikely(read_mems_allowed_retry(cpuset_mems_cookie)))
> +	if (unlikely(read_mems_allowed_retry(cpuset_mems_cookie) ||
> +		     check_retry_zonelist(zonelist_cookie)))
>  		goto retry_cpuset;
>  
>  	return NULL;
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 33aab9deaaa1..70830689048a 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -4411,7 +4411,7 @@ EXPORT_SYMBOL_GPL(fs_reclaim_release);
>   */
>  static DEFINE_SEQLOCK(zonelist_update_seq);
>  
> -static unsigned int zonelist_iter_begin(void)
> +unsigned int zonelist_iter_begin(void)
>  {
>  	if (IS_ENABLED(CONFIG_MEMORY_HOTPLUG))
>  		return read_seqbegin(&zonelist_update_seq);
> @@ -4432,7 +4432,7 @@ static unsigned int zonelist_iter_sample(void)
>  	return 0;
>  }
>  
> -static unsigned int check_retry_zonelist(unsigned int seq)
> +unsigned int check_retry_zonelist(unsigned int seq)
>  {
>  	if (IS_ENABLED(CONFIG_MEMORY_HOTPLUG))
>  		return read_seqretry(&zonelist_update_seq, seq);
> diff --git a/mm/page_alloc.h b/mm/page_alloc.h
> index 2b1017a91928..53df6b8a0c00 100644
> --- a/mm/page_alloc.h
> +++ b/mm/page_alloc.h
> @@ -265,6 +265,9 @@ extern void zone_pcp_disable(struct zone *zone);
>  extern void zone_pcp_enable(struct zone *zone);
>  extern void zone_pcp_init(struct zone *zone);
>  
> +unsigned int zonelist_iter_begin(void);
> +unsigned int check_retry_zonelist(unsigned int seq);

... is it possible to inline these here instead?

> +
>  enum fallback_result {
>  	/* Found suitable migratetype, *mt_out is valid. */
>  	FALLBACK_FOUND,

Re: [PATCH v4 6/8] mm/hugetlb: retry the pool walk when the zonelist is rebuilt

[Gregory Price (Meta)]

On 2026-10-29 19:08 +0100, Vlastimil Babka (SUSE) wrote:
> >  
> > +unsigned int zonelist_iter_begin(void);
> > +unsigned int check_retry_zonelist(unsigned int seq);
> 
> ... is it possible to inline these here instead?
> 

Reasonable, let me take a look.

-- 
~Gregory

[PATCH v4 7/8] mm/page_alloc: retry alloc_contig_pages() when the zonelist is rebuilt

[Gregory Price (Meta)]

Onlining the first block of a zone rebuilds every zonelist in place,
shifting the slots after the new zone.
alloc_contig_frozen_pages_noprof() approves a slot by the caller's gfp
and nodemask, then loads the zone from it:

  CPU 0                             CPU 1
  alloc_contig_frozen_pages_noprof(GFP_HIGHUSER | __GFP_THISNODE)
    slot 0 (NORMAL1) approved
                                    online_pages()
                                      build_all_zonelists()
                                        [NORMAL1]
                                        -> [MOVABLE1, NORMAL1]
    zone = zonelist_zone(slot 0)    /* MOVABLE1 */
    alloc_contig_frozen_range(MOVABLE1 pfns)

A gigantic hugetlb page can then land in ZONE_MOVABLE, and a nodemask
caller can get a range from another node.  Gigantic pages are kept out
of ZONE_MOVABLE since commit 79b63f12abcb ("mm, hugetlb: do not allocate
non-migrateable gigantic pages from movable zones").

Check zonelist_update_seq before using each zone, and restart the walk
if it changed, as the hugetlb pool walk does.  Check again before
escalating to the pass that may migrate hugetlb pages, so a pass a
rebuild cut short is redone rather than escalated.

With kprobes holding the walk across the online, the base kernel put the
1G page in ZONE_MOVABLE in 3 of 3 runs, and this change put it in
ZONE_NORMAL in 3 of 3.  Direct GFP_KERNEL calls with nodemask {1} and
{2} got a ZONE_MOVABLE range and a node 1 range on base, 3 of 3 each,
and neither with this change.

Fixes: 5e27a2df03b8 ("mm/page_alloc: add alloc_contig_pages()")
Cc: stable@vger.kernel.org
Signed-off-by: Gregory Price (Meta) <gourry@gourry.net>
---
 mm/page_alloc.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 70830689048a..7d06c6c4011a 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -7593,6 +7593,7 @@ struct page *alloc_contig_frozen_pages_noprof(unsigned long nr_pages,
 		gfp_t gfp_mask, int nid, nodemask_t *nodemask)
 {
 	unsigned long ret, pfn, flags;
+	unsigned int zonelist_cookie;
 	struct zonelist *zonelist;
 	struct zone *zone;
 	struct zoneref *z;
@@ -7601,8 +7602,13 @@ struct page *alloc_contig_frozen_pages_noprof(unsigned long nr_pages,
 
 retry:
 	zonelist = node_zonelist(nid, gfp_mask);
+	zonelist_cookie = zonelist_iter_begin();
 	for_each_zone_zonelist_nodemask(zone, z, zonelist,
 					gfp_zone(gfp_mask), nodemask) {
+		/* A rebuild may have shifted an ineligible zone into this slot. */
+		if (unlikely(check_retry_zonelist(zonelist_cookie)))
+			goto retry;
+
 		spin_lock_irqsave(&zone->lock, flags);
 
 		pfn = ALIGN(zone->zone_start_pfn, nr_pages);
@@ -7637,6 +7643,9 @@ struct page *alloc_contig_frozen_pages_noprof(unsigned long nr_pages,
 	 * second pass, we will try moving HugeTLB pages when those pages are
 	 * smaller than the requested contiguous region size.
 	 */
+	/* Redo a pass that a rebuild cut short before escalating. */
+	if (unlikely(check_retry_zonelist(zonelist_cookie)))
+		goto retry;
 	if (skip_hugetlb && skipped_hugetlb) {
 		skip_hugetlb = false;
 		goto retry;
-- 
2.56.0

Re: [PATCH v4 7/8] mm/page_alloc: retry alloc_contig_pages() when the zonelist is rebuilt

[Vlastimil Babka (SUSE)]

On 10/28/26 22:19, Gregory Price (Meta) wrote:
> Onlining the first block of a zone rebuilds every zonelist in place,
> shifting the slots after the new zone.
> alloc_contig_frozen_pages_noprof() approves a slot by the caller's gfp
> and nodemask, then loads the zone from it:
> 
>   CPU 0                             CPU 1
>   alloc_contig_frozen_pages_noprof(GFP_HIGHUSER | __GFP_THISNODE)
>     slot 0 (NORMAL1) approved
>                                     online_pages()
>                                       build_all_zonelists()
>                                         [NORMAL1]
>                                         -> [MOVABLE1, NORMAL1]
>     zone = zonelist_zone(slot 0)    /* MOVABLE1 */
>     alloc_contig_frozen_range(MOVABLE1 pfns)
> 
> A gigantic hugetlb page can then land in ZONE_MOVABLE, and a nodemask
> caller can get a range from another node.  Gigantic pages are kept out
> of ZONE_MOVABLE since commit 79b63f12abcb ("mm, hugetlb: do not allocate
> non-migrateable gigantic pages from movable zones").
> 
> Check zonelist_update_seq before using each zone, and restart the walk
> if it changed, as the hugetlb pool walk does.  Check again before
> escalating to the pass that may migrate hugetlb pages, so a pass a
> rebuild cut short is redone rather than escalated.
> 
> With kprobes holding the walk across the online, the base kernel put the
> 1G page in ZONE_MOVABLE in 3 of 3 runs, and this change put it in
> ZONE_NORMAL in 3 of 3.  Direct GFP_KERNEL calls with nodemask {1} and
> {2} got a ZONE_MOVABLE range and a node 1 range on base, 3 of 3 each,
> and neither with this change.
> 
> Fixes: 5e27a2df03b8 ("mm/page_alloc: add alloc_contig_pages()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Gregory Price (Meta) <gourry@gourry.net>

Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>

> ---
>  mm/page_alloc.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 70830689048a..7d06c6c4011a 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -7593,6 +7593,7 @@ struct page *alloc_contig_frozen_pages_noprof(unsigned long nr_pages,
>  		gfp_t gfp_mask, int nid, nodemask_t *nodemask)
>  {
>  	unsigned long ret, pfn, flags;
> +	unsigned int zonelist_cookie;
>  	struct zonelist *zonelist;
>  	struct zone *zone;
>  	struct zoneref *z;
> @@ -7601,8 +7602,13 @@ struct page *alloc_contig_frozen_pages_noprof(unsigned long nr_pages,
>  
>  retry:
>  	zonelist = node_zonelist(nid, gfp_mask);
> +	zonelist_cookie = zonelist_iter_begin();
>  	for_each_zone_zonelist_nodemask(zone, z, zonelist,
>  					gfp_zone(gfp_mask), nodemask) {
> +		/* A rebuild may have shifted an ineligible zone into this slot. */
> +		if (unlikely(check_retry_zonelist(zonelist_cookie)))
> +			goto retry;
> +
>  		spin_lock_irqsave(&zone->lock, flags);
>  
>  		pfn = ALIGN(zone->zone_start_pfn, nr_pages);
> @@ -7637,6 +7643,9 @@ struct page *alloc_contig_frozen_pages_noprof(unsigned long nr_pages,
>  	 * second pass, we will try moving HugeTLB pages when those pages are
>  	 * smaller than the requested contiguous region size.
>  	 */
> +	/* Redo a pass that a rebuild cut short before escalating. */
> +	if (unlikely(check_retry_zonelist(zonelist_cookie)))
> +		goto retry;
>  	if (skip_hugetlb && skipped_hugetlb) {
>  		skip_hugetlb = false;
>  		goto retry;

[PATCH v4 8/8] mm/mempolicy: read the zoneref slot once in mempolicy_slab_node()

[Gregory Price (Meta)]

For a bind or preferred-many policy, mempolicy_slab_node() tests the
first matching slot's zone for NULL, then reads the slot again to look
up the zone's node. Offlining a zone's last block rebuilds the
zonelists in place, so the second read can return the terminator:

  CPU 0                             CPU 1
  mempolicy_slab_node()
    zonelist_zone(slot 4)
      = node 2 NORMAL
                                    offline_pages()
                                      build_all_zonelists()
                                        slot 4 = NULL
    zonelist_node_idx(slot 4)
      zone_to_nid(NULL)

Current GCC merges the two reads, but nothing requires it. With
mm/mempolicy.o built with -fno-inline -fno-ipa-sra, holding a bound
task's slab_strict_numa allocation between the reads while a movable
block earlier in the zonelist goes offline oopses every time:

  BUG: kernel NULL pointer dereference, address: 0000000000000060
  RIP: 0010:zone_to_nid+0x0/0x10
  Call Trace:
   kmem_cache_alloc_noprof+0x294/0x610
   do_getname+0x34/0x230

mempolicy_slab_node() and zonelist_node_idx() are tail calls here, so
neither appears in the trace.

Read the slot once and use that zone.

Fixes: 11cd8638c37f ("mm, page_alloc: remove stop_machine from build_all_zonelists")
Cc: stable@vger.kernel.org
Signed-off-by: Gregory Price (Meta) <gourry@gourry.net>
---
 mm/mempolicy.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 0884c629670d..cd63c3e2c029 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -2176,6 +2176,7 @@ unsigned int mempolicy_slab_node(void)
 	case MPOL_PREFERRED_MANY:
 	{
 		struct zoneref *z;
+		struct zone *zone;
 
 		/*
 		 * Follow bind policy behavior and start allocation at the
@@ -2186,7 +2187,8 @@ unsigned int mempolicy_slab_node(void)
 		zonelist = &NODE_DATA(node)->node_zonelists[ZONELIST_FALLBACK];
 		z = first_zones_zonelist(zonelist, highest_zoneidx,
 							&policy->nodes);
-		return zonelist_zone(z) ? zonelist_node_idx(z) : node;
+		zone = zonelist_zone(z);
+		return zone ? zone_to_nid(zone) : node;
 	}
 	case MPOL_LOCAL:
 		return node;
-- 
2.56.0

Re: [PATCH v4 8/8] mm/mempolicy: read the zoneref slot once in mempolicy_slab_node()

[Vlastimil Babka (SUSE)]

On 10/28/26 22:19, Gregory Price (Meta) wrote:
> For a bind or preferred-many policy, mempolicy_slab_node() tests the
> first matching slot's zone for NULL, then reads the slot again to look
> up the zone's node. Offlining a zone's last block rebuilds the
> zonelists in place, so the second read can return the terminator:
> 
>   CPU 0                             CPU 1
>   mempolicy_slab_node()
>     zonelist_zone(slot 4)
>       = node 2 NORMAL
>                                     offline_pages()
>                                       build_all_zonelists()
>                                         slot 4 = NULL
>     zonelist_node_idx(slot 4)
>       zone_to_nid(NULL)
> 
> Current GCC merges the two reads, but nothing requires it. With
> mm/mempolicy.o built with -fno-inline -fno-ipa-sra, holding a bound
> task's slab_strict_numa allocation between the reads while a movable
> block earlier in the zonelist goes offline oopses every time:
> 
>   BUG: kernel NULL pointer dereference, address: 0000000000000060
>   RIP: 0010:zone_to_nid+0x0/0x10
>   Call Trace:
>    kmem_cache_alloc_noprof+0x294/0x610
>    do_getname+0x34/0x230
> 
> mempolicy_slab_node() and zonelist_node_idx() are tail calls here, so
> neither appears in the trace.
> 
> Read the slot once and use that zone.
> 
> Fixes: 11cd8638c37f ("mm, page_alloc: remove stop_machine from build_all_zonelists")
> Cc: stable@vger.kernel.org
> Signed-off-by: Gregory Price (Meta) <gourry@gourry.net>

Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>

> ---
>  mm/mempolicy.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index 0884c629670d..cd63c3e2c029 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -2176,6 +2176,7 @@ unsigned int mempolicy_slab_node(void)
>  	case MPOL_PREFERRED_MANY:
>  	{
>  		struct zoneref *z;
> +		struct zone *zone;
>  
>  		/*
>  		 * Follow bind policy behavior and start allocation at the
> @@ -2186,7 +2187,8 @@ unsigned int mempolicy_slab_node(void)
>  		zonelist = &NODE_DATA(node)->node_zonelists[ZONELIST_FALLBACK];
>  		z = first_zones_zonelist(zonelist, highest_zoneidx,
>  							&policy->nodes);
> -		return zonelist_zone(z) ? zonelist_node_idx(z) : node;
> +		zone = zonelist_zone(z);
> +		return zone ? zone_to_nid(zone) : node;
>  	}
>  	case MPOL_LOCAL:
>  		return node;