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

[Liu, Yuan1]

> -----Original Message-----
> From: David Hildenbrand (Arm) <david@xxxxxxxxxx>
> Sent: Wednesday, August 5, 2026 7:02 PM
> To: Liu, Yuan1 <yuan1.liu@xxxxxxxxx>; Oscar Salvador <osalvador@xxxxxxx>;
> Mike Rapoport <rppt@xxxxxxxxxx>; Wei Yang <richard.weiyang@xxxxxxxxx>
> Cc: 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 7/23/26 10:49, 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;
> >  	}
> >
> >  	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;
> 
> 
> I think improving that should be patch #1.
> 
> But I think we can do much better code-wise. What about the following
> cleanup instead:
> 
> 
> From 6e9ea094daf8d04c0b119b780572a846334f8164 Mon Sep 17 00:00:00 2001
> From: "David Hildenbrand (Arm)" <david@xxxxxxxxxx>
> Date: Wed, 5 Aug 2026 12:52:55 +0200
> Subject: [PATCH] mm/memory_hotplug: make shrink_zone_span() more robust
> 
> Let's make shrink_zone_span() more robust by checking in
> find_smallest_section_pfn() / find_biggest_section_pfn() that start+end
> of the subsection.
> 
> While at it, clean up the function heavily, factoring the core check
> out into subsection_overlaps_zone().
> 
> There likely is no need to check for the nid first: we require
> SPARSEMEM_VMEMMAP_ENABLE where pfn_to_page() is cheap, and the
> pfn_to_nid() on CONFIG_NUMA would do a pfn_to_page() either way. So
> let's just drop that for now.
> 
> Signed-off-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>
> ---
>  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 226ab9cb078ad..79937f9672d01 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;

Hi David

Thanks for drafting this patch and for the guidance. I'll fold it into
the next revision as patch 1.

> 
> 
> --
> Cheers,
> 
> David