[PATCH 0/9] mm, page_alloc: several stable fixes

[PATCH 0/9] mm, page_alloc: several stable fixes

[Vlastimil Babka (SUSE)]

A bunch of fixes from Josef, filtered for page allocator by David.

They seem worthy of Cc: stable to me, but IMHO don't need to be sent
urgently as the issues mostly need root actions (memory offline) to
trigger and/or CONFIG_DEBUG_VM to result in a denial of service.

I have tweaked some commit logs and a comment in one case.

Based on mm/mm-unstable.

Signed-off-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
---
Josef Bacik (9):
      mm/page_alloc: don't undo a racing start_isolate_page_range()
      mm/page_alloc: clear ->private of deferred free pages
      mm: page_frag: refill the cache for large fragments
      mm/page_alloc: keep the pagesets of an empty zone
      mm, page_alloc: pass zone pointer to trace_reclaim_retry_zone
      mm/compaction: trust zone->contiguous only in the span
      mm: page_isolation: do not take slab pages for free
      mm/memory_hotplug: resize zones under their seqlock
      mm/page_alloc: check pageblock pfns under span seqlock

 include/trace/events/oom.h |  8 ++++----
 mm/memory_hotplug.c        | 51 +++++++++++++++++++++++++++++++---------------
 mm/mm_init.c               |  4 ++--
 mm/page_alloc.c            | 43 ++++++++++++++++++++++++++++++--------
 mm/page_alloc.h            |  7 ++++++-
 mm/page_frag_cache.c       | 21 +++++++++++--------
 mm/page_isolation.c        |  7 +++++++
 7 files changed, 101 insertions(+), 40 deletions(-)
---
base-commit: 39274b454e9993cea319f4ac342a8082a9ee1730
change-id: 20261027-pgalloc-ai-hotfixes-59bcd873a355

[PATCH 1/9] mm/page_alloc: don't undo a racing start_isolate_page_range()

[Vlastimil Babka (SUSE)]

From: Josef Bacik <josef@toxicpanda.com>

start_isolate_page_range() fails with -EBUSY if one of the pageblocks is
already isolated, i.e. if somebody else is working on it. It restores
the pageblocks which it has isolated itself before it returns, as its
kernel-doc says:

  We then clean up by restoring the migration type on pageblocks we may
  have modified and return -EBUSY to caller. This prevents two threads
  from simultaneously working on overlapping ranges.

alloc_contig_frozen_range() nevertheless calls undo_isolate_page_range()
for the whole range in this case, which clears every isolated pageblock
it finds. The only ones left at this point belong to the other user of
the range:

  CPU0 offline_pages()                CPU1 alloc_contig_frozen_range()
  start_isolate_page_range(A)
    pageblock X: isolated
  migrate ...
                                      start_isolate_page_range(B)
                                        X is isolated: -EBUSY
                                        (own pageblocks restored)
                                      undo_isolate_page_range(B)
                                        X: not isolated any more
  test_pages_isolated(A): -EBUSY
  (again and again)

offline_pages() repeats the migration and test_pages_isolated() until the
latter succeeds, which is now impossible and the task spins until it gets
a signal, while the free pages of X are handed out by the allocator again.

If X is released after the last test_pages_isolated(),
__offline_isolated_pages() takes pages which are accounted as free off
the free lists as MIGRATE_ISOLATE. Thus NR_FREE_PAGES stays too high, or
hits BUG_ON(!PageBuddy(page)) if one of them has been allocated.

For another alloc_contig_frozen_range() or cma_alloc() the result is a
spurious -EBUSY.

Return the error, like the function did until it was reworked.

Fixes: b2c9e2fbba32 ("mm: make alloc_contig_range work at pageblock granularity")
Cc: <stable@vger.kernel.org>
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
Signed-off-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
---
 mm/page_alloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 36079e092875..ee013db45ed1 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -7329,7 +7329,7 @@ int alloc_contig_frozen_range_noprof(unsigned long start, unsigned long end,
 
 	ret = start_isolate_page_range(start, end, mode);
 	if (ret)
-		goto done;
+		return ret;
 
 	drain_all_pages(cc.zone);
 

-- 
2.56.0

[PATCH 2/9] mm/page_alloc: clear ->private of deferred free pages

[Vlastimil Babka (SUSE)]

From: Josef Bacik <josef@toxicpanda.com>

free_pages_nolock() and free_frozen_pages_nolock() put the page on
zone->trylock_free_pages if they get neither the pcp lock nor zone->lock.
add_page_to_zone_llist() keeps the order of the page in page->private.
The next free_one_page() which does get zone->lock passes these pages to
split_large_buddy() with ->private still set.

__free_one_page() writes ->private of the head of the resulting free page
only. If the deferred page is merged with its lower buddy, it becomes a
tail of that free page and keeps the order in ->private.

All other free pages which are not the head of a buddy have a ->private
of zero, and since commit 819bfbd47e49 ("mm/page_alloc: remove
set_page_private() in prep_compound_tail()") the allocator relies on it.
When the stale page is allocated again as a tail of a larger page:

 - prep_compound_tail() hits VM_WARN_ON_ONCE(tail->private);
 - with the page checks enabled, __free_pages_prepare() reports
   "BUG: Bad page state", "nonzero private", and leaks the whole page;
 - without them the value is seen by whoever uses that page next, e.g. as
   folio->private of a folio which is split off a large folio.

Only pages of an order above zero are affected. Those can be freed
through _nolock() by the stack depot (order 2) and by SLUB (a new slab
which cannot be added to the partial list by kmalloc_nolock()).

Clear ->private when the page is taken off the list.

Fixes: 8c57b687e833 ("mm, bpf: Introduce free_pages_nolock()")
Cc: <stable@vger.kernel.org>
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
Signed-off-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
---
 mm/page_alloc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index ee013db45ed1..9a4d3352c037 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1562,6 +1562,8 @@ static void free_one_page(struct zone *zone, struct page *page,
 		llist_for_each_entry_safe(p, tmp, llnode, pcp_llist) {
 			unsigned int p_order = p->private;
 
+			/* It may end up as a tail of the merged free page. */
+			p->private = 0;
 			split_large_buddy(zone, p, page_to_pfn(p), p_order, fpi_flags);
 			__count_vm_events(PGFREE, 1 << p_order);
 		}

-- 
2.56.0

[PATCH 3/9] mm: page_frag: refill the cache for large fragments

[Vlastimil Babka (SUSE)]

From: Josef Bacik <josef@toxicpanda.com>

A fragment which is larger than PAGE_SIZE can only be served from the
32K page of a page_frag_cache. If memory is low and the cache had to fall
back to a single page, __page_frag_alloc_align() returns NULL and keeps
that page.

Since the offset counts up, this check comes first and is made whenever
the fragment does not fit behind nc->offset, no matter how big the page
of the cache is. So the cache is never refilled on behalf of a large
fragment: with fragsz = 10000 the first three allocations succeed, the
fourth one and all the following ones fail although memory is plentiful.
nc->offset does not move either, so this lasts until smaller fragments
have used up the rest of the page, which never happens if the cache has
no other user.

The nfp driver allocates its receive buffers with napi_alloc_frag() and
netdev_alloc_frag(). They are larger than a page with a jumbo MTU.

Only give up if the fragment does not fit into the page at all, which is
what the comment describes. The fourth allocation in the scenario above
with a 32k page will thus now proceed to refill the cache.

Fixes: 8218f62c9c9b ("mm: page_frag: use initial zero offset for page_frag_alloc_align()")
Cc: <stable@vger.kernel.org>
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
Reviewed-by: Johannes Weiner <hannes@cmpxchg.org>
[vbabka@kernel.org: make the comment more obvious]
Signed-off-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
---
 mm/page_frag_cache.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/mm/page_frag_cache.c b/mm/page_frag_cache.c
index e63efe78b7d4..8914437ddc51 100644
--- a/mm/page_frag_cache.c
+++ b/mm/page_frag_cache.c
@@ -119,15 +119,20 @@ void *__page_frag_alloc_align(struct page_frag_cache *nc,
 	size = PAGE_SIZE << encoded_page_decode_order(encoded_page);
 	offset = __ALIGN_KERNEL_MASK(nc->offset, ~align_mask);
 	if (unlikely(offset + fragsz > size)) {
-		if (unlikely(fragsz > PAGE_SIZE)) {
+		if (unlikely(fragsz > size)) {
 			/*
-			 * The caller is trying to allocate a fragment
-			 * with fragsz > PAGE_SIZE but the cache isn't big
-			 * enough to satisfy the request, this may
-			 * happen in low memory conditions.
-			 * We don't release the cache page because
-			 * it could make memory pressure worse
-			 * so we simply return NULL here.
+			 * The caller is trying to allocate a fragment that's
+			 * larger than the whole cache page.
+			 *
+			 * Either fragsz is larger than PAGE_FRAG_CACHE_MAX_SIZE
+			 * so this cannot ever succeed.
+			 *
+			 * Or the refill has fallen back to order-0 due to a low
+			 * memory condition. Then we don't want to release the
+			 * cache page to attempt a new large-size refill because
+			 * it could make memory pressure worse.
+			 *
+			 * So in both cases we simply return NULL here.
 			 */
 			return NULL;
 		}

-- 
2.56.0

[PATCH 4/9] mm/page_alloc: keep the pagesets of an empty zone

[Vlastimil Babka (SUSE)]

From: Josef Bacik <josef@toxicpanda.com>

When the last memory of a zone goes offline, zone_pcp_reset() frees
zone->per_cpu_pageset and zone->per_cpu_zonestats. Nothing waits for those
who found the zone populated and still use the two pointers. They take
neither the memory hotplug lock nor any other lock which offline_pages()
holds, and some of them can sleep:

  CPU0 vmstat_update()                CPU1 offline_pages()
  refresh_cpu_vm_stats()
    for_each_populated_zone(zone)
      pcp = zone->per_cpu_pageset
      cond_resched()
                                      adjust_present_page_count()
                                      zone_pcp_reset(zone)
                                        free_percpu(zone->per_cpu_pageset)
      decay_pcp_high(zone, pcp)
      __this_cpu_write(pcp->expire, 0)

The same goes for drain_all_pages(NULL), which takes pcp->lock, for
zone_pcp_update(), show_free_areas() and the readers and writers of the
zonestats. If the per-cpu memory has got a new owner in the meantime, it
is corrupted.

The pagesets are only freed because setup_zone_pageset() would otherwise
leak them when the zone gets memory again. Keep them instead and let
setup_zone_pageset() and zone_pcp_init() use them again, like it is done
with pgdat->per_cpu_nodestats. Their lists are empty, and their limits
have been calculated for a zone without memory by the
init_per_zone_wmark_min() of offline_pages(), which is what
setup_zone_pageset() would come up with as well.

Fixes: 340175b7d14d ("mm/hotplug: free zone->pageset when a zone becomes empty")
Cc: <stable@vger.kernel.org>
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
Reviewed-by: Johannes Weiner <hannes@cmpxchg.org>
Signed-off-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
---
 mm/page_alloc.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 9a4d3352c037..13211b8caada 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6312,6 +6312,10 @@ void __meminit setup_zone_pageset(struct zone *zone)
 {
 	int cpu;
 
+	/* The zone has been populated before, see zone_pcp_reset(). */
+	if (zone->per_cpu_pageset != &boot_pageset)
+		return;
+
 	/* Size may be 0 on !SMP && !NUMA */
 	if (sizeof(struct per_cpu_zonestat) > 0)
 		zone->per_cpu_zonestats = alloc_percpu(struct per_cpu_zonestat);
@@ -6404,6 +6408,10 @@ void __init setup_per_cpu_pageset(void)
 
 __meminit void zone_pcp_init(struct zone *zone)
 {
+	/* A node which is added again, see zone_pcp_reset(). */
+	if (zone->per_cpu_pageset && zone->per_cpu_pageset != &boot_pageset)
+		return;
+
 	/*
 	 * per cpu subsystem is not up at this point. The following code
 	 * relies on the ability of the linker to provide the
@@ -7684,6 +7692,12 @@ void zone_pcp_enable(struct zone *zone)
 	mutex_unlock(&pcp_batch_high_lock);
 }
 
+/*
+ * The zone has become empty. Its pagesets and zonestats are not freed: nothing
+ * keeps somebody who found the zone populated, such as refresh_cpu_vm_stats()
+ * or drain_all_pages(), from still using them. They are used again when the
+ * zone gets memory again.
+ */
 void zone_pcp_reset(struct zone *zone)
 {
 	int cpu;
@@ -7694,12 +7708,6 @@ void zone_pcp_reset(struct zone *zone)
 			pzstats = per_cpu_ptr(zone->per_cpu_zonestats, cpu);
 			drain_zonestat(zone, pzstats);
 		}
-		free_percpu(zone->per_cpu_pageset);
-		zone->per_cpu_pageset = &boot_pageset;
-		if (zone->per_cpu_zonestats != &boot_zonestats) {
-			free_percpu(zone->per_cpu_zonestats);
-			zone->per_cpu_zonestats = &boot_zonestats;
-		}
 	}
 }
 

-- 
2.56.0

[PATCH 5/9] mm, page_alloc: pass zone pointer to trace_reclaim_retry_zone

[Vlastimil Babka (SUSE)]

From: Josef Bacik <josef@toxicpanda.com>

should_reclaim_retry() walks a zonelist and passes the zoneref pointer
to the reclaim_retry_zone tracepoint, which loads the zone from the
entry again to find the node.

Memory hotplug rebuilds the zonelists in place while allocations are
under way. When a zone in front of the zoneref entry has become empty,
the entries move up, and this one can be the end marker by the time the
tracepoint looks at it. zonelist_node_idx() then dereferences a NULL
zone. Short of that, the event can name another zone than the one its
numbers belong to.

Pass the zone pointer instead of the zoneref pointer.

Fixes: d379f01de095 ("oom, trace: add oom detection tracepoints")
Cc: <stable@vger.kernel.org>
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
Signed-off-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
---
 include/trace/events/oom.h | 8 ++++----
 mm/page_alloc.c            | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/trace/events/oom.h b/include/trace/events/oom.h
index 9f0a5d1482c4..cbcf6fe4cc61 100644
--- a/include/trace/events/oom.h
+++ b/include/trace/events/oom.h
@@ -33,7 +33,7 @@ TRACE_EVENT(oom_score_adj_update,
 
 TRACE_EVENT(reclaim_retry_zone,
 
-	TP_PROTO(struct zoneref *zoneref,
+	TP_PROTO(struct zone *zone,
 		int order,
 		unsigned long reclaimable,
 		unsigned long available,
@@ -41,7 +41,7 @@ TRACE_EVENT(reclaim_retry_zone,
 		int no_progress_loops,
 		bool wmark_check),
 
-	TP_ARGS(zoneref, order, reclaimable, available, min_wmark, no_progress_loops, wmark_check),
+	TP_ARGS(zone, order, reclaimable, available, min_wmark, no_progress_loops, wmark_check),
 
 	TP_STRUCT__entry(
 		__field(	int, node)
@@ -55,8 +55,8 @@ TRACE_EVENT(reclaim_retry_zone,
 	),
 
 	TP_fast_assign(
-		__entry->node = zonelist_node_idx(zoneref);
-		__entry->zone_idx = zonelist_zone_idx(zoneref);
+		__entry->node = zone_to_nid(zone);
+		__entry->zone_idx = zone_idx(zone);
 		__entry->order = order;
 		__entry->reclaimable = reclaimable;
 		__entry->available = available;
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 13211b8caada..8f8885165045 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -4675,7 +4675,7 @@ should_reclaim_retry(gfp_t gfp_mask, unsigned order,
 		 */
 		wmark = __zone_watermark_ok(zone, order, min_wmark,
 				ac->highest_zoneidx, alloc_flags, available);
-		trace_reclaim_retry_zone(z, order, reclaimable,
+		trace_reclaim_retry_zone(zone, order, reclaimable,
 				available, min_wmark, *no_progress_loops, wmark);
 		if (wmark) {
 			ret = true;

-- 
2.56.0

[PATCH 6/9] mm/compaction: trust zone->contiguous only in the span

[Vlastimil Babka (SUSE)]

From: Josef Bacik <josef@toxicpanda.com>

compact_zone() keeps the positions of its scanners in cc->migrate_pfn and
cc->free_pfn for the whole run. Memory offlining can shrink the zone in
the meantime. At the end of remove_pfn_range_from_zone(),
set_zone_contiguous() only looks at what is left of the zone. If that has
no hole, or if nothing is left, zone->contiguous is set again, and
pageblock_pfn_to_page() returns the memmap of any pfn unchecked, also of
one that the zone does not span any more.

The memmap of the range that left the zone is poisoned if CONFIG_DEBUG_VM
is set. Once the memory has been removed, the memmap and the pageblock
flags are freed. Ejecting DIMMs that were online in ZONE_MOVABLE while
other tasks write to /proc/sys/vm/compact_memory, no debug options set:

  BUG: unable to handle page fault for address: ffffea0004000008
  #PF: supervisor read access in kernel mode
  #PF: error_code(0x0000) - not-present page
  PGD 3fee9067 P4D 3fee9067 PUD 3fee8067 PMD 0
  RIP: 0010:isolate_migratepages_block+0x32f/0x1280
  Call Trace:
   compact_zone+0x673/0x1140
   compact_node+0xb6/0x130
   sysctl_compaction_handler+0x77/0xd0

kcompactd and direct compaction walk into such ranges in the same way.

Only trust the flag for a pfn that the zone spans. Everything else goes
to __pageblock_pfn_to_page(), which finds the section offline or gone.
The span is written before the flag is set, so read them in the opposite
order.

Fixes: 7cf91a98e607 ("mm/compaction: speed up pageblock_pfn_to_page() when zone is contiguous")
Cc: <stable@vger.kernel.org>
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
Signed-off-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
---
 mm/mm_init.c    | 4 ++--
 mm/page_alloc.h | 7 ++++++-
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/mm/mm_init.c b/mm/mm_init.c
index df1f888e59b8..262e4e5e97be 100644
--- a/mm/mm_init.c
+++ b/mm/mm_init.c
@@ -2270,8 +2270,8 @@ void set_zone_contiguous(struct zone *zone)
 		cond_resched();
 	}
 
-	/* We confirm that there is no hole */
-	zone->contiguous = true;
+	/* We confirm that there is no hole, see pageblock_pfn_to_page() */
+	smp_store_release(&zone->contiguous, true);
 }
 
 /*
diff --git a/mm/page_alloc.h b/mm/page_alloc.h
index 2b1017a91928..e24d1203c514 100644
--- a/mm/page_alloc.h
+++ b/mm/page_alloc.h
@@ -216,7 +216,12 @@ extern struct page *__pageblock_pfn_to_page(unsigned long start_pfn,
 static inline struct page *pageblock_pfn_to_page(unsigned long start_pfn,
 				unsigned long end_pfn, struct zone *zone)
 {
-	if (zone->contiguous)
+	/*
+	 * A scanner may still be at a pfn that has left the zone: the zone is
+	 * shrunk before set_zone_contiguous() says that it has no holes.
+	 */
+	if (smp_load_acquire(&zone->contiguous) &&
+	    zone_spans_pfn(zone, start_pfn))
 		return pfn_to_page(start_pfn);
 
 	return __pageblock_pfn_to_page(start_pfn, end_pfn, zone);

-- 
2.56.0

[PATCH 7/9] mm: page_isolation: do not take slab pages for free

[Vlastimil Babka (SUSE)]

From: Josef Bacik <josef@toxicpanda.com>

page_is_unmovable() regards a page with a reference count of zero as free
or about to be freed. Since slab allocates frozen pages, they will
be wrongly considered to be freed if order 0 (higher-order compound
pages are caught earlier).

If such slabs are the only unmovable pages of a memory block, memory
offlining now isolates the block, this can lead to offline_pages()
looping without progress.

Thus detect the slab pages as removable to return -EBUSY, as would
happen before commit 9aec2fb0fd5e ("slab: allocate frozen pages").

Fixes: 9aec2fb0fd5e ("slab: allocate frozen pages")
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
Signed-off-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
---
 mm/memory_hotplug.c | 7 +++++++
 mm/page_isolation.c | 7 +++++++
 2 files changed, 14 insertions(+)

diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 796af1028ee2..e773a7d54165 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1847,6 +1847,13 @@ static int scan_movable_pages(unsigned long start, unsigned long end,
 		if (PageOffline(page) && page_count(page))
 			return -EBUSY;
 
+		/*
+		 * A page that was allocated, but was not a slab page yet, when
+		 * its pageblock was isolated.
+		 */
+		if (PageSlab(page))
+			return -EBUSY;
+
 		folio = page_folio(page);
 		if (!folio_test_hugetlb(folio))
 			continue;
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index 5aaad037384e..f4f0571607ab 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -74,6 +74,13 @@ bool page_is_unmovable(struct zone *zone, struct page *page,
 		return false;
 	}
 
+	/*
+	 * Slab pages are frozen: do not take their reference count of zero
+	 * for that of a free page.
+	 */
+	if (PageSlab(page))
+		return true;
+
 	/*
 	 * We can't use page_count without pin a page
 	 * because another CPU can free compound page.

-- 
2.56.0

[PATCH 8/9] mm/memory_hotplug: resize zones under their seqlock

[Vlastimil Babka (SUSE)]

From: Josef Bacik <josef@toxicpanda.com>

With CONFIG_DEBUG_VM the page allocator checks that a page which is
freed, split or allocated lies within its zone. The check reads
zone->zone_start_pfn and zone->spanned_pages under zone->span_seqlock, as
memory hotplug can change the two at any time.

Commit 27cacaad16c5 ("mm,memory_hotplug: drop unneeded locking") removed
the write side, since all resizing is serialized by the hotplug lock. That
leaves the reader without protection. When memory below the start of a
zone goes online, the start is lowered first and the size is raised
afterwards. When the lowest memory of a zone goes offline, the size is
reduced first. In between, the zone seems to end before its last pages. If
one of them goes through the allocator at that moment, it is reported as
being outside of the zone and VM_BUG_ON_PAGE() kills the kernel.

Write the two fields in a write section of the seqlock. Interrupts are
disabled as pages are freed from interrupts, too, and the reader would
wait for the writer which it has interrupted.

Fixes: 27cacaad16c5 ("mm,memory_hotplug: drop unneeded locking")
Cc: <stable@vger.kernel.org>
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
Signed-off-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
---
 mm/memory_hotplug.c | 44 ++++++++++++++++++++++++++++----------------
 1 file changed, 28 insertions(+), 16 deletions(-)

diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index e773a7d54165..66f181d98ebd 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -467,6 +467,22 @@ static unsigned long find_biggest_section_pfn(int nid, struct zone *zone,
 	return 0;
 }
 
+/*
+ * The page allocator checks pages against the span of their zone while the
+ * zone is resized, see page_outside_zone_boundaries(). It must not see the
+ * new value of one field together with the old value of the other one.
+ */
+static void zone_set_span(struct zone *zone, unsigned long start_pfn,
+			  unsigned long spanned_pages)
+{
+	unsigned long flags;
+
+	write_seqlock_irqsave(&zone->span_seqlock, flags);
+	zone->zone_start_pfn = start_pfn;
+	zone->spanned_pages = spanned_pages;
+	write_sequnlock_irqrestore(&zone->span_seqlock, flags);
+}
+
 static void shrink_zone_span(struct zone *zone, unsigned long start_pfn,
 			     unsigned long end_pfn)
 {
@@ -482,13 +498,10 @@ static void shrink_zone_span(struct zone *zone, unsigned long start_pfn,
 		 */
 		pfn = find_smallest_section_pfn(nid, zone, end_pfn,
 						zone_end_pfn(zone));
-		if (pfn) {
-			zone->spanned_pages = zone_end_pfn(zone) - pfn;
-			zone->zone_start_pfn = pfn;
-		} else {
-			zone->zone_start_pfn = 0;
-			zone->spanned_pages = 0;
-		}
+		if (pfn)
+			zone_set_span(zone, pfn, zone_end_pfn(zone) - pfn);
+		else
+			zone_set_span(zone, 0, 0);
 	} else if (zone_end_pfn(zone) == end_pfn) {
 		/*
 		 * If the section is biggest section in the zone, it need
@@ -499,11 +512,10 @@ static void shrink_zone_span(struct zone *zone, unsigned long start_pfn,
 		pfn = find_biggest_section_pfn(nid, zone, zone->zone_start_pfn,
 					       start_pfn);
 		if (pfn)
-			zone->spanned_pages = pfn - zone->zone_start_pfn + 1;
-		else {
-			zone->zone_start_pfn = 0;
-			zone->spanned_pages = 0;
-		}
+			zone_set_span(zone, zone->zone_start_pfn,
+				      pfn - zone->zone_start_pfn + 1);
+		else
+			zone_set_span(zone, 0, 0);
 	}
 }
 
@@ -696,12 +708,12 @@ static void online_pages_range(unsigned long start_pfn, unsigned long nr_pages)
 static void __meminit resize_zone_range(struct zone *zone, unsigned long start_pfn,
 		unsigned long nr_pages)
 {
-	unsigned long old_end_pfn = zone_end_pfn(zone);
+	unsigned long end_pfn = max(start_pfn + nr_pages, zone_end_pfn(zone));
 
-	if (zone_is_empty(zone) || start_pfn < zone->zone_start_pfn)
-		zone->zone_start_pfn = start_pfn;
+	if (!zone_is_empty(zone) && start_pfn > zone->zone_start_pfn)
+		start_pfn = zone->zone_start_pfn;
 
-	zone->spanned_pages = max(start_pfn + nr_pages, old_end_pfn) - zone->zone_start_pfn;
+	zone_set_span(zone, start_pfn, end_pfn - start_pfn);
 }
 
 static void __meminit resize_pgdat_range(struct pglist_data *pgdat, unsigned long start_pfn,

-- 
2.56.0

[PATCH 9/9] mm/page_alloc: check pageblock pfns under span seqlock

[Vlastimil Babka (SUSE)]

From: Josef Bacik <josef@toxicpanda.com>

Whenever the flags of a pageblock are read or written, a kernel with
CONFIG_DEBUG_VM makes sure that the pfn lies within the zone of the page.
Unlike bad_range(), that check reads zone->zone_start_pfn and
zone->spanned_pages without zone->span_seqlock.

Memory hotplug changes the two fields one after the other. If the lower
end of a zone moves, the zone seems to end before its last pages in
between. A page of that part which is freed at that moment, on another
CPU, ends in

  page dumped because: VM_BUG_ON_PAGE(!zone_spans_pfn(page_zone(page), pfn))
  kernel BUG at mm/page_alloc.c:372!
   free_unref_folios+0x55e/0xed0
   folios_put_refs+0x2b5/0x530

Read the span in a read section of the seqlock.

Fixes: 86051ca5eaf5 ("mm: fix usemap initialization")
Cc: <stable@vger.kernel.org>
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
Signed-off-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
---
 mm/page_alloc.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 8f8885165045..f7bb0d59efc8 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -357,6 +357,21 @@ static __always_inline bool is_standalone_pb_bit(enum pageblock_bits pb_bit)
 	return pb_bit >= PB_compact_skip && pb_bit < __NR_PAGEBLOCK_BITS;
 }
 
+/* Memory hotplug can resize the zone while we look at its span */
+static bool __maybe_unused zone_spans_pfn_stable(struct zone *zone,
+						 unsigned long pfn)
+{
+	unsigned int seq;
+	bool ret;
+
+	do {
+		seq = zone_span_seqbegin(zone);
+		ret = zone_spans_pfn(zone, pfn);
+	} while (zone_span_seqretry(zone, seq));
+
+	return ret;
+}
+
 static __always_inline void
 get_pfnblock_bitmap_bitidx(const struct page *page, unsigned long pfn,
 			   unsigned long **bitmap_word, unsigned long *bitidx)
@@ -370,7 +385,7 @@ get_pfnblock_bitmap_bitidx(const struct page *page, unsigned long pfn,
 	BUILD_BUG_ON(NR_PAGEBLOCK_BITS != 4);
 #endif
 	BUILD_BUG_ON(__MIGRATE_TYPE_END > PAGEBLOCK_MIGRATETYPE_MASK);
-	VM_BUG_ON_PAGE(!zone_spans_pfn(page_zone(page), pfn), page);
+	VM_BUG_ON_PAGE(!zone_spans_pfn_stable(page_zone(page), pfn), page);
 
 	bitmap = get_pageblock_bitmap(page, pfn);
 	*bitidx = pfn_to_bitidx(page, pfn);

-- 
2.56.0

Re: [PATCH 1/9] mm/page_alloc: don't undo a racing start_isolate_page_range()

[Zi Yan]

On 27 Oct 2026, at 11:46, Vlastimil Babka (SUSE) wrote:

> From: Josef Bacik <josef@toxicpanda.com>
>
> start_isolate_page_range() fails with -EBUSY if one of the pageblocks is
> already isolated, i.e. if somebody else is working on it. It restores
> the pageblocks which it has isolated itself before it returns, as its
> kernel-doc says:
>
>   We then clean up by restoring the migration type on pageblocks we may
>   have modified and return -EBUSY to caller. This prevents two threads
>   from simultaneously working on overlapping ranges.
>
> alloc_contig_frozen_range() nevertheless calls undo_isolate_page_range()
> for the whole range in this case, which clears every isolated pageblock
> it finds. The only ones left at this point belong to the other user of
> the range:
>
>   CPU0 offline_pages()                CPU1 alloc_contig_frozen_range()
>   start_isolate_page_range(A)
>     pageblock X: isolated
>   migrate ...
>                                       start_isolate_page_range(B)
>                                         X is isolated: -EBUSY
>                                         (own pageblocks restored)
>                                       undo_isolate_page_range(B)
>                                         X: not isolated any more
>   test_pages_isolated(A): -EBUSY
>   (again and again)
>
> offline_pages() repeats the migration and test_pages_isolated() until the
> latter succeeds, which is now impossible and the task spins until it gets
> a signal, while the free pages of X are handed out by the allocator again.
>
> If X is released after the last test_pages_isolated(),
> __offline_isolated_pages() takes pages which are accounted as free off
> the free lists as MIGRATE_ISOLATE. Thus NR_FREE_PAGES stays too high, or
> hits BUG_ON(!PageBuddy(page)) if one of them has been allocated.
>
> For another alloc_contig_frozen_range() or cma_alloc() the result is a
> spurious -EBUSY.
>
> Return the error, like the function did until it was reworked.
>
> Fixes: b2c9e2fbba32 ("mm: make alloc_contig_range work at pageblock granularity")
> Cc: <stable@vger.kernel.org>
> Assisted-by: LLM
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
> Signed-off-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
> ---
>  mm/page_alloc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

Reviewed-by: Zi Yan <ziy@nvidia.com>

Best Regards,
Yan, Zi

Re: [PATCH 1/9] mm/page_alloc: don't undo a racing start_isolate_page_range()

[Gregory Price (Meta)]

On 2026-10-27 16:46 +0100, Vlastimil Babka (SUSE) wrote:
> From: Josef Bacik <josef@toxicpanda.com>
> 
> start_isolate_page_range() fails with -EBUSY if one of the pageblocks is
> already isolated, i.e. if somebody else is working on it. It restores
> the pageblocks which it has isolated itself before it returns, as its
> kernel-doc says:
> 
>   We then clean up by restoring the migration type on pageblocks we may
>   have modified and return -EBUSY to caller. This prevents two threads
>   from simultaneously working on overlapping ranges.
> 
> alloc_contig_frozen_range() nevertheless calls undo_isolate_page_range()
> for the whole range in this case, which clears every isolated pageblock
> it finds. The only ones left at this point belong to the other user of
> the range:
> 
>   CPU0 offline_pages()                CPU1 alloc_contig_frozen_range()
>   start_isolate_page_range(A)
>     pageblock X: isolated
>   migrate ...
>                                       start_isolate_page_range(B)
>                                         X is isolated: -EBUSY
>                                         (own pageblocks restored)
>                                       undo_isolate_page_range(B)
>                                         X: not isolated any more
>   test_pages_isolated(A): -EBUSY
>   (again and again)
> 
> offline_pages() repeats the migration and test_pages_isolated() until the
> latter succeeds, which is now impossible and the task spins until it gets
> a signal, while the free pages of X are handed out by the allocator again.
> 
> If X is released after the last test_pages_isolated(),
> __offline_isolated_pages() takes pages which are accounted as free off
> the free lists as MIGRATE_ISOLATE. Thus NR_FREE_PAGES stays too high, or
> hits BUG_ON(!PageBuddy(page)) if one of them has been allocated.
> 
> For another alloc_contig_frozen_range() or cma_alloc() the result is a
> spurious -EBUSY.
> 
> Return the error, like the function did until it was reworked.
> 
> Fixes: b2c9e2fbba32 ("mm: make alloc_contig_range work at pageblock granularity")
> Cc: <stable@vger.kernel.org>
> Assisted-by: LLM
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
> Signed-off-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>

Given i separately found this exact patch and tested it

Reviewed-by: Gregory Price (Meta) <gourry@gourry.net>
Tested-by: Gregory Price (Meta) <gourry@gourry.net>

-- 
~Gregory

Re: [PATCH 5/9] mm, page_alloc: pass zone pointer to trace_reclaim_retry_zone

[Gregory Price (Meta)]

On 2026-10-27 16:46 +0100, Vlastimil Babka (SUSE) wrote:
> From: Josef Bacik <josef@toxicpanda.com>
> 
> should_reclaim_retry() walks a zonelist and passes the zoneref pointer
> to the reclaim_retry_zone tracepoint, which loads the zone from the
> entry again to find the node.
> 
> Memory hotplug rebuilds the zonelists in place while allocations are
> under way. When a zone in front of the zoneref entry has become empty,
> the entries move up, and this one can be the end marker by the time the
> tracepoint looks at it. zonelist_node_idx() then dereferences a NULL
> zone. Short of that, the event can name another zone than the one its
> numbers belong to.
> 
> Pass the zone pointer instead of the zoneref pointer.
> 
> Fixes: d379f01de095 ("oom, trace: add oom detection tracepoints")
> Cc: <stable@vger.kernel.org>
> Assisted-by: LLM
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
> Signed-off-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>

Similarly found separately and tested the fix.

Reviewed-by: Gregory Price (Meta) <gourry@gourry.net>
Tested-by: Gregory Price (Meta) <gourry@gourry.net>

-- 
~Gregory

Re: [PATCH 2/9] mm/page_alloc: clear ->private of deferred free pages

[Zi Yan]

On 27 Oct 2026, at 11:46, Vlastimil Babka (SUSE) wrote:

> From: Josef Bacik <josef@toxicpanda.com>
>
> free_pages_nolock() and free_frozen_pages_nolock() put the page on
> zone->trylock_free_pages if they get neither the pcp lock nor zone->lock.
> add_page_to_zone_llist() keeps the order of the page in page->private.
> The next free_one_page() which does get zone->lock passes these pages to
> split_large_buddy() with ->private still set.
>
> __free_one_page() writes ->private of the head of the resulting free page
> only. If the deferred page is merged with its lower buddy, it becomes a
> tail of that free page and keeps the order in ->private.
>
> All other free pages which are not the head of a buddy have a ->private
> of zero, and since commit 819bfbd47e49 ("mm/page_alloc: remove
> set_page_private() in prep_compound_tail()") the allocator relies on it.
> When the stale page is allocated again as a tail of a larger page:
>
>  - prep_compound_tail() hits VM_WARN_ON_ONCE(tail->private);
>  - with the page checks enabled, __free_pages_prepare() reports
>    "BUG: Bad page state", "nonzero private", and leaks the whole page;
>  - without them the value is seen by whoever uses that page next, e.g. as
>    folio->private of a folio which is split off a large folio.
>
> Only pages of an order above zero are affected. Those can be freed
> through _nolock() by the stack depot (order 2) and by SLUB (a new slab
> which cannot be added to the partial list by kmalloc_nolock()).
>
> Clear ->private when the page is taken off the list.
>
> Fixes: 8c57b687e833 ("mm, bpf: Introduce free_pages_nolock()")
> Cc: <stable@vger.kernel.org>
> Assisted-by: LLM
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
> Signed-off-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
> ---
>  mm/page_alloc.c | 2 ++
>  1 file changed, 2 insertions(+)
>
Makes sense.

Reviewed-by: Zi Yan <ziy@nvidia.com>

Best Regards,
Yan, Zi

Re: [PATCH 5/9] mm, page_alloc: pass zone pointer to trace_reclaim_retry_zone

[Zi Yan]

On 27 Oct 2026, at 11:46, Vlastimil Babka (SUSE) wrote:

> From: Josef Bacik <josef@toxicpanda.com>
>
> should_reclaim_retry() walks a zonelist and passes the zoneref pointer
> to the reclaim_retry_zone tracepoint, which loads the zone from the
> entry again to find the node.
>
> Memory hotplug rebuilds the zonelists in place while allocations are
> under way. When a zone in front of the zoneref entry has become empty,
> the entries move up, and this one can be the end marker by the time the
> tracepoint looks at it. zonelist_node_idx() then dereferences a NULL
> zone. Short of that, the event can name another zone than the one its
> numbers belong to.
>
> Pass the zone pointer instead of the zoneref pointer.
>
> Fixes: d379f01de095 ("oom, trace: add oom detection tracepoints")
> Cc: <stable@vger.kernel.org>
> Assisted-by: LLM
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
> Signed-off-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
> ---
>  include/trace/events/oom.h | 8 ++++----
>  mm/page_alloc.c            | 2 +-
>  2 files changed, 5 insertions(+), 5 deletions(-)
>
Makes sense to me.

Acked-by: Zi Yan <ziy@nvidia.com>

Best Regards,
Yan, Zi

Re: [PATCH 7/9] mm: page_isolation: do not take slab pages for free

[Zi Yan]

On 27 Oct 2026, at 11:46, Vlastimil Babka (SUSE) wrote:

> From: Josef Bacik <josef@toxicpanda.com>
>
> page_is_unmovable() regards a page with a reference count of zero as free
> or about to be freed. Since slab allocates frozen pages, they will
> be wrongly considered to be freed if order 0 (higher-order compound
> pages are caught earlier).
>
> If such slabs are the only unmovable pages of a memory block, memory
> offlining now isolates the block, this can lead to offline_pages()
> looping without progress.
>
> Thus detect the slab pages as removable to return -EBUSY, as would
> happen before commit 9aec2fb0fd5e ("slab: allocate frozen pages").
>
> Fixes: 9aec2fb0fd5e ("slab: allocate frozen pages")
> Assisted-by: LLM
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
> Signed-off-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
> ---
>  mm/memory_hotplug.c | 7 +++++++
>  mm/page_isolation.c | 7 +++++++
>  2 files changed, 14 insertions(+)
>
LGTM.

Reviewed-by: Zi Yan <ziy@nvidia.com>

Best Regards,
Yan, Zi

Re: [PATCH 6/9] mm/compaction: trust zone->contiguous only in the span

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] mm/compaction: pageblock_pfn_to_page() ignores end_pfn in span check
- [High] mm/compaction: racy zone_spans_pfn() check in pageblock_pfn_to_page()
--

commit fc93d5609d6ae896a622cb4ad47040ba39704072
Author: Josef Bacik <josef@toxicpanda.com>

mm/compaction: trust zone->contiguous only in the span

This commit restricts the fast path in pageblock_pfn_to_page() so that
zone->contiguous is only trusted when start_pfn falls within the zone span,
and pairs smp_store_release() in set_zone_contiguous() with
smp_load_acquire() in pageblock_pfn_to_page().

> diff --git a/mm/page_alloc.h b/mm/page_alloc.h
> index 2b1017a91928c..e24d1203c5148 100644
> --- a/mm/page_alloc.h
> +++ b/mm/page_alloc.h
> @@ -216,7 +216,12 @@ extern struct page *__pageblock_pfn_to_page(unsigned long start_pfn,
>  static inline struct page *pageblock_pfn_to_page(unsigned long start_pfn,
>  				unsigned long end_pfn, struct zone *zone)
>  {
> -	if (zone->contiguous)
> +	/*
> +	 * A scanner may still be at a pfn that has left the zone: the zone is
> +	 * shrunk before set_zone_contiguous() says that it has no holes.
> +	 */
> +	if (smp_load_acquire(&zone->contiguous) &&
> +	    zone_spans_pfn(zone, start_pfn))
>  		return pfn_to_page(start_pfn);

[Severity: High]
Can pageblock_pfn_to_page() still return a non-NULL page for a range that has
left the zone because it only checks start_pfn and ignores end_pfn?

When the lowest memory block at the start of a multi-block zone is offlined
and removed while compact_zone() is running with cc->migrate_pfn in that
removed lower block, remove_pfn_range_from_zone() advances
zone->zone_start_pfn via shrink_zone_span() and sets zone->contiguous = true
via set_zone_contiguous() when the remaining zone has no holes.

On the next call to compact_zone() -> isolate_migratepages() in
mm/compaction.c, block_start_pfn is clamped up to cc->zone->zone_start_pfn
while low_pfn and block_end_pfn remain in the removed range below
cc->zone->zone_start_pfn:

mm/compaction.c:isolate_migratepages() {
	low_pfn = fast_find_migrateblock(cc);
	block_start_pfn = pageblock_start_pfn(low_pfn);
	if (block_start_pfn < cc->zone->zone_start_pfn)
		block_start_pfn = cc->zone->zone_start_pfn;
	...
	block_end_pfn = pageblock_end_pfn(low_pfn);
	...
		page = pageblock_pfn_to_page(block_start_pfn,
						block_end_pfn, cc->zone);
	...
		if (isolate_migratepages_block(cc, low_pfn, block_end_pfn,
						isolate_mode))
			return ISOLATE_ABORT;
}

Because block_start_pfn was clamped to cc->zone->zone_start_pfn,
zone_spans_pfn(zone, start_pfn) evaluates to true even though
start_pfn >= end_pfn and end_pfn is in the removed range.
pageblock_pfn_to_page() then returns pfn_to_page(start_pfn), and
isolate_migratepages() passes low_pfn and block_end_pfn to
isolate_migratepages_block(), which dereferences the unmapped or poisoned
memmap for low_pfn.

The same start_pfn clamping occurs in isolate_migratepages_range(),
isolate_freepages_range(), and fast_isolate_around():

mm/compaction.c:fast_isolate_around() {
	start_pfn = max(pageblock_start_pfn(pfn), cc->zone->zone_start_pfn);
	end_pfn = min(pageblock_end_pfn(pfn), zone_end_pfn(cc->zone));

	page = pageblock_pfn_to_page(start_pfn, end_pfn, cc->zone);
}

When pfn < cc->zone->zone_start_pfn, start_pfn > end_pfn, so
pageblock_pfn_to_page() returns pfn_to_page(cc->zone->zone_start_pfn) and
fast_isolate_around() -> isolate_freepages_block() falsely sets the pageblock
skip hint at the new start of the zone. Additionally, if zone_end_pfn(zone) is
not pageblock-aligned or shrinks from the top while remaining contiguous,
start_pfn < zone_end_pfn(zone) < end_pfn allows isolate_migratepages() to scan
past zone_end_pfn(zone).

Note that __pageblock_pfn_to_page() in mm/page_alloc.c also does not check
that start_pfn < end_pfn or that end_pfn - 1 is online when start_pfn has been
clamped into the online zone:

mm/page_alloc.c:__pageblock_pfn_to_page() {
	end_pfn--;

	if (!pfn_valid(end_pfn))
		return NULL;

	start_page = pfn_to_online_page(start_pfn);
}

Should pageblock_pfn_to_page() and __pageblock_pfn_to_page() verify that
start_pfn < end_pfn and that both start_pfn and end_pfn - 1 are spanned and
online?

[Severity: High]
Is reading zone->contiguous before zone_spans_pfn(zone, start_pfn) without
zone->span_seqlock safe against concurrent zone span changes?

Reading zone->contiguous first and the zone span second only pairs with the
false to true transition in set_zone_contiguous(). On the true to false
transition in move_pfn_range_to_zone() and remove_pfn_range_from_zone() in
mm/memory_hotplug.c, clear_zone_contiguous() clears zone->contiguous with a
plain store before the zone span is modified:

mm/mm_init.h:clear_zone_contiguous() {
	zone->contiguous = false;
}

mm/memory_hotplug.c:move_pfn_range_to_zone() {
	clear_zone_contiguous(zone);

	if (zone_is_empty(zone))
		init_currently_empty_zone(zone, start_pfn, nr_pages);
	resize_zone_range(zone, start_pfn, nr_pages);
	resize_pgdat_range(pgdat, start_pfn, nr_pages);
	...
	memmap_init_range(nr_pages, nid, zone_idx(zone), start_pfn, 0,
			 MEMINIT_HOTPLUG, altmap, migratetype,
			 isolate_pageblock);
}

Because the writer clears zone->contiguous first and updates the span second,
a reader in pageblock_pfn_to_page() executing in the same order can load
zone->contiguous == true just before clear_zone_contiguous(zone) and then
observe the expanded span from resize_zone_range() in
zone_spans_pfn(zone, start_pfn). When a contiguous zone is expanded across a
PFN hole or before memmap_init_range() initializes the new range and its
sections are marked online, a compaction scanner with a PFN in that hole or
new range can bypass __pageblock_pfn_to_page() and access an unmapped hole or
uninitialized memmap.

Furthermore, zone_spans_pfn() in pageblock_pfn_to_page() reads
zone->zone_start_pfn and zone->spanned_pages without zone->span_seqlock
(which remains unguarded even after later commits in the series resize zones
under span_seqlock and check pageblock pfns under span_seqlock in
__pageblock_pfn_to_page()):

mm/memory_hotplug.c:shrink_zone_span() {
		if (pfn) {
			zone->spanned_pages = zone_end_pfn(zone) - pfn;
			zone->zone_start_pfn = pfn;
		} else {
			zone->zone_start_pfn = 0;
			zone->spanned_pages = 0;
		}
}

When shrink_zone_span() raises zone->zone_start_pfn and reduces
zone->spanned_pages, a reader in pageblock_pfn_to_page() can observe the new
zone->zone_start_pfn together with the old zone->spanned_pages, causing
zone_end_pfn(zone) to transiently extend past the actual upper end of the
zone into a previously removed PFN range.

Should pageblock_pfn_to_page() read the zone span and zone->contiguous under
zone->span_seqlock, and ensure clear_zone_contiguous() is properly ordered
against span updates?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261027-pgalloc-ai-hotfixes-v1-0-9c7d1aca801e@kernel.org?part=6

Re: [PATCH 9/9] mm/page_alloc: check pageblock pfns under span seqlock

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mm: NMI deadlock and invalid wait context in zone_spans_pfn_stable()
--

commit 46b6bec2b8c6bc868d5bf37583c51e88e5fe1429
Author: Josef Bacik <josef@toxicpanda.com>

mm/page_alloc: check pageblock pfns under span seqlock

This patch adds zone_spans_pfn_stable() to read zone->zone_start_pfn and
zone->spanned_pages under zone->span_seqlock when checking pageblock PFNs in
get_pfnblock_bitmap_bitidx() under CONFIG_DEBUG_VM.

> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 8f8885165045b..f7bb0d59efc81 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -357,6 +357,21 @@ static __always_inline bool is_standalone_pb_bit(enum pageblock_bits pb_bit)
>  	return pb_bit >= PB_compact_skip && pb_bit < __NR_PAGEBLOCK_BITS;
>  }
>  
> +/* Memory hotplug can resize the zone while we look at its span */
> +static bool __maybe_unused zone_spans_pfn_stable(struct zone *zone,
> +						 unsigned long pfn)
> +{
> +	unsigned int seq;
> +	bool ret;
> +
> +	do {
> +		seq = zone_span_seqbegin(zone);
> +		ret = zone_spans_pfn(zone, pfn);
> +	} while (zone_span_seqretry(zone, seq));
> +
> +	return ret;
> +}
> +
>  static __always_inline void
>  get_pfnblock_bitmap_bitidx(const struct page *page, unsigned long pfn,
>  			   unsigned long **bitmap_word, unsigned long *bitidx)
> @@ -370,7 +385,7 @@ get_pfnblock_bitmap_bitidx(const struct page *page, unsigned long pfn,
>  	BUILD_BUG_ON(NR_PAGEBLOCK_BITS != 4);
>  #endif
>  	BUILD_BUG_ON(__MIGRATE_TYPE_END > PAGEBLOCK_MIGRATETYPE_MASK);
> -	VM_BUG_ON_PAGE(!zone_spans_pfn(page_zone(page), pfn), page);
> +	VM_BUG_ON_PAGE(!zone_spans_pfn_stable(page_zone(page), pfn), page);

[Severity: High]
Can calling zone_spans_pfn_stable() here cause an NMI deadlock or an invalid
wait context when freeing or allocating pages through the nolock paths with
CONFIG_DEBUG_VM and CONFIG_MEMORY_HOTPLUG enabled?

free_pages_nolock(), free_frozen_pages_nolock(), and alloc_pages_nolock() can
be called from NMI, hardirq, or while holding a raw_spinlock_t. On the free
path, __free_frozen_pages() calls get_pfnblock_migratetype() before checking
can_spin_trylock():

free_pages_nolock()
  ___free_pages()
    __free_frozen_pages()
      get_pfnblock_migratetype()
        __get_pfnblock_flags_mask()
          get_pfnblock_bitmap_bitidx()
            zone_spans_pfn_stable()
              zone_span_seqbegin()

In mm/page_alloc.c:__free_frozen_pages():
	zone = page_zone(page);
	migratetype = get_pfnblock_migratetype(page, pfn);
	if (unlikely(migratetype >= MIGRATE_PCPTYPES)) {
		if (unlikely(is_migrate_isolate(migratetype))) {
			free_one_page(zone, page, pfn, order, fpi_flags);
			return;
		}
		migratetype = MIGRATE_MOVABLE;
	}

	if (unlikely((fpi_flags & FPI_NOLOCK) && !can_spin_trylock())) {
		add_page_to_zone_llist(zone, page, order);
		return;
	}

On the write side, mm/memory_hotplug.c:zone_set_span() updates the span using
write_seqlock_irqsave(&zone->span_seqlock, flags), which disables regular
interrupts but cannot mask NMIs:

	write_seqlock_irqsave(&zone->span_seqlock, flags);
	zone->zone_start_pfn = start_pfn;
	zone->spanned_pages = spanned_pages;
	write_sequnlock_irqrestore(&zone->span_seqlock, flags);

This seems to introduce three regressions on the nolock paths:

1. If an NMI interrupts zone_set_span() while the sequence count is odd and
the NMI handler calls free_pages_nolock(), free_frozen_pages_nolock(), or
alloc_pages_nolock() on a page in that zone, read_seqbegin() in
zone_span_seqbegin() will spin forever waiting for the interrupted writer on
the same CPU.

2. With CONFIG_PROVE_LOCKING enabled, read_seqbegin() calls
seqcount_lockdep_reader_access(), which records a non-trylock lock_acquire()
on span_seqlock and triggers a lockdep inconsistent NMI usage splat once
zone_set_span() has used the lock.

3. On CONFIG_PREEMPT_RT, zone->span_seqlock is a seqlock_t backed by a
sleeping spinlock_t. If a concurrent writer is active (seq & 1),
read_seqbegin() takes spin_lock(&zone->span_seqlock.lock) via
__seqprop_spinlock_sequence(), attempting to acquire a sleeping lock from
NMI, hardirq, or raw_spinlock_t context.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261027-pgalloc-ai-hotfixes-v1-0-9c7d1aca801e@kernel.org?part=9