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 4E73DC3DA4A for ; Thu, 1 Aug 2024 14:02:11 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B1BB66B008A; Thu, 1 Aug 2024 10:02:10 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id AC9AF6B0092; Thu, 1 Aug 2024 10:02:10 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9B8B36B0093; Thu, 1 Aug 2024 10:02:10 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 7ECF86B008A for ; Thu, 1 Aug 2024 10:02:10 -0400 (EDT) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 176AA120E4C for ; Thu, 1 Aug 2024 14:02:10 +0000 (UTC) X-FDA: 82403840820.12.B884104 Received: from mail-4316.protonmail.ch (mail-4316.protonmail.ch [185.70.43.16]) by imf10.hostedemail.com (Postfix) with ESMTP id 1CEBAC0025 for ; Thu, 1 Aug 2024 14:02:06 +0000 (UTC) Authentication-Results: imf10.hostedemail.com; dkim=pass header.d=proton.me header.s=protonmail header.b=B6EY1Z51; dmarc=pass (policy=quarantine) header.from=proton.me; spf=pass (imf10.hostedemail.com: domain of benno.lossin@proton.me designates 185.70.43.16 as permitted sender) smtp.mailfrom=benno.lossin@proton.me ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1722520922; a=rsa-sha256; cv=none; b=nqvlOJuElam8AGvJelpgm1M/fqR14XKOArzqon1RMWbJcmC1T5kBaENcicUurCynha7D6k pZBy2+gC+VXhQmMdQ5HoinnLh3UZP+nXkMSO4QgOvx+U8vSki+etsj+s8PEfvRtsfRz3x0 9py8KgwzICsqc5QsgmQEDvKrsOMNkP0= ARC-Authentication-Results: i=1; imf10.hostedemail.com; dkim=pass header.d=proton.me header.s=protonmail header.b=B6EY1Z51; dmarc=pass (policy=quarantine) header.from=proton.me; spf=pass (imf10.hostedemail.com: domain of benno.lossin@proton.me designates 185.70.43.16 as permitted sender) smtp.mailfrom=benno.lossin@proton.me ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1722520922; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=sr4WeHf3fgnQqbWXVbnaeKVIjMkwrY2mcG/IiWbhHvk=; b=d6FX83+2ggny+fDGndv+AKwVAok9VqrzskQ975oqdmJihA2zVt0kulXP8N1MPpVZ5/Ip0c a4RV4YMW1vuHoGVsN1nk2/utigJ9K21fHdNaw62ZeFYiXO/Mkv8UrE7bV/yPuV8wNJdJxQ GhTwzCG5RaKWbUE0nDTS3S3QKn+OLYk= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=proton.me; s=protonmail; t=1722520924; x=1722780124; bh=sr4WeHf3fgnQqbWXVbnaeKVIjMkwrY2mcG/IiWbhHvk=; h=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References: Feedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID: Message-ID:BIMI-Selector; b=B6EY1Z514XkP1xCh7HFWOPDR6AIbBoD9RS9Tosoy3rr0QxTWaQd9s79qogyE1Omtn h4RLrLYnTyFKrN+pV3rVd43Xg9n1HrxvUGNO4Gd8EyMiEg++nQ+bsl8b24BT39bnlO 8HvhnsiQrtyyXssh2ZxujaSZuRphpOUhrBH/FgrA8inQvO4c0SAyNKmvDPF9GnWz6X VLTcRrdkvkwMSDYfkiA8O8TDmywa3oRIxZX47ULJd2mf++V3wAh11zCFrVpmf6cbzk 0eSK9z5pzpZCpqOcMsngcBQgZRaXCXMdzK//KNhmPhIUO6DRQWNpYX4GMWnaleUuV0 CGYH553uw2jOw== Date: Thu, 01 Aug 2024 14:01:45 +0000 To: Alice Ryhl , Miguel Ojeda , Andrew Morton From: Benno Lossin Cc: Alex Gaynor , Wedson Almeida Filho , Boqun Feng , Gary Guo , =?utf-8?Q?Bj=C3=B6rn_Roy_Baron?= , Andreas Hindborg , Matthew Wilcox , "Liam R. Howlett" , Vlastimil Babka , Lorenzo Stoakes , linux-kernel@vger.kernel.org, linux-mm@kvack.org, rust-for-linux@vger.kernel.org Subject: Re: [PATCH v3] rust: mm: add abstractions for mm_struct and vm_area_struct Message-ID: <82e4816c-cada-46f3-bebf-882ae8ded118@proton.me> In-Reply-To: <20240801-vma-v3-1-db6c1c0afda9@google.com> References: <20240801-vma-v3-1-db6c1c0afda9@google.com> Feedback-ID: 71780778:user:proton X-Pm-Message-ID: 26750703ee0fd483967e00faa06470dcd6368aaa MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Queue-Id: 1CEBAC0025 X-Rspamd-Server: rspam01 X-Stat-Signature: 1xo3spzme6ewigoa1jz6fnd73j73t7uj X-HE-Tag: 1722520926-6037 X-HE-Meta: U2FsdGVkX1+dyTt8kvNdEbWPyyel3/dtS5NGszUartIzPd5jT5o1UkpkzUVD4TZxxDjalaCXB3O6S1xNKVi7ueDsO6NIBmU4J2AqtQFzi3lzESnYYeX2UKHHTTq2ywoYNtkFVUeJQIYFxdulpXSDEdxmgfPoen9ygicCBwm+LoVrTOo+16vBccuCfhC6TOpMcRexNbzxSw0/xMyFj04Vwxedna82Acc6LrqCAkbZzbEZtCOOGXDrIVgx/IMwJVubR10F95+pth+Zepeehv8VXfOiLI//pw0wHioeWQOxo043Nh7kRvONHsESvCR6sgXuKYqDHE4F9P57wtR4YmpkdZ1k29ZUkg1dkrIkWQSbI1Qb0H6Wx2EAgo9nuJPDT6CTUxe1AC36dYWg4frPBh5sjUpMcnazU7eFUV2OqQ+v6G4ycY/Rd5YSPxZB/LswANYF41PvbYF9WjGYpa3XB16ZSvrulW2NjRIeHXdIlukzEaTcYfSGIY/MzwfLYV1y5XIzD2eORpKgkB+F0+JDTk5WIsg/Obgw60NDdqm/P7UP4piIFMFWIRqoQrALIn8VxTa/ijKCejQ6zt3/Kx4mJXZR0ZBTcYRCBz6de/bIsgFNMVayjDypfu/S+7FgQkeTT3X6gdzRjruNWBe2/enbpscuGPWzbVF5TXxzvqy2267AqZmJJe4GrIhGi1nDJK1teeqRM2jjNVSp9cxxrno0s9BQw/6S462WEfcqtjD8qNW/wFNSb35UUWavHXMNV2ieAWGSjgqmtCVqxR9A0+4LtFHnRoi6/rIrlVms6mE21kwfTn6FR9zwwpD7kXkKAB71D6KavvviZ0bXo7hq7sPohAIuxPZPCJQmTwegcp+Ke3qEiL1FL0NI0Gyin8cn3PvZ7xUzKQC+vdfeEPvB/XQWXIo8+1V4zSyuLMCzWjG1/8JMi3BPexhiJnDdh/2Bu+GX3rQPI2ZO9DOBijrfPDIrexM TVXaX+LL W7nQ9ASr0QzjhMBEYhleXKabOnH+OvbIoBGTA+gDr2NnFwXTi2GM9EDqEH3T3n6se37cje4vwW5XlTqjQHTpzX3gSmnQcH2/BltAN+XYEMWmNY8aBsLjg+7DI6uCsfGkwkOMT0UiKoi23tp03mHuL7I9f9gPcx6HryVq+HyAB87C9VApJti4wvggv4fxzg7rENCe3RXcVb/f7quM/tVldplVyYZKEELVAcMImNPf2lfjszQC/HTlg2g+t6c/NPvqLHFj46wGT2WS3M6RSKVFlS4oHUNmhW6JsSlrlAfJiyyrLSpJ6nJkbCYXwDF1IPrAuYLrXIgTyaIFQU+x5HnggJhQHxDYzUmDastUcFCFcKW0gWjmJEuPYhASfWXs86J9b/8AzdCbEQZGtf+xM3j9ndu3cyw== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On 01.08.24 14:58, Alice Ryhl wrote: > diff --git a/rust/kernel/mm.rs b/rust/kernel/mm.rs > new file mode 100644 > index 000000000000..ed2db893fb79 > --- /dev/null > +++ b/rust/kernel/mm.rs > @@ -0,0 +1,337 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +// Copyright (C) 2024 Google LLC. > + > +//! Memory management. > +//! > +//! C header: [`include/linux/mm.h`](../../../../include/linux/mm.h) > + > +use crate::{ > + bindings, > + types::{ARef, AlwaysRefCounted, Opaque}, > +}; > + > +use core::{ > + ops::Deref, > + ptr::{self, NonNull}, > +}; > + > +pub mod virt; > + > +/// A wrapper for the kernel's `struct mm_struct`. > +/// > +/// Since `mm_users` may be zero, the associated address space may not e= xist anymore. You must use Can it also be the case that the space never existed to begin with? Then I would write "the associated address space may not exist." Also I think it makes more sense to use "You can use [`mmget_not_zero`] to be able to access the address space." instead of the second sentence. > +/// [`mmget_not_zero`] before accessing the address space. > +/// > +/// The `ARef` smart pointer holds an `mmgrab` refcount. Its destruc= tor may sleep. > +/// > +/// # Invariants > +/// > +/// Values of this type are always refcounted. Would be good to record the refcount used in the invariant. > +/// > +/// [`mmget_not_zero`]: Mm::mmget_not_zero > +pub struct Mm { > + mm: Opaque, > +} > + > +// SAFETY: It is safe to call `mmdrop` on another thread than where `mmg= rab` was called. > +unsafe impl Send for Mm {} > +// SAFETY: All methods on `Mm` can be called in parallel from several th= reads. > +unsafe impl Sync for Mm {} > + > +// SAFETY: By the type invariants, this type is always refcounted. > +unsafe impl AlwaysRefCounted for Mm { > + fn inc_ref(&self) { > + // SAFETY: The pointer is valid since self is a reference. > + unsafe { bindings::mmgrab(self.as_raw()) }; > + } > + > + 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 used only when `mm_users` is known to be non-zero at co= mpile-time. It can be used I find the "This type is used only when" a bit weird, what about "Like an [`Mm`], but with non-zero `mm_users`."? > +/// 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. The value of `mm_users` i= s non-zero. > +#[repr(transparent)] > +pub struct MmWithUser { > + mm: Mm, > +} > + > +// SAFETY: It is safe to call `mmput` on another thread than where `mmge= t` was called. > +unsafe impl Send for MmWithUser {} > +// SAFETY: All methods on `MmWithUser` can be called in parallel from se= veral threads. > +unsafe impl Sync for MmWithUser {} > + > +// SAFETY: By the type invariants, this type is always refcounted. > +unsafe impl AlwaysRefCounted for MmWithUser { > + fn inc_ref(&self) { > + // SAFETY: The pointer is valid since self is a reference. > + unsafe { bindings::mmget(self.as_raw()) }; > + } > + > + 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 =3D Mm; > + > + #[inline] > + fn deref(&self) -> &Mm { > + &self.mm > + } > +} > + > +/// A wrapper for the kernel's `struct mm_struct`. > +/// > +/// This type is identical to `MmWithUser` except that it uses `mmput_as= ync` when dropping a > +/// refcount. This means that the destructor of `ARef` = is safe to call in atomic > +/// context. Missing Invariants. > +#[repr(transparent)] > +pub struct MmWithUserAsync { > + mm: MmWithUser, > +} > + > +// SAFETY: It is safe to call `mmput_async` on another thread than where= `mmget` was called. > +unsafe impl Send for MmWithUserAsync {} > +// SAFETY: All methods on `MmWithUserAsync` can be called in parallel fr= om several threads. > +unsafe impl Sync for MmWithUserAsync {} > + > +// SAFETY: By the type invariants, this type is always refcounted. > +unsafe impl AlwaysRefCounted for MmWithUserAsync { > + fn inc_ref(&self) { > + // SAFETY: The pointer is valid since self is a reference. > + unsafe { bindings::mmget(self.as_raw()) }; > + } > + > + unsafe fn dec_ref(obj: NonNull) { > + // SAFETY: The caller is giving up their refcount. > + unsafe { bindings::mmput_async(obj.cast().as_ptr()) }; > + } > +} > + > +// Make all `MmWithUser` methods available on `MmWithUserAsync`. > +impl Deref for MmWithUserAsync { > + type Target =3D MmWithUser; > + > + #[inline] > + fn deref(&self) -> &MmWithUser { > + &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 =3D unsafe { > + let current =3D bindings::get_current(); > + (*current).mm > + }; > + > + let mm =3D NonNull::new(mm)?; > + > + // SAFETY: We just checked that `mm` is not null. > + unsafe { bindings::mmgrab(mm.as_ptr()) }; > + > + // SAFETY: We just created an `mmgrab` refcount. Layouts are com= patible due to > + // repr(transparent). > + Some(unsafe { ARef::from_raw(mm.cast()) }) > + } > + > + /// 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_mm<'a>(ptr: *const bindings::mm_struct) -> &'= a Mm { Why not just `from_raw`? --- Cheers, Benno > + // SAFETY: Caller promises that the pointer is valid for 'a. Lay= outs are compatible due to > + // repr(transparent). > + unsafe { &*ptr.cast() } > + }