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

On Sat, Aug 8, 2026 at 12:24 AM Matthew Wilcox [off-list ref] wrote:

On Thu, Aug 06, 2026 at 01:05:46PM -0700, Suren Baghdasaryan wrote:

quoted

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

== Background ==

I think we can do without the headings?

quoted

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.

Umm.  I don't see how this avoids deadlock.  Assuming the next patch
converts copy_from_user() to use the VMA lock, surely the following
situation would obtain:

A takes mmap_read_lock
B tries to take mmap_write_lock, blocks
A calls copy_from_user()
A calls vma_start_read_unlocked() (because it doesn't know A actually
holds the mmap_read_lock() already)
A does a lookup under RCU, but gets NULL back (maybe it's calling
c_f_u() with an invalid address?)
A tries to take the mmap_read_lock again to make sure.  Deadlock
because B is waiting for A to release the mmap_read_lock.

Am I missing something?

I think Dave's idea was that A would take a VMA lock (which might
require taking mmap read lock temporarily) and then call
copy_from_user() with VMA lock only, which allows copy_from_user() to
take the mmap lock. This was used in the last patch [1] of the
original RFC. That patch is dropped from the series because this
approach has a conceptual lock ordering problem - it takes VMA lock
and then mmap_lock (see explanation at [2]).

[1] https://lore.kernel.org/all/20260429182005.00BF70D8@davehans-spike.ostc.intel.com/ (local)
[2] https://lore.kernel.org/all/CAJuCfpHBzYUNEe_LBpeERf68_fso=Hy+UZWtqejTUkm1O4NLfA@mail.gmail.com/ (local)

So, yeah. I need to rewrite this commit message.

quoted

+/**
+ * vma_start_read_unlocked() - Find the VMA covering 'address' and read-lock it.
+ * @mm: the mm_struct of the address space to search
+ * @address: address that the vma should contain
+ *
+ * The fast path does not take mmap_lock. Waits for writers to finish if the
+ * VMA is being modified by taking mmap_lock.
+ * Use when mmap_lock is not held, otherwise use vma_start_read_locked().
+ * Nothing prevents VMAs being unmapped/mapped before or after the VMA is
+ * looked up, if a stronger guarantee is required, take an mmap_lock.
+ *
+ * Return: 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.
+ */
+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;
+}