Re: [PATCH RESEND v3 02/10] mm/vma: document possible vma->vm_refcnt values and reference comment

[Lorenzo Stoakes <lorenzo.stoakes@oracle.com>]

Sorry missed this before moving on to 3/10, responses below.

On Thu, Jan 22, 2026 at 09:28:05AM -0800, Suren Baghdasaryan wrote:
> On Thu, Jan 22, 2026 at 8:48 AM Vlastimil Babka <vbabka@suse.cz> wrote:
> >
> > On 1/22/26 14:01, Lorenzo Stoakes wrote:
> > > The possible vma->vm_refcnt values are confusing and vague, explain in
> > > detail what these can be in a comment describing the vma->vm_refcnt field
> > > and reference this comment in various places that read/write this field.
> > >
> > > No functional change intended.
> > >
> > > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> >
> > Thanks, very useful. Forgive my nitpicks :) It's because it's tricky so best
> > try to be as precise as possible, I believe.
>
> Another thanks from me.

Ack

>
> >
> > > ---
> > >  include/linux/mm_types.h  | 39 +++++++++++++++++++++++++++++++++++++--
> > >  include/linux/mmap_lock.h |  7 +++++++
> > >  mm/mmap_lock.c            |  6 ++++++
> > >  3 files changed, 50 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> > > index 94de392ed3c5..e5ee66f84d9a 100644
> > > --- a/include/linux/mm_types.h
> > > +++ b/include/linux/mm_types.h
> > > @@ -758,7 +758,8 @@ static inline struct anon_vma_name *anon_vma_name_alloc(const char *name)
> > >   * set the VM_REFCNT_EXCLUDE_READERS_FLAG in vma->vm_refcnt to indiciate to
> > >   * vma_start_read() that the reference count should be left alone.
> > >   *
> > > - * Once the operation is complete, this value is subtracted from vma->vm_refcnt.
> > > + * See the comment describing vm_refcnt in vm_area_struct for details as to
> > > + * which values the VMA reference count can be.
> > >   */
> > >  #define VM_REFCNT_EXCLUDE_READERS_BIT        (30)
> > >  #define VM_REFCNT_EXCLUDE_READERS_FLAG       (1U << VM_REFCNT_EXCLUDE_READERS_BIT)
> > > @@ -989,7 +990,41 @@ struct vm_area_struct {
> > >       struct vma_numab_state *numab_state;    /* NUMA Balancing state */
> > >  #endif
> > >  #ifdef CONFIG_PER_VMA_LOCK
> > > -     /* Unstable RCU readers are allowed to read this. */
> > > +     /*
> > > +      * Used to keep track of the number of references taken by VMA read or
> > > +      * write locks. May have the VM_REFCNT_EXCLUDE_READERS_FLAG set
> >
> > I wonder about the "or write locks" part. The process of acquiring it uses
> > VM_REFCNT_EXCLUDE_READERS_FLAG but then the writer doesn't hold a 1
> > refcount? (the sentence could be read it way IMHO) It's vma being attached
> > that does, AFAIK?
>
> Yes, since there can be only one write-locker it only has to set
> VM_REFCNT_EXCLUDE_READERS_FLAG bit to announce its presence, without
> incrementing the refcount.

See reply to Vlastimil, I've reworked this to:

	 * Used to keep track of firstly, whether the VMA is attached, secondly,
	 * if attached, how many read locks are taken, and thirdly, if the
	 * VM_REFCNT_EXCLUDE_READERS_FLAG is set, whether any read locks held
	 * are currently in the process of being excluded.

>
> >
> > > +      * indicating that a thread has entered __vma_enter_locked() and is
> > > +      * waiting on any outstanding read locks to exit.
> > > +      *
> > > +      * This value can be equal to:
> > > +      *
> > > +      * 0 - Detached.
> >
> > Is it worth saying that readers can't increment the refcount?
>
> Yes, you mention that for VM_REFCNT_EXCLUDE_READERS_FLAG value. The
> same IMPORTANT notice applies here.

Yup already done as per reply to Vlastimil.

>
> >
> > > +      * 1 - Unlocked or write-locked.
> >
> > "Attached and either unlocked or write-locked." ?
>
> Agree. That's more specific.
> Should we also mention here that unlocked vs write-locked distinction
> is determined using the vm_lock_seq member?

Good idea. Have updated to:

	 * 1 - Attached and either unlocked or write-locked. Write locks are
	 * identified via __is_vma_write_locked() which checks for equality of
	 * vma->vm_lock_seq and mm->mm_lock_seq.

Note I felt it'd be distracting to say 'vma->vm_mm->mm_lock_seq.sequence' :)
mm->mm_lock_seq and hey go look at the function to see the gory details.

>
> >
> > (see how "write-locked" isn't reflected, I argued above)
> >
> > > +      *
> > > +      * >1, < VM_REFCNT_EXCLUDE_READERS_FLAG - Read-locked or (unlikely)
> > > +      * write-locked with other threads having temporarily incremented the
> > > +      * reference count prior to determining it is write-locked and
> > > +      * decrementing it again.
> >
> > Ack.
> >
> > > +      * VM_REFCNT_EXCLUDE_READERS_FLAG - Detached, pending
> > > +      * __vma_exit_locked() completion which will decrement the reference
> > > +      * count to zero. IMPORTANT - at this stage no further readers can
> > > +      * increment the reference count. It can only be reduced.
> > > +      *
> > > +      * VM_REFCNT_EXCLUDE_READERS_FLAG + 1 - Either an attached VMA pending
> > > +      * __vma_exit_locked() completion which will decrement the reference
> > > +      * count to one, OR a detached VMA waiting on a single spurious reader
> > > +      * to decrement reference count. IMPORTANT - as above, no further
> > > +      * readers can increment the reference count.
> > > +      *
> > > +      * > VM_REFCNT_EXCLUDE_READERS_FLAG + 1 - VMA is waiting on readers,
> >
> > "VMA is waiting" sounds weird? a thread might be, but VMA itself?
> > (similarly in the previous paragraph)
>
> Maybe "VMA in the process of being write-locked or detached, which got
> blocked due to the spurious readers that temporarily raised the
> refcount"?
>

Well no, because you might have legit read locks held that you're waiting on
right? That are not spurious? Can be a mix of spurious and legit (though
unlikely, since by then you'd hope the READ_ONCE() check would succeed, but hey
there's weird out-of-order arches out there).

As per reply to Vlastimil I updated it to:

	 * VM_REFCNT_EXCLUDE_READERS_FLAG + 1 - A thread is either write-locking
	 * an attached VMA and has yet to invoke __vma_exit_locked(), OR a
	 * thread is detaching a VMA and is waiting on a single spurious reader
	 * in order to decrement the reference count. IMPORTANT - as above, no
	 * further readers can increment the reference count.
	 *
	 * > VM_REFCNT_EXCLUDE_READERS_FLAG + 1 - A thread is either
	 * write-locking or detaching a VMA is waiting on readers to
	 * exit. IMPORTANT - as above, no ruther readers can increment the
	 * reference count.

Thanks, Lorenzo