RE: [PATCH v8 2/2] mm/memory_hotplug: optimize zone contiguous check when changing pfn range

[Liu, Yuan1]

> -----Original Message-----
> From: Wei Yang <richard.weiyang@xxxxxxxxx>
> Sent: Friday, September 4, 2026 3:39 PM
> To: Liu, Yuan1 <yuan1.liu@xxxxxxxxx>
> Cc: David Hildenbrand <david@xxxxxxxxxx>; Oscar Salvador
> <osalvador@xxxxxxx>; Mike Rapoport <rppt@xxxxxxxxxx>; Wei Yang
> <richard.weiyang@xxxxxxxxx>; linux-mm@xxxxxxxxx; Zou, Nanhai
> <nanhai.zou@xxxxxxxxx>; Chen Zhang <zhangchen.kidd@xxxxxx>; Zeng, Jason
> <jason.zeng@xxxxxxxxx>; Chen, Yu C <yu.c.chen@xxxxxxxxx>; Deng, Pan
> <pan.deng@xxxxxxxxx>; Li, Tianyou <tianyou.li@xxxxxxxxx>; linux-
> kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v8 2/2] mm/memory_hotplug: optimize zone contiguous
> check when changing pfn range
> 
> On Tue, Sep 01, 2026 at 01:29:50AM -0400, Yuan Liu wrote:
> >When move_pfn_range_to_zone() or remove_pfn_range_from_zone() updates a
> >zone, set_zone_contiguous() rescans the entire zone pageblock-by-
> pageblock
> >to rebuild zone->contiguous. For large zones this is a significant cost
> >during memory hotplug and hot-unplug.
> >
> >Add a new zone member, pages_with_online_memmap, that tracks the
> >number of pages within the zone span that have an online memory map,
> >including present pages and memory holes whose memory map has been
> >initialized and for which pfn_to_online_page() succeeds.
> >
> >For early boot memory, pages_with_online_memmap is calculated in
> >memmap_init_zone_range(). PFNs initialized by memmap_init_range() are
> >included in pages_with_online_memmap, and hole PFNs for which
> >pfn_to_online_page() succeeds are also counted in
> >init_unavailable_range(). For hotplugged memory,
> >pages_with_online_memmap is updated through adjust_present_page_count(),
> >which is called during memory online and offline operations. When
> >spanned_pages == pages_with_online_memmap, every PFN in the zone span
> >has a valid memmap entry, so pfn_to_page() can be called for any PFN
> >within the zone span without an additional pfn_valid() check.
> >
> >The counter may temporarily undercount when pages with an online
> >memory map exist outside the current zone span. This can only happen
> >during boot, when initializing the memory map of pages that do not
> >fall into any zone span. Growing the zone to cover such pages and
> >later shrinking it back may result in a value that is too small.
> >This is safe, as it merely prevents detecting a contiguous zone.
> >
> >The contiguity check using pages_with_online_memmap is stricter than
> >the old pageblock-by-pageblock scan. The old set_zone_contiguous()
> >iterated at pageblock granularity via pageblock_pfn_to_page(), so a
> >zone could be marked contiguous even if a subsection-sized hole
> >existed within a pageblock. The new check requires
> >spanned_pages == pages_with_online_memmap, meaning every PFN in the
> >zone span must satisfy pfn_to_online_page().
[...]

> >+/**
> >+ * 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 requiring pfn_valid() or pfn_to_online_page()
> checks.
> >+ *
> >+ * Note that missing synchronization with memory offlining makes any PFN
> >+ * traversal prone to races.
> >+ *
> >+ * ZONE_DEVICE zones are always marked non-contiguous.
> >+ *
> >+ * Return: true if contiguous, otherwise false.
> >+ */
> >+static inline bool zone_is_contiguous(const struct zone *zone)
> >+{
> >+	return READ_ONCE(zone->contiguous);
> >+}
> >+
> >+static inline void set_zone_contiguous(struct zone *zone)
> >+{
> >+	if (zone_is_zone_device(zone))
> >+		return;
> 
> After this patch, set_zone_contiguous() is only used in two cases:
> 
>   * memory_block_online()
>   * page_alloc_init_late()
> 
> If I understand correctly:
> 
>   * zone_for_pfn_range() won't return ZONE_DEVICE
>   * there is no ZONE_DEVICE memory populated at this point, device memory
> is
>     populated during do_initcalls()
> 
> So we don't expect ZONE_DEVICE here?

The code does indeed work as you described. 
However, I would prefer to keep this check. The zone_is_contiguous comments
explicitly states that "ZONE_DEVICE zones are always marked as non-contiguous",
so I think making this explicit here is clearer

> >+	if (zone->spanned_pages == zone->pages_with_online_memmap)
> >+		WRITE_ONCE(zone->contiguous, true);
> >+}
> >+
> >+static inline void clear_zone_contiguous(struct zone *zone)
> >+{
> >+	WRITE_ONCE(zone->contiguous, false);
> >+}
> >+
> > /*
> >  * Returns true if a zone has pages managed by the buddy allocator.
> >  * All the reclaim decisions have to use this function rather than
> >diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> >index 9f19876ec3ec..100941d1b828 100644
> >--- a/mm/memory_hotplug.c
> >+++ b/mm/memory_hotplug.c
> >@@ -549,18 +549,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 it.
> > 	 */
> > 	if (zone_is_zone_device(zone))
> > 		return;
> 
> One question not closely related to this patch.
> 
> This check is introduced in commit 7ce700bf11b5 ("mm/memory_hotplug: don't
> access uninitialized memmaps in shrink_zone_span()"), at that time
> pfn_to_online_page() couldn't handle ZONE_DEVICE pfn correctly.
> 
> Then commit 1f90a3477df3 ("mm: teach pfn_to_online_page() about
> ZONE_DEVICE
> section collisions") enables it.

That's a good question. However, I don't think it is directly related to
this patch, so I'd prefer to leave this discussion for a separate patch.

> So could we remove the restriction now?
> 
> >
> >-	clear_zone_contiguous(zone);
> >-
> > 	shrink_zone_span(zone, start_pfn, start_pfn + nr_pages);
> > 	update_pgdat_span(pgdat);
> >-
> >-	set_zone_contiguous(zone);
> > }
> >
[...]

> >diff --git a/mm/mm_init.c b/mm/mm_init.c
> >index 1533aebafb68..d40a8ff23370 100644
> >--- a/mm/mm_init.c
> >+++ b/mm/mm_init.c
> >@@ -817,22 +817,39 @@ void __meminit init_deferred_page(unsigned long
> pfn, int nid)
> >  *   zone/node above the hole except for the trailing pages in the last
> >  *   section that will be appended to the zone/node below.
> >  */
> >-static void __init init_unavailable_range(unsigned long spfn,
> >-					  unsigned long epfn,
> >-					  int zone, int node)
> >+static unsigned long __init init_unavailable_range(unsigned long spfn,
> >+						   unsigned long epfn,
> >+						   int zone, int node)
> > {
> >+	unsigned long next_chunk_pfn __maybe_unused = spfn;
> > 	unsigned long pfn;
> >-	u64 pgcnt = 0;
> >+	u64 online_pgcnt = 0, pgcnt = 0;
> >+	bool is_online = true;
> >
> > 	for_each_valid_pfn(pfn, spfn, epfn) {
> > 		__init_single_page(pfn_to_page(pfn), pfn, zone, node);
> > 		__SetPageReserved(pfn_to_page(pfn));
> > 		pgcnt++;
> >+
> >+		/*
> >+		 * With vmemmap, at this stage all pages in an early section
> >+		 * have a valid memmap and are marked as online. However, only
> >+		 * subsections in the subsection map are actually online.
> >+		 */
> >+#ifdef CONFIG_SPARSEMEM_VMEMMAP
> >+		if (pfn >= next_chunk_pfn) {
> >+			is_online = pfn_section_valid(__pfn_to_section(pfn),
> pfn);
> >+			next_chunk_pfn = min(SUBSECTION_ALIGN_UP(pfn + 1),
> epfn);
> 
> The range iterates by for_each_valid_pfn() is [spfn, epfn - 1], so we
> don't
> expect pfn exceed epfn?
> 
> 			next_chunk_pfn = SUBSECTION_ALIGN_UP(pfn + 1);
> 
> Could be enough?

Yes, that's correct. Given the range [spfn, epfn), pfn should not exceed epfn.

I'd still prefer to keep min(..., epfn), as it keeps next_chunk_pfn within the
scanned window[spfn, epfn)  and makes the intended range explicit.

Best Regards,
Liu, Yuan

> >--
> >2.47.3
> 
> --
> Wei Yang
> Help you, Help me