From 03989773a94490383b062912feb0c4d175f20845 Mon Sep 17 00:00:00 2001 From: Wedson Almeida Filho Date: Wed, 27 Mar 2024 22:35:55 -0300 Subject: rust: alloc: introduce the `VecExt` trait Make `try_with_capacity`, `try_push`, and `try_extend_from_slice` methods available in `Vec` even though it doesn't implement them. It is implemented with `try_reserve` and `push_within_capacity`. This is in preparation for switching to the upstream `alloc` crate. Reviewed-by: Benno Lossin Suggested-by: Gary Guo Signed-off-by: Wedson Almeida Filho Link: https://lore.kernel.org/r/20240328013603.206764-3-wedsonaf@gmail.com Signed-off-by: Miguel Ojeda --- rust/kernel/alloc/vec_ext.rs | 48 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 rust/kernel/alloc/vec_ext.rs (limited to 'rust/kernel/alloc/vec_ext.rs') diff --git a/rust/kernel/alloc/vec_ext.rs b/rust/kernel/alloc/vec_ext.rs new file mode 100644 index 000000000000..311e62cc5784 --- /dev/null +++ b/rust/kernel/alloc/vec_ext.rs @@ -0,0 +1,48 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! Extensions to [`Vec`] for fallible allocations. + +use alloc::{collections::TryReserveError, vec::Vec}; +use core::result::Result; + +/// Extensions to [`Vec`]. +pub trait VecExt: Sized { + /// Creates a new [`Vec`] instance with at least the given capacity. + fn try_with_capacity(capacity: usize) -> Result; + + /// Appends an element to the back of the [`Vec`] instance. + fn try_push(&mut self, v: T) -> Result<(), TryReserveError>; + + /// Pushes clones of the elements of slice into the [`Vec`] instance. + fn try_extend_from_slice(&mut self, other: &[T]) -> Result<(), TryReserveError> + where + T: Clone; +} + +impl VecExt for Vec { + fn try_with_capacity(capacity: usize) -> Result { + let mut v = Vec::new(); + v.try_reserve(capacity)?; + Ok(v) + } + + fn try_push(&mut self, v: T) -> Result<(), TryReserveError> { + if let Err(retry) = self.push_within_capacity(v) { + self.try_reserve(1)?; + let _ = self.push_within_capacity(retry); + } + Ok(()) + } + + fn try_extend_from_slice(&mut self, other: &[T]) -> Result<(), TryReserveError> + where + T: Clone, + { + self.try_reserve(other.len())?; + for item in other { + self.try_push(item.clone())?; + } + + Ok(()) + } +} -- cgit v1.2.3 From 5ab560ce12ed0df3450968cfe4211e398ff2a8d7 Mon Sep 17 00:00:00 2001 From: Wedson Almeida Filho Date: Wed, 27 Mar 2024 22:36:00 -0300 Subject: rust: alloc: update `VecExt` to take allocation flags We also rename the methods by removing the `try_` prefix since the names are available due to our usage of the `no_global_oom_handling` config when building the `alloc` crate. Reviewed-by: Boqun Feng Signed-off-by: Wedson Almeida Filho Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20240328013603.206764-8-wedsonaf@gmail.com Signed-off-by: Miguel Ojeda --- rust/kernel/alloc/vec_ext.rs | 158 +++++++++++++++++++++++++++++++++++++++---- 1 file changed, 143 insertions(+), 15 deletions(-) (limited to 'rust/kernel/alloc/vec_ext.rs') diff --git a/rust/kernel/alloc/vec_ext.rs b/rust/kernel/alloc/vec_ext.rs index 311e62cc5784..e24d7c7675ca 100644 --- a/rust/kernel/alloc/vec_ext.rs +++ b/rust/kernel/alloc/vec_ext.rs @@ -2,47 +2,175 @@ //! Extensions to [`Vec`] for fallible allocations. -use alloc::{collections::TryReserveError, vec::Vec}; +use super::Flags; +use alloc::{alloc::AllocError, vec::Vec}; use core::result::Result; /// Extensions to [`Vec`]. pub trait VecExt: Sized { /// Creates a new [`Vec`] instance with at least the given capacity. - fn try_with_capacity(capacity: usize) -> Result; + /// + /// # Examples + /// + /// ``` + /// let v = Vec::::with_capacity(20, GFP_KERNEL)?; + /// + /// assert!(v.capacity() >= 20); + /// # Ok::<(), Error>(()) + /// ``` + fn with_capacity(capacity: usize, flags: Flags) -> Result; /// Appends an element to the back of the [`Vec`] instance. - fn try_push(&mut self, v: T) -> Result<(), TryReserveError>; + /// + /// # Examples + /// + /// ``` + /// let mut v = Vec::new(); + /// v.push(1, GFP_KERNEL)?; + /// assert_eq!(&v, &[1]); + /// + /// v.push(2, GFP_KERNEL)?; + /// assert_eq!(&v, &[1, 2]); + /// # Ok::<(), Error>(()) + /// ``` + fn push(&mut self, v: T, flags: Flags) -> Result<(), AllocError>; /// Pushes clones of the elements of slice into the [`Vec`] instance. - fn try_extend_from_slice(&mut self, other: &[T]) -> Result<(), TryReserveError> + /// + /// # Examples + /// + /// ``` + /// let mut v = Vec::new(); + /// v.push(1, GFP_KERNEL)?; + /// + /// v.extend_from_slice(&[20, 30, 40], GFP_KERNEL)?; + /// assert_eq!(&v, &[1, 20, 30, 40]); + /// + /// v.extend_from_slice(&[50, 60], GFP_KERNEL)?; + /// assert_eq!(&v, &[1, 20, 30, 40, 50, 60]); + /// # Ok::<(), Error>(()) + /// ``` + fn extend_from_slice(&mut self, other: &[T], flags: Flags) -> Result<(), AllocError> where T: Clone; + + /// Ensures that the capacity exceeds the length by at least `additional` elements. + /// + /// # Examples + /// + /// ``` + /// let mut v = Vec::new(); + /// v.push(1, GFP_KERNEL)?; + /// + /// v.reserve(10, GFP_KERNEL)?; + /// let cap = v.capacity(); + /// assert!(cap >= 10); + /// + /// v.reserve(10, GFP_KERNEL)?; + /// let new_cap = v.capacity(); + /// assert_eq!(new_cap, cap); + /// + /// # Ok::<(), Error>(()) + /// ``` + fn reserve(&mut self, additional: usize, flags: Flags) -> Result<(), AllocError>; } impl VecExt for Vec { - fn try_with_capacity(capacity: usize) -> Result { + fn with_capacity(capacity: usize, flags: Flags) -> Result { let mut v = Vec::new(); - v.try_reserve(capacity)?; + >::reserve(&mut v, capacity, flags)?; Ok(v) } - fn try_push(&mut self, v: T) -> Result<(), TryReserveError> { - if let Err(retry) = self.push_within_capacity(v) { - self.try_reserve(1)?; - let _ = self.push_within_capacity(retry); - } + fn push(&mut self, v: T, flags: Flags) -> Result<(), AllocError> { + >::reserve(self, 1, flags)?; + let s = self.spare_capacity_mut(); + s[0].write(v); + + // SAFETY: We just initialised the first spare entry, so it is safe to increase the length + // by 1. We also know that the new length is <= capacity because of the previous call to + // `reserve` above. + unsafe { self.set_len(self.len() + 1) }; Ok(()) } - fn try_extend_from_slice(&mut self, other: &[T]) -> Result<(), TryReserveError> + fn extend_from_slice(&mut self, other: &[T], flags: Flags) -> Result<(), AllocError> where T: Clone, { - self.try_reserve(other.len())?; - for item in other { - self.try_push(item.clone())?; + >::reserve(self, other.len(), flags)?; + for (slot, item) in core::iter::zip(self.spare_capacity_mut(), other) { + slot.write(item.clone()); } + // SAFETY: We just initialised the `other.len()` spare entries, so it is safe to increase + // the length by the same amount. We also know that the new length is <= capacity because + // of the previous call to `reserve` above. + unsafe { self.set_len(self.len() + other.len()) }; + Ok(()) + } + + #[cfg(any(test, testlib))] + fn reserve(&mut self, additional: usize, _flags: Flags) -> Result<(), AllocError> { + Vec::reserve(self, additional); Ok(()) } + + #[cfg(not(any(test, testlib)))] + fn reserve(&mut self, additional: usize, flags: Flags) -> Result<(), AllocError> { + let len = self.len(); + let cap = self.capacity(); + + if cap - len >= additional { + return Ok(()); + } + + if core::mem::size_of::() == 0 { + // The capacity is already `usize::MAX` for SZTs, we can't go higher. + return Err(AllocError); + } + + // We know cap is <= `isize::MAX` because `Layout::array` fails if the resulting byte size + // is greater than `isize::MAX`. 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 (ptr, len, cap) = destructure(self); + + // SAFETY: `ptr` is valid because it's either NULL or comes from a previous call to + // `krealloc_aligned`. We also verified that the type is not a ZST. + let new_ptr = unsafe { super::allocator::krealloc_aligned(ptr.cast(), layout, flags) }; + if new_ptr.is_null() { + // SAFETY: We are just rebuilding the existing `Vec` with no changes. + unsafe { rebuild(self, ptr, len, cap) }; + Err(AllocError) + } else { + // SAFETY: `ptr` has been reallocated with the layout for `new_cap` elements. New cap + // is greater than `cap`, so it continues to be >= `len`. + unsafe { rebuild(self, new_ptr.cast::(), len, new_cap) }; + Ok(()) + } + } +} + +#[cfg(not(any(test, testlib)))] +fn destructure(v: &mut Vec) -> (*mut T, usize, usize) { + let mut tmp = Vec::new(); + core::mem::swap(&mut tmp, v); + let mut tmp = core::mem::ManuallyDrop::new(tmp); + let len = tmp.len(); + let cap = tmp.capacity(); + (tmp.as_mut_ptr(), len, cap) +} + +/// Rebuilds a `Vec` from a pointer, length, and capacity. +/// +/// # Safety +/// +/// The same as [`Vec::from_raw_parts`]. +#[cfg(not(any(test, testlib)))] +unsafe fn rebuild(v: &mut Vec, ptr: *mut T, len: usize, cap: usize) { + // SAFETY: The safety requirements from this function satisfy those of `from_raw_parts`. + let mut tmp = unsafe { Vec::from_raw_parts(ptr, len, cap) }; + core::mem::swap(&mut tmp, v); } -- cgit v1.2.3 From 2c1092853f163762ef0aabc551a630ef233e1be3 Mon Sep 17 00:00:00 2001 From: Wedson Almeida Filho Date: Wed, 27 Mar 2024 22:36:03 -0300 Subject: rust: kernel: remove usage of `allocator_api` unstable feature With the adoption of `BoxExt` and `VecExt`, we don't need the functions provided by this feature (namely the methods prefixed with `try_` and different allocator per collection instance). We do need `AllocError`, but we define our own as it is a trivial empty struct. Reviewed-by: Benno Lossin Signed-off-by: Wedson Almeida Filho Link: https://lore.kernel.org/r/20240328013603.206764-11-wedsonaf@gmail.com Signed-off-by: Miguel Ojeda --- rust/kernel/alloc/vec_ext.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'rust/kernel/alloc/vec_ext.rs') diff --git a/rust/kernel/alloc/vec_ext.rs b/rust/kernel/alloc/vec_ext.rs index e24d7c7675ca..6a916fcf8bf1 100644 --- a/rust/kernel/alloc/vec_ext.rs +++ b/rust/kernel/alloc/vec_ext.rs @@ -2,8 +2,8 @@ //! Extensions to [`Vec`] for fallible allocations. -use super::Flags; -use alloc::{alloc::AllocError, vec::Vec}; +use super::{AllocError, Flags}; +use alloc::vec::Vec; use core::result::Result; /// Extensions to [`Vec`]. -- cgit v1.2.3 From 00280272a0e5d98055e4d47db38a9b4b5517520e Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Mon, 1 Apr 2024 23:23:02 +0200 Subject: rust: kernel: remove redundant imports Rust's `unused_imports` lint covers both unused and redundant imports. In the upcoming 1.78.0, the lint detects more cases of redundant imports [1], e.g.: error: the item `bindings` is imported redundantly --> rust/kernel/print.rs:38:9 | 38 | use crate::bindings; | ^^^^^^^^^^^^^^^ the item `bindings` is already defined by prelude Most cases are `use crate::bindings`, plus a few other items like `Box`. Thus clean them up. Note that, in the `bindings` case, the message "defined by prelude" above means the extern prelude, i.e. the `--extern` flags we pass. Link: https://github.com/rust-lang/rust/pull/117772 [1] Reviewed-by: Alice Ryhl Link: https://lore.kernel.org/r/20240401212303.537355-3-ojeda@kernel.org Signed-off-by: Miguel Ojeda --- rust/kernel/alloc/vec_ext.rs | 1 - 1 file changed, 1 deletion(-) (limited to 'rust/kernel/alloc/vec_ext.rs') diff --git a/rust/kernel/alloc/vec_ext.rs b/rust/kernel/alloc/vec_ext.rs index 6a916fcf8bf1..25025a36e250 100644 --- a/rust/kernel/alloc/vec_ext.rs +++ b/rust/kernel/alloc/vec_ext.rs @@ -4,7 +4,6 @@ use super::{AllocError, Flags}; use alloc::vec::Vec; -use core::result::Result; /// Extensions to [`Vec`]. pub trait VecExt: Sized { -- cgit v1.2.3 From 97ab3e8eec0ce79d9e265e6c9e4c480492180409 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Wed, 1 May 2024 15:47:43 +0200 Subject: rust: alloc: fix dangling pointer in VecExt::reserve() Currently, a Vec's ptr value, after calling Vec::new(), is initialized to Unique::dangling(). Hence, in VecExt::reserve(), we're passing a dangling pointer (instead of NULL) to krealloc() whenever a new Vec's backing storage is allocated through VecExt extension functions. This only works as long as align_of::(), used by Unique::dangling() to derive the dangling pointer, resolves to a value between 0x0 and ZERO_SIZE_PTR (0x10) and krealloc() hence treats it the same as a NULL pointer however. This isn't a case we should rely on, since there may be types whose alignment may exceed the range still covered by krealloc(), plus other kernel allocators are not as tolerant either. Instead, pass a real NULL pointer to krealloc_aligned() if Vec's capacity is zero. Fixes: 5ab560ce12ed ("rust: alloc: update `VecExt` to take allocation flags") Reviewed-by: Alice Ryhl Reviewed-by: Boqun Feng Reviewed-by: Benno Lossin Signed-off-by: Danilo Krummrich Reviewed-by: Wedson Almeida Filho Link: https://lore.kernel.org/r/20240501134834.22323-1-dakr@redhat.com [ Solved `use` conflict and applied the `if`-instead-of-`match` change discussed in the list. - Miguel ] Signed-off-by: Miguel Ojeda --- rust/kernel/alloc/vec_ext.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) (limited to 'rust/kernel/alloc/vec_ext.rs') diff --git a/rust/kernel/alloc/vec_ext.rs b/rust/kernel/alloc/vec_ext.rs index 25025a36e250..e9a81052728a 100644 --- a/rust/kernel/alloc/vec_ext.rs +++ b/rust/kernel/alloc/vec_ext.rs @@ -4,6 +4,7 @@ use super::{AllocError, Flags}; use alloc::vec::Vec; +use core::ptr; /// Extensions to [`Vec`]. pub trait VecExt: Sized { @@ -134,14 +135,20 @@ impl VecExt for Vec { 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 (ptr, len, cap) = destructure(self); + let (old_ptr, len, cap) = destructure(self); + + // We need to make sure that `ptr` is either NULL or comes from a previous call to + // `krealloc_aligned`. 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. + let ptr = if cap == 0 { ptr::null_mut() } else { old_ptr }; // SAFETY: `ptr` is valid because it's either NULL or comes from a previous call to // `krealloc_aligned`. We also verified that the type is not a ZST. let new_ptr = unsafe { super::allocator::krealloc_aligned(ptr.cast(), layout, flags) }; if new_ptr.is_null() { // SAFETY: We are just rebuilding the existing `Vec` with no changes. - unsafe { rebuild(self, ptr, len, cap) }; + unsafe { rebuild(self, old_ptr, len, cap) }; Err(AllocError) } else { // SAFETY: `ptr` has been reallocated with the layout for `new_cap` elements. New cap -- cgit v1.2.3