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

[David Hildenbrand (Arm)]

> 
> 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.

Thanks for reminding me. I think the right direction is to finally clean up the
pfn_valid() handling.

> 
> 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.
Let me take a stab at just having pfn_valid() / for_each_valid_pfn() respecting
the subsection map.

-- 
Cheers,

David