From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ed1-f48.google.com (mail-ed1-f48.google.com [209.85.208.48]) (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 A877F2868B4 for ; Sat, 15 Aug 2026 01:55:37 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.208.48 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786758939; cv=none; b=bbg7+06YV4OCYFFh5g4I3/IGiAkD94cNA7yuXCetwfsMd+T9jwBGVe8iyu0woRDv1zOk3E6jO1YihfXX4f33k4tHaKsi8Y+sx9rf1RbIGliA71TkcMBcx4SaXwlpVdziMQvpYkQs3ezINCcIKCGfCnt5b7JxrAdQkTRwe8+a0kA= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786758939; c=relaxed/simple; bh=4l59T0BDYJ5NK/1SK4WiKIZQHOB3vRIBYhoVhx6dbQY=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Kev1Btmgyrd+A/QNoPxH+VZB5aJFEWmtroQB4Z8NyXt2QW+8HzD/B5m6ROsvcxzkgA+kBytXViAoE0OFRy2Ab+6lERJ8tWH+Og6s3jY83phQav/GYyUiHSVbHuYqxIIcltEPFhC2Yli6G22QiFzyJQFgLwaCyfflvX6+o8j4Fv4= 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=UKAAXKgT; arc=none smtp.client-ip=209.85.208.48 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="UKAAXKgT" Received: by mail-ed1-f48.google.com with SMTP id 4fb4d7f45d1cf-6a0a4a28cbdso2695225a12.3 for ; Fri, 14 Aug 2026 18:55:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1786758936; x=1787363736; 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=KJfedCHwXV6kZVr0Cknx5A9jpM+kExPkRG8SEE/c37w=; b=UKAAXKgTL5MjMigrygn34VEebG8SXEQhWMboSO/YjnJpBT6yBRn/Dq1SLzEPIi5a44 1t7Q/ts2qFMk32QrEwFcYd5XPJAO5i41xcmHRtphTvufti00KXjbTHC3B5fVcFTvBtJg 76BljPuWZXlOSQumjuSObcmSW/b7RICdhta1BEjF3SQguu6lu/w4tGBut0Jw0+7fQsdb 2ZbKrzNL/al6M+EmPfEqd1TaDgRlbOmmIHaDQFA+nKztng9511mLSOyk0fhJ82Ws1pC1 P3FiyXmGNyiYFpM4XZUuN7rVOdLoPkfoTitFymli5fSv1Kl/2KyZeF6asFA0zeQZ4ui9 jAMQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1786758936; x=1787363736; 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=KJfedCHwXV6kZVr0Cknx5A9jpM+kExPkRG8SEE/c37w=; b=JWiIadPlIrabBeswe9W1vDxOTU+jJ2RjZO4ABVsbY/YVuQE+gNCfo46w9ljfM6Msbx 0jyNES4JIoUtG7bGB9WwbSz1CwfWPJLDc0aDALErFT4RNQSwmShHT69EoClX/EGbUypQ lkNlTDTVejvh4nH32q3mBBlrneA6VLc9icNEOv8rdSqfpTaGa73lIERBf6rw2tZzEVqb MRn1Nt0hfA2Xlnb6rmEWhOBsOKg1Mq082NZ/9U1DMJBKbyLoRGGw2sbdJWMWSIrExBig Wq4ZlaOJKj2sNSnwGLbPxf7ndSx0L2FIhKq4IveI6qX5hk4NxNM4ymcqZ6CH+iZJEHgK /5yg== X-Forwarded-Encrypted: i=1; AHgh+RrhEryr8b4Efe/TIxZpwUXzW9hiEwruzrblGZec0giqA5OEZB1jUbd0GnD1VfZBi6RRyAvVNZnydlHxwKc=@vger.kernel.org X-Gm-Message-State: AOJu0Yx+g6MTR4aXo5ptke0dYcc43AO7qk46KicRki3b39MrnT8HqBOh wvFgBAKf82qDZUr54d3QWepm2tARI9JMEJ8lTyGA5cZTkUurZ3msdCMY X-Gm-Gg: AR+sD13HT4xgHrKkfLdDTnPdj6YEDFqp50NKwdQ85DgkktEu9MqLWqIzw9L7+yY+cOu MjFEJmJg39Waa1v5oUxg5IYLghsz6mUZRyhIbK58ybLS8mjEwcQItepU+R4Yz4OBPltksza3AFx dS6gSzFmFj5XAxDxinPRlFNVyp7BawQru96G3ZgizFUe93bOjzZa36skP03SmF8VrczmJyMRui5 1OtR231y1m9LUQHw7DSaIYrlHBi6hLbUnzm4tk8TbWc34qhf9H+/I1WDAiyDboI0bmIuevDUQe2 AGPGad5oHReDgoCgu+JK2SB8sYxvW5efm8Uw6IWfhe5wGzxqEBrP0MW4hogkZezjpwSPtFNuDn4 IvJ0vmmkJHVRafWy3gVPF3lc8RpLFfgngV68oD2U2Nclh8vngCNU9FJWEB4OnvzaImaByap9T9B SJ8o9j3qD5P1+gdrEz5lHt2/5wh7jdJKNTRne0utqaAlp7oJ88wNVMlEfK6X9Cj0fCE6G6AQ== X-Received: by 2002:a17:907:7b9c:b0:c19:6104:e5e4 with SMTP id a640c23a62f3a-c212a2b216cmr427391966b.20.1786758935575; Fri, 14 Aug 2026 18:55:35 -0700 (PDT) Received: from localhost ([185.92.221.13]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-c212378d9fcsm168007966b.34.2026.08.14.18.55.33 (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Fri, 14 Aug 2026 18:55:34 -0700 (PDT) Date: Sat, 15 Aug 2026 01:55:32 +0000 From: Wei Yang To: "David Hildenbrand (Arm)" Cc: "Liu, Yuan1" , 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 1/2] mm/memory_hotplug: optimize zone contiguous check when changing pfn range Message-ID: <20260815015532.ei4abygk4erdtlhh@master> Reply-To: Wei Yang References: <20260723084946.189392-1-yuan1.liu@intel.com> <20260723084946.189392-2-yuan1.liu@intel.com> <2cbd42e1-b5bb-474c-b0e7-ce3f46541891@kernel.org> <0b02ef3e-363f-4b49-a208-54cdfea56c75@kernel.org> <01795519-54e8-4ed8-a94e-2b77780abc3a@kernel.org> 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: <01795519-54e8-4ed8-a94e-2b77780abc3a@kernel.org> User-Agent: NeoMutt/20170113 (1.7.2) On Tue, Aug 11, 2026 at 02:23:18PM +0200, David Hildenbrand (Arm) wrote: >On 8/10/26 16:04, David Hildenbrand (Arm) wrote: >>> >>> Hi David >>> >>> My understanding is that init_unavailable_range() initializes all >>> PFNs that satisfy pfn_valid(), but not all of them satisfy >>> pfn_to_online_page(), since some PFNs belong to subsections that are >>> not online. >> >> Thanks for reminding me. I think the right direction is to finally clean up the >> pfn_valid() handling. >> >>> >>> You previously mentioned: >>> >>> pfn_valid() says early sections always have a full memmap, so even invalid >>> subsections have a memmap. pfn_to_online_page() says an invalid subsection >>> cannot be online and its content must be stale. for_each_valid_pfn() follows >>> pfn_valid() semantics, and we use it to initialize memmap that is not going >>> to be online and account it as pages_with_online_memmap, which is wrong. >>> >>> The cleanest approach is to avoid allocating memmap for subsections, which >>> also removes the special early-section handling from pfn_valid() and >>> for_each_valid_pfn(). >>> >>> I also share the concerns raised by Sashiko in the analysis below [1]: >>> >>> Scanners like isolate_migratepages_block() will then blindly iterate through >>> the pageblock and access the completely uninitialized struct pages of the hole, >>> leading to functional errors or kernel panics when reading these zero-filled >>> structures via macros like PageHuge() or page_zone(). >>> >>> That's why we went with the current approach in v6 instead of your >>> earlier suggestion. I'd really appreciate your guidance on which >>> direction you think would be more appropriate. >> Let me take a stab at just having pfn_valid() / for_each_valid_pfn() respecting >> the subsection map. > >... and that turns complicated very quickly. The problem is that we have some users, >in particular the buddy, that just assumes that MAX_PAGE_ORDER regions are fully >accessible. > >The fun begins once we have MAX_PAGE_ORDER span multiple subsections. So we'd actually >want to initialize the memmap. > >The pfn_valid() vs. pfn_to_online_page() inconsistency is really nasty :( > One thing I'd like to confirm. pfn_to_online_page() is expected to return an "online page", which is in buddy, right? >I mean, in init_unavailable_range() we could actually figure out fairly easily >whether we are dealing with holes where pfn_to_online_page() would succeed. > >diff --git a/mm/mm_init.c b/mm/mm_init.c >index e9c4204b73adb..54e71e17f2c3c 100644 >--- a/mm/mm_init.c >+++ b/mm/mm_init.c >@@ -843,11 +843,13 @@ static void __init init_unavailable_range(unsigned long spfn, > int zone, int node) > { > unsigned long pfn; >- u64 pgcnt = 0; >+ u64 pgcnt = 0, online_pgcnt = 0; > > for_each_valid_pfn(pfn, spfn, epfn) { > __init_single_page(pfn_to_page(pfn), pfn, zone, node); > __SetPageReserved(pfn_to_page(pfn)); >+ if (pfn_to_online_page(pfn)) >+ online_pgcnt++; But a hole in early section could still return a valid page if the hole is less than a subsection. Is this an expected behavior? > pgcnt++; > } > >If it's a problem performance-wise, we can always try optimizing by skipping >checks within the same (sub)section. > > >-- >Cheers, > >David -- Wei Yang Help you, Help me