From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ej1-f54.google.com (mail-ej1-f54.google.com [209.85.218.54]) (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 659621F3D56 for ; Sat, 1 Aug 2026 01:00:00 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.218.54 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785546002; cv=none; b=Olq3XIWmORPRbiwfpxq1HQcH+l7dZHwGvs4RbSstGYpW0f5e4N64vSdCAd1p5DBE4Pla7BePPb9wWjF35rjzvsl/fyGWRQq/D2cisueCBJNwoQEutV0yd0YgYfOII3YxOdwM8p9Lm0TwbWF2ZZcbiQ/M3FfxAnr/888UeFImiTw= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785546002; c=relaxed/simple; bh=SQo+7n+JYa2GrRHrsJRCmPr9eXqurie+mbwBl2EjGnc=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=s5Rq87Ku3Tshy69GkEV/lIpKuV7kmargTx3YO+fr6NLf9bP86pZqTwqOoqSkHfGRKjdSG9HQuiv+gmkz408QcJ7S9OFSyaJ1aEw4/YBDZo/QUc9MUOwegVjlRDSOPggILtNgpzMFK7MxPx1h4Lexhjv7SumRxF3D5FkG8Vt1Gi0= 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=ZB+xucqD; arc=none smtp.client-ip=209.85.218.54 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="ZB+xucqD" Received: by mail-ej1-f54.google.com with SMTP id a640c23a62f3a-c15ba3a2b4bso190000766b.1 for ; Fri, 31 Jul 2026 18:00:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1785545999; x=1786150799; 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=a1kBDM70IBvvYkk/+MFqrnirdBa212qXnM+40/RiAlg=; b=ZB+xucqD6ZvN1PJZJGn/JOis0tPRTeXKqy/aVG/vbVtrwvF+9uVGnjrjQ+79muVjMb 69jooqwYvVUAmqQhkfjRf6uTmQq5rW5TJWQRSJvoQRbbXw6rI3LQuBKrcXdi2UgL/V5y qICJSMhgxxVd8cGYhYCzDQziCVIUttHRIPSUQteLgoi8CRxPKxJLArPpQkN8SiwUq+4P SstSpPbGVlPd1DF2i3XFXo5zkNYVUlbkstlSWleoYfd5btPGx0Qy74RuLr+0CNMjpneH U6DSJCd82QcwTa1f/SW/9Kt690A9RwkUXtbv981rQhCU+oKGRfW2ZiWtYs+5CLmMOH+v D6dQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1785545999; x=1786150799; 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=a1kBDM70IBvvYkk/+MFqrnirdBa212qXnM+40/RiAlg=; b=BFaUzwj8UL6FgCl6ZwhIJwqSbh29Fd8tRcfvd8dladRUeZGGBEs2DRr9ssdP5K8CX9 OOAKHn1Z1UFV287SdxsbQxaxrJqOG1ZKRF8wmmavHtdJgFxY4mGzELewkYSnsJnZR4oB vpCV8hBeSN4FkJ1Max2n6GEu68zX8BUv4nhCi6m6kjQcGjWe3nqeveQyi/82P6W78Mpp rY5TnIeWPncf1TlMuI3TvtiGNwFqlGx/Ojw2zoCo6kR8MjP76SjyhIkHnFO5iRr9DKQY qrk7Ol5iDwa9EVuLiDWISFhn0uM3WNS0xX49esZLrgHrJdnKL8l2p6uaWD4+iwHhYwDq Mweg== X-Forwarded-Encrypted: i=1; AHgh+RqUtIuhK0J1+ecis+QaVOfDjBSB2HeKK7cTcC4yBZg5PjnOW3HqcaefEV05ZLZYeozIjbb8VuTOEM5GQ+A=@vger.kernel.org X-Gm-Message-State: AOJu0Yy+n63kXUfdr7Lxc7AM6fRhSkTlIyb9EKyUOZRnGssHtye6Em4D E5ujmgKKEjULnCQcX16rD9susxquZSaeI0Nrt4BNq0hMUWE8hFkfRfHm X-Gm-Gg: AR+sD13izbfd43kEU5kt/EwoHMV/kRMmgacIALlEWJO7Q2V9Cu7z+yKmvxLmobBi2m3 gR4t0PGPRw0s8DCzJL0v2Y8lWNJJx6cNo80dzriI++n96fBnnljH9QpvyaSbpQjqgCYrVdyVELw adugcXq6IPmTfiP7UgtCf1EbxGXP8G+Z/9eKXzRva+A+o0Eq8XzhdE1eVqWxjTlbV3+QtdRyf2g 5Vj9/hSklBZlE/IbIX8AzkXNDSDN2yO9QY039hjUS3rPZXS8sQ87PCd/6mwa53b8b1hyWtHft+U VYXbBbRPzKso+A2aQPCSdmiRBMwrl3yNS2YMAjZzB08n0YOb8f2obEJ5ygAazI/WCPmPiJoibo3 OoK3Napc8ppMRAwE8hlH/tejBaOstKGH3jWeT7A19MnZLPt23ZzkkwkIBZ1c37pgWc1EWudXmSz jw41vES5RyZ4xJKrioWdBNx2x6LjGAzpxOOSaKwjXXqJNBIxYTjTClTxvE+/E= X-Received: by 2002:a17:906:6bc8:b0:c15:f4aa:f30e with SMTP id a640c23a62f3a-c1fe816e548mr71943466b.37.1785545998452; Fri, 31 Jul 2026 17:59:58 -0700 (PDT) Received: from localhost ([185.92.221.13]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-c1fd455ab7dsm270629066b.58.2026.07.31.17.59.56 (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Fri, 31 Jul 2026 17:59:57 -0700 (PDT) Date: Sat, 1 Aug 2026 00:59:56 +0000 From: Wei Yang To: "Liu, Yuan1" Cc: Wei Yang , David Hildenbrand , Oscar Salvador , Mike Rapoport , "linux-mm@kvack.org" , "Zou, Nanhai" , "Deng, Pan" , "Li, Tianyou" , Chen Zhang , "Zeng, Jason" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH v6 2/2] mm/memory_hotplug: improve shrink_zone_span() subsection boundary checks Message-ID: <20260801005956.6twsbk6cergenv57@master> Reply-To: Wei Yang References: <20260723084946.189392-1-yuan1.liu@intel.com> <20260723084946.189392-3-yuan1.liu@intel.com> <20260725024952.65lfm2s676obpoue@master> <20260730023614.fasdjzfl5wz4tsfr@master> 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 Thu, Jul 30, 2026 at 07:57:13AM +0000, Liu, Yuan1 wrote: >> -----Original Message----- >> From: Wei Yang >> Sent: Thursday, July 30, 2026 10:36 AM >> To: Liu, Yuan1 >> Cc: Wei Yang ; David Hildenbrand >> ; Oscar Salvador ; Mike Rapoport >> ; linux-mm@kvack.org; Zou, Nanhai ; >> Deng, Pan ; Li, Tianyou ; Chen >> Zhang ; Zeng, Jason ; linux- >> kernel@vger.kernel.org >> 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 >> >> Sent: Saturday, July 25, 2026 10:50 AM >> >> To: Liu, Yuan1 >> >> Cc: David Hildenbrand ; Oscar Salvador >> >> ; Mike Rapoport ; Wei Yang >> >> ; linux-mm@kvack.org; Zou, Nanhai >> >> ; Deng, Pan ; Li, Tianyou >> >> ; Chen Zhang ; Zeng, Jason >> >> ; linux-kernel@vger.kernel.org >> >> 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 >> >> >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; >> >> >> >> 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@kernel.org/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. > IIUC, for memory hotplug, the alignment is guaranteed by check_hotplug_memory_range() with min block size which is section aligned. >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. > I am trying to see what the real case it would be. Hmm.. First I don't expect there would be memory hotplug happens between zones or nodes. So the new added memory range always sits after boot memory(early sections). Then let's assume below memory layout after bootup. |< Zone Normal >| |< Zone Movable >| +----------------+.....+----------------+.....+ | |hole1| |hole2| +----------------+.....+----------------+.....+ Both hole is less than subsection size, and the hole2 expand to section aligned address. According to the init_unavailable_range() during memmap_init(), both holes are init to Zone Movable. Then we can hot-add next section and hot-online it to: 1) Zone Movable 2) Zone Normal For case 1), if we offline this section, we run into find_biggest_section_pfn() for Zone Movable. It test the last pfn in hole2, and see it meets the condition. So Zone Movable results in spanning hole2, which is larger than original value. This is not that accurate, but still could work. For case 2), if we offline this section, we run into find_biggest_section_pfn() for Zone Normal. Here we also have two cases based on the location of the hole: a) hole starts at the beginning of subsection (hole + memory) b) hole ends at the end of subsection (memory + hole) For a), it looks good, because the subsection hole is treated as Zone Movable, and Zone Normal is correctly sized. For b), it seems not right, as we only test subsection's last pfn. As it belongs to Zone Movable we would skip the subsection and set Normal Zone span to previous subsection. Even some of the pfn belongs to Zone Normal. Hmm... I guess this is the case David mentioned. And with your patch, it checks head_pfn of this subsection, so it fixes this by extend Zone Normal to just ahead Zone Movable. This works something like a), but make Zone Normal span to some pfn initialized to Zone Movable. Based on above analysis, let's see when find_smallest_section_pfn() will have trouble. The case in my mind is like below: |< Zone Normal >|< Zone Movable >| +----------------+----------------+.....+----------------+ | | |hole | | +----------------+----------------+.....+----------------+ Hole should sits at the beginning of a section, otherwise offline_pages() would prevent offline the range. Then when offline those beginning Movable memory, find_smallest_section_pfn() is triggered. But as hole is init to Zone Movable, it looks good. On summary: * find_biggest_section_pfn() may truncate zone span range if there is a hole less than subsection sits at the end of subsection * find_smallest_section_pfn() looks good even there is a hole less than subsection Above is my understanding about the possible cases when re-sizing zone. In case missing some point, just let me know :-) -- Wei Yang Help you, Help me