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

Suren Baghdasaryan
2026-01-22T22:12:39Z

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