Re: [PATCH v2] mm/vma: avoid spurious write locks when mm->mm_lock_seq overflows

[Andrew Morton]

On Fri, 16 Oct 2026 12:34:33 +0100 "Lorenzo Stoakes (ARM)" <ljs@xxxxxxxxxx> wrote:

> So far the assumption has been that mm->mm_lock_seq overflow is benign,
> resulting only in readers spuriously falling back to the mmap lock, however
> it turns out that this was an unsafe assumption.
> 
> If a VMA can be spuriously considered write locked that means read lock
> exclusion does not take place, and thus a VMA can end up write-locked with
> concurrent readers.
> 
> Since overflow only occurs every 2^31 write lock/unlock cycles, resolve the
> issue by resetting every VMA's vma->vm_lock_seq each time such an overflow
> occurs.
> 
> This is done in vma_end_write_all(), which is called when the mmap write
> lock is released, i.e. in mmap_write_unlock() and mmap_write_downgrade().
> 
> Care must be taken, as resetting the sequence count at overflow means
> changing the seqcount of VMAs which are already attached and observable.
> 

Updated mm-unstable, thanks.

> Fixes: 5e31275cc997 ("mm: add per-VMA lock and helper functions to control it")
> Andrew - no stable tag please.
> 
> I will do the stable backports manually, they can be solved with a different
> approach.
> 

Wilco.  It's a little unusual - we'd normally add the cc:stable and
wait for -stable people to say "hey this needs rework".  But if the
upstream patch happens to apply to the -stable tree then yes, that
would be a problem.

> Also, since, we're so close to the merge window I'm applying this against
> mm-unstable.
> 
> So I think it's fine to just send it (early :) in the merge window.

Yeah, I'm adding late-arriving backportable fixes into the second-week
pile.  21 cc:stables in there so far...

> v2:
> * Updated comment in vma_seqcount_is_write_locked() to mention the VMA
>   being attached.
> * Renamed reset_vma_seqcounts() to __mm_reset_vma_seqcounts().
> * Used acquire semantics in vma_start_reads() to avoid race with reset -
>   there is another acquire immediately after this one so it should just be
>   noise perf-wise.
> * Removed stable tag, will do the backports manually (just make the value
>   64 bits...)
> * Updated comments to reflect changes.
> * Added user-facing effects as per Vlasta ;)

Here's how v2 altered mm.git:


 Documentation/mm/process_addrs.rst |    6 ++--
 include/linux/mm_types.h           |    6 ++--
 include/linux/mmap_lock.h          |    4 +--
 mm/mmap_lock.c                     |   33 +++++++++++++++------------
 4 files changed, 27 insertions(+), 22 deletions(-)

--- a/Documentation/mm/process_addrs.rst~a
+++ a/Documentation/mm/process_addrs.rst
@@ -827,9 +827,9 @@ is odd, then the VMA is write-locked. If
 A VMA's sequence count is not updated on unlock, so after 2^31 write lock/unlock
 cycles the mm sequence count would wrap and match stale VMA values again. To
 prevent this, when the mm sequence count wraps to zero,
-:c:func:`!vma_end_write_all` invokes :c:func:`!reset_vma_seqcounts` to reset
-every VMA's sequence count to zero, an even value which can never indicate a
-write lock.
+:c:func:`!vma_end_write_all` invokes :c:func:`!__mm_reset_vma_seqcounts` to
+reset every VMA's sequence count to zero, an even value which can never
+indicate a write lock.
 
 Each time the mmap write lock is released in :c:func:`!mmap_write_unlock` or
 :c:func:`!mmap_write_downgrade`, :c:func:`!vma_end_write_all` is invoked which
--- a/include/linux/mmap_lock.h~a
+++ a/include/linux/mmap_lock.h
@@ -554,7 +554,7 @@ static inline int __must_check mmap_writ
 	return ret;
 }
 
-void reset_vma_seqcounts(struct mm_struct *mm);
+void __mm_reset_vma_seqcounts(struct mm_struct *mm);
 
 /*
  * Drop all currently-held per-VMA locks.
@@ -577,7 +577,7 @@ static inline void vma_end_write_all(str
 	 * seqcount so that no stale value survives the wrap.
 	 */
 	if (unlikely(!mm->mm_lock_seq.sequence))
-		reset_vma_seqcounts(mm);
+		__mm_reset_vma_seqcounts(mm);
 }
 
 static inline void mmap_write_unlock(struct mm_struct *mm)
--- a/include/linux/mm_types.h~a
+++ a/include/linux/mm_types.h
@@ -992,9 +992,9 @@ struct vm_area_struct {
 	 * Can only be written (using WRITE_ONCE()) while holding both:
 	 *  - mmap_lock (in write mode)
 	 *  - vm_refcnt bit at VM_REFCNT_EXCLUDE_READERS_FLAG is set
-	 * The one exception is reset_vma_seqcounts(), which stores zero (an
-	 * even value, which can never indicate a write lock) under the mmap
-	 * write lock alone once the mm seqcount has wrapped, so a reader
+	 * The one exception is __mm_reset_vma_seqcounts(), which stores zero
+	 * (an even value, which can never indicate a write lock) under the
+	 * mmap write lock alone once the mm seqcount has wrapped, so a reader
 	 * holding only vm_refcnt may observe the value change from odd to zero.
 	 * Can be read reliably while holding one of:
 	 *  - mmap_lock (in read or write mode)
--- a/mm/mmap_lock.c~a
+++ a/mm/mmap_lock.c
@@ -202,8 +202,9 @@ static bool vma_seqcount_is_write_locked
 		return false;
 
 	/*
-	 * The VMA seqcount is always odd if locked. An even value can only be
-	 * the zero stored by reset_vma_seqcounts(), never a write lock.
+	 * The VMA seqcount is always odd if locked. For an attached VMA an even
+	 * value can only be the zero stored by __mm_reset_vma_seqcounts(),
+	 * never a write lock.
 	 */
 	return vma_seq & 1;
 }
@@ -268,17 +269,20 @@ static struct vm_area_struct *vma_start_
 	 * vma->vm_lock_seq under vma->vm_refcnt protection and mm->mm_lock_seq
 	 * modification invalidates all existing locks.
 	 *
-	 * The only other writer is reset_vma_seqcounts(), which stores zero
-	 * (even, so never a write lock) after the mmap write lock's seqcount
-	 * has been released and before any new write lock can be taken, hence
-	 * the READ_ONCE().
+	 * ACQUIRE semantics are required for both reads.
 	 *
-	 * We must use ACQUIRE semantics for the mm_lock_seq so that if we are
-	 * racing with vma_end_write_all(), we only start reading from the VMA
-	 * after it has been unlocked.
-	 * This pairs with RELEASE semantics in vma_end_write_all().
+	 * __mm_reset_vma_seqcounts() might reset vma->vm_lock_seq to zero
+	 * after an overflow. A reader observing that zero must also observe
+	 * the VMA modifications made under the write lock which just ended, so
+	 * the ACQUIRE pairs with the smp_wmb() in do_raw_write_seqcount_end()
+	 * that precedes the reset stores.
+	 *
+	 * The ACQUIRE semantics for the mm_lock_seq read is required so that,
+	 * if we are racing with vma_end_write_all(), we only start reading from
+	 * the VMA after it has been unlocked. This pairs with the RELEASE
+	 * semantics in vma_end_write_all().
 	 */
-	vma_seq = READ_ONCE(vma->vm_lock_seq);
+	vma_seq = smp_load_acquire(&vma->vm_lock_seq);
 	mm_seq = raw_read_seqcount(&mm->mm_lock_seq);
 	if (unlikely(vma_seqcount_is_write_locked(vma_seq, mm_seq))) {
 		vma_refcount_put(vma);
@@ -496,9 +500,10 @@ fallback:
  *
  * The stores below follow the smp_wmb() in do_raw_write_seqcount_end(), so
  * a reader that observes a reset value also observes this cycle's VMA
- * modifications (ordered by the smp_rmb() in raw_read_seqcount()).
+ * modifications (ordered by the ACQUIRE on vma->vm_lock_seq in
+ * vma_start_read()).
  */
-void reset_vma_seqcounts(struct mm_struct *mm)
+void __mm_reset_vma_seqcounts(struct mm_struct *mm)
 {
 	struct vm_area_struct *vma;
 	VMA_ITERATOR(vmi, mm, 0);
@@ -524,7 +529,7 @@ void reset_vma_seqcounts(struct mm_struc
 	for_each_vma(vmi, vma)
 		WRITE_ONCE(vma->vm_lock_seq, 0);
 }
-EXPORT_SYMBOL(reset_vma_seqcounts);
+EXPORT_SYMBOL(__mm_reset_vma_seqcounts);
 
 #ifdef CONFIG_LOCK_MM_AND_FIND_VMA
 #include <linux/extable.h>
_