Re: [PATCH v3] mm/memory hotplug/unplug: Optimize zone contiguous check when changing pfn range

["David Hildenbrand (Arm)" <david@kernel.org>]

On 4/14/26 09:06, Liu, Yuan1 wrote:
>> -----Original Message-----
>> From: David Hildenbrand (Arm) <david@kernel.org>
>> Sent: Thursday, April 9, 2026 11:08 PM
>> To: Mike Rapoport <rppt@kernel.org>
>> Cc: Liu, Yuan1 <yuan1.liu@intel.com>; Oscar Salvador <osalvador@suse.de>;
>> Wei Yang <richard.weiyang@gmail.com>; linux-mm@kvack.org; Hu, Yong
>> <yong.hu@intel.com>; Zou, Nanhai <nanhai.zou@intel.com>; Tim Chen
>> <tim.c.chen@linux.intel.com>; Zhuo, Qiuxu <qiuxu.zhuo@intel.com>; Chen, Yu
>> C <yu.c.chen@intel.com>; Deng, Pan <pan.deng@intel.com>; Li, Tianyou
>> <tianyou.li@intel.com>; Chen Zhang <zhangchen.kidd@jd.com>; linux-
>> kernel@vger.kernel.org
>> Subject: Re: [PATCH v3] mm/memory hotplug/unplug: Optimize zone contiguous
>> check when changing pfn range
>>
>> On 4/9/26 16:40, Mike Rapoport wrote:
>> a
>> pageblock
>> cost
>> of
>> present
>> further
>> towards
>> prevents
>> the
>> --+
>> Reduction |
>> --+
>> |
>> --+
>> |
>> --+
>> --+
>> Reduction |
>> --+
>> |
>> --+
>> |
>> --+
>> \
>> of
>> hotplugged
>> long *pfn)
>> last
>> spfn,
>> long spfn,
>> ranges\n",
>> zone *zone,
>>>
>>> Yeah, this looks better :)
>>>
>>> sashiko had several comments
>>> https://sashiko.dev/#/patchset/20260408031615.1831922-1-
>> yuan1.liu%40intel.com
>>>
>>> I skipped the ones related to hotplug, but in the mm_init part the
>> comment
>>> about zones that can have overlapping physical spans when mirrored
>>> kernelcore is enabled seems valid.
> 
> Hi David & Mike
> 
> I’ve spent some time working through these issues to better understand them.
> For the overlapping physical spans(mirrored kernelcore), should I avoid counting 
> overlap_memmap_init in memmap_init_range in the next version? 
> For example, change it as follows: 
> 
> +unsigned long __meminit
> +memmap_init_range(unsigned long size, int nid, unsigned long zone,
> +               unsigned long start_pfn,
> +               unsigned long zone_end_pfn,
>                 enum meminit_context context,
>                 struct vmem_altmap *altmap, int migratetype,
>                 bool isolate_pageblock)
>  {
>         unsigned long pfn, end_pfn = start_pfn + size;
> +       unsigned long nr_init = 0;
>         struct page *page;
> 
>         if (highest_memmap_pfn < end_pfn - 1)
> @@ -893,7 +897,7 @@ void __meminit memmap_init_range(unsigned long size, int nid, unsigned long zone
>         if (zone == ZONE_DEVICE) {
>                 if (!altmap)
> -                       return;
> +                       return 0;
> 
>                 if (start_pfn == altmap->base_pfn)
>                         start_pfn += altmap->reserve;
> @@ -911,6 +915,7 @@ void __meminit memmap_init_range(unsigned long size, int nid, unsigned long zone
>                         if (defer_init(nid, pfn, zone_end_pfn)) {
>                                 deferred_struct_pages = true;
> +                               nr_init += end_pfn - pfn;

It's confusing. Could the remaining range also include overlapping inits?

Maybe the whole "skip overlapping init" should actually be handled on a
higher level?

I guess we'd want to skip any memblock_is_mirror(r) regions entirely.

@Mike?

>                                 break;
>                         }
>                 }
> @@ -935,8 +940,10 @@ void __meminit memmap_init_range(unsigned long size, int nid, unsigned long zone
>                         cond_resched();
>                 }
> +               nr_init++;
>                 pfn++;
>         }
> +       return nr_init;
> }
> 
> @ -954,13 +962,21 @@ static void __init memmap_init_zone_range(struct zone *zone,
>         if (start_pfn >= end_pfn)
>                 return;
> +       nr_pages = memmap_init_range(end_pfn - start_pfn, nid, zone_id,
> +                                    start_pfn, zone_end_pfn, MEMINIT_EARLY,
> +                                    NULL, MIGRATE_MOVABLE, false);
> +       zone->pages_with_online_memmap += nr_pages;
> +
> 
>> The set_zone_contiguous/clear_zone_contiguous can be ignored I think.
>>
>> The comment about shrink_zone_span() is likely not realistic.
>> shrink_zone_span() would not shrink over boot holes.
>>
>> 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.
>>
>> No need to test pfn_to_online_page() twice, as that is the same result
>> for all pages in a PAGES_PER_SUBSECTION chunk.
> 
> Hi David
> 
> I think I understand your point. 
> Just to confirm, should the improvement to shrink_zone_span be included in this patch?

I would do it in a separate patch that you move before this patch.

-- 
Cheers,

David