Re: [PATCH v3 6/6] mm/mm_init: add zone mismatch warning during page init

[Muchun Song <muchun.song@linux.dev>]

> On Oct 6, 2026, at 12:52, Mike Rapoport <rppt@kernel.org> wrote:
> 
> On Mon, Oct 05, 2026 at 06:27:52PM +0200, Muchun Song wrote:
>>> On Oct 2, 2026, at 20:12, Mike Rapoport <rppt@kernel.org> wrote:
>>> 
>>> On Fri, Oct 02, 2026 at 05:56:40PM +0800, Muchun Song wrote:
>>>>> On Oct 2, 2026, at 16:22, Mike Rapoport <rppt@kernel.org> 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.

Yeah, factoring this into a helper makes sense.

Since it currently has only one caller, how about keeping it as a local
static inline in mm_init.c instead of putting it in sparse.h?

Thanks,
Muchun

> 
>> Thanks,
>> Muchun
>> 
>>> 
>>>> [1] https://lore.kernel.org/20260513132044.41690-18-songmuchun@bytedance.com/
>>>> 
>>>> Thanks,
>>>> Muchun
>>> 
>>> -- 
>>> Sincerely yours,
>>> Mike.
>> 
> 
> -- 
> Sincerely yours,
> Mike.