On Mon, Oct 05, 2026 at 06:27:52PM +0200, Muchun Song wrote:
> > On Oct 2, 2026, at 20:12, Mike Rapoport <[email protected]> wrote:
> > 
> > On Fri, Oct 02, 2026 at 05:56:40PM +0800, Muchun Song wrote:
> >>> On Oct 2, 2026, at 16:22, Mike Rapoport <[email protected]> wrote:
> >> 
> >>>>>> diff --git a/mm/mm_init.c b/mm/mm_init.c
> >>>>>> index 1650d6bc1211..bd02e8d06965 100644
> >>>>>> --- a/mm/mm_init.c
> >>>>>> +++ b/mm/mm_init.c
> >>>>>> @@ -609,6 +609,9 @@ void __meminit __init_single_page(struct page 
> >>>>>> *page, unsigned long pfn,
> >>>>>> if (!is_highmem_idx(zone))
> >>>>>> set_page_address(page, __va(pfn << PAGE_SHIFT));
> >>>>>> #endif
> >>>>>> + 
> >>>>>> VM_WARN_ON_ONCE(vmemmap_optimizable_order(pfn_to_section_compound_order(pfn))
> >>>>>>  &&
> >>>>>> + page_zone_id(page + VMEMMAP_OPTIMIZATION_NR_STRUCT_PAGES) !=
> >>>>>> + page_zone_id(page));
> >>>>> 
> >>>>> Hmm, page + VMEMMAP_OPTIMIZATION_NR_STRUCT_PAGES is initialized a tad 
> >>>>> later
> >>>>> than page so it'll have stale data in the page->flags, won't it?
> >>>> 
> >>>> Lance is right. The shared tail struct pages are already initialized by
> >>>> vmemmap_shared_tail_page() during vmemmap population, so they're not 
> >>>> stale.
> >>>> The head 64 struct pages are initialized later — right here, after 
> >>>> vmemmap
> >>>> population.
> >>> 
> >>> Still it looks out of place here, can this check be done in sparse-vmemmap
> >>> somehow?
> >> 
> >> The struct page entries of a vmemmap-optimizable compound page
> >> are currently initialized in two stages. During vmemmap
> >> population, the shared tail entries are initialized first. The
> >> retained head area—normally 64—is initialized later through
> >> __init_single_page().
> >> 
> >> This warning connects the two stages: while initializing the
> >> retained entries in the second stage, it verifies that their zone
> >> information is consistent with the shared entries initialized in
> >> the first stage. Therefore, the same check cannot be performed
> >> during vmemmap population.
> >> 
> >> I am planning to first unify the HugeTLB and Device DAX
> >> compound-page initialization through a common helper [1]. Once that
> >> work is complete, maybe it will be easy to move the initialization
> >> of the retained head area into vmemmap population. With both the
> >> retained and shared entries initialized in the same stage, there
> >> will be no cross-stage inconsistency to check, and this warning
> >> can be removed.
> >> 
> >> Would keeping the check here for now and removing it as part of
> >> that follow-up sound reasonable to you?
> > 
> > While it feels really out of place in __init_single_page(), but having it
> > memmap_init_range() close to the if that skips shared tail pages makes
> > sense.
> > 
> > What do you say?
> 
> Sorry for the late reply because of traveling to LPC.
> 
> Putting the check in memmap_init_range() would validate the
> invariant for the shared-tail range skipped there. It would
> not, however, cover all vmemmap-optimized initialization
> paths: optimized Device DAX bypasses memmap_init_range()
> when there is no altmap, and deferred boot-time HugeTLB
> initialization may also initialize retained entries
> elsewhere.
> 
> If the intention is to validate only the skip in
> memmap_init_range(), moving it there makes sense. If we want
> the warning to cover both HugeTLB and Device DAX generally,
> it still needs to remain in __init_single_page() or be
> called from the individual initialization paths.

Yeah, putting this into all the callers sucks too :(

With all the callers,__init_single_page() is the right place for this
check, but I think we can add a static inline in sparse.h, like e.g.

vmemmap_optimization_verify_zone(page, pfn)

to make it a bit nicer.
 
> Thanks,
> Muchun
> 
> > 
> >> [1] 
> >> https://lore.kernel.org/[email protected]/
> >> 
> >> Thanks,
> >> Muchun
> > 
> > -- 
> > Sincerely yours,
> > Mike.
> 

-- 
Sincerely yours,
Mike.

Reply via email to