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

[Lorenzo Stoakes (ARM)]

On Mon, Sep 14, 2026 at 03:24:47PM +0100, Pedro Falcato wrote:
> On Thu, Sep 10, 2026 at 02:18:35PM +0100, 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_anon_tracked() 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_anon_tracked() 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>
> > ---
> > v2:
> > - Renamed vma_is_faulted() to vma_anon_tracked() as per David.
> >
> > v1:
> > https://lore.kernel.org/r/20260909-vma-is-faulted-v1-1-3a701f48984a@xxxxxxxxxx
> > ---
> >  include/linux/mmap_lock.h         | 12 ++++++------
> >  mm/huge_memory.c                  |  4 ++--
> >  mm/internal.h                     |  2 +-
> >  mm/khugepaged.c                   |  3 +++
> >  mm/ksm.c                          | 10 +++++-----
> >  mm/madvise.c                      |  4 ++--
> >  mm/memory.c                       |  2 +-
> >  mm/mprotect.c                     |  2 +-
> >  mm/mremap.c                       |  4 ++--
> >  mm/rmap.c                         | 22 +++++++++++-----------
> >  mm/swapfile.c                     |  2 +-
> >  mm/userfaultfd.c                  |  2 +-
> >  mm/vma.c                          | 29 +++++++++++++++--------------
> >  mm/vma.h                          | 22 +++++++++++++++++++++-
> >  tools/testing/vma/include/dup.h   |  2 +-
> >  tools/testing/vma/include/stubs.h |  4 ++++
> >  16 files changed, 77 insertions(+), 49 deletions(-)
> >
> > diff --git a/include/linux/mmap_lock.h b/include/linux/mmap_lock.h
> > index 00eae65b74bd..03e1eb136111 100644
> > --- a/include/linux/mmap_lock.h
> > +++ b/include/linux/mmap_lock.h
> > @@ -273,7 +273,7 @@ static inline void vma_end_read(struct vm_area_struct *vma)
> >  	vma_refcount_put(vma);
> >  }
> >
> > -static inline unsigned int __vma_raw_mm_seqnum(struct vm_area_struct *vma)
> > +static inline unsigned int __vma_raw_mm_seqnum(const struct vm_area_struct *vma)
> >  {
> >  	const struct mm_struct *mm = vma->vm_mm;
> >
> > @@ -288,7 +288,7 @@ static inline unsigned int __vma_raw_mm_seqnum(struct vm_area_struct *vma)
> >   *
> >   * Returns true if write-locked, otherwise false.
> >   */
> > -static inline bool __is_vma_write_locked(struct vm_area_struct *vma)
> > +static inline bool __is_vma_write_locked(const struct vm_area_struct *vma)
> >  {
> >  	/*
> >  	 * current task is holding mmap_write_lock, both vma->vm_lock_seq and
> > @@ -344,7 +344,7 @@ int vma_start_write_killable(struct vm_area_struct *vma)
> >   * vma_assert_write_locked() - assert that @vma holds a VMA write lock.
> >   * @vma: The VMA to assert.
> >   */
> > -static inline void vma_assert_write_locked(struct vm_area_struct *vma)
> > +static inline void vma_assert_write_locked(const struct vm_area_struct *vma)
> >  {
> >  	if (!IS_ENABLED(CONFIG_MMU)) {
> >  		mmap_assert_write_locked(vma->vm_mm);
> > @@ -359,7 +359,7 @@ static inline void vma_assert_write_locked(struct vm_area_struct *vma)
> >   * lock and is not detached.
> >   * @vma: The VMA to assert.
> >   */
> > -static inline void vma_assert_locked(struct vm_area_struct *vma)
> > +static inline void vma_assert_locked(const struct vm_area_struct *vma)
> >  {
> >  	unsigned int refcnt;
> >
> > @@ -410,7 +410,7 @@ static inline void vma_assert_locked(struct vm_area_struct *vma)
> >   * With lockdep disabled we may sometimes race with other threads acquiring the
> >   * mmap read lock simultaneous with our VMA read lock.
> >   */
> > -static inline void vma_assert_stabilised(struct vm_area_struct *vma)
> > +static inline void vma_assert_stabilised(const struct vm_area_struct *vma)
> >  {
> >  	/*
> >  	 * If another thread owns an mmap lock, it may go away at any time, and
> > @@ -445,7 +445,7 @@ static inline void vma_assert_stabilised(struct vm_area_struct *vma)
> >  	vma_assert_locked(vma);
> >  }
> >
> > -static inline bool vma_is_attached(struct vm_area_struct *vma)
> > +static inline bool vma_is_attached(const struct vm_area_struct *vma)
> >  {
> >  	return refcount_read(&vma->vm_refcnt);
> >  }
>
> nit: I would suggest you do all of this constification in a separate patch,
> otherwise it can become tricky for backports. Things can naturally start depending
> on const correctness, and you either need to manually adjust those future patches,
> or try to backport this one.

Ok will break out.

>
> The rest looks vaguely good to me. I admit I'm not a gigantic fan of the name,
> but I suspect it fits better with your future anon rmap ideas :)

Naming's hard and it's hard to make everybody happy, unfortunately :(

vma_has_anon_rmap() better?

>
> Reviewed-by: Pedro Falcato <pfalcato@xxxxxxx>

Thanks!

>
>
> --
> Pedro

--
Cheers, Lorenzo