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 5C917C3DA64 for ; Thu, 1 Aug 2024 12:27:03 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C58576B0083; Thu, 1 Aug 2024 08:27:02 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C087A6B0088; Thu, 1 Aug 2024 08:27:02 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id ACFAD6B0089; Thu, 1 Aug 2024 08:27:02 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 8E80F6B0083 for ; Thu, 1 Aug 2024 08:27:02 -0400 (EDT) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 4285F1A0C9C for ; Thu, 1 Aug 2024 12:27:02 +0000 (UTC) X-FDA: 82403601084.12.9FA0CD4 Received: from sin.source.kernel.org (sin.source.kernel.org [145.40.73.55]) by imf17.hostedemail.com (Postfix) with ESMTP id D81AA40030 for ; Thu, 1 Aug 2024 12:26:59 +0000 (UTC) Authentication-Results: imf17.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=K0HL747l; spf=pass (imf17.hostedemail.com: domain of dakr@kernel.org designates 145.40.73.55 as permitted sender) smtp.mailfrom=dakr@kernel.org; dmarc=pass (policy=none) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1722515164; 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=hXayb9TO0WCdsCdljqz0J4wJhmzHaaU/luIPeLo0fYQ=; b=XJI0KCj8qe7xaxG7uQEsVPJiygJF7uRkscdXw10dLsQmFVnV5z1IrOLJa35ba6KF7+i0FY x5SxQR6AMOUY9oJhA3aAJ2yFKoM4GCCLObz34a52BQ/ShstJ5NM/DP7dB7KJfJ902ogSaZ ZoKKRPY0w1YeLl4UL5zAFjbJhWpl+Fw= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1722515164; a=rsa-sha256; cv=none; b=cCcUXV3TLXqU5cd+EGgIhh82nXiqdkmgzmQlPalNV00VLVdQBPIgOAEftuJ1qc9VSMfGFp 3jr7/2s+3961BrUSmFyEYN9KdqLKtRJsbWurW9oiL8c0q4fV7bv7PMZ9HzSYxGENkH3llF xohvuQpVxlh1HCbgGvxh5b//e1sqJE0= ARC-Authentication-Results: i=1; imf17.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=K0HL747l; spf=pass (imf17.hostedemail.com: domain of dakr@kernel.org designates 145.40.73.55 as permitted sender) smtp.mailfrom=dakr@kernel.org; dmarc=pass (policy=none) header.from=kernel.org Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id C39D2CE0B55; Thu, 1 Aug 2024 12:26:55 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 3896AC32786; Thu, 1 Aug 2024 12:26:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1722515215; bh=ko15EhQwm6KYZ8dfClKPHYYoNPMirgGgRV4mpM7ac0o=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=K0HL747ltC9X04VfLIHkN2XEFIWEwV3eTrltLBqTO1zz+S/IZ74h5EtpjMzxEMW96 6k47pjMBx4e44sGBKY7a7CmH59BcERjCr2Z7TWywCGJLDE+htE0tj+4S5tfchZjrxW 0WTR8xN3r9+RQrd4jsaVKAr1o8xji2uaWh0xeXFRbMSkmRrDn74RdUdZIqjXHSLxeK c95I/gwN7rkxOJL02U3LKzubI23/zcP3tH83E4hVqAdf+Oyi90dOXl1F7r5lFUc0K2 gPPQsfY9m6yhw+ioXp6Is5A7Fn3GQ5m3acjvqEcmZDCS5332KCPmhXH5+B6PybSxpp DGo7zJ6S/a/uA== Date: Thu, 1 Aug 2024 14:26:46 +0200 From: Danilo Krummrich To: Alice Ryhl Cc: ojeda@kernel.org, alex.gaynor@gmail.com, wedsonaf@gmail.com, boqun.feng@gmail.com, gary@garyguo.net, bjorn3_gh@protonmail.com, benno.lossin@proton.me, a.hindborg@samsung.com, akpm@linux-foundation.org, 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 v3 01/25] rust: alloc: add `Allocator` trait Message-ID: References: <20240801000641.1882-1-dakr@kernel.org> <20240801000641.1882-2-dakr@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Rspamd-Queue-Id: D81AA40030 X-Stat-Signature: 8z4z9azzdt9cjgi9xa5fm6omdy9gdb9d X-Rspamd-Server: rspam09 X-Rspam-User: X-HE-Tag: 1722515219-160434 X-HE-Meta: U2FsdGVkX19EVVlFV59iSMA9EmnHF/9LD8zyevuN1DBuRclOztUb7rrepzCYod1j/c64cr5dgUZHqdgEDCZXLIIwSVipAkMmoaomJPdK1M7KIktRy0NT3bJP0wwOsrZ9cYMv5ohIDJ2/sxK+FI0Hs5pqTfYjReZO/HHFvL5S/LF6zwOxh5NQh0XH1vF1CbCf1WPxHaZ10cS1dItchhnbV/lm+mXAEGbVe75ioFp+2ll8GRNGOFFkUEZiE+P8PHxPUzGVkkh8CK4H1IBtHp4RRys4r55hzOjWlIUL8QhClgD6QlzmN1ti4z+EjeQg8DCjTYSqwGUDhmpDuRMmnPOyWtRNh8NKno9l0ejSJfPdEgjAr3N2ggM8rXlZDiLcD4yTQhLNWHi61kSbpE+mBdjlM3Jyw5IsEpyLF0uAVI4Q06VqhrCgV5kRNgltN8uOXSMTeEc/tVa33NLmCiSscWB3td6b9Pzm1VpwWAT5Ww33uVKn7f1JWjxaMFXZeQaEFRaewRcoUpInyVSHT7txyrRPic3NAbd2q+KmRdBcyrNChZvCD5EY1tv6qoIDlEME+FNrL1r2VW7LqIxxXXTnAwpf+ZiH22qJ/Zf37BDuyGpHHGulR5KcxTYE43sd0y2zU5bI+xQUntrc0yumBuGIBrrwXq7vSWy0QuAefDsge7jD6aS5CfTqGybU4GlRLejcO7nTjIT5TeMk8rf75poSCpImQQS6x94EqoGR/lKvxZ6JxnJ+jYz0jwlR7OQ9nCMky5CI10+2Hsn6CkS689WxPAOG9YoP534zMx1JSHS+NngoSyGCcxjhLwIFXQbdhLKl9rlCtZnhUMKIKZUpPFBrbtVoCIaNLyLAqCAnSRWjoFKLArjjdgizGgPv8Yzxcdx8yu3VwWywpKIVxSwipiVdMtj+i17lrJKGYqXqJwJyI/2ovD/IT0sLGAbpN0GWMRz7rpHttSMFyeTr4ztdhyi2Emm SCspH0wc Rf3JZfWFOplv/UjNkG5WvgLxi7A6WNJWgL/msexhW3ImatfJAoOGvZoAxXPQ/JDSJG9YIjD4i/4LU4ta4SKf1EktfFrtOUvnmYaGR6XZ3T1FWh99FOZ5sZ3wWEBoctD/IO7veJM6sN9iHl+R3dGYN6Aqj9M1xEvoGwAEDSJmdazEBnnWjA3232AV6naPrV4jc10GhVPHF2k5zOFGyhP4e948HE2Kk43s6syAXMT2QpUi37SnFYKR1pL6PsgSPdWHlZ5sgZm3niKs0+Kgv5g5h0Mo1/iMwU8QeOw8Tw0L3n4ZBEmjshhZBtxyDQHSp3x5Od8WbvodgqcFrFpOp0E39hB41O/8X13kB3ld7WivvmoLv8UGw1ZlhsKtw0g== 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 Thu, Aug 01, 2024 at 10:19:41AM +0200, Alice Ryhl wrote: > On Thu, Aug 1, 2024 at 2:07 AM Danilo Krummrich wrote: > > > > Add a kernel specific `Allocator` trait, that in contrast to the one in > > Rust's core library doesn't require unstable features and supports GFP > > flags. > > > > Subsequent patches add the following trait implementors: `Kmalloc`, > > `Vmalloc` and `KVmalloc`. > > > > Signed-off-by: Danilo Krummrich > > It could be nice to mention that the design without `self` parameters > is necessary for compatibility with #[derive(SmartPointer)]. Agreed, will do. > > > rust/kernel/alloc.rs | 73 ++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 73 insertions(+) > > > > diff --git a/rust/kernel/alloc.rs b/rust/kernel/alloc.rs > > index 1966bd407017..b79dd2c49277 100644 > > --- a/rust/kernel/alloc.rs > > +++ b/rust/kernel/alloc.rs > > @@ -11,6 +11,7 @@ > > /// Indicates an allocation error. > > #[derive(Copy, Clone, PartialEq, Eq, Debug)] > > pub struct AllocError; > > +use core::{alloc::Layout, ptr::NonNull}; > > > > /// Flags to be used when allocating memory. > > /// > > @@ -86,3 +87,75 @@ pub mod flags { > > /// small allocations. > > pub const GFP_NOWAIT: Flags = Flags(bindings::GFP_NOWAIT); > > } > > + > > +/// The kernel's [`Allocator`] trait. > > +/// > > +/// An implementation of [`Allocator`] can allocate, re-allocate and free memory buffer described > > +/// via [`Layout`]. > > +/// > > +/// [`Allocator`] is designed to be implemented as a ZST; [`Allocator`] functions do not operate on > > +/// an object instance. > > +/// > > +/// # Safety > > +/// > > +/// Memory returned from an allocator must point to a valid memory buffer and remain valid until > > +/// it is explicitly freed. > > +/// > > +/// Any pointer to a memory buffer which is currently allocated must be valid to be passed to any > > +/// other [`Allocator`] function. The same applies for a NULL pointer. > > +/// > > +/// If `realloc` is called with: > > +/// - a size of zero, the given memory allocation, if any, must be freed > > +/// - a NULL pointer, a new memory allocation must be created > > +pub unsafe trait Allocator { > > + /// Allocate memory based on `layout` and `flags`. > > + /// > > + /// On success, returns a buffer represented as `NonNull<[u8]>` that satisfies the layout > > + /// constraints (i.e. minimum size and alignment as specified by `layout`). > > + /// > > + /// This function is equivalent to `realloc` when called with a NULL pointer. > > + fn alloc(layout: Layout, flags: Flags) -> Result, AllocError> { > > + // SAFETY: Passing a NULL pointer to `realloc` is valid by it's safety requirements and asks > > + // for a new memory allocation. > > + unsafe { Self::realloc(None, layout, flags) } > > + } > > + > > + /// Re-allocate an existing memory allocation to satisfy the requested `layout`. If the > > + /// requested size is zero, `realloc` behaves equivalent to `free`. > > + /// > > + /// If the requested size is larger than the size of the existing allocation, a successful call > > + /// to `realloc` guarantees that the new or grown buffer has at least `Layout::size` bytes, but > > + /// may also be larger. > > + /// > > + /// If the requested size is smaller than the size of the existing allocation, `realloc` may or > > + /// may not shrink the buffer; this is implementation specific to the allocator. > > + /// > > + /// On allocation failure, the existing buffer, if any, remains valid. > > + /// > > + /// The buffer is represented as `NonNull<[u8]>`. > > + /// > > + /// # Safety > > + /// > > + /// `ptr` must point to an existing and valid memory allocation created by this allocator > > + /// instance. > > + /// > > + /// Additionally, `ptr` is allowed to be a NULL pointer; in this case a new memory allocation is > > + /// created. > > + unsafe fn realloc( > > + ptr: Option>, > > + layout: Layout, > > + flags: Flags, > > + ) -> Result, AllocError>; > > Is it intentional that this allows you to change the alignment of an > allocation? If so, that could use a note in the docs. Yes, it's intentional and yes it really misses a note in the documentation. The idea is to allow a change of alignment as long as the new alignment is smaller than the old alignment. In terms of safety, it is the callers responsibility to ensure constant alignment throughout re-allocations (if required). > > > + /// Free an existing memory allocation. > > + /// > > + /// # Safety > > + /// > > + /// `ptr` must point to an existing and valid memory allocation created by this `Allocator` > > + /// instance. > > + unsafe fn free(ptr: NonNull) { > > + // SAFETY: `ptr` is guaranteed to be previously allocated with this `Allocator` or NULL. > > + // Calling `realloc` with a buffer size of zero, frees the buffer `ptr` points to. > > + let _ = unsafe { Self::realloc(Some(ptr), Layout::new::<()>(), Flags(0)) }; > > + } > > At the very least, the provided implementation of `free` changes the > alignment when it calls `realloc`. Yes, I think that's fine though. Hopefully no one attempts to use the memory anymore once `free` is being called. > > Alice >