Re: [PATCH 4/4] fsdax: materialize pmem vmemmap metadata on faults

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Global TLB flush broadcast inside global page table lock causing severe lock contention.
- [High] PMD fault materialization failure incorrectly returns VM_FAULT_OOM instead of falling back to PTE.
- [High] __vmemmap_materialize_page dereferences a potentially NULL PTE pointer if the DAX device is concurrently hot-unplugged.
--

commit 9e2a623911317e382af87ef795b2fc34f1686554
Author: Muchun Song <muchun.song@linux.dev>

fsdax: materialize pmem vmemmap metadata on faults

This patch opts pmem FS-DAX into the shared read-only vmemmap mode and 
materializes private metadata before inserting a PFN into a userspace 
mapping. It shifts private vmemmap allocation from device registration 
to the first DAX fault for each metadata page.

[ ... ]

> diff --git a/fs/dax.c b/fs/dax.c
> index 1fbba0d21c13d..89377301d51b0 100644
> --- a/fs/dax.c
> +++ b/fs/dax.c

[ ... ]

> @@ -1877,6 +1878,10 @@ static vm_fault_t dax_fault_iter(struct vm_fault *vmf,
>  	if (err)
>  		return pmd ? VM_FAULT_FALLBACK : dax_fault_return(err);
>  
> +	err = vmemmap_materialize_page(pfn_to_page(pfn), pmd ? PMD_ORDER : 0);

[Severity: High]
Is there a risk of severe lock contention and RCU stalls here?

When a PMD fault triggers vmemmap_materialize_page(), it loops 512 times.
Inside that loop, __vmemmap_materialize_page() acquires the global
init_mm.page_table_lock and performs a synchronous TLB flush:

mm/sparse-vmemmap.c:__vmemmap_materialize_page() {
    ...
    set_pte_at(&init_mm, addr, pte, mk_pte(candidate, PAGE_KERNEL));
    flush_tlb_kernel_range(addr, addr + PAGE_SIZE);
    put_page(template);
out:
    spin_unlock(&init_mm.page_table_lock);
}

Could this cause the system to become unusable under load due to sequential
global IPI broadcasts while holding the global kernel page table lock?


[Severity: High]
Is there a race condition with DAX device hot-unplug here?

The prior call to dax_iomap_direct_access() acquires and then releases the
DAX SRCU read lock. By the time we call vmemmap_materialize_page(), there
is no lock preventing the DAX device from being hot-unplugged, which tears
down the vmemmap page tables.

If that happens concurrently, __vmemmap_materialize_page() will find
pmd_none(*pmd) is true in virt_to_kpte(), resulting in a NULL pte pointer.
Will the subsequent ptep_get(pte) dereference this NULL pointer and panic
the kernel?

> +	if (err)
> +		return dax_fault_return(err);

[Severity: High]
Does returning dax_fault_return(err) on a -ENOMEM error incorrectly trigger
an OOM kill for PMD faults?

dax_fault_return(err) translates -ENOMEM to VM_FAULT_OOM. If this
allocation fails during a PMD fault, shouldn't it return VM_FAULT_FALLBACK
instead, allowing the kernel to gracefully split the huge page and retry at
the PTE level (similar to how dax_iomap_direct_access() handles PMD fallback
just above this)?

> +
>  	*entry = dax_insert_entry(xas, vmf, iter, *entry, pfn, entry_flags);

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903122128.12264-1-songmuchun@bytedance.com?part=4