From patchwork Sat Sep 21 15:32:46 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Danilo Krummrich X-Patchwork-Id: 13808934 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 E4D43CF9C5B for ; Sat, 21 Sep 2024 15:33:38 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 263B66B007B; Sat, 21 Sep 2024 11:33:38 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 2138B6B0082; Sat, 21 Sep 2024 11:33:38 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0DB4C6B0085; Sat, 21 Sep 2024 11:33:38 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id DE3596B007B for ; Sat, 21 Sep 2024 11:33:37 -0400 (EDT) Received: from smtpin23.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 7E450AB174 for ; Sat, 21 Sep 2024 15:33:37 +0000 (UTC) X-FDA: 82589140074.23.BA80BD3 Received: from nyc.source.kernel.org (nyc.source.kernel.org [147.75.193.91]) by imf23.hostedemail.com (Postfix) with ESMTP id C383F140020 for ; Sat, 21 Sep 2024 15:33:35 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=BWH+XVg7; spf=pass (imf23.hostedemail.com: domain of dakr@kernel.org designates 147.75.193.91 as permitted sender) smtp.mailfrom=dakr@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1726932700; 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=AVXASP8N7Zbfb+28V9NfwM2H/R/MjN3Yib7Wa3+bYQY=; b=cd8sP3oNHZYNuGe6IbQfGVaW5ZShPr8bM2RS+oRrQrJQgg298n/GxU9v0ph/r4+v9KXFl8 ai0TASct5Pq6yY3pVl0HPyG9XGcDzw6SlX49IakS29RnW3CCiHtts3KafKcJHgpEfr8ENn +TiV/HazpgT84UFMDwtaz34pM8FnuAU= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1726932700; a=rsa-sha256; cv=none; b=AlznsTa4bWcMy+vZihbtKWu7YSTLpcCjOC7xjhS6W25P3fZrseubi41Y7l0yy4JlB1LcIs 0CCShw448MwZOt0WgIDjaz1pXGk/vHJlpd0oxFbU6O44elnxStL4B9p2Ktc+FKoZ08iFT7 HsosW2ZpEvf9o2E2AnCz+p0foxO5J5Q= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=BWH+XVg7; spf=pass (imf23.hostedemail.com: domain of dakr@kernel.org designates 147.75.193.91 as permitted sender) smtp.mailfrom=dakr@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by nyc.source.kernel.org (Postfix) with ESMTP id 6849AA401EA; Sat, 21 Sep 2024 15:33:26 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 28443C4CEC2; Sat, 21 Sep 2024 15:33:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1726932814; bh=u6SkMDARmiJpubJd+bhVyKAuAzfP2dIXTmGflsC+2IE=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=BWH+XVg7nDKLhIgvleuHSahCfA+asylLR9Zt71HpOfLhRlUcTcUlvIp6VvXbI2tm6 qDCJPwb3vcVgWZKDkRq5NJhbogiP02zgXBWBMCGk3stpdn1OeM7z3heXgMZs7aW+Hs eyDqlWevxv/3FaT9HlNGB3ZRjesckEknNAJWckjuETs9eYA+7wH3qwAa1qIXjel+eX aOPMB3sFckQo2wJSy4jw7Mkt90vZJopAWvMGlZkm83Lj56D3HTnU7gpOOW5Mhga8b0 avGDtR2xK+g72hcnFN+B4PSv0ftu18BFQQZDomU1yKluoBe6Uh+YTBQH+kSewe8dAy WusEkD7ACd2zQ== From: Danilo Krummrich To: 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, aliceryhl@google.com, akpm@linux-foundation.org Cc: daniel.almeida@collabora.com, faith.ekstrand@collabora.com, boris.brezillon@collabora.com, lina@asahilina.net, mcanal@igalia.com, zhiw@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, Danilo Krummrich Subject: [RFC PATCH] rust: alloc: pass `old_layout` to `Allocator` Date: Sat, 21 Sep 2024 17:32:46 +0200 Message-ID: <20240921153315.70860-1-dakr@kernel.org> X-Mailer: git-send-email 2.46.1 In-Reply-To: <20240911225449.152928-2-dakr@kernel.org> References: <20240911225449.152928-2-dakr@kernel.org> MIME-Version: 1.0 X-Rspam-User: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: C383F140020 X-Stat-Signature: tmb1998jnngo8utc5e56affo1ipw4p1n X-HE-Tag: 1726932815-61150 X-HE-Meta: U2FsdGVkX183rT4WkHI+coKNngA3FXaYk2bj2D08mAhhXagdaWgXK03Ni60m9F0dmlU+OvwZAov+x92ygu8jj2SZfCL7cDNq7CfFneER5GuSXs1qLe6T2EzY0Gmnlh951oC/eZSDIk6MpuE+5GFny7AJFiF8J7m9p+UuHmfSxqsvy+OMi+qC+E89MHj76B0l62qoh9UwLd7j+pVrEkL8SY4N7ZdDHjKcpvoaYJkGrm0GdxYev5mccFdEJMDl3dfat07k16fhlQ1CSmAZDzXjMW6S2+gVuLZTuOnkc82bh/3GaVL9dlC6Z/Y0TrgpVifgUrrbzDWLc8b78XhsehuRqu1sq9zH03qa+A1rumgRXH/Hzyjj/55bfMXeFLlWCVkyX1zCd4/MBOH93GFJBr4NGi+XurSJ3doFCR2/yEYScsF55ESq1OkEnqEBnzhyMUKL9FRuOJ8wzyIq2YiA3KGWonLtWSlVrK+ax74VwgMc2gWgALqpeZGeXY8eu1gfqLW+Mowe1yN2jaCDMhY/oglgLMSbimtH8DnWQrfM25WQdjpYZCEnZ44wzWh7yMuQnm/hk2HPxAQn0YwAKyyXPuPs9Oev/3b8HLjDCa1DrlX0OmXxyq26Hy5FHwdPwEhNYSiSoDllGPg9+WLfyUXWxrHMAEamtICqCna2uz4dWXeDUzlLLVxQV586JNBateJrCSNZGTgdedm12VJnGrgak5BgdjqmDOpvXMMERwLhy+DtKxqr2QDZ/KGhnWaGs0hDp3JRmNOG+DA7j36XP2L1WeAIVwt+Q5XiHVOZLrCajAxaN9j2NJpcYzxF8V0Cvn0XW0EtI+m7bj0dlFV54k7ogWFVCZSnYpYwzxFznPqXa8NYIHlpOKzDjOFbA4SNDpW5TSytmJa5xL857N8bUWieHP2+RS64aWNwg1Z2J2tSaOBTdsA/vSXl4kDXGg7XX/BCoHFw16WkJXNruzPgXQCF0A1 lLQkcQ2Y GR9kVzI0/aUkAlDaKv0pYDDkCALNZ6xO6yMcY5DLF+iNuUiX55rjh3buVoC/fwRS/+/rETr8tBSSb3uaJ2fG4ZlayVjSvTITspH4OlJsuo8V3lXxNPioABhrEw6F6OokE7aH0WjYeQHXM+PJa1OBgF/KJqPRduQ+y8nhHJg2M6MXdcL17T2FjNdYE1YlVBjsn63YzgZEa5DeaMsPJdNMT93VJ+G9KgHpDIjISykNK6kyvYyi52FyFt2hQYnbBedSd0hscqEy68JpmCgkGKdHMSPdqnpgF+j8gL3H0tVfGgOTNw+R943j9vy7zKX06VCJwujwfoAgXDNrSCKiyqoGfJ86xDZpLjcTa3LQit5WmNyQ9ZpKaE2toCvYFIefixwCCSOZH 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: Since this came up a few times, this patch shows how the implementation looks like with an `old_layout` argument. Signed-off-by: Danilo Krummrich --- rust/kernel/alloc.rs | 32 ++++++++++++-------------- rust/kernel/alloc/allocator.rs | 28 +++++++++++++++++++---- rust/kernel/alloc/kbox.rs | 13 ++++++----- rust/kernel/alloc/kvec.rs | 42 +++++++++++++++++++++++++++------- 4 files changed, 78 insertions(+), 37 deletions(-) diff --git a/rust/kernel/alloc.rs b/rust/kernel/alloc.rs index 2170b53acd0c..78564eeb987d 100644 --- a/rust/kernel/alloc.rs +++ b/rust/kernel/alloc.rs @@ -166,7 +166,7 @@ pub unsafe trait Allocator { fn alloc(layout: Layout, flags: Flags) -> Result, AllocError> { // SAFETY: Passing `None` to `realloc` is valid by it's safety requirements and asks for a // new memory allocation. - unsafe { Self::realloc(None, layout, flags) } + unsafe { Self::realloc(None, layout, Layout::new::<()>(), flags) } } /// Re-allocate an existing memory allocation to satisfy the requested `layout`. @@ -186,26 +186,23 @@ fn alloc(layout: Layout, flags: Flags) -> Result, AllocError> { /// /// # Safety /// - /// If `ptr == Some(p)`, then `p` must point to an existing and valid memory allocation created - /// by this allocator. The alignment encoded in `layout` must be smaller than or equal to the - /// alignment requested in the previous `alloc` or `realloc` call of the same allocation. - /// - /// Additionally, `ptr` is allowed to be `None`; in this case a new memory allocation is - /// created. + /// - If `ptr == Some(p)`, then `p` must point to an existing and valid memory allocation + /// created by this allocator. + /// - `ptr` is allowed to be `None`; in this case a new memory allocation is created. + /// - `old_layout` must match the `Layout` the allocation has been created with. /// /// # Guarantees /// /// This function has the same guarantees as [`Allocator::alloc`]. When `ptr == Some(p)`, then /// it additionally guarantees that: /// - the contents of the memory pointed to by `p` are preserved up to the lesser of the new - /// and old size, - /// and old size, i.e. - /// `ret_ptr[0..min(layout.size(), old_size)] == p[0..min(layout.size(), old_size)]`, where - /// `old_size` is the size of the allocation that `p` points at. - /// - when the return value is `Err(AllocError)`, then `p` is still valid. + /// and old size, i.e. `ret_ptr[0..min(layout.size(), old_layout.size())] == + /// p[0..min(layout.size(), old_layout.size())]`. + /// - when the return value is `Err(AllocError)`, then `ptr` is still valid. unsafe fn realloc( ptr: Option>, layout: Layout, + old_layout: Layout, flags: Flags, ) -> Result, AllocError>; @@ -213,14 +210,13 @@ unsafe fn realloc( /// /// # Safety /// - /// `ptr` must point to an existing and valid memory allocation created by this `Allocator` and - /// must not be a dangling pointer. - /// - /// The memory allocation at `ptr` must never again be read from or written to. - unsafe fn free(ptr: NonNull) { + /// - `ptr` must point to an existing and valid memory allocation created by this `Allocator`. + /// - `layout` must match the `Layout` the allocation has been created with. + /// - The memory allocation at `ptr` must never again be read from or written to. + unsafe fn free(ptr: NonNull, layout: Layout) { // SAFETY: The caller guarantees that `ptr` points at a valid allocation created by this // allocator. We are passing a `Layout` with the smallest possible alignment, so it is // smaller than or equal to the alignment previously used with this allocation. - let _ = unsafe { Self::realloc(Some(ptr), Layout::new::<()>(), Flags(0)) }; + let _ = unsafe { Self::realloc(Some(ptr), Layout::new::<()>(), layout, Flags(0)) }; } } diff --git a/rust/kernel/alloc/allocator.rs b/rust/kernel/alloc/allocator.rs index 0b586c0361f4..07820c8c4e17 100644 --- a/rust/kernel/alloc/allocator.rs +++ b/rust/kernel/alloc/allocator.rs @@ -54,6 +54,14 @@ fn aligned_size(new_layout: Layout) -> usize { layout.size() } +/// Returns a properly aligned dangling pointer from the given `layout`. +fn zst_realloc(layout: Layout) -> NonNull { + let ptr = layout.align() as *mut u8; + + // SAFETY: `layout.align()` (and hence `ptr`) is guaranteed to be non-zero. + unsafe { NonNull::new_unchecked(ptr) } +} + /// # Invariants /// /// One of the following `krealloc`, `vrealloc`, `kvrealloc`. @@ -84,11 +92,18 @@ unsafe fn call( &self, ptr: Option>, layout: Layout, + old_layout: Layout, flags: Flags, ) -> Result, AllocError> { let size = aligned_size(layout); let ptr = match ptr { - Some(ptr) => ptr.as_ptr(), + Some(ptr) => { + if old_layout.size() == 0 { + ptr::null() + } else { + ptr.as_ptr() + } + } None => ptr::null(), }; @@ -106,7 +121,7 @@ unsafe fn call( }; let ptr = if size == 0 { - NonNull::dangling() + zst_realloc(layout) } else { NonNull::new(raw_ptr).ok_or(AllocError)? }; @@ -124,10 +139,11 @@ unsafe impl Allocator for Kmalloc { unsafe fn realloc( ptr: Option>, layout: Layout, + old_layout: Layout, flags: Flags, ) -> Result, AllocError> { // SAFETY: `ReallocFunc::call` has the same safety requirements as `Allocator::realloc`. - unsafe { ReallocFunc::KREALLOC.call(ptr, layout, flags) } + unsafe { ReallocFunc::KREALLOC.call(ptr, layout, old_layout, flags) } } } @@ -140,6 +156,7 @@ unsafe impl Allocator for Vmalloc { unsafe fn realloc( ptr: Option>, layout: Layout, + old_layout: Layout, flags: Flags, ) -> Result, AllocError> { // TODO: Support alignments larger than PAGE_SIZE. @@ -150,7 +167,7 @@ unsafe fn realloc( // SAFETY: If not `None`, `ptr` is guaranteed to point to valid memory, which was previously // allocated with this `Allocator`. - unsafe { ReallocFunc::VREALLOC.call(ptr, layout, flags) } + unsafe { ReallocFunc::VREALLOC.call(ptr, layout, old_layout, flags) } } } @@ -163,6 +180,7 @@ unsafe impl Allocator for KVmalloc { unsafe fn realloc( ptr: Option>, layout: Layout, + old_layout: Layout, flags: Flags, ) -> Result, AllocError> { // TODO: Support alignments larger than PAGE_SIZE. @@ -173,6 +191,6 @@ unsafe fn realloc( // SAFETY: If not `None`, `ptr` is guaranteed to point to valid memory, which was previously // allocated with this `Allocator`. - unsafe { ReallocFunc::KVREALLOC.call(ptr, layout, flags) } + unsafe { ReallocFunc::KVREALLOC.call(ptr, layout, old_layout, flags) } } } diff --git a/rust/kernel/alloc/kbox.rs b/rust/kernel/alloc/kbox.rs index 6188494f040d..e9e2e94430ef 100644 --- a/rust/kernel/alloc/kbox.rs +++ b/rust/kernel/alloc/kbox.rs @@ -5,6 +5,7 @@ #[allow(unused_imports)] // Used in doc comments. use super::allocator::{KVmalloc, Kmalloc, Vmalloc}; use super::{AllocError, Allocator, Flags}; +use core::alloc::Layout; use core::fmt; use core::marker::PhantomData; use core::mem::ManuallyDrop; @@ -233,7 +234,7 @@ pub fn new_uninit(flags: Flags) -> Result, A>, AllocError> { let ptr = if Self::is_zst() { NonNull::dangling() } else { - let layout = core::alloc::Layout::new::>(); + let layout = Layout::new::>(); let ptr = A::alloc(layout, flags)?; ptr.cast() @@ -452,14 +453,14 @@ impl Drop for Box A: Allocator, { fn drop(&mut self) { - let size = core::mem::size_of_val::(self); + let layout = Layout::for_value::(self); // SAFETY: The pointer in `self.0` is guaranteed to be valid by the type invariant. unsafe { core::ptr::drop_in_place::(self.deref_mut()) }; - if size != 0 { - // SAFETY: As `size` is not zero, `self.0` was previously allocated with `A`. - unsafe { A::free(self.0.cast()) }; - } + // SAFETY: + // - `self.0` was previously allocated with `A`. + // - `layout` is equal to the `Layout´ `self.0` was allocated with. + unsafe { A::free(self.0.cast(), layout) }; } } diff --git a/rust/kernel/alloc/kvec.rs b/rust/kernel/alloc/kvec.rs index 686e969463f8..fb979013562c 100644 --- a/rust/kernel/alloc/kvec.rs +++ b/rust/kernel/alloc/kvec.rs @@ -7,6 +7,7 @@ AllocError, Allocator, Box, Flags, }; use core::{ + alloc::Layout, fmt, marker::PhantomData, mem::{ManuallyDrop, MaybeUninit}, @@ -452,21 +453,28 @@ pub fn reserve(&mut self, additional: usize, flags: Flags) -> Result<(), AllocEr // We know `cap` is <= `isize::MAX` because of the type invariants of `Self`. So the // multiplication by two won't overflow. let new_cap = core::cmp::max(cap * 2, len.checked_add(additional).ok_or(AllocError)?); - let layout = core::alloc::Layout::array::(new_cap).map_err(|_| AllocError)?; + let layout = Layout::array::(new_cap).map_err(|_| AllocError)?; + + let old_layout = Layout::array::(self.cap).map_err(|_| AllocError)?; // We need to make sure that `ptr` is either NULL or comes from a previous call to // `realloc_flags`. A `Vec`'s `ptr` value is not guaranteed to be NULL and might be // dangling after being created with `Vec::new`. Instead, we can rely on `Vec`'s // capacity to be zero if no memory has been allocated yet. + // + // Still required? In `Vec::new` we use `NonNull::dangling`, which effectively would be + // valid to pass to `A::realloc`, but was never created by one the `Allocator`'s functions. let ptr = if cap == 0 { None } else { Some(self.ptr.cast()) }; - // SAFETY: `ptr` is valid because it's either `None` or comes from a previous call to - // `A::realloc`. We also verified that the type is not a ZST. - let ptr = unsafe { A::realloc(ptr, layout, flags)? }; + // SAFETY: + // - `ptr` is valid because it's either `None` or comes from a previous call to + // `A::realloc`. + // - `old_layout` matches the `Layout` of the preceeding allocation, if any. + let ptr = unsafe { A::realloc(ptr, layout, old_layout, flags)? }; self.ptr = ptr.cast(); @@ -528,9 +536,16 @@ fn drop(&mut self) { }; // If `cap == 0` we never allocated any memory in the first place. + // + // Same here, theoretically, we know that `NonNull::dangling` from `Vec::new` is valid for + // `A::free`, but it was never created by any of the `Allocator` functions. if self.cap != 0 { + // This can never fail; since this `Layout` is equivalent to the one of the original + // allocation. + let layout = Layout::array::(self.cap).unwrap(); + // SAFETY: `self.ptr` was previously allocated with `A`. - unsafe { A::free(self.ptr.cast()) }; + unsafe { A::free(self.ptr.cast(), layout) }; } } } @@ -751,13 +766,17 @@ pub fn collect(self, flags: Flags) -> Vec { ptr = buf.as_ptr(); } + // This can never fail; since this `Layout` is equivalent to the one of the original + // allocation. + let old_layout = Layout::array::(cap).unwrap(); + // This can never fail, `len` is guaranteed to be smaller than `cap`. - let layout = core::alloc::Layout::array::(len).unwrap(); + let layout = Layout::array::(len).unwrap(); // SAFETY: `buf` points to the start of the backing buffer and `len` is guaranteed to be // smaller than `cap`. Depending on `alloc` this operation may shrink the buffer or leaves // it as it is. - ptr = match unsafe { A::realloc(Some(buf.cast()), layout, flags) } { + ptr = match unsafe { A::realloc(Some(buf.cast()), layout, old_layout, flags) } { // If we fail to shrink, which likely can't even happen, continue with the existing // buffer. Err(_) => ptr, @@ -846,9 +865,16 @@ fn drop(&mut self) { unsafe { ptr::drop_in_place(self.as_raw_mut_slice()) }; // If `cap == 0` we never allocated any memory in the first place. + // + // Same here, theoretically, we know that `NonNull::dangling` from `Vec::new` is valid for + // `A::free`, but it was never created by any of the `Allocator` functions. if self.cap != 0 { + // This can never fail; since this `Layout` is equivalent to the one of the original + // allocation. + let layout = Layout::array::(self.cap).unwrap(); + // SAFETY: `self.buf` was previously allocated with `A`. - unsafe { A::free(self.buf.cast()) }; + unsafe { A::free(self.buf.cast(), layout) }; } } }