Re: [PATCH v6 1/2] mm/memory_hotplug: optimize zone contiguous check when changing pfn range

[Wei Yang <richard.weiyang@gmail.com>]

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