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 54F92C52D70 for ; Tue, 6 Aug 2024 16:51:39 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id BF8296B0085; Tue, 6 Aug 2024 12:51:38 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id BA71F6B0088; Tue, 6 Aug 2024 12:51:38 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A6E766B0089; Tue, 6 Aug 2024 12:51:38 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 8472A6B0085 for ; Tue, 6 Aug 2024 12:51:38 -0400 (EDT) Received: from smtpin18.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 23094A0365 for ; Tue, 6 Aug 2024 16:51:38 +0000 (UTC) X-FDA: 82422411876.18.7A4BEF5 Received: from mail-40134.protonmail.ch (mail-40134.protonmail.ch [185.70.40.134]) by imf18.hostedemail.com (Postfix) with ESMTP id 345AC1C000E for ; Tue, 6 Aug 2024 16:51:35 +0000 (UTC) Authentication-Results: imf18.hostedemail.com; dkim=pass header.d=proton.me header.s=protonmail header.b=lzweRFsj; spf=pass (imf18.hostedemail.com: domain of benno.lossin@proton.me designates 185.70.40.134 as permitted sender) smtp.mailfrom=benno.lossin@proton.me; dmarc=pass (policy=quarantine) header.from=proton.me ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1722963033; 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=j+sH+MxUzfSpzuSfOiW6jt862OOTAwTU+CEagi5eEQM=; b=MFjtryi40atyZxRNZryM4LlZ/14x2UlvfLVF3TH0gOI/BtG2zkfxxgLNq08GG8pDUOv/Fr 0/i2cNR4vIKVXeroQ51FJXhv283/pD3CB4L1XDAwz5w5T+JH5FQe/4o3a9zDOB0qbFTnUi bnvFScMNs2HjG6lMlI2OwUz8BThq7sM= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1722963033; a=rsa-sha256; cv=none; b=4V37YfCjlbwYOiBFUxDfRPJNL18Xxz9vr2HkLp6yZNi+woa3hEOVK2wKSIi89qNArFGwFd kDmSdVjjC5PNWlSswWL+HNBryQc09DiVzgoXdVjYxrb9Pek9ymqut+hceDAW+Omq+mMCjg IOEzwyAX2bvvecSKoYr0Cv+060PsDqQ= ARC-Authentication-Results: i=1; imf18.hostedemail.com; dkim=pass header.d=proton.me header.s=protonmail header.b=lzweRFsj; spf=pass (imf18.hostedemail.com: domain of benno.lossin@proton.me designates 185.70.40.134 as permitted sender) smtp.mailfrom=benno.lossin@proton.me; dmarc=pass (policy=quarantine) header.from=proton.me DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=proton.me; s=protonmail; t=1722963094; x=1723222294; bh=j+sH+MxUzfSpzuSfOiW6jt862OOTAwTU+CEagi5eEQM=; 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=lzweRFsj7ZhgrySbT1kblqrMe7241NZ2TAcVRcW3CrdMBFx37UfWYaYaYUjA95p9v R/MOKjHBSsZeolLhFK3HmgMsxlz6TUrqCfReAPh3aXB2+mjM0KvU8re9JQXJJyoXT0 x86EFZ+bSr9epilX+yPdg+De1rOwIf5aNSyH7WCAtoox/ztKv3/1hhki/K+xOJuv9F nfd5Dqjirta6y8bg87B0+MAqgyz/vExjIMarRsZ7jFTPv9t9yQbNQUU4+pgF2foqEC 2U5nCbkgT0X+yUDYhLKEVXYLAnmg4uKnuxmjuHexeDTD1e5vFhlIIUuCgdSNiC9+gy XvsJa2XKIBbtQ== Date: Tue, 06 Aug 2024 16:51:28 +0000 To: Danilo Krummrich , ojeda@kernel.org, alex.gaynor@gmail.com, wedsonaf@gmail.com, boqun.feng@gmail.com, gary@garyguo.net, bjorn3_gh@protonmail.com, a.hindborg@samsung.com, aliceryhl@google.com, akpm@linux-foundation.org From: Benno Lossin Cc: daniel.almeida@collabora.com, faith.ekstrand@collabora.com, boris.brezillon@collabora.com, lina@asahilina.net, mcanal@igalia.com, zhiw@nvidia.com, acurrid@nvidia.com, cjia@nvidia.com, jhubbard@nvidia.com, airlied@redhat.com, ajanulgu@redhat.com, lyude@redhat.com, linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH v4 04/28] rust: alloc: implement `Allocator` for `Kmalloc` Message-ID: In-Reply-To: <20240805152004.5039-5-dakr@kernel.org> References: <20240805152004.5039-1-dakr@kernel.org> <20240805152004.5039-5-dakr@kernel.org> Feedback-ID: 71780778:user:proton X-Pm-Message-ID: 60940ad762dd5fe06aa5d084a0527ef2e1169f23 MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Stat-Signature: subj5ieggt133zrt6zhxjt4xbhtszbx3 X-Rspamd-Queue-Id: 345AC1C000E X-Rspam-User: X-Rspamd-Server: rspam08 X-HE-Tag: 1722963095-901021 X-HE-Meta: U2FsdGVkX1+u0KyVX4TVyfUZVCHS6h7HukBbyrJrQ52iD9S+R/Q5vl1TAuvm2agjJUgtby0KINwcK1f6vK2Nyd6CdSfJgQ2E0vTTvITEDlv0qx3cOyMTxHoibd6w5ae5E8gYylEKbBfRZ6yqJ5GPa5iHOJPFqaDkvuS8/HXAYAOOT/7/xPAWP9AIS6fRCSNaegK4clpabjIg1OLES9/coK/3A650fhyu3elkxLgsHDVC4kaxukQwjGKmr0fzCW5TN+XRViCztSyuZZp5+JIa3NKP7MiQTbKLhC6/kr/KmRh5Mn2J0oAbXAqAR9p+YA1hQJNUGg1kp0T77PL58jRxDaPl6s8+etLoSlxk/E8ZcMCuKp5rXu63W2zMAiyeRJ36p58Hy/d08sembfDNLI4vkkxiLXiwndRRG9pMBeVj3QsfpsYFbX1AoNF2MAw//7e+bEiFlVFLBUT+mRk4tkxdp9GhRWrLjNDKIhhdWQ63uKZanv/9WsViFedMFwkuxmMqbTk67Fbfyz7u5IpOz7E54SgShvYDKkqkPgXdnaO925+fSFASbql2y9PHMtU05Oa3NYQ98wRfVElz4/G6AB679wazxj54dtQbvGyqgJEaSS0sZ/qO0eezjKXO6hcG80wLoYft3jJKTScSgfoJ2cNNwFYpD3LQNHvOWF5HI0utHxtQ70qSl/q25T1U5nYClVb216zlAW9k71i2ab97DMSMx+nj0x2vUef1K8wgf9Xvb0UjfYQW4/uOswpwV+/evrGfYvx4GWHgbpDK3ljjQQKzxJvPmK09c3L/l2gDlSnN4CT0EmRZwfBIW2temi+WSN/72I3vaWVGMe3ixJ74CrJiZElzvGvsO3w3DpJQMut3rfkoOBEUvFLhdaiVbAUc95BrWdh4huiGqRRXUneVRGimsygsYQhkYmvZhllLhlncs10vrfmq9KBWeWjJu2DIzxzZtnPSd/PVhVzqKkEwLfy kWvmY4Om 5u8m/527wuUMgOvZ1l9Hz5fPWyi8VFEZB72nURZg6i6VrHIoHwaWDh+j0kbjagAX4piYN5IL3FIw1dw4MPEvhoyHEvFqY1msCyzdNah3jEygcWtX0Yy1KO2h8HScXxblVVwNd2GjZgD4dD2kpK/0Bl0on3JZHk4ngj1esrYVXHPkY9huAphmUVB3Pk88gbRg+VSl0OkF/baeckCcGz6vJ+mOW3MFc9iNS1UQ8Agc3wfqQdNTSnC31PJlx6qN1KHtJzzCXcyDM72ao9KevwhtTl5N/hGzGEyHTX+gwst07tYCARDEyRauHrQULYdmezx8Le6af5B239ODhph932NbWjpqZjIJneCL+zstGX7yMTVGANzlU8aBfkMcMyhD25DybkC5ImbQD1apjAT0uZjd498RMWpLZ/uKNBBdB 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 05.08.24 17:19, Danilo Krummrich wrote: > Implement `Allocator` for `Kmalloc`, the kernel's default allocator, > typically used for objects smaller than page size. >=20 > All memory allocations made with `Kmalloc` end up in `krealloc()`. >=20 > It serves as allocator for the subsequently introduced types `KBox` and > `KVec`. >=20 > Signed-off-by: Danilo Krummrich > --- > rust/helpers.c | 3 +- > rust/kernel/alloc.rs | 2 +- > rust/kernel/alloc/allocator.rs | 68 +++++++++++++++++++++++++++++++++- > 3 files changed, 69 insertions(+), 4 deletions(-) >=20 > diff --git a/rust/helpers.c b/rust/helpers.c > index 92d3c03ae1bd..9f7275493365 100644 > --- a/rust/helpers.c > +++ b/rust/helpers.c > @@ -193,8 +193,7 @@ void rust_helper_init_work_with_key(struct work_struc= t *work, work_func_t func, > } > EXPORT_SYMBOL_GPL(rust_helper_init_work_with_key); >=20 > -void * __must_check __realloc_size(2) > -rust_helper_krealloc(const void *objp, size_t new_size, gfp_t flags) > +void *rust_helper_krealloc(const void *objp, size_t new_size, gfp_t flag= s) > { > =09return krealloc(objp, new_size, flags); > } > diff --git a/rust/kernel/alloc.rs b/rust/kernel/alloc.rs > index 8a71a589469d..bc01a17df5e0 100644 > --- a/rust/kernel/alloc.rs > +++ b/rust/kernel/alloc.rs > @@ -4,7 +4,7 @@ >=20 > #[cfg(not(test))] > #[cfg(not(testlib))] > -mod allocator; > +pub mod allocator; > pub mod box_ext; > pub mod vec_ext; >=20 > diff --git a/rust/kernel/alloc/allocator.rs b/rust/kernel/alloc/allocator= .rs > index 2c1eae25da84..c6ad1dd59dd0 100644 > --- a/rust/kernel/alloc/allocator.rs > +++ b/rust/kernel/alloc/allocator.rs > @@ -5,8 +5,16 @@ > use super::{flags::*, Flags}; > use core::alloc::{GlobalAlloc, Layout}; > use core::ptr; > +use core::ptr::NonNull; >=20 > -struct Kmalloc; > +use crate::alloc::{AllocError, Allocator}; > +use crate::bindings; > + > +/// The contiguous kernel allocator. > +/// > +/// The contiguous kernel allocator only ever allocates physically conti= guous memory through > +/// `bindings::krealloc`. > +pub struct Kmalloc; >=20 > /// Returns a proper size to alloc a new object aligned to `new_layout`'= s alignment. > fn aligned_size(new_layout: Layout) -> usize { > @@ -40,6 +48,64 @@ pub(crate) unsafe fn krealloc_aligned(ptr: *mut u8, ne= w_layout: Layout, flags: F > } > } >=20 > +/// # Invariants > +/// > +/// One of the following `krealloc`, `vrealloc`, `kvrealloc`. > +struct ReallocFunc( > + unsafe extern "C" fn(*const core::ffi::c_void, usize, u32) -> *mut c= ore::ffi::c_void, > +); > + > +impl ReallocFunc { > + // INVARIANT: `krealloc` satisfies the type invariants. This INVARIANT comment should be moved one line downwards. > + fn krealloc() -> Self { > + Self(bindings::krealloc) > + } > + > + /// # Safety > + /// > + /// This method has the exact same safety requirements as `Allocator= ::realloc`. I would remove "exact", I don't think we want to mean "almost the same" when we write just "same". > + unsafe fn call( > + &self, > + ptr: Option>, > + layout: Layout, > + flags: Flags, > + ) -> Result, AllocError> { > + let size =3D aligned_size(layout); > + let ptr =3D match ptr { > + Some(ptr) =3D> ptr.as_ptr(), > + None =3D> ptr::null(), > + }; > + > + // SAFETY: `ptr` is valid by the safety requirements of this fun= ction. "`ptr` is either NULL or valid by the safety requirements of this function." > + let raw_ptr =3D unsafe { > + // If `size =3D=3D 0` and `ptr !=3D NULL` the memory behind = the pointer is freed. > + self.0(ptr.cast(), size, flags.0).cast() > + }; > + > + let ptr =3D if size =3D=3D 0 { > + NonNull::dangling() If we call `realloc(Some(ptr), , ...)`, then this leaks the pointer returned by the call to `self.0` above. I don't know what the return value of the different functions are that can appear in `self.0`, do they return NULL? What about the following sequence: let ptr =3D realloc(None, , ...); let ptr =3D realloc(Some(ptr), , ...); Then the above call to `self.0` is done with a dangling pointer, can the functions that appear in `self.0` handle that? > + } else { > + NonNull::new(raw_ptr).ok_or(AllocError)? > + }; > + > + Ok(NonNull::slice_from_raw_parts(ptr, size)) > + } > +} > + > +unsafe impl Allocator for Kmalloc { > + unsafe fn realloc( > + ptr: Option>, > + layout: Layout, > + flags: Flags, > + ) -> Result, AllocError> { > + let realloc =3D ReallocFunc::krealloc(); > + > + // SAFETY: If not `None`, `ptr` is guaranteed to point to valid = memory, which was previously > + // allocated with this `Allocator`. What about the other requirements? (they should be satisfied, since they are also requirements for calling this function) > + unsafe { realloc.call(ptr, layout, flags) } If you make `ReallocFunc::krealloc()` into a constant `ReallocFunc::KREALLOC`, then we could avoid the let binding above. --- Cheers, Benno > + } > +} > + > unsafe impl GlobalAlloc for Kmalloc { > unsafe fn alloc(&self, layout: Layout) -> *mut u8 { > // SAFETY: `ptr::null_mut()` is null and `layout` has a non-zero= size by the function safety > -- > 2.45.2 >=20