Re: [PATCH v8 1/2] mm/memory_hotplug: make shrink_zone_span() more robust

[Wei Yang <richard.weiyang@gmail.com>]

On Tue, Sep 01, 2026 at 01:29:49AM -0400, Yuan Liu wrote:
>From: "David Hildenbrand (Arm)" <david@kernel.org>
>
>Let's make shrink_zone_span() more robust by checking in
>find_smallest_section_pfn() / find_biggest_section_pfn() that the
>start and end PFNs of the subsection are within the zone.

Either the start or end PFN is within the zone?

>
>While at it, clean up the function by factoring the core check out
>into subsection_overlaps_zone().
>
>There likely is no need to check the nid first. We require
>SPARSEMEM_VMEMMAP_ENABLE, where pfn_to_page() is cheap, and
>pfn_to_nid() on CONFIG_NUMA would call pfn_to_page() either way.
>So let's just drop that for now.
>
>Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
>Tested-by: Yuan Liu <yuan1.liu@intel.com>
>Signed-off-by: Yuan Liu <yuan1.liu@intel.com>

Otherwise, LGTM.

Reviewed-by: Wei Yang <richard.weiyang@gmail.com>

>---
> mm/memory_hotplug.c | 59 ++++++++++++++++++---------------------------
> 1 file changed, 24 insertions(+), 35 deletions(-)
>
>diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
>index 226ab9cb078a..9f19876ec3ec 100644
>--- a/mm/memory_hotplug.c
>+++ b/mm/memory_hotplug.c
>@@ -425,49 +425,39 @@ int __add_pages(int nid, unsigned long pfn, unsigned long nr_pages,
> 	return err;
> }
> 
>-/* find the smallest valid pfn in the range [start_pfn, end_pfn) */
>-static unsigned long find_smallest_section_pfn(int nid, struct zone *zone,
>-				     unsigned long start_pfn,
>-				     unsigned long end_pfn)
>+static bool subsection_overlaps_zone(unsigned long pfn, struct zone *zone)
> {
>-	for (; start_pfn < end_pfn; start_pfn += PAGES_PER_SUBSECTION) {
>-		if (unlikely(!pfn_to_online_page(start_pfn)))
>-			continue;
>+	const unsigned long start_pfn = ALIGN_DOWN(pfn, PAGES_PER_SUBSECTION);
>+	const unsigned long end_pfn = start_pfn + PAGES_PER_SUBSECTION - 1;
> 
>-		if (unlikely(pfn_to_nid(start_pfn) != nid))
>-			continue;
>+	/* All pages in a subsection are either online or offline. */
>+	if (unlikely(!pfn_to_online_page(start_pfn)))
>+		return false;
> 
>-		if (zone != page_zone(pfn_to_page(start_pfn)))
>-			continue;
>+	/* Checking start+end is sufficient. */
>+	return zone == page_zone(pfn_to_page(start_pfn)) ||
>+	       zone == page_zone(pfn_to_page(end_pfn));
>+}
> 
>-		return start_pfn;
>+/* find the smallest valid pfn in the range [start_pfn, end_pfn) */
>+static unsigned long find_smallest_section_pfn(struct zone *zone,
>+		unsigned long start_pfn, unsigned long end_pfn)
>+{
>+	for (; start_pfn < end_pfn; start_pfn += PAGES_PER_SUBSECTION) {
>+		if (subsection_overlaps_zone(start_pfn, zone))
>+			return start_pfn;
> 	}
>-
> 	return 0;
> }
> 
> /* find the biggest valid pfn in the range [start_pfn, end_pfn). */
>-static unsigned long find_biggest_section_pfn(int nid, struct zone *zone,
>-				    unsigned long start_pfn,
>-				    unsigned long end_pfn)
>+static unsigned long find_biggest_section_pfn(struct zone *zone,
>+		unsigned long start_pfn, unsigned long end_pfn)
> {
>-	unsigned long 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;
>-
>-		if (unlikely(pfn_to_nid(pfn) != nid))
>-			continue;
>-
>-		if (zone != page_zone(pfn_to_page(pfn)))
>-			continue;
>-
>-		return pfn;
>+	for (; end_pfn > start_pfn; end_pfn -= PAGES_PER_SUBSECTION) {
>+		if (subsection_overlaps_zone(end_pfn - 1, zone))
>+			return end_pfn - 1;
> 	}
>-
> 	return 0;
> }
> 
>@@ -475,7 +465,6 @@ static void shrink_zone_span(struct zone *zone, unsigned long start_pfn,
> 			     unsigned long end_pfn)
> {
> 	unsigned long pfn;
>-	int nid = zone_to_nid(zone);
> 
> 	if (zone->zone_start_pfn == start_pfn) {
> 		/*
>@@ -484,7 +473,7 @@ static void shrink_zone_span(struct zone *zone, unsigned long start_pfn,
> 		 * In this case, we find second smallest valid mem_section
> 		 * for shrinking zone.
> 		 */
>-		pfn = find_smallest_section_pfn(nid, zone, end_pfn,
>+		pfn = find_smallest_section_pfn(zone, end_pfn,
> 						zone_end_pfn(zone));
> 		if (pfn) {
> 			zone->spanned_pages = zone_end_pfn(zone) - pfn;
>@@ -500,7 +489,7 @@ static void shrink_zone_span(struct zone *zone, unsigned long start_pfn,
> 		 * In this case, we find second biggest valid mem_section for
> 		 * shrinking zone.
> 		 */
>-		pfn = find_biggest_section_pfn(nid, zone, zone->zone_start_pfn,
>+		pfn = find_biggest_section_pfn(zone, zone->zone_start_pfn,
> 					       start_pfn);
> 		if (pfn)
> 			zone->spanned_pages = pfn - zone->zone_start_pfn + 1;
>-- 
>2.47.3

-- 
Wei Yang
Help you, Help me