Re: [PATCH 2/4] mm: don't report OOM when a huge PMD race blocks a pfn insert

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] mm: spurious VM_FAULT_OOM in insert_pfn() on concurrent PMD
  zap or split
--

commit 0ac526524e0ffcc35b96005d118a4d79edeae689
Author: Eliav Farber <farbere@amazon.com>

mm: don't report OOM when a huge PMD race blocks a pfn insert

This commit updates insert_pfn() to walk to the PMD and check for a
concurrently installed huge PMD leaf when pte_alloc_map_lock() returns NULL,
returning VM_FAULT_NOPAGE instead of VM_FAULT_OOM.

> diff --git a/mm/memory.c b/mm/memory.c
> index 09ae7e21ff0f5..509a984f17f8f 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -2803,10 +2803,25 @@ static vm_fault_t insert_pfn(struct vm_area_struct *vma, unsigned long addr,
>  	struct mm_struct *mm = vma->vm_mm;
>  	pte_t *pte, entry;
>  	spinlock_t *ptl;
> +	pmd_t *pmd;
>  
> -	pte = get_locked_pte(mm, addr, &ptl);
> -	if (!pte)
> +	pmd = walk_to_pmd(mm, addr);
> +	if (!pmd)
> +		return VM_FAULT_OOM;
> +
> +	pte = pte_alloc_map_lock(mm, pmd, addr, &ptl);
> +	if (!pte) {
> +		/*
> +		 * A concurrent ->huge_fault may have installed a PMD leaf over
> +		 * addr, which pte_offset_map_lock() refuses to walk. The
> +		 * mapping is fine, so let the access refault instead of
> +		 * reporting a failed allocation.
> +		 */
> +		if (pmd_trans_huge(pmdp_get_lockless(pmd)))
> +			return VM_FAULT_NOPAGE;
>  		return VM_FAULT_OOM;
> +	}

[Severity: Medium]
Can this still return a spurious VM_FAULT_OOM if the huge PMD leaf is split
or zapped before pmdp_get_lockless() re-reads it?

CPU0                              CPU1
insert_pfn()
  pte_alloc_map_lock()
    pte_alloc() -> 0
                                  unmap_mapping_range()
                                    ...
                                      zap_huge_pmd()
                                        pmdp_huge_get_and_clear_full()
    pte_offset_map_lock() -> NULL
  pmd_trans_huge(pmdp_get_lockless(pmd)) -> false
  return VM_FAULT_OOM;            <-- spurious OOM

pte_alloc_map_lock() expands to pte_alloc() followed by
pte_offset_map_lock().  Only pte_alloc() allocates memory and can fail with
OOM; pte_offset_map_lock() never allocates and returns NULL whenever the PMD
is a leaf or pmd_none().  If a concurrent huge PMD is cleared back to
pmd_none() (for example by unmap_mapping_range() or __split_huge_pmd())
after pte_alloc() sees !pmd_none() and before pmdp_get_lockless(pmd) runs,
pmd_trans_huge() is false and insert_pfn() triggers the OOM killer even
though no allocation failed.

Calling pte_alloc(mm, pmd) first (returning VM_FAULT_OOM on failure) and
then pte_offset_map_lock(mm, pmd, addr, &ptl) (returning VM_FAULT_NOPAGE
when NULL), as do_anonymous_page() and finish_fault() do, avoids re-reading
*pmd.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111140655.34281-1-farbere@amazon.com?part=2