Re: [PATCH] mm: implement and use vma_is_faulted(), silence KCSAN

[Lorenzo Stoakes (ARM)]

On Wed, Sep 09, 2026 at 07:32:57PM +0200, David Hildenbrand (Arm) wrote:
> On 9/9/26 18:36, Lorenzo Stoakes (ARM) wrote:
> > On Wed, Sep 09, 2026 at 06:24:52PM +0200, David Hildenbrand (Arm) wrote:
> >> On 9/9/26 18:14, Lorenzo Stoakes (ARM) wrote:
> >>> Provide a function to abstract the common task of checking whether
> >>> a VMA is faulted in or not.
> >>>
> >>> A VMA or mmap lock must be held when calling this function. For an attached
> >>> VMA the transitions between unfaulted/faulted state are:
> >>>
> >>>        Transition        |              VMA/mmap Lock state
> >>>  ------------------------|-----------------------------------------------
> >>>    unfaulted to faulted  | write lock OR read lock + mm->page_table_lock
> >>>    faulted to unfaulted  | write lock
> >>>
> >>> So vma_is_faulted() never provides a false positive (the lock precludes
> >>> it), but if only a read lock is held, a negative result must be re-checked
> >>> with mm->page_table_lock held.
> >>>
> >>> Detached VMAs cannot be concurrently manipulated as they are removed from
> >>> the maple tree so require no guarantees.
> >>>
> >>> Use data_race() to silence KCSAN about non-existent data races between
> >>> concurrent vma->anon_vma read/write on optimistic fault tests.
> >>>
> >>> Also while here, const-ify vma_is_attached(), vma_assert_stabilised() and
> >>> dependants.
> >>>
> >>> Finally, update the core VMA merge/split, rmap, mremap, KSM and fault
> >>> preparation callers which test vma->anon_vma directly to use
> >>> vma_is_faulted() instead.
> >>>
> >>> Note that the lockless read in reusable_anon_vma() is doing more than
> >>> checking whether the VMA is faulted - it is returning the anon_vma to be
> >>> used on fault, so this check is not altered.
> >>>
> >>> There is one odd one out - file_backed_vma_is_retractable() - which holds
> >>> neither a VMA nor mmap lock and is stabilised by the file rmap lock only.
> >>>
> >>> Therefore just add a comment to explain why the direct vma->anon_vma check
> >>> is required.
> >>>
> >>> Reported-by: Guilherme Giacomo Simoes <trintaeoitogc@xxxxxxxxx>
> >>> Closes: https://lore.kernel.org/all/20260829100034.423064-1-trintaeoitogc@xxxxxxxxx/
> >>> Closes: https://lore.kernel.org/all/20260909115723.528501-1-trintaeoitogc@xxxxxxxxx/
> >>> Signed-off-by: Lorenzo Stoakes (ARM) <ljs@xxxxxxxxxx>
> >>> ---
> >>
> >>
> >> Is vma_is_faulted() really the right thing to use when wanting to say that we
> >> (likely) faulted in an anon page?
> >>
> >> I think that's highly confusing, considering just MAP_SHARED mappings where that
> >> will never be true.
> >
> > Hmm, yeah.
> >
> > I mean there's nowhere you'd do this check where you weren't checking something
> > that couldn't at least in theory be anon-faulted. But it's confusing vs. shared
> > you're right.
> >
> > vma_is_anon_faulted()?
>
> Hm, not sure.
>
> 	vma_had_anon_fault()
>
> Might sound better.

vma_anon_faulted()

>
> Alternatively:
>
> 	vma_has_anon_vma()
>
> is the obvious thing we're checking.

No, I want to abstract the mechanism (so later it can be replaced :)

>
> 	vma_might_have_anon_folios()
>
> would be the clearest (no anon_vma -> no anon folios). But semantically that's
> likely not what you want to check in the code?

That kind of implies you want to interact with those and generally you're asking
whether an anon fault has occurred.

Also a bit of a mouthful.

>
>
> The whole faulted/unfaulted terminology is a bit confusing ...

To me not really?

Actually how about:

vma_anon_rmap_tracked()?

Then it speaks to what anon_vma is actually for, the fact the VMA has an
anon_vma assigned like that means it is tracked by the anon_vma, and it
abstracts the actual mechanism?

>
> --
> Cheers,
>
> David

--
Cheers, Lorenzo