Re: [PATCH v9 2/2] mm/memory hotplug/unplug: Optimize zone->contiguous update when changes pfn range

[David Hildenbrand (Arm)]

On 2/9/26 11:52, David Hildenbrand (Arm) wrote:
> On 2/8/26 20:39, Mike Rapoport wrote:
> > On Sat, Feb 07, 2026 at 12:00:09PM +0100, David Hildenbrand (Arm) wrote:
> > > Thanks for all your work on this and sorry for being slower with
> > > review the last month.
> > >
> > > While I was in the shower I was thinking about how much I hate
> > > zone->contiguous + the pageblock walking, and how we could just get
> > > rid of it.
> > >
> > > You know, just what you do while having a relaxing shower.
> > >
> > >
> > > And I was wondering:
> > >
> > > (a) in which case would we have zone_spanned_pages == zone_present_pages
> > > and the zone *not* being contiguous? I assume this just cannot happen,
> > > otherwise BUG.
> > >
> > > (b) in which case would we have zone_spanned_pages != zone_present_pages
> > > and the zone *being* contiguous? I assume in some cases where we have 
> > > small
> > > holes within a pageblock?
> > >
> > > Reading the doc of __pageblock_pfn_to_page(), there are some weird
> > > scenarios with holes in pageblocks.
> > It seems that "zone->contigous" is really bad name for what this thing
> > represents.
> >
> > tl;dr I don't think zone_spanned_pages == zone_present_pages is 
> > related to
> > zone->contigous at all :)
>
> My point in (a) was that with "zone_spanned_pages == zone_present_pages" 
> there are no holes so -> contiguous.
>
> (b), and what I said further below, is exactly about memory holes where 
> we have a memmap, but it's not present memory.
>
> > If you look at pageblock_pfn_to_page() and __pageblock_pfn_to_page(), the
> > check for zone->contigous should guarantee that the entire pageblock 
> > has a
> > valid memory map and that the entire pageblock fits a zone and does not
> > cross zone/node boundaries.
>
> Right. But that must hold for each and ever pageblock in the spanned 
> zone range for it to be contiguous.
>
> zone->contigous tells you "pfn_to_page()" is valid on the complete zone 
> range"
>
> That's why set_zone_contiguous() probes __pageblock_pfn_to_page() on ech 
> and ever pageblock.
>
> > For coldplug memory the memory map is valid for every section that has
> > present memory, i.e. even it there is a hole in a section, it's memory 
> > map
> > will be populated and will have struct pages.
>
> There is this sub-section thing, and holes larger than a section might 
> not have a memmap (unless reserved I guess).
>
> > When zone->contigous is false, the slow path in __pageblock_pfn_to_page()
> > essentially checks if the first page in a pageblock is online and if 
> > first
> > and last pages are in the zone being compacted.
> > AFAIU, in the hotplug case an entire pageblock is always onlined to the
> > same zone, so zone->contigous won't change after the hotplug is complete.
>
> I think you are missing a point: hotp(un)plug might create holes in the 
> zone span. Then, pfn_to_page() is no longer valid to be called on 
> arbitrary pageblocks within the zone.
>
> > We might set it to false in the beginning of the hotplug to avoid 
> > scanning
> > offline pages, although I'm not sure if it's possible.
> >
> > But in the end of hotplug we can simply restore the old value and move 
> > on.
>
> No, you might create holes.
>
> > For the coldplug case I'm also not sure it's worth the hassle, we could
> > just let compaction scan a few more pfns for those rare weird pageblocks
> > and bail out on wrong page conditions.
>
> To recap:
>
> My idea is that "zone_spanned_pages == zone_present_pages" tells you 
> that the zone is contiguous because there are no holes.
>
> To handle "non-memory with a struct page", you'd have to check
>
>      "zone_spanned_pages == zone_present_pages +
>           zone_non_present_memmap_pages"
>
> Or shorter
>
>      "zone_spanned_pages == zone_pages_with_memmap"
>
> Then, pfn_to_page() is valid within the complete zone.
>
> The question is how to best calculate the "zone_pages_with_memmap" 
> during boot.
>
> During hot(un)plug we only add/remove zone_present_pages. The 
> zone_non_present_memmap_pages will not change due to hot(un)plug later.

The following hack does the trick. But

(a) I wish we could get rid of the pageblock walking in calc_online_pages().
(b) "online_pages" has weird semantics due to the pageblock handling.
    "online_pageblock_pages"? not sure.
(c) Calculating "online_pages" when we know there is a hole does not make sense,
    as we could just keep it 0 if there are holes and simply set it to
    zone->online_pageblock_pages->zone->spanned_pages in case all are online.


From d4cb825e91a6363afc68fb994c5d9b29c38c5f42 Mon Sep 17 00:00:00 2001
From: "David Hildenbrand (Arm)" <david@xxxxxxxxxx>
Date: Mon, 9 Feb 2026 13:40:24 +0100
Subject: [PATCH] tmp

Signed-off-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>
---
 include/linux/mmzone.h | 25 +++++++++++++++++++++++--
 mm/internal.h          |  8 +-------
 mm/memory_hotplug.c    | 20 ++++++--------------
 mm/mm_init.c           | 12 ++++++------
 4 files changed, 36 insertions(+), 29 deletions(-)

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index fc5d6c88d2f0..3f7d8d88c597 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -943,6 +943,11 @@ struct zone {
 	 * cma pages is present pages that are assigned for CMA use
 	 * (MIGRATE_CMA).
 	 *
+	 * online_pages is pages within the zone that have an online memmap.
+	 * online_pages include present pages and memory holes that have a
+	 * memmap. When spanned_pages == online_pages, pfn_to_page() can be
+	 * performed without further checks on any pfn within the zone span.
+	 *
 	 * So present_pages may be used by memory hotplug or memory power
 	 * management logic to figure out unmanaged pages by checking
 	 * (present_pages - managed_pages). And managed_pages should be used
@@ -967,6 +972,7 @@ struct zone {
 	atomic_long_t		managed_pages;
 	unsigned long		spanned_pages;
 	unsigned long		present_pages;
+	unsigned long		online_pages;
 #if defined(CONFIG_MEMORY_HOTPLUG)
 	unsigned long		present_early_pages;
 #endif
@@ -1051,8 +1057,6 @@ struct zone {
 	bool			compact_blockskip_flush;
 #endif
 
-	bool			contiguous;
-
 	CACHELINE_PADDING(_pad3_);
 	/* Zone statistics */
 	atomic_long_t		vm_stat[NR_VM_ZONE_STAT_ITEMS];
@@ -1124,6 +1128,23 @@ static inline bool zone_spans_pfn(const struct zone *zone, unsigned long pfn)
 	return zone->zone_start_pfn <= pfn && pfn < zone_end_pfn(zone);
 }
 
+/**
+ * zone_is_contiguous - test whether a zone is contiguous
+ * @zone: the zone to test.
+ *
+ * In a contiguous zone, it is valid to call pfn_to_page() on any pfn in the
+ * spanned zone without requiting pfn_valid() or pfn_to_online_page() checks.
+ *
+ * Returns: true if contiguous, otherwise false.
+ */
+static inline bool zone_is_contiguous(const struct zone *zone)
+{
+	return READ_ONCE(zone->spanned_pages) == READ_ONCE(zone->online_pages);
+}
+
 static inline bool zone_is_initialized(const struct zone *zone)
 {
 	return zone->initialized;
diff --git a/mm/internal.h b/mm/internal.h
index f35dbcf99a86..6062f9b8ee62 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -716,21 +716,15 @@ 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)
+	if (zone_is_contiguous(zone))
 		return pfn_to_page(start_pfn);
 
 	return __pageblock_pfn_to_page(start_pfn, end_pfn, zone);
 }
 
-void set_zone_contiguous(struct zone *zone);
 bool pfn_range_intersects_zones(int nid, unsigned long start_pfn,
 			   unsigned long nr_pages);
 
-static inline void clear_zone_contiguous(struct zone *zone)
-{
-	zone->contiguous = false;
-}
-
 extern int __isolate_free_page(struct page *page, unsigned int order);
 extern void __putback_isolated_page(struct page *page, unsigned int order,
 				    int mt);
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index a63ec679d861..76496c1039a9 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -492,11 +492,11 @@ 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;
+			WRITE_ONCE(zone->spanned_pages, zone_end_pfn(zone) - pfn);
 			zone->zone_start_pfn = pfn;
 		} else {
 			zone->zone_start_pfn = 0;
-			zone->spanned_pages = 0;
+			WRITE_ONCE(zone->spanned_pages, 0);
 		}
 	} else if (zone_end_pfn(zone) == end_pfn) {
 		/*
@@ -508,10 +508,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;
+			WRITE_ONCE(zone->spanned_pages, pfn - zone->zone_start_pfn + 1);
 		else {
 			zone->zone_start_pfn = 0;
-			zone->spanned_pages = 0;
+			WRITE_ONCE(zone->spanned_pages, 0);
 		}
 	}
 }
@@ -565,18 +565,13 @@ void remove_pfn_range_from_zone(struct zone *zone,
 
 	/*
 	 * Zone shrinking code cannot properly deal with ZONE_DEVICE. So
-	 * we will not try to shrink the zones - which is okay as
-	 * set_zone_contiguous() cannot deal with ZONE_DEVICE either way.
+	 * we will not try to shrink the zones.
 	 */
 	if (zone_is_zone_device(zone))
 		return;
 
-	clear_zone_contiguous(zone);
-
 	shrink_zone_span(zone, start_pfn, start_pfn + nr_pages);
 	update_pgdat_span(pgdat);
-
-	set_zone_contiguous(zone);
 }
 
 /**
@@ -753,8 +748,6 @@ void move_pfn_range_to_zone(struct zone *zone, unsigned long start_pfn,
 	struct pglist_data *pgdat = zone->zone_pgdat;
 	int nid = pgdat->node_id;
 
-	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);
@@ -782,8 +775,6 @@ void move_pfn_range_to_zone(struct zone *zone, unsigned long start_pfn,
 	memmap_init_range(nr_pages, nid, zone_idx(zone), start_pfn, 0,
 			 MEMINIT_HOTPLUG, altmap, migratetype,
 			 isolate_pageblock);
-
-	set_zone_contiguous(zone);
 }
 
 struct auto_movable_stats {
@@ -1079,6 +1070,7 @@ void adjust_present_page_count(struct page *page, struct memory_group *group,
 	if (early_section(__pfn_to_section(page_to_pfn(page))))
 		zone->present_early_pages += nr_pages;
 	zone->present_pages += nr_pages;
+	WRITE_ONCE(zone->online_pages,  zone->online_pages + nr_pages);
 	zone->zone_pgdat->node_present_pages += nr_pages;
 
 	if (group && movable)
diff --git a/mm/mm_init.c b/mm/mm_init.c
index 2a809cd8e7fa..e33caa6fb6fc 100644
--- a/mm/mm_init.c
+++ b/mm/mm_init.c
@@ -2263,9 +2263,10 @@ void __init init_cma_pageblock(struct page *page)
 }
 #endif
 
-void set_zone_contiguous(struct zone *zone)
+static void calc_online_pages(struct zone *zone)
 {
 	unsigned long block_start_pfn = zone->zone_start_pfn;
+	unsigned long online_pages = 0;
 	unsigned long block_end_pfn;
 
 	block_end_pfn = pageblock_end_pfn(block_start_pfn);
@@ -2277,12 +2278,11 @@ void set_zone_contiguous(struct zone *zone)
 
 		if (!__pageblock_pfn_to_page(block_start_pfn,
 					     block_end_pfn, zone))
-			return;
+			continue;
 		cond_resched();
+		online_pages += block_end_pfn - block_start_pfn;
 	}
-
-	/* We confirm that there is no hole */
-	zone->contiguous = true;
+	zone->online_pages = online_pages;
 }
 
 /*
@@ -2348,7 +2348,7 @@ void __init page_alloc_init_late(void)
 		shuffle_free_memory(NODE_DATA(nid));
 
 	for_each_populated_zone(zone)
-		set_zone_contiguous(zone);
+		calc_online_pages(zone);
 
 	/* Initialize page ext after all struct pages are initialized. */
 	if (deferred_struct_pages)
--
2.43.0


--
Cheers,

David