RE: [PATCH v6 2/2] mm/memory_hotplug: improve shrink_zone_span() subsection boundary checks

[Liu, Yuan1]

> -----Original Message-----
> From: Wei Yang <richard.weiyang@xxxxxxxxx>
> Sent: Saturday, July 25, 2026 10:50 AM
> 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>; Deng, Pan <pan.deng@xxxxxxxxx>; Li, Tianyou
> <tianyou.li@xxxxxxxxx>; Chen Zhang <zhangchen.kidd@xxxxxx>; Zeng, Jason
> <jason.zeng@xxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v6 2/2] mm/memory_hotplug: improve shrink_zone_span()
> subsection boundary checks
> 
> On Thu, Jul 23, 2026 at 04:49:46AM -0400, Yuan Liu wrote:
> >When shrinking a zone span after removing a PFN range,
> >find_smallest_section_pfn() and find_biggest_section_pfn()
> >only checked one edge PFN in each subsection for nid/zone matching.
> >
> >If a memory or hole boundary falls in the middle of a subsection,
> >that edge PFN may belong to a different nid/zone, causing the helpers
> >to miss a valid PFN within that subsection.
> >
> >Fix this by checking both subsection edge PFNs for nid/zone matching.
> >Keep a single pfn_to_online_page() check per subsection, since online
> >state is the same for all PFNs in a subsection.
> >
> >Reviewed-by: Jason Zeng <jason.zeng@xxxxxxxxx>
> >Signed-off-by: Yuan Liu <yuan1.liu@xxxxxxxxx>
> >---
> > mm/memory_hotplug.c | 42 +++++++++++++++++++++++++++---------------
> > 1 file changed, 27 insertions(+), 15 deletions(-)
> >
> >diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> >index 4c699fd9c479..3a281d595207 100644
> >--- a/mm/memory_hotplug.c
> >+++ b/mm/memory_hotplug.c
> >@@ -427,17 +427,24 @@ static unsigned long find_smallest_section_pfn(int
> nid, struct zone *zone,
> > 				     unsigned long start_pfn,
> > 				     unsigned long end_pfn)
> > {
> >-	for (; start_pfn < end_pfn; start_pfn += PAGES_PER_SUBSECTION) {
> >-		if (unlikely(!pfn_to_online_page(start_pfn)))
> >-			continue;
> >+	unsigned long next_pfn;
> >
> >-		if (unlikely(pfn_to_nid(start_pfn) != nid))
> >-			continue;
> >+	for (; start_pfn < end_pfn; start_pfn = next_pfn) {
> >+		unsigned long tail_pfn;
> >
> >-		if (zone != page_zone(pfn_to_page(start_pfn)))
> >+		next_pfn = start_pfn + PAGES_PER_SUBSECTION;
> >+		tail_pfn = next_pfn - 1;
> >+
> >+		if (unlikely(!pfn_to_online_page(start_pfn)))
> > 			continue;
> >
> >-		return start_pfn;
> >+		if (likely(pfn_to_nid(start_pfn) == nid) &&
> >+		    zone == page_zone(pfn_to_page(start_pfn)))
> >+			return start_pfn;
> >+
> >+		if (likely(pfn_to_nid(tail_pfn) == nid) &&
> >+		    zone == page_zone(pfn_to_page(tail_pfn)))
> >+			return start_pfn;
> 
> Here we are checking range [start_pfn, tail_pfn]. When we come here, it
> means
> start_pfn's nid or zone doesn't match our expectation. But if tail_pfn
> does,
> why it still return start_pfn?

Hi Wei

If start_pfn falls into a hole while tail_pfn still belongs to a valid
memblock in this zone, skipping the subsection would cause
shrink_zone_span() to shrink the zone span too aggressively, excluding
valid PFNs from the zone.

Since init_unavailable_range() initializes hole pages with the correct
zone/nid, the start_pfn check always succeeds here, making the tail_pfn
check redundant today.

That said, I wonder if it is still worth keeping this check so that
shrink_zone_span() does not depend on how hole pages are initialized.

> 
> > 	}
> >
> > 	return 0;
> >@@ -448,21 +455,26 @@ static unsigned long find_biggest_section_pfn(int
> nid, struct zone *zone,
> > 				    unsigned long start_pfn,
> > 				    unsigned long end_pfn)
> > {
> >-	unsigned long pfn;
> >+	unsigned long pfn, prev_pfn;
> >
> > 	/* pfn is the end pfn of a memory section. */
> > 	pfn = end_pfn - 1;
> >-	for (; pfn >= start_pfn; pfn -= PAGES_PER_SUBSECTION) {
> >-		if (unlikely(!pfn_to_online_page(pfn)))
> >-			continue;
> >+	for (; pfn >= start_pfn; pfn = prev_pfn) {
> >+		unsigned long head_pfn;
> >
> >-		if (unlikely(pfn_to_nid(pfn) != nid))
> >-			continue;
> >+		prev_pfn = pfn - PAGES_PER_SUBSECTION;
> >+		head_pfn = prev_pfn + 1;
> >
> >-		if (zone != page_zone(pfn_to_page(pfn)))
> >+		if (unlikely(!pfn_to_online_page(pfn)))
> > 			continue;
> >
> >-		return pfn;
> >+		if (likely(pfn_to_nid(pfn) == nid) &&
> >+		    zone == page_zone(pfn_to_page(pfn)))
> >+			return pfn;
> >+
> >+		if (likely(pfn_to_nid(head_pfn) == nid) &&
> >+		    zone == page_zone(pfn_to_page(head_pfn)))
> >+			return pfn;
> > 	}
> >
> > 	return 0;
> >--
> >2.47.3
> 
> --
> Wei Yang
> Help you, Help me