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

[Muchun Song]

> On Oct 8, 2026, at 09:45, Mike Rapoport <rppt@xxxxxxxxxx> wrote:
> 
> On Tue, Oct 06, 2026 at 03:32:43PM +0200, Muchun Song wrote:
>>> On Oct 6, 2026, at 12:52, Mike Rapoport <rppt@xxxxxxxxxx> wrote:
>>> 
>>> On Mon, Oct 05, 2026 at 06:27:52PM +0200, Muchun Song wrote:
>>>>> On Oct 2, 2026, at 20:12, Mike Rapoport <rppt@xxxxxxxxxx> wrote:
>>>>> 
>>>>> On Fri, Oct 02, 2026 at 05:56:40PM +0800, Muchun Song wrote:
>>>>>>> On Oct 2, 2026, at 16:22, Mike Rapoport <rppt@xxxxxxxxxx> 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?
> 
> Presuming we don't expect new callers any time soon let's put it just above
> __init_single_page().

Make sense.

Thanks,
Muchun

> 
> -- 
> Sincerely yours,
> Mike.