Re: [PATCH] mm/pagewalk: fix race between concurrent split and refault

[Boone, Max]

Hey Qi,

[…]

> 
> Why not check pudval directly here? Like the following:
> 
> if (pud_leaf(*pud) || !pud_present(*pud))
> goto again;
> 

Good point, my initial idea [1] was also to put it there (although I
checked on pud_special instead and continued instead of retrying).
I wasn’t sure whether I could link to a thread in a patch message,
but there’s some discussion between David and me there.

Making sure that a passed-in PMD range can be walked by checking 
if the parent PUD is present & not a leaf feels better suited as a guard
in the walk_pmd_range() function to me. After all, the failure originates
from inside that function, and potential other callers won’t need to 
incorporate the check which has to be done for safety anyways.

It also makes the logic of walk_pud_range() more similar to 
walk_pmd_range() - which also has the retry if it gets an ACTION_AGAIN
from the walk_pte_range() call.

Finally, doesn’t feel very natural to me to have:

if (walk->vma)
    split_huge_pud(walk->vma, pud, addr);
else if (pud_leaf(*pud) || !pud_present(*pud))
    continue; /* Nothing to do. */
if (pud_leaf(*pud) || !pud_present(*pud))
    goto again; /* Retry on concurrent refault as leaf */

[1] https://lore.kernel.org/all/20260309174949.2514565-1-mboone@xxxxxxxxxx/

Attachment: smime.p7s
Description: S/MIME cryptographic signature