Message-Id: <20260126113806.5b0942a6fd0a3554ae5e21fd@linux-foundation.org>
Date: Mon, 26 Jan 2026 11:38:06 -0800
From: Andrew Morton <akpm@...ux-foundation.org>
To: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
Cc: Vlastimil Babka <vbabka@...e.cz>, David Hildenbrand <david@...nel.org>,
 "Liam R . Howlett" <Liam.Howlett@...cle.com>, Mike Rapoport
 <rppt@...nel.org>, Suren Baghdasaryan <surenb@...gle.com>, Michal Hocko
 <mhocko@...e.com>, Shakeel Butt <shakeel.butt@...ux.dev>, Jann Horn
 <jannh@...gle.com>, linux-mm@...ck.org, linux-kernel@...r.kernel.org,
 linux-rt-devel@...ts.linux.dev, Peter Zijlstra <peterz@...radead.org>, Ingo
 Molnar <mingo@...hat.com>, Will Deacon <will@...nel.org>, Boqun Feng
 <boqun.feng@...il.com>, Waiman Long <longman@...hat.com>, Sebastian Andrzej
 Siewior <bigeasy@...utronix.de>, Clark Williams <clrkwllms@...nel.org>,
 Steven Rostedt <rostedt@...dmis.org>
Subject: Re: [PATCH v4 07/10] mm/vma: introduce helper struct + thread
 through exclusive lock fns

On Mon, 26 Jan 2026 16:09:24 +0000 Lorenzo Stoakes <lorenzo.stoakes@...cle.com> wrote:

> Andrew - could we change the commit message to:
> 
> -->
> 
> It is confusing to have __vma_start_exclude_readers() return 0, 1 or an
> error (but only when waiting for readers in TASK_KILLABLE state), and
> having the return value be stored in a stack variable called 'locked' is
> further confusion.
> 
> More generally, we are doing a lot of rather finnicky things during the
> acquisition of a state in which readers are excluded and moving out of this
> state, including tracking whether we are detached or not or whether an
> error occurred.
> 
> We are implementing logic in __vma_start_exclude_readers() that effectively
> acts as if 'if one caller calls us do X, if another then do Y', which is
> very confusing from a control flow perspective.
> 
> Introducing the shared helper object state helps us avoid this, as we can
> now handle the 'an error arose but we're detached' condition correctly in
> both callers - a warning if not detaching, and treating the situation as if
> no error arose in the case of a VMA detaching.
> 
> This also acts to help document what's going on and allows us to add some
> more logical debug asserts.
> 
> Also update vma_mark_detached() to add a guard clause for the likely
> 'already detached' state (given we hold the mmap write lock), and add a
> comment about ephemeral VMA read lock reference count increments to clarify
> why we are entering/exiting an exclusive locked state here.
> 
> Finally, separate vma_mark_detached() into its fast-path component and make
> it inline, then place the slow path for excluding readers in mmap_lock.c.
> 
> No functional change intended.

Pasted in.

> <--
> 
> Please as per Vlasta's comments below? Thanks!
> 
> Also could you sed the patch with:
> 
> s/__vma_exit_exclusive_locked/__vma_end_exclude_readers/
> s/__vma_[enter, exit]_exclusive_locked/__vma_[start, end]_exclude_readers/
> 
> As per Vlasta's comments below?
>
> As I have clearly forgotten to do this bit myself... doh!
> 
> Also at the bottom there is one small correction to a comment there too.

I added this -fix:


From: Andrew Morton <akpm@...ux-foundation.org>
Subject: mm-vma-introduce-helper-struct-thread-through-exclusive-lock-fns-fix
Date: Fri, 23 Jan 2026 20:12:17 +0000

fix function naming in comments, add comment per Vlastimil per Lorenzo

Link: https://lkml.kernel.org/r/7d3084d596c84da10dd374130a5055deba6439c0.1769198904.git.lorenzo.stoakes@oracle.com
Cc: Boqun Feng <boqun.feng@...il.com>
Cc: Liam Howlett <liam.howlett@...cle.com>
Cc: Michal Hocko <mhocko@...e.com>
Cc: Mike Rapoport <rppt@...nel.org>
Cc: Shakeel Butt <shakeel.butt@...ux.dev>
Cc: Suren Baghdasaryan <surenb@...gle.com>
Cc: Vlastimil Babka <vbabka@...e.cz>
Cc: Waiman Long <longman@...hat.com>
Signed-off-by: Andrew Morton <akpm@...ux-foundation.org>
---

 include/linux/mm_types.h |    4 ++--
 mm/mmap_lock.c           |    4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

--- a/include/linux/mm_types.h~mm-vma-introduce-helper-struct-thread-through-exclusive-lock-fns-fix
+++ a/include/linux/mm_types.h
@@ -1011,12 +1011,12 @@ struct vm_area_struct {
 	 * decrementing it again.
 	 *
 	 * VM_REFCNT_EXCLUDE_READERS_FLAG - Detached, pending
-	 * __vma_exit_exclusive_locked() completion which will decrement the
+	 * __vma_end_exclude_readers() 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 - A thread is either write-locking
-	 * an attached VMA and has yet to invoke __vma_exit_exclusive_locked(),
+	 * an attached VMA and has yet to invoke __vma_end_exclude_readers(),
 	 * 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.
--- a/mm/mmap_lock.c~mm-vma-introduce-helper-struct-thread-through-exclusive-lock-fns-fix
+++ a/mm/mmap_lock.c
@@ -46,7 +46,7 @@ EXPORT_SYMBOL(__mmap_lock_do_trace_relea
 #ifdef CONFIG_MMU
 #ifdef CONFIG_PER_VMA_LOCK
 
-/* State shared across __vma_[enter, exit]_exclusive_locked(). */
+/* State shared across __vma_[start, end]_exclude_readers. */
 struct vma_exclude_readers_state {
 	/* Input parameters. */
 	struct vm_area_struct *vma;
@@ -100,7 +100,7 @@ static unsigned int get_target_refcnt(st
  *
  * If ves->state is set to something other than TASK_UNINTERRUPTIBLE, the
  * function may also return -EINTR to indicate a fatal signal was received while
- * waiting.
+ * waiting.  Otherwise, the function returns 0.
  */
 static int __vma_start_exclude_readers(struct vma_exclude_readers_state *ves)
 {
_

Powered by blists - more mailing lists