From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ej1-f46.google.com (mail-ej1-f46.google.com [209.85.218.46]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 920CA23D7F4 for ; Fri, 7 Aug 2026 03:22:10 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.218.46 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786072932; cv=none; b=If8fcX4wXgWBLR8jw/wQCqQTVo9su1ZvKZjW5Q/2/mGvGxYchylpUB5CoUYGnhtzqQFtb/Lw1KGtS9wo1j2JwAgPacdl4T39Yr9jRIEjWDkQzdWzrIZf7ZzkB2i1xrL0QlxZSAnRB6DAhhxwdVXkfL9gpsM6f+0MGyCyoTOSXF8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786072932; c=relaxed/simple; bh=TYkdZlNai1TbqCBEHYdECqI/0+tdtS7PjyC+TmldCBU=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=SL2b8RZEdOOqUynEj5EdA2EyEQzDwygnOQo9PZ73zzp1grHvZeMhHvcASbLctlLLC9IHf6cL0oXKGzU0T8c4Y+rOO/UIhPbxAHncm5FBKb9JVV3H8BVIEGT318FzqiojzfcbX00sEvSKjXgKaWo3uisqjeNTz6V4ey6oPjYil3M= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=pZHwjLw2; arc=none smtp.client-ip=209.85.218.46 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="pZHwjLw2" Received: by mail-ej1-f46.google.com with SMTP id a640c23a62f3a-c169ae1cb26so290698666b.1 for ; Thu, 06 Aug 2026 20:22:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1786072929; x=1786677729; darn=vger.kernel.org; h=user-agent:in-reply-to:content-disposition:content-type :mime-version:references:reply-to:message-id:subject:cc:to:from:date :from:to:cc:subject:date:message-id:reply-to:content-type; bh=kHc5p3lKJbmPBh7ge2UhBNPTqES+fAoQf4O+7yGB4mc=; b=pZHwjLw2Foo8Cgr/AhLwmy60qpHRsmMPUlSyymbYbJrRsB8IeaJN9uLlF3GnBc6SiY DZfAKExaLsRuwlM5+Lh3gkiHWFM3U/Mr2G5ZlMvQz96sFbiWz0q4FacW9od6+et1uxAM 7jrpDNJ4KSbrCzdwAh52HqXrZfVlGMC7Bo2FIw/81qdtJMKrXrAeW/wZxBOqZ4MSSJWC ZIV9M5k9DTvXpj7+VwN/TKzThJawUIQIGJ/XVBqJ7Ub4iRTVkWCLD6bVLBMed039ITMt 9bXnl0+NYSFlk+9GnfwMDI8H87XiKq2b8HouGasDudzu8i5xXpVUVXTNVc2aeWuKWcOs TZ0A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1786072929; x=1786677729; h=user-agent:in-reply-to:content-disposition:content-type :mime-version:references:reply-to:message-id:subject:cc:to:from:date :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to:content-type; bh=kHc5p3lKJbmPBh7ge2UhBNPTqES+fAoQf4O+7yGB4mc=; b=mg0TuKZbPXUOKng4MJCrok0vYzBsSwuFbl9vgxmjt36o2YsFRYGrM68k4vhDxFuXI2 UIjs3yfYfwQ3vbcdep82PfcJmtdf7WlxO+2z1FFtQuFQUvyZrCDoGjis0tHehgZldrXb kYx2OQoGp2dG+i1CBzUCUzwx3+fax+r7CGyxjy+BU3zuWHzukn2sja12p0pkIhRPJvoW 8YKNA32qjkg5kWWwbdkYHgyQhgos/pTtla1yt8XexSGMYCdHBqlMtR0pO7CaCOo5vTPc 9+UMHIVLuwnjQ4erom7yuupLw2dFk0vWGV7nn1djGJc8bE93Hrtn1g66Kx2/+msQJjg0 03rA== X-Forwarded-Encrypted: i=1; AHgh+RoHrTzY3Pu6duNQjaF/3SKM+2WcD9gEeq73imeS0w0vqnmU+mns+wZNJBnzh5RSeAmhfCjrSUoIYIk+ttI=@vger.kernel.org X-Gm-Message-State: AOJu0YztT9SWCur5ZYC+wbbgRVMW4ADLz6unApj0RziSsFMP0C6YO1cE b+lna++P3GBC2NgDQtYh03OyvbtYNRyP0s7TO1x3DeK/AaNfS+W0LpSQm+QATA== X-Gm-Gg: AR+sD10bE7y4SpuXSfxhFvSMVhrQAZXyu4l8juXaSF5gSUWUxNzkn3ANXSureJS8NUc nq78nABDOhEeRlYAN0qzLuh7REYrcVdiQabmJfDpL9Zq+NvNmuibXY6a4ge68s/Qt2JwWFYsX4F o3W+w6MvXo+UU82T3Flp0HXxRVmaOJQKFwLYItRlYcqCkK4J2TKMUg1N00IIxBLIBPKUcEdIZTq c4iBxnP7jkqQat2IWBMZOF7AEmxUWB0aSG4ZEReDyxeS036SY7flfNavCL/y5UkcLA41DQsMYTm b0UfURmgj/DaFTFIJdjRAWqtWQ/xVfwsyYv4QvEYwQ8hHKGCGrkTAM+r0onjnRn8r2y+1YTJGxw pyxdwTNWVtdJZHCwSYIll/gXAjM+MhT1eTWXahhdmdOhEje45F/jvDxm0H2H3HLTIiOOJbiJRss XO851BcZJAzzv1Gr18mgKWcwreJwJFz8wx1fsZ3eTVOQ1ffuKWkeioEYfktJOTB+78P8Bcmg== X-Received: by 2002:a17:907:d9e:b0:c1c:2218:f317 with SMTP id a640c23a62f3a-c2057d85210mr515418366b.2.1786072928434; Thu, 06 Aug 2026 20:22:08 -0700 (PDT) Received: from localhost ([185.92.221.13]) by smtp.gmail.com with ESMTPSA id 4fb4d7f45d1cf-6a1967117c7sm1589944a12.28.2026.08.06.20.22.06 (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Thu, 06 Aug 2026 20:22:07 -0700 (PDT) Date: Fri, 7 Aug 2026 03:22:05 +0000 From: Wei Yang To: "David Hildenbrand (Arm)" Cc: Yuan Liu , Oscar Salvador , Mike Rapoport , Wei Yang , linux-mm@kvack.org, Nanhai Zou , Pan Deng , Tianyou Li , Chen Zhang , Jason Zeng , linux-kernel@vger.kernel.org Subject: Re: [PATCH v6 2/2] mm/memory_hotplug: improve shrink_zone_span() subsection boundary checks Message-ID: <20260807032205.ykhxbapx6nhg5hpt@master> Reply-To: Wei Yang References: <20260723084946.189392-1-yuan1.liu@intel.com> <20260723084946.189392-3-yuan1.liu@intel.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) On Wed, Aug 05, 2026 at 01:02:29PM +0200, David Hildenbrand (Arm) wrote: >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 >> Signed-off-by: Yuan Liu >> --- >> 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)" >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) >--- > 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)); >+} Hi, David I am thinking if we could initialize each subsection to only one zone, we may reduce the complexity? For example, in init_unavailable_range() align spfn to subsection and init the head unaligned part to previous zone? Do you think this is reasonable? > >- 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.43.0 > > >-- >Cheers, > >David -- Wei Yang Help you, Help me