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

[Wei Yang]

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

-- 
Wei Yang
Help you, Help me