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

David Hildenbrand (Arm)
2026-04-14T09:24:28Z

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