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

[David Hildenbrand (Arm)]

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.

Alternatively:

	vma_has_anon_vma()

is the obvious thing we're checking.

	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?


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

-- 
Cheers,

David