Re: [PATCH v6 1/2] mm/memory_hotplug: optimize zone contiguous check when changing pfn range
"David Hildenbrand (Arm)" <[email protected]>
| Newsgroups | org.kvack.linux-mm,org.kernel.vger.linux-kernel |
|---|---|
| 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