Re: [PATCH v3 3/5] mm: Add RCU-based VMA lookup helper that waits for writers

On Mon, Aug 3, 2026 at 9:44 AM Lorenzo Stoakes (ARM) [off-list ref] wrote:

On Mon, Aug 03, 2026 at 06:24:34PM +0200, Vlastimil Babka (SUSE) wrote:

quoted

On 8/3/26 17:00, Lorenzo Stoakes (ARM) wrote:

quoted

On Mon, Aug 03, 2026 at 04:55:19PM +0200, Vlastimil Babka (SUSE) wrote:

quoted

On 8/2/26 23:54, Suren Baghdasaryan wrote:

quoted

From: Dave Hansen <dave.hansen@linux.intel.com>

== Background ==

There are basically two parallel ways to look up a VMA: the
traditional way, which is protected by mmap_read_lock, and the RCU-based
per-VMA lock way which is based on RCU and refcounts.

== Problem ==

The mmap_lock one is more straightforward to use but it has a big
disadvantage in that it can not be mixed with page faults since those
can take mmap_lock for read, which can deadlock when mixed with nested
page faults and parallel writers.
For example:

 mmap_read_lock(mm);
 // Another thread does mmap_write_lock().
 // New mmap_lock readers are blocked.
 vma = vma_lookup(mm, address);
 // This deadlocks on mmap_read_lock() if it faults:
 copy_from_user(address);
 mmap_read_unlock(mm);

The per-VMA lock can be mixed with faults, but they can fail and need to
be able to fall back to the traditional way.

== Solution ==

Add a variant of the RCU-based lookup that waits for writers. This is
basically the same as the existing RCU-based lookup, but on a failure to
lock it temporarily takes mmap_lock for read and waits for writers
to finish before locking the VMA, dropping the mmap_lock and returning
the locked VMA. This has some advantages:

Maybe mention that the helper is called vma_start_read_unlocked()?

Ack.

quoted

quoted

quoted

quoted

 1. Callers do not need to have a fallback path for when they
    collide with writers.
 2. It can be used in contexts where page faults can happen because
    it can take the mmap_lock for read but never *holds* it.
 3. Its fast path does not require taking mmap_lock for read.

Basically, when applied correctly, this approach results in faster
*and* simpler code.

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
Cc: Suren Baghdasaryan <surenb@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: "Liam R. Howlett" <redacted>
Cc: Lorenzo Stoakes <ljs@kernel.org>
Cc: Vlastimil Babka <vbabka@kernel.org>
Cc: Shakeel Butt <shakeel.butt@linux.dev>
Cc: linux-mm@kvack.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Arve Hjønnevåg <arve@android.com>
Cc: Todd Kjos <tkjos@android.com>
Cc: Christian Brauner <christian@brauner.io>
Cc: Carlos Llamas <cmllamas@google.com>
Cc: Alice Ryhl <aliceryhl@google.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: David Ahern <dsahern@kernel.org>
Cc: netdev@vger.kernel.org
---
 include/linux/mmap_lock.h | 15 +++++++++++----
 mm/mmap_lock.c            | 29 +++++++++++++++++++++++++++++
 mm/userfaultfd.c          |  6 ++++--
 3 files changed, 44 insertions(+), 6 deletions(-)

diff --git a/include/linux/mmap_lock.h b/include/linux/mmap_lock.h
index eb32b482434e..fdd8f5cf5722 100644
--- a/include/linux/mmap_lock.h
+++ b/include/linux/mmap_lock.h

@@ -228,10 +228,12 @@ static inline void vma_refcount_put(struct vm_area_struct *vma)
 }

 /*
- * Use only while holding mmap read lock which guarantees that locking will not
- * fail (nobody can concurrently write-lock the vma). vma_start_read() should
+ * Use only while holding mmap read lock which guarantees that vma lock is not
+ * contended (nobody can concurrently write-lock the vma). vma_start_read() should
  * not be used in such cases because it might fail due to mm_lock_seq overflow.
  * This functionality is used to obtain vma read lock and drop the mmap read lock.
+ * VMA can't be detached while we are holding mmap lock, therefore in practice this
+ * function can fail only when there are so many readers that vm_refcnt overflows.
  */
 static inline bool vma_start_read_locked_nested(struct vm_area_struct *vma, int subclass)
 {

@@ -247,16 +249,21 @@ static inline bool vma_start_read_locked_nested(struct vm_area_struct *vma, int
 }

 /*
- * Use only while holding mmap read lock which guarantees that locking will not
- * fail (nobody can concurrently write-lock the vma). vma_start_read() should
+ * Use only while holding mmap read lock which guarantees that vma lock is not
+ * contended (nobody can concurrently write-lock the vma). vma_start_read() should
  * not be used in such cases because it might fail due to mm_lock_seq overflow.
  * This functionality is used to obtain vma read lock and drop the mmap read lock.
+ * VMA can't be detached while we are holding mmap lock, therefore in practice this
+ * function can fail only when there are so many readers that vm_refcnt overflows.
  */
 static inline bool vma_start_read_locked(struct vm_area_struct *vma)
 {
         return vma_start_read_locked_nested(vma, 0);
 }

+struct vm_area_struct *vma_start_read_unlocked(struct mm_struct *mm,
+                                               unsigned long address);
+
 static inline void vma_end_read(struct vm_area_struct *vma)
 {
         vma_refcount_put(vma);

diff --git a/mm/mmap_lock.c b/mm/mmap_lock.c
index e20d01e8d38f..6ff05e68e61b 100644
--- a/mm/mmap_lock.c
+++ b/mm/mmap_lock.c

@@ -338,6 +338,35 @@ struct vm_area_struct *lock_vma_under_rcu(struct mm_struct *mm,
         return NULL;
 }

+/*
+ * Find the VMA covering 'address' and lock it for reading. Waits for writers to
+ * finish if the VMA is being modified. Returns NULL if there is no VMA covering
+ * 'address'.

Hm but it can also return NULL when vm_refcnt overflows, in theory.
Should we also return -EAGAIN (like uffd_lock_vma() below), or just retry in
here and hope for the best? The latter would be simpler for the users.
(AFAICS due to VM_REFCNT_LIMIT we never end up triggering the refcount
saturation)

The problem is everything's unlocked so 'didn't find a VMA' doesn't really mean
much more than 'something went wrong' because hey maybe if you check again now
you'll find something :)

Well there might be use cases where you know that either there's a vma with
your address and then you need to do something with it, or there's not and
then you don't. And it can't suddenly appear after you check.

You don't hold a lock that prevents new VMAs appearing/disappearing
spontaneously at the point you call lock_vma_under_rcu(), or after you drop the
mmap read lock, only that at the point of checking a VMA spans address, so
there's nothing preventing a VMA suddenly appearing after you check right? Or it
not being the one you wanted?

And checking to see if it's 'really the one you meant' is itself fraught (see
the whole uffd saga on that).

Point I'm making is that in any case where you'd actually care you'd need to
take a stronger lock anyway, so it's actually potentially dangerous to
differentiate between the two.

Yeah, I tend to agree with Lorenzo that  when lock_vma_under_rcu()
fails, we should not make any assumptions about the reason because the
range is not locked and therefore is not stable. Any assumption risks
being wrong if a race occurs.

Given the overflow is very very unlikely I think it's also not a big deal to not
differentiate anyway.

quoted

So in that case treating that spurious NULL as "there's no vma so I don't
need to do anything" would be wrong.

The usages in 4/5 and 5/5 seem like they are not this case though. So it's
fine. But perhaps worth just mentioning it in the comment then.

Agree this is worth spelling out in the comment (I raised similarly).

Maybe something like:

        If a VMA exists which spans @address, return that VMA, read-locked.

        If no VMA is mapped there or, very unlikely, a reference count overflow
        occurred, return NULL.

        Nothing prevents VMAs being unmapped/mapped before or after the VMA is
        looked up, if a stronger guarantee is required, take an mmap lock.

This last statement is true only if the function returns NULL, so I
think it should be in the same paragraph as the "If no VMA is
mapped..." sentence and prepended with "In this case...". So:

       If no VMA is mapped there or, very unlikely, a reference count overflow
       occurred, return NULL. In this case, nothing prevents VMAs
being unmapped/
       mapped before or after the VMA is looked up, if a stronger guarantee is
       required, take an mmap lock.

Does that sounds good?

quoted

quoted

So I think this might be a feature more than a bug, especially given overflow is
not exactly likely.

quoted

quoted

+ *
+ * Use only in code paths where no mmap_lock and no VMA lock is held.
+ *
+ * The fast path does not take mmap_lock.
+ */
+struct vm_area_struct *vma_start_read_unlocked(struct mm_struct *mm,
+                                               unsigned long address)
+{
+        struct vm_area_struct *vma;
+
+        /* Fast path: return stable VMA covering 'address': */
+        vma = lock_vma_under_rcu(mm, address);
+        if (vma)
+                return vma;
+
+        /* Slow path: preclude VMA writers by temporarily getting mmap read lock. */
+        mmap_read_lock(mm);
+        vma = vma_lookup(mm, address);
+        if (vma && !vma_start_read_locked(vma))
+                vma = NULL;
+        mmap_read_unlock(mm);
+
+        return vma;
+}
+
 static struct vm_area_struct *lock_next_vma_under_mmap_lock(struct mm_struct *mm,
                                                             struct vma_iterator *vmi,
                                                             unsigned long from_addr)

diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c
index edd90892f8cc..c3a0c38a3dc3 100644
--- a/mm/userfaultfd.c
+++ b/mm/userfaultfd.c

@@ -129,8 +129,10 @@ struct vm_area_struct *find_vma_and_prepare_anon(struct mm_struct *mm,
  *
  * Should be called without holding mmap_lock.
  *
- * Return: A locked vma containing @address, -ENOENT if no vma is found, or
- * -ENOMEM if anon_vma couldn't be allocated.
+ * Return: A locked vma containing @address, -ENOENT if no vma is found,
+ * -ENOMEM if anon_vma couldn't be allocated, or -EAGAIN if vma refcount
+ * overflow happened due to high number of readers and the caller should
+ * retry later.
  */
 static struct vm_area_struct *uffd_lock_vma(struct mm_struct *mm,
                                        unsigned long address)

--
Cheers, Lorenzo

--
Cheers, Lorenzo