linux-kernel - Re: [PATCH v11 4/8] mm: rust: add lock_vma_under

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: <CAH5fLghYSL7TWUKa+Nj5_wyBBJpfo=0AStHSbC=aDD9Hj6VEfQ@mail.gmail.com>
Date: Mon, 13 Jan 2025 11:04:36 +0100
From: Alice Ryhl <aliceryhl@...gle.com>
To: Andreas Hindborg <a.hindborg@...nel.org>
Cc: Miguel Ojeda <ojeda@...nel.org>, Matthew Wilcox <willy@...radead.org>, 
	Lorenzo Stoakes <lorenzo.stoakes@...cle.com>, Vlastimil Babka <vbabka@...e.cz>, 
	John Hubbard <jhubbard@...dia.com>, "Liam R. Howlett" <Liam.Howlett@...cle.com>, 
	Andrew Morton <akpm@...ux-foundation.org>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, 
	Arnd Bergmann <arnd@...db.de>, Christian Brauner <brauner@...nel.org>, Jann Horn <jannh@...gle.com>, 
	Suren Baghdasaryan <surenb@...gle.com>, Alex Gaynor <alex.gaynor@...il.com>, 
	Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>, 
	Björn Roy Baron <bjorn3_gh@...tonmail.com>, 
	Benno Lossin <benno.lossin@...ton.me>, Trevor Gross <tmgross@...ch.edu>, linux-kernel@...r.kernel.org, 
	linux-mm@...ck.org, rust-for-linux@...r.kernel.org
Subject: Re: [PATCH v11 4/8] mm: rust: add lock_vma_under_rcu

On Mon, Dec 16, 2024 at 3:50 PM Andreas Hindborg <a.hindborg@...nel.org> wrote:
>
> "Alice Ryhl" <aliceryhl@...gle.com> writes:
>
> > Currently, the binder driver always uses the mmap lock to make changes
> > to its vma. Because the mmap lock is global to the process, this can
> > involve significant contention. However, the kernel has a feature called
> > per-vma locks, which can significantly reduce contention. For example,
> > you can take a vma lock in parallel with an mmap write lock. This is
> > important because contention on the mmap lock has been a long-term
> > recurring challenge for the Binder driver.
> >
> > This patch introduces support for using `lock_vma_under_rcu` from Rust.
> > The Rust Binder driver will be able to use this to reduce contention on
> > the mmap lock.
> >
> > Acked-by: Lorenzo Stoakes <lorenzo.stoakes@...cle.com> (for mm bits)
> > Reviewed-by: Jann Horn <jannh@...gle.com>
> > Signed-off-by: Alice Ryhl <aliceryhl@...gle.com>
> > ---
> >  rust/helpers/mm.c |  5 +++++
> >  rust/kernel/mm.rs | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 61 insertions(+)
> >
> > diff --git a/rust/helpers/mm.c b/rust/helpers/mm.c
> > index 7b72eb065a3e..81b510c96fd2 100644
> > --- a/rust/helpers/mm.c
> > +++ b/rust/helpers/mm.c
> > @@ -43,3 +43,8 @@ struct vm_area_struct *rust_helper_vma_lookup(struct mm_struct *mm,
> >  {
> >       return vma_lookup(mm, addr);
> >  }
> > +
> > +void rust_helper_vma_end_read(struct vm_area_struct *vma)
> > +{
> > +     vma_end_read(vma);
> > +}
> > diff --git a/rust/kernel/mm.rs b/rust/kernel/mm.rs
> > index ace8e7d57afe..425b73a9dfe6 100644
> > --- a/rust/kernel/mm.rs
> > +++ b/rust/kernel/mm.rs
> > @@ -13,6 +13,7 @@
> >  use core::{ops::Deref, ptr::NonNull};
> >
> >  pub mod virt;
> > +use virt::VmAreaRef;
> >
> >  /// A wrapper for the kernel's `struct mm_struct`.
> >  ///
> > @@ -170,6 +171,32 @@ pub unsafe fn from_raw<'a>(ptr: *const bindings::mm_struct) -> &'a MmWithUser {
> >          unsafe { &*ptr.cast() }
> >      }
> >
> > +    /// Attempt to access a vma using the vma read lock.
> > +    ///
> > +    /// This is an optimistic trylock operation, so it may fail if there is contention. In that
> > +    /// case, you should fall back to taking the mmap read lock.
> > +    ///
> > +    /// When per-vma locks are disabled, this always returns `None`.
> > +    #[inline]
> > +    pub fn lock_vma_under_rcu(&self, vma_addr: usize) -> Option<VmaReadGuard<'_>> {
> > +        #[cfg(CONFIG_PER_VMA_LOCK)]
> > +        {
> > +            // SAFETY: Calling `bindings::lock_vma_under_rcu` is always okay given an mm where
> > +            // `mm_users` is non-zero.
> > +            let vma = unsafe { bindings::lock_vma_under_rcu(self.as_raw(), vma_addr as _) };
>
> Is `as _` the right approach here?

We can drop it once the FFI integer types are fixed. It's late in the
cycle, so this patch probably won't make it for 6.14. This means I can
remove the casts entirely before this is merged. Otherwise we can
remove them in a follow-up.

Alice