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

[Suren Baghdasaryan]

On Sat, Aug 8, 2026 at 12:24 AM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:
>
> On Thu, Aug 06, 2026 at 01:05:46PM -0700, Suren Baghdasaryan wrote:
> > From: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
> >
> > == Background ==
>
> I think we can do without the headings?
>
> > 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@xxxxxxxxxxxxxxxxxxxxxxxxxxxxx/
[2] https://lore.kernel.org/all/CAJuCfpHBzYUNEe_LBpeERf68_fso=Hy+UZWtqejTUkm1O4NLfA@xxxxxxxxxxxxxx/

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

>
> > +/**
> > + * 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;
> > +}