Re: [PATCH 1/2] powerpc/64s/radix: don't dedup vmemmap covered by an existing PMD leaf

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

[-- Attachment #1: Type: text/plain, Size: 8258 bytes --]



> On Oct 29, 2026, at 11:50, Ritesh Harjani (IBM) <ritesh.list@gmail.com> wrote:
> 
> Muchun Song <muchun.song@linux.dev <mailto:muchun.song@linux.dev>> writes:
> 
>> Hi Ritesh,
>> 
>>> On Oct 27, 2026, at 11:22, Ritesh Harjani (IBM) <ritesh.list@gmail.com> wrote:
>>> 
>>> radix__vmemmap_populate() is asked for one 16MB section, which is 16KB of
>>> struct pages. With no altmap it rounds the request down to a PMD leaf.
>>> That leaf describes 2GB of PFNs with 64KB pages (128MB with 4KB pages),
>>> so it also covers sections that are not present yet. It cannot be freed
>>> while any of those sections are still in use.
>>> 
>>> A devdax namespace that lands in the same window is marked for vmemmap dedup.
>>> vmemmap_populate_compound_pages() treated the leaf as an existing huge mapping
>>> and skipped it. The core then initializes only the head vmemmap page and assumes
>>> the rest is the shared tail. Those remaining tail slots are real struct pages
>>> still holding the page_init_poison() pattern and are not the shared tail pages.
>> 
>> Right.
>> 
>>> __init_single_page() warns on the zone mismatch and the following pageblock init
>>> faults in __set_pfnblock_flags_mask().
>> 
>> Yeah, the new adding warning is useful.
>> 
>>> 
>>> If the section's vmemmap is already a PMD leaf, clear its compound order
>> 
>> I think it's enough to fix this issue.
>> 
> 
> Oh yes, right! since the page init is done later and the optimizable
> order can be read from mem_section.
> 
>>> and reuse the leaf, so every struct page in the section is initialized.
>>> The shared-tail layout needs base-page PTEs, so it cannot be built on top of
>>> this leaf and dropping the leaf is not an option while the neighbouring range
>>> is still using it. So, fail the compound populate if a leaf is still seen there.
>>> 
>>> <Call Trace>
>>> ============
>>> [    3.467060][    T1] WARNING: mm/mm_init.c:600 at __init_single_page+0xd0/0xf4, CPU#0: swapper/0/1
>>> <...>
>>> [    3.477419][    T1] NIP [c000000000576b30] __init_single_page+0xd0/0xf4
>>> [    3.477827][    T1] LR [c0000000005778b0] __init_zone_device_page+0x30/0xb8
>>> [    3.478444][    T1] Call Trace:
>>> [    3.478764][    T1]  0x10000 (unreliable)
>>> [    3.479874][    T1]  memmap_init_zone_device+0x164/0x430
>>> [    3.480392][    T1]  memremap_pages+0x43c/0x93c
>>> [    3.480836][    T1]  devm_memremap_pages+0x3c/0xa4
>>> [    3.481328][    T1]  dev_dax_probe+0x264/0x3d0
>>> [    3.481817][    T1]  dax_bus_probe+0x140/0x1c8
>>> [    3.482237][    T1]  really_probe+0x108/0x580
>>> [    3.482681][    T1]  __driver_probe_device+0xcc/0x284
>>> [    3.483186][    T1]  driver_probe_device+0x5c/0x108
>>> [    3.483656][    T1]  __device_attach_driver+0x10c/0x1e8
>>> [    3.484134][    T1]  bus_for_each_drv+0xc0/0x13c
>>> <...>
>>> [    3.499972][    T1] BUG: Kernel NULL pointer dereference on read at 0x00001400
>>> [    3.500417][    T1] Faulting instruction address: 0xc0000000005f4ecc
>>> [    3.500417][    T1] Faulting instruction address: 0xc0000000005f4ecc
>>> cpu 0x0: Vector: 300 (Data Access) at [c000000006756b50]
>>>   pc: c0000000005f4ecc: __set_pfnblock_flags_mask.constprop.0+0x34/0x160
>>>   lr: c000000000577f38: memmap_init_zone_device+0x358/0x430
>>>   pid   = 1, comm = swapper/0
>>> enter ? for help
>>> memmap_init_zone_device+0x358/0x430
>>> memmap_init_zone_device+0x220/0x430 (unreliable)
>>> memremap_pages+0x43c/0x93c
>>> devm_memremap_pages+0x3c/0xa4
>>> dev_dax_probe+0x264/0x3d0
>>> dax_bus_probe+0x140/0x1c8
>>> really_probe+0x108/0x580
>>> <...>
>>> 
>>> Fixes: bd0862382adb ("mm/sparse-vmemmap: drop Device DAX-specific population path")
>> 
>> Actually, the right Fixes is:
>> 
>> Fixes: f2b79c0d7968 ("powerpc/book3s64/radix: add support for vmemmap optimization for radix")
>> 
>> On older kernels, these errors are silent. They trigger a kernel panic when the user
>> actually uses Device DAX, rather than during the initialization phase. 
>> 
>> So, for older kernels, we should guide them to backport the patch.
>> 
>>> Reported-by: Pavithra <pavrampu@linux.ibm.com>
>>> Assisted-by: LLM
>>> Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
>>> ---
>>> Fixes tag here is added since the fix is dependent upon the change mentioned in
>>> "Fixes" tag. The Fixes commit-id is from mm-new branch. Although this could be
>>> an older pre-existing issue, but got exposed after this patch series.
>> 
>> I knew this issue, so I have a local commit to fix it as well. I'd like to share
>> it with you, because it looks a little simple, does it work for you?
>> 
>> powerpc/mm: fix uninitialized ZONE_DEVICE struct pages with PMDs
>> 
>> PowerPC radix vmemmap population can reuse an existing PMD mapping while
>> populating a device DAX range whose section is marked as HVO-enabled. The
>> reused mapping is an ordinary vmemmap mapping: the redundant tail struct
>> pages are not remapped to the shared tail page.
>> 
>> Leaving the section marked as HVO-enabled makes later ZONE_DEVICE
>> initialization treat the range as optimized and initialize only the struct
>> pages that would be backed by a real HVO mapping. The remaining tail struct
>> pages can then be left uninitialized even though they are still used as
>> metadata for the device DAX compound page.
>> 
> 
> Here, feel free to use the call trace from my commit msg to show what goes
> wrong w/o this fix.

No problem. Thanks for your detailed msg.

> 
>> The Fixes target predates the section-based vmemmap optimization
>> infrastructure, so older kernels do not have section state that can be
>> marked non-HVO. Backports to such kernels need to handle the PMD-reuse
>> case in powerpc radix code directly, for example by initializing the tail
>> struct pages that core ZONE_DEVICE initialization would otherwise skip.
>> 
>> Mark the section as non-HVO when reusing an existing PMD mapping, so the
>> section's HVO state matches the actual vmemmap layout and all required
>> struct pages are initialized.
>> 
>> Fixes: f2b79c0d7968 ("powerpc/book3s64/radix: add support for vmemmap optimization for radix")
>> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
>> ---
>> arch/powerpc/mm/book3s64/radix_pgtable.c | 11 ++++++++++-
>> 1 file changed, 10 insertions(+), 1 deletion(-)
> 
> yes, love this approach. Thanks for sharing the fix.
> 
>> 
>> diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c b/arch/powerpc/mm/book3s64/radix_pgtable.c
>> index 72793ace1fb6..fc3a2d4b422d 100644
>> --- a/arch/powerpc/mm/book3s64/radix_pgtable.c
>> +++ b/arch/powerpc/mm/book3s64/radix_pgtable.c
>> @@ -1200,7 +1200,16 @@ static int __meminit vmemmap_populate_compound_pages(unsigned long start_pfn,
>> 			return -ENOMEM;
>> 
>> 		if (pmd_leaf(READ_ONCE(*pmd))) {
>> -			/* existing huge mapping. Skip the range */
>> +			unsigned long addr_pfn = page_to_pfn((struct page *)addr);
>> +			struct mem_section *ms = __pfn_to_section(addr_pfn);
>> +
>> +			/*
>> +			 * Existing PMD mappings can cover the edges of a valid
>> +			 * HVO populate range.  They are ordinary vmemmap mappings,
>> +			 * so the current section must not be treated as HVO-backed
>> +			 * when its struct pages are initialized later.
>> +			 */
>> +			section_set_order(ms, 0);
> 
> I think you meant "section_set_compound_order(ms, 0);"

Yes, because it's been sitting on my disk for a while, and I forgot that
this part needed to be adapted.

> 
> yes, otherwise the patch looks good to me. Can you please post this
> patch separately then? Feel free to add:
> 
> Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com <mailto:ritesh.list@gmail.com>>

Thanks for your review. I'll send it ASAP.

> 
> I have validated the fix with my custom reproducer and I see the issue
> is fixed with this. Meanwhile, Pavithra (cc'd) is testing this and few
> other patches of your series. Once her testing is finished, she can help
> provide her Tested-by/feedback if any.

Thanks a lot to both of you for testing this.

Thanks,
Muchun

> 
>> 			next = pmd_addr_end(addr, end);
>> 			continue;
>> 		}
> 
> -ritesh


[-- Attachment #2: Type: text/html, Size: 31657 bytes --]