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

[Farber, Eliav]

On 11/11/26 4:54 PM, Jose A. Perez de Azpillaga wrote:
> On 11/11/26 3:06 PM, Eliav Farber wrote:
>> +    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;
> 
> we should split the allocation from the map, which removes the re-read
> and the leaf special case. something like:
> 
>       if (pte_alloc(mm, pmd))
>               return VM_FAULT_OOM;
> 
>       pte = pte_offset_map_lock(mm, pmd, addr, &ptl);
>       if (!pte)
>               return VM_FAULT_NOPAGE;
> 
> maybe could work better :)

Agreed, that is better. pte_alloc_map_lock() is only
pte_alloc() ? NULL : pte_offset_map_lock(), so splitting it separates
the two failures at the source instead of working out afterwards which
one happened. insert_pages() already builds the PTE this way
(mm/memory.c:2546).

It also widens the fix. __pte_offset_map() returns NULL for a leaf, for
a PMD that is not present, and for a bad PMD it has just cleared. v1
recognized only the leaf and still reported the other two as OOM, though
all three want a refault.

Taken in v2, with a Suggested-by.

--
Thanks,
Eliav