Re: [PATCH v11 1/8] mm: rust: add abstraction for struct mm_struct

[Alice Ryhl <aliceryhl@google.com>]

On Mon, Dec 16, 2024 at 3:50 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> "Alice Ryhl" <aliceryhl@google.com> writes:
>
> > These abstractions allow you to reference a `struct mm_struct` using
> > both mmgrab and mmget refcounts. This is done using two Rust types:
> >
> > * Mm - represents an mm_struct where you don't know anything about the
> >   value of mm_users.
> > * MmWithUser - represents an mm_struct where you know at compile time
> >   that mm_users is non-zero.
> >
> > This allows us to encode in the type system whether a method requires
> > that mm_users is non-zero or not. For instance, you can always call
> > `mmget_not_zero` but you can only call `mmap_read_lock` when mm_users is
> > non-zero.
> >
> > It's possible to access current->mm without a refcount increment, but
> > that is added in a later patch of this series.
> >
> > Acked-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com> (for mm bits)
> > Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> > ---
> >  rust/helpers/helpers.c |   1 +
> >  rust/helpers/mm.c      |  39 +++++++++
> >  rust/kernel/lib.rs     |   1 +
> >  rust/kernel/mm.rs      | 219 +++++++++++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 260 insertions(+)
> >
> > diff --git a/rust/kernel/mm.rs b/rust/kernel/mm.rs
> > new file mode 100644
> > index 000000000000..84cba581edaa
> > --- /dev/null
> > +++ b/rust/kernel/mm.rs
> > @@ -0,0 +1,219 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +// Copyright (C) 2024 Google LLC.
> > +
> > +//! Memory management.
>
> Could you add a little more context here?

How about this?

//! Memory management.
//!
//! This module deals with managing the address space of userspace
processes. Each process has an
//! instance of [`Mm`], which keeps track of multiple VMAs (virtual
memory areas). Each VMA
//! corresponds to a region of memory that the userspace process can
access, and the VMA lets you
//! control what happens when userspace reads or writes to that region
of memory.
//!
//! C header: [`include/linux/mm.h`](srctree/include/linux/mm.h)

> > +//!
> > +//! C header: [`include/linux/mm.h`](srctree/include/linux/mm.h)
> > +
> > +use crate::{
> > +    bindings,
> > +    types::{ARef, AlwaysRefCounted, NotThreadSafe, Opaque},
> > +};
> > +use core::{ops::Deref, ptr::NonNull};
> > +
> > +/// A wrapper for the kernel's `struct mm_struct`.
>
> Could you elaborate the data structure use case? When do I need it, what
> does it do?

How about this?

/// A wrapper for the kernel's `struct mm_struct`.
///
/// This represents the address space of a userspace process, so each
process has one `Mm`
/// instance. It may hold many VMAs internally.
///
/// There is a counter called `mm_users` that counts the users of the
address space; this includes
/// the userspace process itself, but can also include kernel threads
accessing the address space.
/// Once `mm_users` reaches zero, this indicates that the address
space can be destroyed. To access
/// the address space, you must prevent `mm_users` from reaching zero
while you are accessing it.
/// The [`MmWithUser`] type represents an address space where this is
guaranteed, and you can
/// create one using [`mmget_not_zero`].
///
/// The `ARef<Mm>` smart pointer holds an `mmgrab` refcount. Its
destructor may sleep.

> > +///
> > +/// Since `mm_users` may be zero, the associated address space may not exist anymore. You can use
> > +/// [`mmget_not_zero`] to be able to access the address space.
> > +///
> > +/// The `ARef<Mm>` smart pointer holds an `mmgrab` refcount. Its destructor may sleep.
> > +///
> > +/// # Invariants
> > +///
> > +/// Values of this type are always refcounted using `mmgrab`.
> > +///
> > +/// [`mmget_not_zero`]: Mm::mmget_not_zero
> > +#[repr(transparent)]
> > +pub struct Mm {
>
> Could we come up with a better name? `MemoryMap` or `MemoryMapping`?. You
> use `MMapReadGuard` later.

Those names seem really confusing to me. The mmap syscall creates a
new VMA, but MemoryMap sounds like it's the thing that mmap creates.

Lorenzo, what do you think? I'm inclined to just call it Mm since
that's what C calls it.

Alice