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

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

On Tue, Apr 14, 2026 at 11:32:13AM +0200, David Hildenbrand (Arm) wrote:
>On 4/14/26 04:12, Wei Yang wrote:
>> On Mon, Apr 13, 2026 at 08:24:05PM +0200, David Hildenbrand (Arm) wrote:
>>>>   With the last memblock region fits in Node 1 Zone Normal. 
>>>>
>>>>   Then I punch a hole in this region with 2M(subsection) size with following
>>>>   change, to mimic there is a hole in memory range:
>>>>
>>>>     @@ -1372,5 +1372,8 @@ __init void e820__memblock_setup(void)
>>>>             /* Throw away partial pages: */
>>>>             memblock_trim_memory(PAGE_SIZE);
>>>>      
>>>>     +       memblock_remove(0x140000000, 0x200000);
>>>>     +
>>>>             memblock_dump_all();
>>>>      }
>>>>     
>>>>   Then the memblock dump shows:
>>>>
>>>>      MEMBLOCK configuration:
>>>>       memory size = 0x000000017fd7dc00 reserved size = 0x0000000005a97 9c2
>>>>       memory.cnt  = 0x4
>>>>       memory[0x0]     [0x0000000000001000-0x000000000009efff], 0x000000000009e000 bytes on node 0 flags: 0x0
>>>>       memory[0x1]     [0x0000000000100000-0x00000000bffdefff], 0x00000000bfedf000 bytes on node 0 flags: 0x0
>>>>   +-  memory[0x2]     [0x0000000100000000-0x000000013fffffff], 0x0000000040000000 bytes on node 1 flags: 0x0
>>>>   +-  memory[0x3]     [0x0000000140200000-0x00000001bfffffff], 0x000000007fe00000 bytes on node 1 flags: 0x0
>>>>
>>>>   We can see the original one memblock region is divided into two, with a hole
>>>>   of 2M in the middle.
>>>
>>> Yes, that makes sense.
>>>
>>>>
>>>>   Not sure this is a reasonable mimic of memory hole. Also I tried to
>>>>   punch a larger hole, e.g. 10M, still see the behavioral change.
>>>>
>>>> The /proc/zoneinfo result:
>>>>
>>>>     w/o patch
>>>>
>>>>     Node 1, zone   Normal
>>>>       pages free     469271
>>>>             boost    0
>>>>             min      8567
>>>>             low      10708
>>>>             high     12849
>>>>             promo    14990
>>>>             spanned  786432
>>>>             present  785920
>>>>             contigu  0           <--- zone is non-contiguous
>>>>             managed  766024
>>>>             cma      0
>>>>     
>>>>     with patch
>>>>
>>>>     Node 1, zone   Normal
>>>>       pages free     121098
>>>>             boost    0
>>>>             min      8665
>>>>             low      10831
>>>>             high     12997
>>>>             promo    15163
>>>>             spanned  786432
>>>>             present  785920
>>>>             contigu  1           <--- zone is contiguous
>>>>             managed  773041
>>>>             cma      0
>>>>
>>>>   This shows we treat Node 1 Zone Normal as non-contiguous before, but treat
>>>>   it a contiguous zone after this patch.
>>>>
>>>> Reason:
>>>>
>>>>   set_zone_contiguous()
>>>>       __pageblock_pfn_to_page()
>>>>           pfn_to_online_page()
>>>> 	      pfn_section_valid() <--- check subsection
>>>>
>>>>   When SPARSEMEM_VMEMMEP is set, pfn_section_valid() checks subsection bit to
>>>>   decide if it is valid. For a hole, the corresponding bit is not set. So it
>>>>   is non-contiguous before the patch.
>>>>
>>>>   After this patch, the memory map in this hole also contributes to
>>>>   pages_with_online_memmap, so it is treated as contiguous.
>>>
>>> That means that mm init code actually initialized a memmap, so there is
>>> a memmap there that is properly initialized?
>>>
>>> So init_unavailable_range()->for_each_valid_pfn() processed these
>>> sub-section holes I guess.
>>>
>> 
>> Yes, I think so.
>> 
>> When memmap_init()->for_each_mem_pfn_range() iterate on the last memblock
>> region, init_unavailable_range() will init the hole.
>> 
>>> subsection_map_init() takes care of initializing the subsections. That
>>> happens before memmap_init() in free_area_init().
>>>
>> 
>> Yes. I guess you mean sparse_init_subsection_map().
>> 
>>> Is there a problem in for_each_valid_pfn()?
>>>
>>> And I think there is in first_valid_pfn:
>>>
>> 
>> You mean there is a problem in first_valid_pfn?
>
>That is my theory.
>
>> 
>>> 	if (valid_section(ms) &&
>>> 	    (early_section(ms) || pfn_section_first_valid(ms, &pfn))) {
>>> 		rcu_read_unlock_sched();
>>> 		return pfn;
>>> 	}
>>>
>>> The PFN is valid, but we actually care about whether it will be online.
>>> So likely, we should skip over sub-sections here also for early sections
>>> (even though the memmap exist, nobody should be looking at it, just like
>>> for an offline memory section).
>>>
>> 
>> And it should be like below?
>> 
>> 	if (valid_section(ms) &&
>> 	    pfn_section_first_valid(ms, &pfn)) {
>> 		rcu_read_unlock_sched();
>> 		return pfn;
>> 	}
>
>Probably, yes. We have to understand if other users would be negatively
>affected.
>
>> 
>> IIUC, this would skip hole and leave allocated memory map uninitialized. And
>> then those pages won't contribute to pages_with_online_memmap, which further
>> leave the zone non-contiguous.
>
>Yes.
>
>> 
>> But we want zone to be contiguous when we have a hole like this, right?
>
>Not if a subsection is marked invalid. That's why the existing scenario
>is that it will not be contiguous.
>

Let me try to understand.

During previous discussion[1], we want to define "zone->contiguous guarantee
pfn_to_page() is valid on the complete zone". So this definition is not true
now, since we found the behavioral change.

Because __pageblock_pfn_to_page() won't treat range with hole as contiguous,
detected by pfn_to_online_page() on invalid subsection.

[1]: https://lkml.org/lkml/2026/2/9/550

Now you are thinking the problem is in the iteration function,
for_each_valid_pfn(), used in init_unavaiable_range(). It should only take
valid subsections into consideration, even for early sections.

So your concern is if we change first_valid_pfn(), it will affect other users.

If the above understanding is correct, maybe we can use spanned == present
to do the trick? Because holes are marked subsection invalid and holes are
counted into absent.

But I see the mirrored_kernel thing, not fully understand yet. This is the
reason to prevent spanned == present approach?

>Note that Wei reported that it was not contiguous but would now be
>contiguous.
>
>
>If you have a DAX device the plugs into that hole through
>memremap_pages()->pagemap_range(), I think this could cause problems. I
>doubt that this would happen in practice for such small holes, but if
>they would be bigger, or at the start/end of the range, it could be
>problematic.
>

Not fully understand yet, will take a look into it.

>-- 
>Cheers,
>
>David

-- 
Wei Yang
Help you, Help me