Re: [PATCH RESEND v3 10/10] mm/vma: add and use vma_assert_stabilised()

[Suren Baghdasaryan <surenb@google.com>]

On Thu, Jan 22, 2026 at 5:02 AM Lorenzo Stoakes
<lorenzo.stoakes@oracle.com> wrote:
>
> Sometimes we wish to assert that a VMA is stable, that is - the VMA cannot
> be changed underneath us. This will be the case if EITHER the VMA lock or
> the mmap lock is held.
>
> In order to do so, we introduce a new assert vma_assert_stablised() - this

s/vma_assert_stablised/vma_assert_stabilised

> will make a lockdep assert if lockdep is enabled AND the VMA is
> read-locked.
>
> Currently lockdep tracking for VMA write locks is not implemented, so it
> suffices to check in this case that we have either an mmap read or write
> semaphore held.
>
> Note that because the VMA lock uses the non-standard vmlock_dep_map naming
> convention, we cannot use lockdep_assert_is_write_held() so have to open
> code this ourselves via lockdep-asserting that
> lock_is_held_type(&vma->vmlock_dep_map, 0).
>
> We have to be careful here - for instance when merging a VMA, we use the
> mmap write lock to stabilise the examination of adjacent VMAs which might
> be simultaneously VMA read-locked whilst being faulted in.
>
> If we were to assert VMA read lock using lockdep we would encounter an
> incorrect lockdep assert.
>
> Also, we have to be careful about asserting mmap locks are held - if we try
> to address the above issue by first checking whether mmap lock is held and
> if so asserting it via lockdep, we may find that we were raced by another
> thread acquiring an mmap read lock simultaneously that either we don't
> own (and thus can be released any time - so we are not stable) or was
> indeed released since we last checked.
>
> So to deal with these complexities we end up with either a precise (if
> lockdep is enabled) or imprecise (if not) approach - in the first instance
> we assert the lock is held using lockdep and thus whether we own it.
>
> If we do own it, then the check is complete, otherwise we must check for
> the VMA read lock being held (VMA write lock implies mmap write lock so the
> mmap lock suffices for this).
>
> If lockdep is not enabled we simply check if the mmap lock is held and risk
> a false negative (i.e. not asserting when we should do).
>
> There are a couple places in the kernel where we already do this
> stabliisation check - the anon_vma_name() helper in mm/madvise.c and
> vma_flag_set_atomic() in include/linux/mm.h, which we update to use
> vma_assert_stabilised().
>
> This change abstracts these into vma_assert_stabilised(), uses lockdep if
> possible, and avoids a duplicate check of whether the mmap lock is held.
>
> This is also self-documenting and lays the foundations for further VMA
> stability checks in the code.

So, is the lockdep addition the only functional change here?

>
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

Reviewed-by: Suren Baghdasaryan <surenb@google.com>

> ---
>  include/linux/mm.h        |  5 +---
>  include/linux/mmap_lock.h | 52 +++++++++++++++++++++++++++++++++++++++
>  mm/madvise.c              |  4 +--
>  3 files changed, 54 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 6029a71a6908..d7ca837dd8a5 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1008,10 +1008,7 @@ static inline void vma_flag_set_atomic(struct vm_area_struct *vma,
>  {
>         unsigned long *bitmap = ACCESS_PRIVATE(&vma->flags, __vma_flags);
>
> -       /* mmap read lock/VMA read lock must be held. */
> -       if (!rwsem_is_locked(&vma->vm_mm->mmap_lock))
> -               vma_assert_locked(vma);
> -
> +       vma_assert_stabilised(vma);
>         if (__vma_flag_atomic_valid(vma, bit))
>                 set_bit((__force int)bit, bitmap);
>  }
> diff --git a/include/linux/mmap_lock.h b/include/linux/mmap_lock.h
> index 92ea07f0da4e..e01161560608 100644
> --- a/include/linux/mmap_lock.h
> +++ b/include/linux/mmap_lock.h
> @@ -374,6 +374,52 @@ static inline void vma_assert_locked(struct vm_area_struct *vma)
>         vma_assert_write_locked(vma);
>  }
>
> +/**
> + * vma_assert_stabilised() - assert that this VMA cannot be changed from
> + * underneath us either by having a VMA or mmap lock held.
> + * @vma: The VMA whose stability we wish to assess.
> + *
> + * If lockdep is enabled we can precisely ensure stability via either an mmap
> + * lock owned by us or a specific VMA lock.
> + *
> + * 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)
> +{
> +       /*
> +        * If another thread owns an mmap lock, it may go away at any time, and
> +        * thus is no guarantee of stability.
> +        *
> +        * If lockdep is enabled we can accurately determine if an mmap lock is
> +        * held and owned by us. Otherwise we must approximate.
> +        *
> +        * It doesn't necessarily mean we are not stabilised however, as we may
> +        * hold a VMA read lock (not a write lock as this would require an owned
> +        * mmap lock).
> +        *
> +        * If (assuming lockdep is not enabled) we were to assert a VMA read
> +        * lock first we may also run into issues, as other threads can hold VMA
> +        * read locks simlutaneous to us.
> +        *
> +        * Therefore if lockdep is not enabled we risk a false negative (i.e. no
> +        * assert fired). If accurate checking is required, enable lockdep.
> +        */
> +       if (IS_ENABLED(CONFIG_LOCKDEP)) {
> +               if (lockdep_is_held(&vma->vm_mm->mmap_lock))
> +                       return;
> +       } else {
> +               if (rwsem_is_locked(&vma->vm_mm->mmap_lock))
> +                       return;
> +       }
> +
> +       /*
> +        * We're not stabilised by the mmap lock, so assert that we're
> +        * stabilised by a VMA lock.
> +        */
> +       vma_assert_locked(vma);
> +}
> +
>  static inline bool vma_is_attached(struct vm_area_struct *vma)
>  {
>         return refcount_read(&vma->vm_refcnt);
> @@ -455,6 +501,12 @@ static inline void vma_assert_locked(struct vm_area_struct *vma)
>         mmap_assert_locked(vma->vm_mm);
>  }
>
> +static inline void vma_assert_stabilised(struct vm_area_struct *vma)
> +{
> +       /* If no VMA locks, then either mmap lock suffices to stabilise. */
> +       mmap_assert_locked(vma->vm_mm);
> +}
> +
>  #endif /* CONFIG_PER_VMA_LOCK */
>
>  static inline void mmap_write_lock(struct mm_struct *mm)
> diff --git a/mm/madvise.c b/mm/madvise.c
> index 4bf4c8c38fd3..1f3040688f04 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -109,9 +109,7 @@ void anon_vma_name_free(struct kref *kref)
>
>  struct anon_vma_name *anon_vma_name(struct vm_area_struct *vma)
>  {
> -       if (!rwsem_is_locked(&vma->vm_mm->mmap_lock))
> -               vma_assert_locked(vma);
> -
> +       vma_assert_stabilised(vma);
>         return vma->anon_name;
>  }
>
> --
> 2.52.0