On Thu, Sep 3, 2026 at 8:53 PM <[email protected]> wrote:
>
> 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 <[email protected]>
>
> 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.
No. It loops 8 times not 512.
> 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?
>
No. The FS-DAX fault path is not relying on that SRCU lock to protect
later struct page accesses. Before dax_fault_iter() is reached,
grab_mapping_entry() has already installed
and locked a DAX entry in mapping->i_pages. DAX invalidation/removal
paths synchronize
through that entry and wait for locked entries before deleting them.
> 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?
There is no reason to fallback since no memory could be allocated.
>
> 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/[email protected]?part=4
