From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 66F1EE7717F for ; Mon, 16 Dec 2024 14:50:56 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DCFE86B0083; Mon, 16 Dec 2024 09:50:55 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id D587C6B0085; Mon, 16 Dec 2024 09:50:55 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BD2FB6B0088; Mon, 16 Dec 2024 09:50:55 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 9B33E6B0083 for ; Mon, 16 Dec 2024 09:50:55 -0500 (EST) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 395C94530D for ; Mon, 16 Dec 2024 14:50:55 +0000 (UTC) X-FDA: 82901108136.07.3871661 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf19.hostedemail.com (Postfix) with ESMTP id 9D9501A0022 for ; Mon, 16 Dec 2024 14:50:22 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=XpN+neue; dmarc=pass (policy=quarantine) header.from=kernel.org; spf=pass (imf19.hostedemail.com: domain of a.hindborg@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=a.hindborg@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1734360640; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=6+ATMHxoHY3/8kkMRxBzJyl3sYt6J16WHbp5zBDO2a0=; b=eTG/G4jZ9lWn8kz5wMQHTZEGbQF1eCBnVOLnxDvFJ69+CxbZ83h+pwd2pIOI0+J8sW7vJh Ea0zrPf2lAM86TR9vJzuzzg82eCy33b83CuIbI6aR2H+AJIg+QBFez/9OYUfbgZRRcKr8n P2YtozscN4+Wq9XYh61Fq3/hKxBHQMw= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1734360640; a=rsa-sha256; cv=none; b=2QJCkeMwEfCJ+Nq3HfUUPrC6eQ103rewzNgrGUdI+D8ABrTExBY2ljda1iB9RMx+AyOF0A re8DN/jOjIQKIj1Fwxj2bl69YdwiU6whPQLJSaBxkDBw5FwBUNyK3EnOZQc3gsThj51+e3 TJD4xBPaeKEcU+803KtmJixwulYWJmM= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=XpN+neue; dmarc=pass (policy=quarantine) header.from=kernel.org; spf=pass (imf19.hostedemail.com: domain of a.hindborg@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=a.hindborg@kernel.org Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 1CE405C0180; Mon, 16 Dec 2024 14:50:10 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id F1F69C4CED0; Mon, 16 Dec 2024 14:50:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1734360652; bh=SpOyFr604TyCPZ97y4iKHmgf4xMBlupcNw2iCHFL5ww=; h=From:To:Cc:Subject:In-Reply-To:Date:References:From; b=XpN+neuex2xSDDR85zpJ3rX/3hBClc+5x4BAjtifdn2jaIifCPN0mp+49NLkw9Uq2 WXxiG/gjZgJr6S2RLIi4lVwZViLeaUsUldK4Z4H2zN8Bip9reiH9P5ZpYcQey2YHBI d34ORfm51N/mrKoLBc1ugCKXWY+kSY21Jk4AS6TGrGRsL2hwnb9LCcwm7DWOvkrqJd QwYK7/z3Kwe2Gu1tg7UgP2W7Ob7YxeXktZQU4rlTMbTSLa6n1QN8uvmIwJ3iw3wUK/ I4HxIGpGqZ9KIWUIO3FK+SjgvEf3HdCRbeZz1zNGyyINHFMTHWkvuQ5Fd45j1C2HaL TYBqbajFyzxSA== From: Andreas Hindborg To: "Alice Ryhl" Cc: "Miguel Ojeda" , "Matthew Wilcox" , "Lorenzo Stoakes" , "Vlastimil Babka" , "John Hubbard" , "Liam R. Howlett" , "Andrew Morton" , "Greg Kroah-Hartman" , "Arnd Bergmann" , "Christian Brauner" , "Jann Horn" , "Suren Baghdasaryan" , "Alex Gaynor" , "Boqun Feng" , "Gary Guo" , =?us-ascii?Q?=3D=3Fus-ascii=3FQ=3F=3D3D?= =?us-ascii?Q?=3D3Fus-ascii=3D3FQ=3D3F=3D3D3D=3F=3D_=3D=3Fus-ascii=3FQ=3F?= =?us-ascii?Q?=3D3D3Fus-ascii=3D3D3FQ=3D3D3F=3D3D3D3D=3D3F=3D3D=5F=3D3D=3D?= =?us-ascii?Q?3Fus-ascii=3D3FQ=3D3F=3F=3D_=3D=3Fus-ascii=3FQ=3F=3D3D3D3Fut?= =?us-ascii?Q?f-8=3D3D3D3FQ=3D3D3D3FBj=3D3D3D3DC3=3D3D3F=3D3D3D=3D5F=3D3D3?= =?us-ascii?Q?D=3D3D3=3F=3D_=3D=3Fus-ascii=3FQ=3FFus-ascii=3D3D3FQ=3D3D3F?= =?us-ascii?Q?=3D3F=3D3D=5F=3D3D=3D3Fus-ascii=3D3FQ=3D3F=3D3D3D3DB6rn=3F?= =?us-ascii?Q?=3D_=3D=3Fus-ascii=3FQ=3F=3D3D3D3F=3D3D3D3D=3D3D3F=3D3D3D=3D?= =?us-ascii?Q?3F=3D3D=3F=3D?= Roy Baron , "Benno Lossin" , "Trevor Gross" , , , Subject: Re: [PATCH v11 1/8] mm: rust: add abstraction for struct mm_struct In-Reply-To: <20241211-vma-v11-1-466640428fc3@google.com> (Alice Ryhl's message of "Wed, 11 Dec 2024 10:37:05 +0000") Date: Mon, 16 Dec 2024 12:31:55 +0100 Message-ID: <878qsfdftg.fsf@kernel.org> References: <20241211-vma-v11-0-466640428fc3@google.com> <20241211-vma-v11-1-466640428fc3@google.com> MIME-Version: 1.0 Content-Type: text/plain X-Stat-Signature: 4pqc4kzbeynn8p1c5zgbxsm8xfdzqsrj X-Rspamd-Queue-Id: 9D9501A0022 X-Rspam-User: X-Rspamd-Server: rspam01 X-HE-Tag: 1734360622-386759 X-HE-Meta: U2FsdGVkX18aFBgYB5h2qbnWHFPJpITPYKJJbxqvDLS6S5Cb6y5nncDrRqHT5lnu4RLrV2tuxQneKb6+JmE5S/a5x+zlRVacdxBcmimW+4uMnImfOIBLezgluYpjMs6hV5MI0MIOg3FftUmMcfYcf70NL+LCbA2HJHJImZNsxbDouRXP48rryQNlx4t/Cv4abbBQg0jcmjnM5WVkygNTxolHVFeFOXsY1RCKXlR/zYabtlwM/94naLRHnfQF3l2gIUaJiDoOsRSUleCs4ko5oxOshj3dethXGgXJSBrR0Y7k+A0yI+P5H/6Ubt8cqSiqGlcOQehLP3/eTj9QXtVttBqNxSUAFQ34mI/Ow2WhVEyuKbT7ecja9y2ewO+fczRjOJDvRfnjD5B95xEF7xJSaFOzBH1GCAVDs63PKj7SVEld1d7/40HREgHFWR/YPDZpPtLtaHcRqA1XfGPBpGbCPBFDi5AIeg+gLqFiu6DpCjS4s6CIJ1la5NaMshyR5FRwuObN9OZKMV7jCncs/op/xgoiz4eLc5+OuZqKXdvRNaFh+mo/0tYUHFgMDd3zX3dyhhX7zBN+ZiSrlwMQwIXFUyDNbsyackUCty0e0S2PjbW2bh4lL6+LbZkk2NUI+LOio6DQiVfwKjW9ssaMjPwnGJBkFkvcRd7ecb/ZEN6jHjYVHqqClr1OLnwkiJN63LkB7cnldsSXVhhOTShugxSGN/rQC1TLkQNxFviUhf5vHv+7MpRUA4FbBigiU3SJuEA92SL5/DGx9aeU4cLpp7KG9abT7pgOlv5QayvGU39XHP0W0hKlfEyBYxHOlnwSYcfVluG3IXk+O3kKVhYYhZckr4oORhVcGWA/d2AYLjYRRwGl4PnIgDu2SeP6zHfHnbb6i3SDQN6l9shnmvqF65NuAnMlfUz6bCF1jZvcEu27AD4kjj1PFGmn+xe7uaTIT1m4GE4wD9LCIWVMvDTfAkV T3ughEAh cWMT6M560N3TI1cO665JPc8XVlchxPg3yMPxiwVbrAV5+6FppBIy132mUP/NTL8FcSzzW3yp+mOXvJNbRpyykVp1uKQ40Tnimo3hM7JoEcvNxr04EjJoymUEWxoed044+VJYsEgF2YCst0N7BsyXdNGmhqjcCgKGQayxlHMvAZO0Kw9oCB/A2YhkFv8yJML8z3NRYTs31xT/mDrk+QGINOuLPU6RtpRJ8Yhz9Sb8APdS6GFZRFYSwxKCgXoQ1xXfxn3kABAk8QygoYNTeRh2cOE9O2xt6H6RzzQ5Oi1nS7GfHyTKwDAherhq/a7dohb1TKnYkms4gnl8giSmKZle8rYjwg8hMzP5kxy5Dhqxe9obaXk1ZgzcvNOivHbi7dF1usUvXWYGXB1mPRJqcDfKM9SKJOw== X-Bogosity: Ham, tests=bogofilter, spamicity=0.119064, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: "Alice Ryhl" 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 (for mm bits) > Signed-off-by: Alice Ryhl > --- > 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? > +//! > +//! 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? > +/// > +/// 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` 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. > + mm: Opaque, > +} > + > +// SAFETY: It is safe to call `mmdrop` on another thread than where `mmgrab` was called. > +unsafe impl Send for Mm {} > +// SAFETY: All methods on `Mm` can be called in parallel from several threads. > +unsafe impl Sync for Mm {} > + > +// SAFETY: By the type invariants, this type is always refcounted. > +unsafe impl AlwaysRefCounted for Mm { > + #[inline] > + fn inc_ref(&self) { > + // SAFETY: The pointer is valid since self is a reference. > + unsafe { bindings::mmgrab(self.as_raw()) }; > + } > + > + #[inline] > + unsafe fn dec_ref(obj: NonNull) { > + // SAFETY: The caller is giving up their refcount. > + unsafe { bindings::mmdrop(obj.cast().as_ptr()) }; > + } > +} > + > +/// A wrapper for the kernel's `struct mm_struct`. > +/// > +/// This type is like [`Mm`], but with non-zero `mm_users`. It can only be used when `mm_users` can > +/// be proven to be non-zero at compile-time, usually because the relevant code holds an `mmget` > +/// refcount. It can be used to access the associated address space. > +/// > +/// The `ARef` smart pointer holds an `mmget` refcount. Its destructor may sleep. > +/// > +/// # Invariants > +/// > +/// Values of this type are always refcounted using `mmget`. The value of `mm_users` is non-zero. > +#[repr(transparent)] > +pub struct MmWithUser { > + mm: Mm, > +} > + > +// SAFETY: It is safe to call `mmput` on another thread than where `mmget` was called. > +unsafe impl Send for MmWithUser {} > +// SAFETY: All methods on `MmWithUser` can be called in parallel from several threads. > +unsafe impl Sync for MmWithUser {} > + > +// SAFETY: By the type invariants, this type is always refcounted. > +unsafe impl AlwaysRefCounted for MmWithUser { > + #[inline] > + fn inc_ref(&self) { > + // SAFETY: The pointer is valid since self is a reference. > + unsafe { bindings::mmget(self.as_raw()) }; > + } > + > + #[inline] > + unsafe fn dec_ref(obj: NonNull) { > + // SAFETY: The caller is giving up their refcount. > + unsafe { bindings::mmput(obj.cast().as_ptr()) }; > + } > +} > + > +// Make all `Mm` methods available on `MmWithUser`. > +impl Deref for MmWithUser { > + type Target = Mm; > + > + #[inline] > + fn deref(&self) -> &Mm { > + &self.mm > + } > +} > + > +// These methods are safe to call even if `mm_users` is zero. > +impl Mm { > + /// Call `mmgrab` on `current.mm`. > + #[inline] > + pub fn mmgrab_current() -> Option> { > + // SAFETY: It's safe to get the `mm` field from current. > + let mm = unsafe { > + let current = bindings::get_current(); > + (*current).mm > + }; > + > + if mm.is_null() { > + return None; > + } > + > + // SAFETY: The value of `current->mm` is guaranteed to be null or a valid `mm_struct`. We > + // just checked that it's not null. Furthermore, the returned `&Mm` is valid only for the > + // duration of this function, and `current->mm` will stay valid for that long. > + let mm = unsafe { Mm::from_raw(mm) }; > + > + // This increments the refcount using `mmgrab`. > + Some(ARef::from(mm)) > + } > + > + /// Returns a raw pointer to the inner `mm_struct`. > + #[inline] > + pub fn as_raw(&self) -> *mut bindings::mm_struct { > + self.mm.get() > + } > + > + /// Obtain a reference from a raw pointer. > + /// > + /// # Safety > + /// > + /// The caller must ensure that `ptr` points at an `mm_struct`, and that it is not deallocated > + /// during the lifetime 'a. > + #[inline] > + pub unsafe fn from_raw<'a>(ptr: *const bindings::mm_struct) -> &'a Mm { > + // SAFETY: Caller promises that the pointer is valid for 'a. Layouts are compatible due to > + // repr(transparent). > + unsafe { &*ptr.cast() } > + } > + > + /// Calls `mmget_not_zero` and returns a handle if it succeeds. > + #[inline] > + pub fn mmget_not_zero(&self) -> Option> { > + // SAFETY: The pointer is valid since self is a reference. > + let success = unsafe { bindings::mmget_not_zero(self.as_raw()) }; > + > + if success { > + // SAFETY: We just created an `mmget` refcount. > + Some(unsafe { ARef::from_raw(NonNull::new_unchecked(self.as_raw().cast())) }) > + } else { > + None > + } > + } > +} Nit: could we put the impl next to the struct definition? Best regards, Andreas Hindborg