| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAJuCfpGK63E-EtX4V7+Mgf9-KdBfvb27kzyZH6pYoYuzUQPnxQ@mail.gmail.com>
Date: Sun, 25 Jan 2026 21:47:38 -0800
From: Suren Baghdasaryan <surenb@...gle.com>
To: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>, David Hildenbrand <david@...nel.org>,
"Liam R . Howlett" <Liam.Howlett@...cle.com>, Vlastimil Babka <vbabka@...e.cz>,
Mike Rapoport <rppt@...nel.org>, 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 06/10] mm/vma: clean up __vma_enter/exit_locked()
On Fri, Jan 23, 2026 at 12:12 PM Lorenzo Stoakes
<lorenzo.stoakes@...cle.com> wrote:
>
> These functions are very confusing indeed. 'Entering' a lock could be
> interpreted as acquiring it, but this is not what these functions are
> interacting with.
>
> Equally they don't indicate at all what kind of lock we are 'entering' or
> 'exiting'. Finally they are misleading as we invoke these functions when we
> already hold a write lock to detach a VMA.
>
> These functions are explicitly simply 'entering' and 'exiting' a state in
> which we hold the EXCLUSIVE lock in order that we can either mark the VMA
> as being write-locked, or mark the VMA detached.
>
> Rename the functions accordingly, and also update
> __vma_end_exclude_readers() to return detached state with a __must_check
> directive, as it is simply clumsy to pass an output pointer here to
> detached state and inconsistent vs. __vma_start_exclude_readers().
>
> Finally, remove the unnecessary 'inline' directives.
>
> No functional change intended.
>
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
Reviewed-by: Suren Baghdasaryan <surenb@...gle.com>
> ---
> include/linux/mmap_lock.h | 4 +--
> mm/mmap_lock.c | 58 +++++++++++++++++++++++++++------------
> 2 files changed, 42 insertions(+), 20 deletions(-)
>
> diff --git a/include/linux/mmap_lock.h b/include/linux/mmap_lock.h
> index 1887ca55ead7..d6df6aad3e24 100644
> --- a/include/linux/mmap_lock.h
> +++ b/include/linux/mmap_lock.h
> @@ -211,8 +211,8 @@ static inline void vma_refcount_put(struct vm_area_struct *vma)
> newcnt = __vma_refcount_put_return(vma);
>
> /*
> - * __vma_enter_locked() may be sleeping waiting for readers to drop
> - * their reference count, so wake it up if we were the last reader
> + * __vma_start_exclude_readers() may be sleeping waiting for readers to
> + * drop their reference count, so wake it up if we were the last reader
> * blocking it from being acquired.
> *
> * We may be raced by other readers temporarily incrementing the
> diff --git a/mm/mmap_lock.c b/mm/mmap_lock.c
> index 1fabda07c922..72f15f606093 100644
> --- a/mm/mmap_lock.c
> +++ b/mm/mmap_lock.c
> @@ -46,19 +46,44 @@ EXPORT_SYMBOL(__mmap_lock_do_trace_released);
> #ifdef CONFIG_MMU
> #ifdef CONFIG_PER_VMA_LOCK
>
> -static inline void __vma_exit_locked(struct vm_area_struct *vma, bool *detached)
> +/*
> + * Now that all readers have been evicted, mark the VMA as being out of the
> + * 'exclude readers' state.
> + *
> + * Returns true if the VMA is now detached, otherwise false.
> + */
> +static bool __must_check __vma_end_exclude_readers(struct vm_area_struct *vma)
> {
> - *detached = refcount_sub_and_test(VM_REFCNT_EXCLUDE_READERS_FLAG,
> - &vma->vm_refcnt);
> + bool detached;
> +
> + detached = refcount_sub_and_test(VM_REFCNT_EXCLUDE_READERS_FLAG,
> + &vma->vm_refcnt);
> __vma_lockdep_release_exclusive(vma);
> + return detached;
> }
>
> /*
> - * __vma_enter_locked() returns 0 immediately if the vma is not
> - * attached, otherwise it waits for any current readers to finish and
> - * returns 1. Returns -EINTR if a signal is received while waiting.
> + * Mark the VMA as being in a state of excluding readers, check to see if any
> + * VMA read locks are indeed held, and if so wait for them to be released.
> + *
> + * Note that this function pairs with vma_refcount_put() which will wake up this
> + * thread when it detects that the last reader has released its lock.
> + *
> + * The state parameter ought to be set to TASK_UNINTERRUPTIBLE in cases where we
> + * wish the thread to sleep uninterruptibly or TASK_KILLABLE if a fatal signal
> + * is permitted to kill it.
> + *
> + * The function will return 0 immediately if the VMA is detached, or wait for
> + * readers and return 1 once they have all exited, leaving the VMA exclusively
> + * locked.
> + *
> + * If the function returns 1, the caller is required to invoke
> + * __vma_end_exclude_readers() once the exclusive state is no longer required.
> + *
> + * If state is set to something other than TASK_UNINTERRUPTIBLE, the function
> + * may also return -EINTR to indicate a fatal signal was received while waiting.
> */
> -static inline int __vma_enter_locked(struct vm_area_struct *vma,
> +static int __vma_start_exclude_readers(struct vm_area_struct *vma,
> bool detaching, int state)
> {
> int err;
> @@ -85,13 +110,10 @@ static inline int __vma_enter_locked(struct vm_area_struct *vma,
> refcount_read(&vma->vm_refcnt) == tgt_refcnt,
> state);
> if (err) {
> - bool detached;
> -
> - __vma_exit_locked(vma, &detached);
> - if (detached) {
> + if (__vma_end_exclude_readers(vma)) {
> /*
> * The wait failed, but the last reader went away
> - * as well. Tell the caller the VMA is detached.
> + * as well. Tell the caller the VMA is detached.
> */
> WARN_ON_ONCE(!detaching);
> err = 0;
> @@ -108,7 +130,7 @@ int __vma_start_write(struct vm_area_struct *vma, unsigned int mm_lock_seq,
> {
> int locked;
>
> - locked = __vma_enter_locked(vma, false, state);
> + locked = __vma_start_exclude_readers(vma, false, state);
> if (locked < 0)
> return locked;
>
> @@ -121,10 +143,10 @@ int __vma_start_write(struct vm_area_struct *vma, unsigned int mm_lock_seq,
> WRITE_ONCE(vma->vm_lock_seq, mm_lock_seq);
>
> if (locked) {
> - bool detached;
> + bool detached = __vma_end_exclude_readers(vma);
>
> - __vma_exit_locked(vma, &detached);
> - WARN_ON_ONCE(detached); /* vma should remain attached */
> + /* The VMA should remain attached. */
> + WARN_ON_ONCE(detached);
> }
>
> return 0;
> @@ -148,14 +170,14 @@ void vma_mark_detached(struct vm_area_struct *vma)
> */
> if (unlikely(__vma_refcount_put_return(vma))) {
> /* Wait until vma is detached with no readers. */
> - if (__vma_enter_locked(vma, true, TASK_UNINTERRUPTIBLE)) {
> + if (__vma_start_exclude_readers(vma, true, TASK_UNINTERRUPTIBLE)) {
> bool detached;
>
> /*
> * Once this is complete, no readers can increment the
> * reference count, and the VMA is marked detached.
> */
> - __vma_exit_locked(vma, &detached);
> + detached = __vma_end_exclude_readers(vma);
> WARN_ON_ONCE(!detached);
> }
> }
> --
> 2.52.0
>
Powered by blists - more mailing lists