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

"David Hildenbrand (Arm)" <[email protected]>
Newsgroups gmane.linux.kernel,gmane.linux.kernel.mm
Message-ID <[email protected]>
> 
> 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.