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

[Matthew Wilcox <willy@infradead.org>]

On Thu, Aug 06, 2026 at 01:05:46PM -0700, Suren Baghdasaryan wrote:
> From: Dave Hansen <dave.hansen@linux.intel.com>
> 
> == 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?

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