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: Thursday, July 30, 2026 10:36 AM
> To: Liu, Yuan1 <yuan1.liu@xxxxxxxxx>
> Cc: Wei Yang <richard.weiyang@xxxxxxxxx>; David Hildenbrand
> <david@xxxxxxxxxx>; Oscar Salvador <osalvador@xxxxxxx>; Mike Rapoport
> <rppt@xxxxxxxxxx>; 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 Mon, Jul 27, 2026 at 09:49:39AM +0000, Liu, Yuan1 wrote:
> >> -----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.
> >
> 
> Looks reasonable.
> 
> I search the discussion history, and found David suggest this fix in [1]
> with
> following statement.
> 
>   Well, unless we have an odd case where the hole+memory starts in the
>   middle of a "PAGES_PER_SUBSECTION". That would already be problematic if
>   memory starts/ends in the middle of a PAGES_PER_SUBSECTION chunk. I
>   don't such a case exists.
> 
>   We could improve shrink_zone_span() to let
>   find_smallest_section_pfn/find_biggest_section_pfn test the pfn_to_nid()
>   and page_zone() not on;y on the smallest/highest pfn, but also on the
>   highest/smallest PFN in a PAGES_PER_SUBSECTION chunk.
> 
> I am trying to understand the exact case David described, but not fully
> get
> it. Would you mind describing more, so we would make sure not missing the
> point.
> 
> [1]: https://lore.kernel.org/all/e86fee84-08d8-4563-8596-
> e40d8e196799@xxxxxxxxxx/T/#u

I am not sure whether this scenario can occur on real systems, but
from reading the current code it seems there may be a corner case.

For example, consider a zone whose tail ends with a single-PFN hole.
init_unavailable_range() initializes this hole page and assigns its
nid/zone to the adjacent next zone, since the PFN itself belongs outside 
the current zone.

When memory is hot-added after this hole, the hotplug granularity
(PAGES_PER_SUBSECTION) extends the zone span to include the entire
subsection containing the hole.

Later, when the hot-added memory is removed,
find_biggest_section_pfn() examines the hole page first. Because its
nid/zone belongs to the adjacent next zone, the boundary subsection may 
be considered removable even though it still contains boot memory pages.

This appears to be asymmetric between the zone head and the zone tail.

At the zone head (find_smallest_section_pfn()), the boundary subsection
has a hole + memory layout. The hole pages belong to the current zone,
so the subsection is retained.

At the zone tail (find_biggest_section_pfn()), the boundary subsection
has a memory + hole layout. The trailing hole pages belong to the
adjacent next zone, so the subsection may be discarded.

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