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

[David Hildenbrand (Arm)]

On 8/12/26 11:17, Liu, Yuan1 wrote:
>> -----Original Message-----
>> From: David Hildenbrand (Arm) <david@xxxxxxxxxx>
>> Sent: Tuesday, August 11, 2026 8:23 PM
>> To: Liu, Yuan1 <yuan1.liu@xxxxxxxxx>; Oscar Salvador <osalvador@xxxxxxx>;
>> Mike Rapoport <rppt@xxxxxxxxxx>; Wei Yang <richard.weiyang@xxxxxxxxx>
>> Cc: 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 1/2] mm/memory_hotplug: optimize zone contiguous
>> check when changing pfn range
>>
>> On 8/10/26 16:04, David Hildenbrand (Arm) wrote:
>>>
>>> Thanks for reminding me. I think the right direction is to finally clean
>> up the
>>> pfn_valid() handling.
>>>
>> invalid
>> subsection
>> follows
>> going
>> wrong.
>> which
>> through
>> the hole,
>> zero-filled
>>> 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 :(
>>
>> 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++;
>>                 pgcnt++;
>>         }
>>
>> If it's a problem performance-wise, we can always try optimizing by
>> skipping
>> checks within the same (sub)section.
>  
> Hi David
> 
> What about the following approach to skipping the check within the same
> (sub)section?
> 
> @@ -827,11 +827,21 @@ 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;
> +       unsigned long last_subsection = -1;
> +       bool is_online = false;
> 
>         for_each_valid_pfn(pfn, spfn, epfn) {
> +               unsigned long subsection = pfn & PAGE_SUBSECTION_MASK;
> +
>                 __init_single_page(pfn_to_page(pfn), pfn, zone, node);
>                 __SetPageReserved(pfn_to_page(pfn));
> +               if (subsection != last_subsection) {
> +                       is_online = !!pfn_to_online_page(pfn);
> +                       last_subsection = subsection;
> +               }
> +               if (is_online)
> +                       online_pgcnt++;
>                 pgcnt++;
> 
> If this looks good, let me prepare and send out the next version series.

Yeah, something like that should do.

-- 
Cheers,

David