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

On Fri, Aug 07, 2026 at 12:15:42PM +0000, Liu, Yuan1 wrote:
>> -----Original Message-----
>> From: David Hildenbrand (Arm) <[email protected]>
>> Sent: Friday, August 7, 2026 7:30 PM
>> To: Liu, Yuan1 <[email protected]>; Oscar Salvador <[email protected]>;
>> Mike Rapoport <[email protected]>; Wei Yang <[email protected]>
>> Cc: [email protected]; Zou, Nanhai <[email protected]>; Deng, Pan
>> <[email protected]>; Li, Tianyou <[email protected]>; Chen Zhang
>> <[email protected]>; Zeng, Jason <[email protected]>; linux-
>> [email protected]
>> Subject: Re: [PATCH v6 1/2] mm/memory_hotplug: optimize zone contiguous
>> check when changing pfn range
>> 
>> On 8/6/26 11:52, Liu, Yuan1 wrote:
>> >> -----Original Message-----
>> >> From: David Hildenbrand (Arm) <[email protected]>
>> >> Sent: Thursday, August 6, 2026 4:46 PM
>> >> To: Liu, Yuan1 <[email protected]>; Oscar Salvador
>> <[email protected]>;
>> >> Mike Rapoport <[email protected]>; Wei Yang <[email protected]>
>> >> Cc: [email protected]; Zou, Nanhai <[email protected]>; Deng, Pan
>> >> <[email protected]>; Li, Tianyou <[email protected]>; Chen Zhang
>> >> <[email protected]>; Zeng, Jason <[email protected]>; linux-
>> >> [email protected]
>> >> Subject: Re: [PATCH v6 1/2] mm/memory_hotplug: optimize zone contiguous
>> >> check when changing pfn range
>> >>
>> >>
>> >>>
>> >>> Will do.
>> >>>
>> >>>
>> >>> pages_with_online_memmap counts all PFNs where pfn_to_online_page() is
>> >>> valid. With CONFIG_SPARSEMEM_VMEMMAP, pfn_section_valid() operates at
>> >>> PAGES_PER_SUBSECTION granularity — when any page in a subsection has
>> >>> memory, the entire subsection is valid/online. So we align to
>> subsection
>> >>> boundaries to include hole pages within partially-populated
>> subsections.
>> >>
>> >> But we must never account exceeding the zone range. So I don't
>> understand
>> >> why we
>> >> would have to care about PAGES_PER_SUBSECTION here at all?
>> >
>> > We never account beyond the zone range, because `sub_start` and
>> > `sub_end` are still clamped to the zone boundaries after the
>> > alignment.
>> >
>> > The subsection alignment is needed for hole PFNs between memblocks
>> > within the zone. These hole PFNs are valid for
>> > `pfn_to_online_page()`, because they sit in a subsection that has
>> > memory, so the whole subsection's memmap is online.
>> >
>> > |----------- zone range -----------|
>> >
>> > +-----------+---------+------------+
>> > + memblock 1|  hole   | memblock 2 |
>> > +-----------+---------+------------+
>> >                ^
>> >                |
>> >                |
>> >      subsection boundary
>> 
>> Right, but init_unavailable_range() can just return how many were actually
>> initalized? Why can't we piggy-back on that?
>> 
>> I think we had something similar previously, why can't we use that?
>> 
>> We know the zone span, so we can just account all the mmap in the zone
>> span
>> that we initialize.
>> 
>> What is the problem with that?
>
>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.
>
>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.
>
>[1] https://sashiko.dev/#/patchset/20260520093457.3719960-1-yuan1.liu%40intel.com
>

Hi, Yuan & David

One thing I found is __pageblock_pfn_to_page() is ambiguous when pageblock
contains 2 subsections.

The subsection size is fixed, which is 2^21=2M. But pageblock is not fixed
size. If !CONFIG_HUGETLB_PAGE && !CONFIG_TRANSPARENT_HUGEPAGE, default
pageblock size is 4K * 2 ^ 10 = 4M, which contains 2 subsection.

For a pageblock in early section, __paeblock_pfn_to_page() result is
different when the hole is at the beginning or at the end.

Like what I did in [1], I punch a hole in Zone Normal with 2M size.

If the hole is at the beginning of the pageblock, the zone is non-contiguous.

  [    0.004079]  memory[0x2]     [0x0000000100000000-0x000000013fffffff], 0x0000000040000000 bytes on node 0 flags: 0x0
  [    0.004080]  memory[0x3]     [0x0000000140200000-0x00000001bfffffff], 0x000000007fe00000 bytes on node 0 flags: 0x0
  
If the hole is at the end of the pageblock, the zone is contiguous.

  [    0.004068]  memory[0x2]     [0x0000000100000000-0x00000001401fffff], 0x0000000040200000 bytes on node 0 flags: 0x0
  [    0.004068]  memory[0x3]     [0x0000000140400000-0x00000001bfffffff], 0x000000007fc00000 bytes on node 0 flags: 0x0

The reason is in __pageblock_pfn_to_page() we have different requirement for
start_pfn and end_pfn.

Hmm... and things would be complicated as pageblock is PAGE based but
subsection is size based. If we have large PAGE_SIZE...

Also when pageblock_pfn_to_page() is introduced in commit 7d49d8868336 ("mm,
compaction: reduce zone checking frequency in the migration scanner"), we
assume there won't be [node0 node1 node0] interleave within a single pageblock.
Would this still stand for large PAGE_SIZE?

[1]: https://lore.kernel.org/all/[email protected]/T/#u

>Best Regards,
>Liu, Yuan
>
>> Cheers,
>> 
>> David

-- 
Wei Yang
Help you, Help me