linux-kernel - Re: [PATCH RESEND v3 08/10] mm/vma: improve and document __is_vma_write

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJuCfpEo9XFFzGL3c-Udy+0GDM12xOtDeFxu2J4L7F-kQ7xohg@mail.gmail.com>
Date: Fri, 23 Jan 2026 09:42:43 -0800
From: Suren Baghdasaryan <surenb@...gle.com>
To: Vlastimil Babka <vbabka@...e.cz>
Cc: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>, Andrew Morton <akpm@...ux-foundation.org>, 
	David Hildenbrand <david@...nel.org>, "Liam R . Howlett" <Liam.Howlett@...cle.com>, 
	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 RESEND v3 08/10] mm/vma: improve and document __is_vma_write_locked()

On Fri, Jan 23, 2026 at 8:21 AM Vlastimil Babka <vbabka@...e.cz> wrote:
>
> On 1/22/26 22:55, Suren Baghdasaryan wrote:
> > On Thu, Jan 22, 2026 at 5:02 AM Lorenzo Stoakes
> > <lorenzo.stoakes@...cle.com> wrote:
> >>
> >> The function is a little confusing, clean it up a little then add a
> >> descriptive comment.
> >
> > I appreciate the descriptive comment but what exactly was confusing in
> > this function?
> >
> >>
> >> No functional change intended.
> >>
> >> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
> >> ---
> >>  include/linux/mmap_lock.h | 23 ++++++++++++++++++-----
> >>  1 file changed, 18 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/include/linux/mmap_lock.h b/include/linux/mmap_lock.h
> >> index 873bc5f3c97c..b00d34b5ad10 100644
> >> --- a/include/linux/mmap_lock.h
> >> +++ b/include/linux/mmap_lock.h
> >> @@ -252,17 +252,30 @@ static inline void vma_end_read(struct vm_area_struct *vma)
> >>         vma_refcount_put(vma);
> >>  }
> >>
> >> -/* WARNING! Can only be used if mmap_lock is expected to be write-locked */
> >> -static inline bool __is_vma_write_locked(struct vm_area_struct *vma, unsigned int *mm_lock_seq)
> >> +/*
> >> + * Determine whether a VMA is write-locked. Must be invoked ONLY if the mmap
> >> + * write lock is held.
> >> + *
> >> + * Returns true if write-locked, otherwise false.
> >> + *
> >> + * Note that mm_lock_seq is updated only if the VMA is NOT write-locked.
>
> Could it also say to what it's updated to? Or is it too obvious?
>
> >
> > True, this does not result in a functional change because we do not
> > use mm_lock_seq if __is_vma_write_locked() succeeds. However this
> > seems to add additional gotcha that you need to remember. Any reason
> > why?
>
> Actually I wonder if it's really worth returning the mm_lock_seq and passing
> it to __vma_start_write(), which could just determine it on its own. It
> would simplify things.

That looks fine to me and indeed would simplify things... Yes, please!

>
> >> + */
> >> +static inline bool __is_vma_write_locked(struct vm_area_struct *vma,
> >> +                                        unsigned int *mm_lock_seq)
> >>  {
> >> -       mmap_assert_write_locked(vma->vm_mm);
> >> +       struct mm_struct *mm = vma->vm_mm;
> >> +       const unsigned int seq = mm->mm_lock_seq.sequence;
> >> +
> >> +       mmap_assert_write_locked(mm);
> >>
> >>         /*
> >>          * current task is holding mmap_write_lock, both vma->vm_lock_seq and
> >>          * mm->mm_lock_seq can't be concurrently modified.
> >>          */
> >> -       *mm_lock_seq = vma->vm_mm->mm_lock_seq.sequence;
> >> -       return (vma->vm_lock_seq == *mm_lock_seq);
> >> +       if (vma->vm_lock_seq == seq)
> >> +               return true;
> >> +       *mm_lock_seq = seq;
> >> +       return false;
> >>  }
> >>
> >>  int __vma_start_write(struct vm_area_struct *vma, unsigned int mm_lock_seq,
> >> --
> >> 2.52.0
>