From fb1bf1067de979c89ae33589e0466d6ce0dde204 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sat, 15 Mar 2025 16:43:02 +0100 Subject: [PATCH 01/78] rust: alloc: add missing invariant in Vec::set_len() When setting a new length, we have to justify that the set length represents the exact number of elements stored in the vector. Reviewed-by: Benno Lossin Reported-by: Alice Ryhl Closes: https://lore.kernel.org/rust-for-linux/20250311-iov-iter-v1-4-f6c9134ea824@google.com Fixes: 2aac4cd7dae3 ("rust: alloc: implement kernel `Vec` type") Link: https://lore.kernel.org/r/20250315154436.65065-2-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/alloc/kvec.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/rust/kernel/alloc/kvec.rs b/rust/kernel/alloc/kvec.rs index ae9d072741ce..b01dabfe35aa 100644 --- a/rust/kernel/alloc/kvec.rs +++ b/rust/kernel/alloc/kvec.rs @@ -193,6 +193,9 @@ pub fn len(&self) -> usize { #[inline] pub unsafe fn set_len(&mut self, new_len: usize) { debug_assert!(new_len <= self.capacity()); + + // INVARIANT: By the safety requirements of this method `new_len` represents the exact + // number of elements stored within `self`. self.len = new_len; } From 81e1c4dab5d0c508907722f18b028102454d52e6 Mon Sep 17 00:00:00 2001 From: Andrew Ballance Date: Sun, 16 Mar 2025 06:16:42 -0500 Subject: [PATCH 02/78] rust: alloc: add Vec::truncate method Implement the equivalent to the std's Vec::truncate on the kernel's Vec type. Link: https://lore.kernel.org/r/20250316111644.154602-2-andrewjballance@gmail.com Signed-off-by: Andrew Ballance [ Rewrote safety comment of set_len(). - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/alloc/kvec.rs | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/rust/kernel/alloc/kvec.rs b/rust/kernel/alloc/kvec.rs index b01dabfe35aa..8fd941429d7c 100644 --- a/rust/kernel/alloc/kvec.rs +++ b/rust/kernel/alloc/kvec.rs @@ -455,6 +455,41 @@ pub fn reserve(&mut self, additional: usize, flags: Flags) -> Result<(), AllocEr Ok(()) } + + /// Shortens the vector, setting the length to `len` and drops the removed values. + /// If `len` is greater than or equal to the current length, this does nothing. + /// + /// This has no effect on the capacity and will not allocate. + /// + /// # Examples + /// + /// ``` + /// let mut v = kernel::kvec![1, 2, 3]?; + /// v.truncate(1); + /// assert_eq!(v.len(), 1); + /// assert_eq!(&v, &[1]); + /// + /// # Ok::<(), Error>(()) + /// ``` + pub fn truncate(&mut self, len: usize) { + if len >= self.len() { + return; + } + + let drop_range = len..self.len(); + + // SAFETY: `drop_range` is a subrange of `[0, len)` by the bounds check above. + let ptr: *mut [T] = unsafe { self.get_unchecked_mut(drop_range) }; + + // SAFETY: By the above bounds check, it is guaranteed that `len < self.capacity()`. + unsafe { self.set_len(len) }; + + // SAFETY: + // - the dropped values are valid `T`s by the type invariant + // - we are allowed to invalidate [`new_len`, `old_len`) because we just changed the + // len, therefore we have exclusive access to [`new_len`, `old_len`) + unsafe { ptr::drop_in_place(ptr) }; + } } impl Vec { From 1679b7159379d11100e4ab7d1de23c8cd7765aa1 Mon Sep 17 00:00:00 2001 From: Andrew Ballance Date: Sun, 16 Mar 2025 06:16:43 -0500 Subject: [PATCH 03/78] rust: alloc: add Vec::resize method Implement the equivalent of the rust std's Vec::resize on the kernel's Vec type. Reviewed-by: Benno Lossin Reviewed-by: Tamir Duberstein Link: https://lore.kernel.org/r/20250316111644.154602-3-andrewjballance@gmail.com Signed-off-by: Andrew Ballance [ Use checked_sub(), as suggested by Tamir. - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/alloc/kvec.rs | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/rust/kernel/alloc/kvec.rs b/rust/kernel/alloc/kvec.rs index 8fd941429d7c..7ebec5c4a277 100644 --- a/rust/kernel/alloc/kvec.rs +++ b/rust/kernel/alloc/kvec.rs @@ -556,6 +556,33 @@ pub fn from_elem(value: T, n: usize, flags: Flags) -> Result { Ok(v) } + + /// Resizes the [`Vec`] so that `len` is equal to `new_len`. + /// + /// If `new_len` is smaller than `len`, the `Vec` is [`Vec::truncate`]d. + /// If `new_len` is larger, each new slot is filled with clones of `value`. + /// + /// # Examples + /// + /// ``` + /// let mut v = kernel::kvec![1, 2, 3]?; + /// v.resize(1, 42, GFP_KERNEL)?; + /// assert_eq!(&v, &[1]); + /// + /// v.resize(3, 42, GFP_KERNEL)?; + /// assert_eq!(&v, &[1, 42, 42]); + /// + /// # Ok::<(), Error>(()) + /// ``` + pub fn resize(&mut self, new_len: usize, value: T, flags: Flags) -> Result<(), AllocError> { + match new_len.checked_sub(self.len()) { + Some(n) => self.extend_with(n, value, flags), + None => { + self.truncate(new_len); + Ok(()) + } + } + } } impl Drop for Vec From c3152988c047a7b6abb10d4dc5e24fafbabe8b7e Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Tue, 18 Mar 2025 10:52:42 -0400 Subject: [PATCH 04/78] rust: alloc: use `spare_capacity_mut` to reduce unsafe Use `spare_capacity_mut` in the implementation of `push` to reduce the use of `unsafe`. Both methods were added in commit 2aac4cd7dae3 ("rust: alloc: implement kernel `Vec` type"). Reviewed-by: Alice Ryhl Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20250318-vec-push-use-spare-v3-1-68741671d1af@gmail.com Signed-off-by: Tamir Duberstein Signed-off-by: Danilo Krummrich --- rust/kernel/alloc/kvec.rs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/rust/kernel/alloc/kvec.rs b/rust/kernel/alloc/kvec.rs index 7ebec5c4a277..6ac8756989e5 100644 --- a/rust/kernel/alloc/kvec.rs +++ b/rust/kernel/alloc/kvec.rs @@ -288,15 +288,10 @@ pub fn spare_capacity_mut(&mut self) -> &mut [MaybeUninit] { pub fn push(&mut self, v: T, flags: Flags) -> Result<(), AllocError> { self.reserve(1, flags)?; - // SAFETY: - // - `self.len` is smaller than `self.capacity` and hence, the resulting pointer is - // guaranteed to be part of the same allocated object. - // - `self.len` can not overflow `isize`. - let ptr = unsafe { self.as_mut_ptr().add(self.len) }; + let spare = self.spare_capacity_mut(); - // SAFETY: - // - `ptr` is properly aligned and valid for writes. - unsafe { core::ptr::write(ptr, v) }; + // SAFETY: The call to `reserve` was successful so the spare capacity is at least 1. + unsafe { spare.get_unchecked_mut(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 From 2cce50f5ab83d01cc3aafa3fb55cba97584aee51 Mon Sep 17 00:00:00 2001 From: Benno Lossin Date: Wed, 16 Apr 2025 22:50:15 +0000 Subject: [PATCH 05/78] rust: pin-init: synchronize README.md The upstream version of the `README.md` differs by this change, so synchronize it. The reason that this wasn't in the original sync patch is that this was a late change that I didn't port back to the kernel repo, since it was generated by `cargo rdme`. Link: https://lore.kernel.org/all/20250416225002.25253-1-benno.lossin@proton.me Signed-off-by: Benno Lossin --- rust/pin-init/README.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/rust/pin-init/README.md b/rust/pin-init/README.md index 3d04796b212b..24d0f0a3f8fb 100644 --- a/rust/pin-init/README.md +++ b/rust/pin-init/README.md @@ -218,11 +218,13 @@ the `kernel` crate. The [`sync`] module is a good starting point. [pinning]: https://doc.rust-lang.org/std/pin/index.html [structurally pinned fields]: https://doc.rust-lang.org/std/pin/index.html#pinning-is-structural-for-field [stack]: https://docs.rs/pin-init/latest/pin_init/macro.stack_pin_init.html -[`Arc`]: https://doc.rust-lang.org/stable/alloc/sync/struct.Arc.html -[`Box`]: https://doc.rust-lang.org/stable/alloc/boxed/struct.Box.html [`impl PinInit`]: https://docs.rs/pin-init/latest/pin_init/trait.PinInit.html [`impl PinInit`]: https://docs.rs/pin-init/latest/pin_init/trait.PinInit.html [`impl Init`]: https://docs.rs/pin-init/latest/pin_init/trait.Init.html [Rust-for-Linux]: https://rust-for-linux.com/ + + +[`Arc`]: https://doc.rust-lang.org/stable/alloc/sync/struct.Arc.html +[`Box`]: https://doc.rust-lang.org/stable/alloc/boxed/struct.Box.html From 31005209b88c83adc4b4ef63fbc02867cddb2689 Mon Sep 17 00:00:00 2001 From: Benno Lossin Date: Mon, 14 Apr 2025 20:00:03 +0000 Subject: [PATCH 06/78] rust: pin-init: internal: skip rustfmt formatting of kernel-only module The `quote` module only is available in the kernel and thus running `cargo fmt` or `rustfmt internal/src/lib.rs` in the user-space repository [1] results in: error: couldn't read `~/pin-init/internal/src/../../../macros/quote.rs`: No such file or directory (os error 2) --> ~/pin-init/internal/src/lib.rs:25:1 | 25 | mod quote; | ^^^^^^^^^^ Error writing files: failed to resolve mod `quote`: ~/pin-init/internal/src/../../../macros/quote.rs does not exist Thus mark it with `rustfmt::skip` when compiling without kernel support. Link: https://github.com/Rust-for-Linux/pin-init [1] Link: https://github.com/Rust-for-Linux/pin-init/pull/33/commits/a6caf1945e51da38761aab4dffa56e63e2baa218 Link: https://lore.kernel.org/all/20250414195928.129040-2-benno.lossin@proton.me Reviewed-by: Christian Schrefl Signed-off-by: Benno Lossin --- rust/pin-init/internal/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/rust/pin-init/internal/src/lib.rs b/rust/pin-init/internal/src/lib.rs index babe5e878550..56aa9ecc1e1a 100644 --- a/rust/pin-init/internal/src/lib.rs +++ b/rust/pin-init/internal/src/lib.rs @@ -22,6 +22,7 @@ #[cfg(kernel)] #[path = "../../../macros/quote.rs"] #[macro_use] +#[cfg_attr(not(kernel), rustfmt::skip)] mod quote; #[cfg(not(kernel))] #[macro_use] From 5c4167b4056c9db951788541e23585dd4a1fa426 Mon Sep 17 00:00:00 2001 From: Benno Lossin Date: Mon, 14 Apr 2025 20:00:20 +0000 Subject: [PATCH 07/78] rust: pin-init: examples: conditionally enable `feature(lint_reasons)` `lint_reasons` is unstable in Rust 1.80 and earlier, enable it conditionally in the examples to allow compiling them with older compilers. Link: https://github.com/Rust-for-Linux/pin-init/pull/33/commits/ec494fe686b0a97d5b59b5be5a42d3858038ea6a Link: https://lore.kernel.org/all/20250414195928.129040-3-benno.lossin@proton.me Signed-off-by: Benno Lossin --- rust/pin-init/examples/linked_list.rs | 1 + rust/pin-init/examples/mutex.rs | 1 + rust/pin-init/examples/pthread_mutex.rs | 2 ++ rust/pin-init/examples/static_init.rs | 1 + 4 files changed, 5 insertions(+) diff --git a/rust/pin-init/examples/linked_list.rs b/rust/pin-init/examples/linked_list.rs index 6d7eb0a0ec0d..0bbc7b8d83a1 100644 --- a/rust/pin-init/examples/linked_list.rs +++ b/rust/pin-init/examples/linked_list.rs @@ -2,6 +2,7 @@ #![allow(clippy::undocumented_unsafe_blocks)] #![cfg_attr(feature = "alloc", feature(allocator_api))] +#![cfg_attr(not(RUSTC_LINT_REASONS_IS_STABLE), feature(lint_reasons))] use core::{ cell::Cell, diff --git a/rust/pin-init/examples/mutex.rs b/rust/pin-init/examples/mutex.rs index 073bb79341d1..3e3630780c96 100644 --- a/rust/pin-init/examples/mutex.rs +++ b/rust/pin-init/examples/mutex.rs @@ -2,6 +2,7 @@ #![allow(clippy::undocumented_unsafe_blocks)] #![cfg_attr(feature = "alloc", feature(allocator_api))] +#![cfg_attr(not(RUSTC_LINT_REASONS_IS_STABLE), feature(lint_reasons))] #![allow(clippy::missing_safety_doc)] use core::{ diff --git a/rust/pin-init/examples/pthread_mutex.rs b/rust/pin-init/examples/pthread_mutex.rs index 5ac22f1880d2..074ebf485a97 100644 --- a/rust/pin-init/examples/pthread_mutex.rs +++ b/rust/pin-init/examples/pthread_mutex.rs @@ -3,6 +3,8 @@ // inspired by #![allow(clippy::undocumented_unsafe_blocks)] #![cfg_attr(feature = "alloc", feature(allocator_api))] +#![cfg_attr(not(RUSTC_LINT_REASONS_IS_STABLE), feature(lint_reasons))] + #[cfg(not(windows))] mod pthread_mtx { #[cfg(feature = "alloc")] diff --git a/rust/pin-init/examples/static_init.rs b/rust/pin-init/examples/static_init.rs index 3487d761aa26..48531413ab94 100644 --- a/rust/pin-init/examples/static_init.rs +++ b/rust/pin-init/examples/static_init.rs @@ -2,6 +2,7 @@ #![allow(clippy::undocumented_unsafe_blocks)] #![cfg_attr(feature = "alloc", feature(allocator_api))] +#![cfg_attr(not(RUSTC_LINT_REASONS_IS_STABLE), feature(lint_reasons))] use core::{ cell::{Cell, UnsafeCell}, From 39051adb070432b283e6c11b2b24937281b9f97f Mon Sep 17 00:00:00 2001 From: Benno Lossin Date: Mon, 14 Apr 2025 20:00:31 +0000 Subject: [PATCH 08/78] rust: pin-init: examples: use `allow` instead of `expect` Rust 1.78 doesn't emit a `dead_code` error on the annotated element, resulting in the `unfulfilled_lint_expectations` error. Rust 1.85 does emit the `dead_code` error, so we still need an `allow`. Link: https://github.com/Rust-for-Linux/pin-init/pull/33/commits/0e28cbb895bd29f896a59b40e8ed506ea7bef13c Link: https://lore.kernel.org/all/20250414195928.129040-4-benno.lossin@proton.me Signed-off-by: Benno Lossin --- rust/pin-init/examples/pthread_mutex.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/pin-init/examples/pthread_mutex.rs b/rust/pin-init/examples/pthread_mutex.rs index 074ebf485a97..5acc5108b954 100644 --- a/rust/pin-init/examples/pthread_mutex.rs +++ b/rust/pin-init/examples/pthread_mutex.rs @@ -42,7 +42,7 @@ fn drop(self: Pin<&mut Self>) { #[derive(Debug)] pub enum Error { - #[expect(dead_code)] + #[allow(dead_code)] IO(std::io::Error), Alloc, } From 85f8e98dbb0135d2bc1999c6015cd374fe2c69fa Mon Sep 17 00:00:00 2001 From: Alexandre Courbot Date: Sat, 12 Apr 2025 15:29:13 +0900 Subject: [PATCH 09/78] rust: alloc: allow coercion from `Box` to `Box` if T implements U This enables the creation of trait objects backed by a Box, similarly to what can be done with the standard library. Suggested-by: Benno Lossin Reviewed-by: Alice Ryhl Reviewed-by: Benno Lossin Signed-off-by: Alexandre Courbot Link: https://lore.kernel.org/r/20250412-box_trait_objs-v3-1-f67ced62d520@nvidia.com Signed-off-by: Danilo Krummrich --- rust/kernel/alloc/kbox.rs | 40 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/rust/kernel/alloc/kbox.rs b/rust/kernel/alloc/kbox.rs index b77d32f3a58b..e043bf2f4687 100644 --- a/rust/kernel/alloc/kbox.rs +++ b/rust/kernel/alloc/kbox.rs @@ -57,12 +57,50 @@ /// assert!(KVBox::::new_uninit(GFP_KERNEL).is_ok()); /// ``` /// +/// [`Box`]es can also be used to store trait objects by coercing their type: +/// +/// ``` +/// trait FooTrait {} +/// +/// struct FooStruct; +/// impl FooTrait for FooStruct {} +/// +/// let _ = KBox::new(FooStruct, GFP_KERNEL)? as KBox; +/// # Ok::<(), Error>(()) +/// ``` +/// /// # Invariants /// /// `self.0` is always properly aligned and either points to memory allocated with `A` or, for /// zero-sized types, is a dangling, well aligned pointer. #[repr(transparent)] -pub struct Box(NonNull, PhantomData); +#[cfg_attr(CONFIG_RUSTC_HAS_COERCE_POINTEE, derive(core::marker::CoercePointee))] +pub struct Box<#[cfg_attr(CONFIG_RUSTC_HAS_COERCE_POINTEE, pointee)] T: ?Sized, A: Allocator>( + NonNull, + PhantomData, +); + +// This is to allow coercion from `Box` to `Box` if `T` can be converted to the +// dynamically-sized type (DST) `U`. +#[cfg(not(CONFIG_RUSTC_HAS_COERCE_POINTEE))] +impl core::ops::CoerceUnsized> for Box +where + T: ?Sized + core::marker::Unsize, + U: ?Sized, + A: Allocator, +{ +} + +// This is to allow `Box` to be dispatched on when `Box` can be coerced into `Box`. +#[cfg(not(CONFIG_RUSTC_HAS_COERCE_POINTEE))] +impl core::ops::DispatchFromDyn> for Box +where + T: ?Sized + core::marker::Unsize, + U: ?Sized, + A: Allocator, +{ +} /// Type alias for [`Box`] with a [`Kmalloc`] allocator. /// From 47a17a63f9e23f7e8f39d0965bcda8fee6c322f8 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Wed, 16 Apr 2025 13:15:40 -0400 Subject: [PATCH 10/78] rust: alloc: add Vec::len() <= Vec::capacity invariant Document the invariant that the vector's length is always less than or equal to its capacity. This is already implied by these other invariants: - `self.len` always represents the exact number of elements stored in the vector. - `self.layout` represents the absolute number of elements that can be stored within the vector without re-allocation. but it doesn't hurt to spell it out. Note that the language references `self.capacity` rather than `self.layout.len` as the latter is zero for a vector of ZSTs. Update a safety comment touched by this patch to correctly reference `realloc` rather than `alloc` and replace "leaves" with "leave" to improve grammar. Reviewed-by: Alice Ryhl Signed-off-by: Tamir Duberstein Link: https://lore.kernel.org/r/20250416-vec-set-len-v4-1-112b222604cd@gmail.com Signed-off-by: Danilo Krummrich --- rust/kernel/alloc/kvec.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/rust/kernel/alloc/kvec.rs b/rust/kernel/alloc/kvec.rs index 6ac8756989e5..ca30fad90de5 100644 --- a/rust/kernel/alloc/kvec.rs +++ b/rust/kernel/alloc/kvec.rs @@ -90,6 +90,8 @@ macro_rules! kvec { /// without re-allocation. For ZSTs `self.layout`'s capacity is zero. However, it is legal for the /// backing buffer to be larger than `layout`. /// +/// - `self.len()` is always less than or equal to `self.capacity()`. +/// /// - The `Allocator` type `A` of the vector is the exact same `Allocator` type the backing buffer /// was allocated with (and must be freed with). pub struct Vec { @@ -262,8 +264,8 @@ pub const fn new() -> Self { /// Returns a slice of `MaybeUninit` for the remaining spare capacity of the vector. pub fn spare_capacity_mut(&mut self) -> &mut [MaybeUninit] { // SAFETY: - // - `self.len` is smaller than `self.capacity` and hence, the resulting pointer is - // guaranteed to be part of the same allocated object. + // - `self.len` is smaller than `self.capacity` by the type invariant and hence, the + // resulting pointer is guaranteed to be part of the same allocated object. // - `self.len` can not overflow `isize`. let ptr = unsafe { self.as_mut_ptr().add(self.len) } as *mut MaybeUninit; @@ -817,12 +819,13 @@ pub fn collect(self, flags: Flags) -> Vec { unsafe { ptr::copy(ptr, buf.as_ptr(), len) }; ptr = buf.as_ptr(); - // SAFETY: `len` is guaranteed to be smaller than `self.layout.len()`. + // SAFETY: `len` is guaranteed to be smaller than `self.layout.len()` by the type + // invariant. let layout = unsafe { ArrayLayout::::new_unchecked(len) }; - // 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. + // SAFETY: `buf` points to the start of the backing buffer and `len` is guaranteed by + // the type invariant to be smaller than `cap`. Depending on `realloc` this operation + // may shrink the buffer or leave it as it is. ptr = match unsafe { A::realloc(Some(buf.cast()), layout.into(), old_layout.into(), flags) } { From dbb0b840a0cd2ebabbc94a0040e366c7f1a70f7b Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Wed, 16 Apr 2025 13:15:41 -0400 Subject: [PATCH 11/78] rust: alloc: add `Vec::dec_len` Add `Vec::dec_len` that reduces the length of the receiver. This method is intended to be used from methods that remove elements from `Vec` such as `truncate`, `pop`, `remove`, and others. This method is intentionally not `pub`. Reviewed-by: Alice Ryhl Signed-off-by: Tamir Duberstein Link: https://lore.kernel.org/r/20250416-vec-set-len-v4-2-112b222604cd@gmail.com [ Add #[expect(unused)] to dec_len(). - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/alloc/kvec.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/rust/kernel/alloc/kvec.rs b/rust/kernel/alloc/kvec.rs index ca30fad90de5..7671867165a0 100644 --- a/rust/kernel/alloc/kvec.rs +++ b/rust/kernel/alloc/kvec.rs @@ -201,6 +201,26 @@ pub unsafe fn set_len(&mut self, new_len: usize) { self.len = new_len; } + /// Decreases `self.len` by `count`. + /// + /// Returns a mutable slice to the elements forgotten by the vector. It is the caller's + /// responsibility to drop these elements if necessary. + /// + /// # Safety + /// + /// - `count` must be less than or equal to `self.len`. + #[expect(unused)] + unsafe fn dec_len(&mut self, count: usize) -> &mut [T] { + debug_assert!(count <= self.len()); + // INVARIANT: We relinquish ownership of the elements within the range `[self.len - count, + // self.len)`, hence the updated value of `set.len` represents the exact number of elements + // stored within `self`. + self.len -= count; + // SAFETY: The memory after `self.len()` is guaranteed to contain `count` initialized + // elements of type `T`. + unsafe { slice::from_raw_parts_mut(self.as_mut_ptr().add(self.len), count) } + } + /// Returns a slice of the entire vector. #[inline] pub fn as_slice(&self) -> &[T] { From 1b04b466c873f62413bf65a05a558f036660aedc Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Wed, 16 Apr 2025 13:15:42 -0400 Subject: [PATCH 12/78] rust: alloc: refactor `Vec::truncate` using `dec_len` Use `checked_sub` to satisfy the safety requirements of `dec_len` and replace nearly the whole body of `truncate` with a call to `dec_len`. Reviewed-by: Andrew Ballance Reviewed-by: Alice Ryhl Signed-off-by: Tamir Duberstein Link: https://lore.kernel.org/r/20250416-vec-set-len-v4-3-112b222604cd@gmail.com [ Remove #[expect(unused)] from dec_len(). - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/alloc/kvec.rs | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/rust/kernel/alloc/kvec.rs b/rust/kernel/alloc/kvec.rs index 7671867165a0..87dc37ecb94d 100644 --- a/rust/kernel/alloc/kvec.rs +++ b/rust/kernel/alloc/kvec.rs @@ -209,7 +209,6 @@ pub unsafe fn set_len(&mut self, new_len: usize) { /// # Safety /// /// - `count` must be less than or equal to `self.len`. - #[expect(unused)] unsafe fn dec_len(&mut self, count: usize) -> &mut [T] { debug_assert!(count <= self.len()); // INVARIANT: We relinquish ownership of the elements within the range `[self.len - count, @@ -489,23 +488,15 @@ pub fn reserve(&mut self, additional: usize, flags: Flags) -> Result<(), AllocEr /// # Ok::<(), Error>(()) /// ``` pub fn truncate(&mut self, len: usize) { - if len >= self.len() { - return; + if let Some(count) = self.len().checked_sub(len) { + // SAFETY: `count` is `self.len() - len` so it is guaranteed to be less than or + // equal to `self.len()`. + let ptr: *mut [T] = unsafe { self.dec_len(count) }; + + // SAFETY: the contract of `dec_len` guarantees that the elements in `ptr` are + // valid elements whose ownership has been transferred to the caller. + unsafe { ptr::drop_in_place(ptr) }; } - - let drop_range = len..self.len(); - - // SAFETY: `drop_range` is a subrange of `[0, len)` by the bounds check above. - let ptr: *mut [T] = unsafe { self.get_unchecked_mut(drop_range) }; - - // SAFETY: By the above bounds check, it is guaranteed that `len < self.capacity()`. - unsafe { self.set_len(len) }; - - // SAFETY: - // - the dropped values are valid `T`s by the type invariant - // - we are allowed to invalidate [`new_len`, `old_len`) because we just changed the - // len, therefore we have exclusive access to [`new_len`, `old_len`) - unsafe { ptr::drop_in_place(ptr) }; } } From 88d5d6a38d5161228fbfe017eb94d777d5e8a0e4 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Wed, 16 Apr 2025 13:15:43 -0400 Subject: [PATCH 13/78] rust: alloc: replace `Vec::set_len` with `inc_len` Rename `set_len` to `inc_len` and simplify its safety contract. Note that the usage in `CString::try_from_fmt` remains correct as the receiver is known to have `len == 0`. Reviewed-by: Alice Ryhl Signed-off-by: Tamir Duberstein Link: https://lore.kernel.org/r/20250416-vec-set-len-v4-4-112b222604cd@gmail.com Signed-off-by: Danilo Krummrich --- rust/kernel/alloc/kvec.rs | 25 ++++++++++++------------- rust/kernel/str.rs | 2 +- rust/kernel/uaccess.rs | 2 +- 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/rust/kernel/alloc/kvec.rs b/rust/kernel/alloc/kvec.rs index 87dc37ecb94d..5798e2c890a2 100644 --- a/rust/kernel/alloc/kvec.rs +++ b/rust/kernel/alloc/kvec.rs @@ -185,20 +185,19 @@ pub fn len(&self) -> usize { self.len } - /// Forcefully sets `self.len` to `new_len`. + /// Increments `self.len` by `additional`. /// /// # Safety /// - /// - `new_len` must be less than or equal to [`Self::capacity`]. - /// - If `new_len` is greater than `self.len`, all elements within the interval - /// [`self.len`,`new_len`) must be initialized. + /// - `additional` must be less than or equal to `self.capacity - self.len`. + /// - All elements within the interval [`self.len`,`self.len + additional`) must be initialized. #[inline] - pub unsafe fn set_len(&mut self, new_len: usize) { - debug_assert!(new_len <= self.capacity()); - - // INVARIANT: By the safety requirements of this method `new_len` represents the exact - // number of elements stored within `self`. - self.len = new_len; + pub unsafe fn inc_len(&mut self, additional: usize) { + // Guaranteed by the type invariant to never underflow. + debug_assert!(additional <= self.capacity() - self.len()); + // INVARIANT: By the safety requirements of this method this represents the exact number of + // elements stored within `self`. + self.len += additional; } /// Decreases `self.len` by `count`. @@ -317,7 +316,7 @@ pub fn push(&mut self, v: T, flags: Flags) -> Result<(), AllocError> { // 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) }; + unsafe { self.inc_len(1) }; Ok(()) } @@ -521,7 +520,7 @@ pub fn extend_with(&mut self, n: usize, value: T, flags: Flags) -> Result<(), Al // SAFETY: // - `self.len() + n < self.capacity()` due to the call to reserve above, // - the loop and the line above initialized the next `n` elements. - unsafe { self.set_len(self.len() + n) }; + unsafe { self.inc_len(n) }; Ok(()) } @@ -552,7 +551,7 @@ pub fn extend_from_slice(&mut self, other: &[T], flags: Flags) -> Result<(), All // the length by the same number. // - `self.len() + other.len() <= self.capacity()` is guaranteed by the preceding `reserve` // call. - unsafe { self.set_len(self.len() + other.len()) }; + unsafe { self.inc_len(other.len()) }; Ok(()) } diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs index 878111cb77bc..d3b0b00e05fa 100644 --- a/rust/kernel/str.rs +++ b/rust/kernel/str.rs @@ -886,7 +886,7 @@ pub fn try_from_fmt(args: fmt::Arguments<'_>) -> Result { // SAFETY: The number of bytes that can be written to `f` is bounded by `size`, which is // `buf`'s capacity. The contents of the buffer have been initialised by writes to `f`. - unsafe { buf.set_len(f.bytes_written()) }; + unsafe { buf.inc_len(f.bytes_written()) }; // Check that there are no `NUL` bytes before the end. // SAFETY: The buffer is valid for read because `f.bytes_written()` is bounded by `size` diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs index 80a9782b1c6e..e4882f113d79 100644 --- a/rust/kernel/uaccess.rs +++ b/rust/kernel/uaccess.rs @@ -290,7 +290,7 @@ pub fn read_all(mut self, buf: &mut Vec, flags: Flags) -> R // SAFETY: Since the call to `read_raw` was successful, so the next `len` bytes of the // vector have been initialized. - unsafe { buf.set_len(buf.len() + len) }; + unsafe { buf.inc_len(len) }; Ok(()) } } From 1116f0c5ff3385658ceb8ae2c5c4cb05bd7836d7 Mon Sep 17 00:00:00 2001 From: FUJITA Tomonori Date: Thu, 24 Apr 2025 04:28:51 +0900 Subject: [PATCH 14/78] rust: hrtimer: Add Ktime temporarily Add Ktime temporarily until hrtimer is refactored to use Instant and Delta types. Reviewed-by: Andreas Hindborg Reviewed-by: Boqun Feng Signed-off-by: FUJITA Tomonori Link: https://lore.kernel.org/r/20250423192857.199712-2-fujita.tomonori@gmail.com Signed-off-by: Andreas Hindborg --- rust/kernel/time/hrtimer.rs | 18 +++++++++++++++++- rust/kernel/time/hrtimer/arc.rs | 2 +- rust/kernel/time/hrtimer/pin.rs | 2 +- rust/kernel/time/hrtimer/pin_mut.rs | 4 ++-- rust/kernel/time/hrtimer/tbox.rs | 2 +- 5 files changed, 22 insertions(+), 6 deletions(-) diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs index ce53f8579d18..17824aa0c0f3 100644 --- a/rust/kernel/time/hrtimer.rs +++ b/rust/kernel/time/hrtimer.rs @@ -68,10 +68,26 @@ //! `start` operation. use super::ClockId; -use crate::{prelude::*, time::Ktime, types::Opaque}; +use crate::{prelude::*, types::Opaque}; use core::marker::PhantomData; use pin_init::PinInit; +/// A Rust wrapper around a `ktime_t`. +// NOTE: Ktime is going to be removed when hrtimer is converted to Instant/Delta. +#[repr(transparent)] +#[derive(Copy, Clone, PartialEq, PartialOrd, Eq, Ord)] +pub struct Ktime { + inner: bindings::ktime_t, +} + +impl Ktime { + /// Returns the number of nanoseconds. + #[inline] + pub fn to_ns(self) -> i64 { + self.inner + } +} + /// A timer backed by a C `struct hrtimer`. /// /// # Invariants diff --git a/rust/kernel/time/hrtimer/arc.rs b/rust/kernel/time/hrtimer/arc.rs index 4a984d85b4a1..ccf1e66e5b2d 100644 --- a/rust/kernel/time/hrtimer/arc.rs +++ b/rust/kernel/time/hrtimer/arc.rs @@ -5,10 +5,10 @@ use super::HrTimerCallback; use super::HrTimerHandle; use super::HrTimerPointer; +use super::Ktime; use super::RawHrTimerCallback; use crate::sync::Arc; use crate::sync::ArcBorrow; -use crate::time::Ktime; /// A handle for an `Arc>` returned by a call to /// [`HrTimerPointer::start`]. diff --git a/rust/kernel/time/hrtimer/pin.rs b/rust/kernel/time/hrtimer/pin.rs index f760db265c7b..293ca9cf058c 100644 --- a/rust/kernel/time/hrtimer/pin.rs +++ b/rust/kernel/time/hrtimer/pin.rs @@ -4,9 +4,9 @@ use super::HrTimer; use super::HrTimerCallback; use super::HrTimerHandle; +use super::Ktime; use super::RawHrTimerCallback; use super::UnsafeHrTimerPointer; -use crate::time::Ktime; use core::pin::Pin; /// A handle for a `Pin<&HasHrTimer>`. When the handle exists, the timer might be diff --git a/rust/kernel/time/hrtimer/pin_mut.rs b/rust/kernel/time/hrtimer/pin_mut.rs index 90c0351d62e4..6033572d35ad 100644 --- a/rust/kernel/time/hrtimer/pin_mut.rs +++ b/rust/kernel/time/hrtimer/pin_mut.rs @@ -1,9 +1,9 @@ // SPDX-License-Identifier: GPL-2.0 use super::{ - HasHrTimer, HrTimer, HrTimerCallback, HrTimerHandle, RawHrTimerCallback, UnsafeHrTimerPointer, + HasHrTimer, HrTimer, HrTimerCallback, HrTimerHandle, Ktime, RawHrTimerCallback, + UnsafeHrTimerPointer, }; -use crate::time::Ktime; use core::{marker::PhantomData, pin::Pin, ptr::NonNull}; /// A handle for a `Pin<&mut HasHrTimer>`. When the handle exists, the timer might diff --git a/rust/kernel/time/hrtimer/tbox.rs b/rust/kernel/time/hrtimer/tbox.rs index 2071cae07234..29526a5da203 100644 --- a/rust/kernel/time/hrtimer/tbox.rs +++ b/rust/kernel/time/hrtimer/tbox.rs @@ -5,9 +5,9 @@ use super::HrTimerCallback; use super::HrTimerHandle; use super::HrTimerPointer; +use super::Ktime; use super::RawHrTimerCallback; use crate::prelude::*; -use crate::time::Ktime; use core::ptr::NonNull; /// A handle for a [`Box>`] returned by a call to From 3caad57d29b5f64fa41cff0b12cc5d9144dacb04 Mon Sep 17 00:00:00 2001 From: FUJITA Tomonori Date: Thu, 24 Apr 2025 04:28:52 +0900 Subject: [PATCH 15/78] rust: time: Add PartialEq/Eq/PartialOrd/Ord trait to Ktime Add PartialEq/Eq/PartialOrd/Ord trait to Ktime so two Ktime instances can be compared to determine whether a timeout is met or not. Use the derive implements; we directly touch C's ktime_t rather than using the C's accessors because it is more efficient and we already do in the existing code (Ktime::sub). Reviewed-by: Trevor Gross Reviewed-by: Alice Ryhl Reviewed-by: Gary Guo Reviewed-by: Fiona Behrens Tested-by: Daniel Almeida Reviewed-by: Andreas Hindborg Signed-off-by: FUJITA Tomonori Link: https://lore.kernel.org/r/20250423192857.199712-3-fujita.tomonori@gmail.com Signed-off-by: Andreas Hindborg --- rust/kernel/time.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs index f509cb0eb71e..9d57e8a5552a 100644 --- a/rust/kernel/time.rs +++ b/rust/kernel/time.rs @@ -29,7 +29,7 @@ pub fn msecs_to_jiffies(msecs: Msecs) -> Jiffies { /// A Rust wrapper around a `ktime_t`. #[repr(transparent)] -#[derive(Copy, Clone)] +#[derive(Copy, Clone, PartialEq, PartialOrd, Eq, Ord)] pub struct Ktime { inner: bindings::ktime_t, } From fae0cdc12340ce402a4681dba0f357b05d167d00 Mon Sep 17 00:00:00 2001 From: FUJITA Tomonori Date: Thu, 24 Apr 2025 04:28:53 +0900 Subject: [PATCH 16/78] rust: time: Introduce Delta type Introduce a type representing a span of time. Define our own type because `core::time::Duration` is large and could panic during creation. time::Ktime could be also used for time duration but timestamp and timedelta are different so better to use a new type. i64 is used instead of u64 to represent a span of time; some C drivers uses negative Deltas and i64 is more compatible with Ktime using i64 too (e.g., ktime_[us|ms]_delta() APIs return i64 so we create Delta object without type conversion. i64 is used instead of bindings::ktime_t because when the ktime_t type is used as timestamp, it represents values from 0 to KTIME_MAX, which is different from Delta. as_millis() method isn't used in this patchset. It's planned to be used in Binder driver. Reviewed-by: Andrew Lunn Reviewed-by: Alice Ryhl Reviewed-by: Gary Guo Reviewed-by: Fiona Behrens Tested-by: Daniel Almeida Reviewed-by: Andreas Hindborg Signed-off-by: FUJITA Tomonori Link: https://lore.kernel.org/r/20250423192857.199712-4-fujita.tomonori@gmail.com Signed-off-by: Andreas Hindborg --- rust/kernel/time.rs | 88 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs index 9d57e8a5552a..e00b9a853e6a 100644 --- a/rust/kernel/time.rs +++ b/rust/kernel/time.rs @@ -10,9 +10,15 @@ pub mod hrtimer; +/// The number of nanoseconds per microsecond. +pub const NSEC_PER_USEC: i64 = bindings::NSEC_PER_USEC as i64; + /// The number of nanoseconds per millisecond. pub const NSEC_PER_MSEC: i64 = bindings::NSEC_PER_MSEC as i64; +/// The number of nanoseconds per second. +pub const NSEC_PER_SEC: i64 = bindings::NSEC_PER_SEC as i64; + /// The time unit of Linux kernel. One jiffy equals (1/HZ) second. pub type Jiffies = crate::ffi::c_ulong; @@ -149,3 +155,85 @@ fn into_c(self) -> bindings::clockid_t { self as bindings::clockid_t } } + +/// A span of time. +/// +/// This struct represents a span of time, with its value stored as nanoseconds. +/// The value can represent any valid i64 value, including negative, zero, and +/// positive numbers. +#[derive(Copy, Clone, PartialEq, PartialOrd, Eq, Ord, Debug)] +pub struct Delta { + nanos: i64, +} + +impl Delta { + /// A span of time equal to zero. + pub const ZERO: Self = Self { nanos: 0 }; + + /// Create a new [`Delta`] from a number of microseconds. + /// + /// The `micros` can range from -9_223_372_036_854_775 to 9_223_372_036_854_775. + /// If `micros` is outside this range, `i64::MIN` is used for negative values, + /// and `i64::MAX` is used for positive values due to saturation. + #[inline] + pub const fn from_micros(micros: i64) -> Self { + Self { + nanos: micros.saturating_mul(NSEC_PER_USEC), + } + } + + /// Create a new [`Delta`] from a number of milliseconds. + /// + /// The `millis` can range from -9_223_372_036_854 to 9_223_372_036_854. + /// If `millis` is outside this range, `i64::MIN` is used for negative values, + /// and `i64::MAX` is used for positive values due to saturation. + #[inline] + pub const fn from_millis(millis: i64) -> Self { + Self { + nanos: millis.saturating_mul(NSEC_PER_MSEC), + } + } + + /// Create a new [`Delta`] from a number of seconds. + /// + /// The `secs` can range from -9_223_372_036 to 9_223_372_036. + /// If `secs` is outside this range, `i64::MIN` is used for negative values, + /// and `i64::MAX` is used for positive values due to saturation. + #[inline] + pub const fn from_secs(secs: i64) -> Self { + Self { + nanos: secs.saturating_mul(NSEC_PER_SEC), + } + } + + /// Return `true` if the [`Delta`] spans no time. + #[inline] + pub fn is_zero(self) -> bool { + self.as_nanos() == 0 + } + + /// Return `true` if the [`Delta`] spans a negative amount of time. + #[inline] + pub fn is_negative(self) -> bool { + self.as_nanos() < 0 + } + + /// Return the number of nanoseconds in the [`Delta`]. + #[inline] + pub const fn as_nanos(self) -> i64 { + self.nanos + } + + /// Return the smallest number of microseconds greater than or equal + /// to the value in the [`Delta`]. + #[inline] + pub const fn as_micros_ceil(self) -> i64 { + self.as_nanos().saturating_add(NSEC_PER_USEC - 1) / NSEC_PER_USEC + } + + /// Return the number of milliseconds in the [`Delta`]. + #[inline] + pub const fn as_millis(self) -> i64 { + self.as_nanos() / NSEC_PER_MSEC + } +} From ddc671506458849c1a1c882208bbffed033e770c Mon Sep 17 00:00:00 2001 From: FUJITA Tomonori Date: Thu, 24 Apr 2025 04:28:54 +0900 Subject: [PATCH 17/78] rust: time: Introduce Instant type Introduce a type representing a specific point in time. We could use the Ktime type but C's ktime_t is used for both timestamp and timedelta. To avoid confusion, introduce a new Instant type for timestamp. Rename Ktime to Instant and modify their methods for timestamp. Implement the subtraction operator for Instant: Delta = Instant A - Instant B Reviewed-by: Boqun Feng Reviewed-by: Gary Guo Reviewed-by: Fiona Behrens Tested-by: Daniel Almeida Reviewed-by: Andreas Hindborg Signed-off-by: FUJITA Tomonori Link: https://lore.kernel.org/r/20250423192857.199712-5-fujita.tomonori@gmail.com Signed-off-by: Andreas Hindborg --- rust/kernel/time.rs | 91 +++++++++++++++++++++++---------------------- 1 file changed, 46 insertions(+), 45 deletions(-) diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs index e00b9a853e6a..a8089a98da9e 100644 --- a/rust/kernel/time.rs +++ b/rust/kernel/time.rs @@ -5,6 +5,22 @@ //! This module contains the kernel APIs related to time and timers that //! have been ported or wrapped for usage by Rust code in the kernel. //! +//! There are two types in this module: +//! +//! - The [`Instant`] type represents a specific point in time. +//! - The [`Delta`] type represents a span of time. +//! +//! Note that the C side uses `ktime_t` type to represent both. However, timestamp +//! and timedelta are different. To avoid confusion, we use two different types. +//! +//! A [`Instant`] object can be created by calling the [`Instant::now()`] function. +//! It represents a point in time at which the object was created. +//! By calling the [`Instant::elapsed()`] method, a [`Delta`] object representing +//! the elapsed time can be created. The [`Delta`] object can also be created +//! by subtracting two [`Instant`] objects. +//! +//! A [`Delta`] type supports methods to retrieve the duration in various units. +//! //! C header: [`include/linux/jiffies.h`](srctree/include/linux/jiffies.h). //! C header: [`include/linux/ktime.h`](srctree/include/linux/ktime.h). @@ -33,59 +49,44 @@ pub fn msecs_to_jiffies(msecs: Msecs) -> Jiffies { unsafe { bindings::__msecs_to_jiffies(msecs) } } -/// A Rust wrapper around a `ktime_t`. +/// A specific point in time. +/// +/// # Invariants +/// +/// The `inner` value is in the range from 0 to `KTIME_MAX`. #[repr(transparent)] #[derive(Copy, Clone, PartialEq, PartialOrd, Eq, Ord)] -pub struct Ktime { +pub struct Instant { inner: bindings::ktime_t, } -impl Ktime { - /// Create a `Ktime` from a raw `ktime_t`. - #[inline] - pub fn from_raw(inner: bindings::ktime_t) -> Self { - Self { inner } - } - +impl Instant { /// Get the current time using `CLOCK_MONOTONIC`. #[inline] - pub fn ktime_get() -> Self { - // SAFETY: It is always safe to call `ktime_get` outside of NMI context. - Self::from_raw(unsafe { bindings::ktime_get() }) - } - - /// Divide the number of nanoseconds by a compile-time constant. - #[inline] - fn divns_constant(self) -> i64 { - self.to_ns() / DIV - } - - /// Returns the number of nanoseconds. - #[inline] - pub fn to_ns(self) -> i64 { - self.inner - } - - /// Returns the number of milliseconds. - #[inline] - pub fn to_ms(self) -> i64 { - self.divns_constant::() - } -} - -/// Returns the number of milliseconds between two ktimes. -#[inline] -pub fn ktime_ms_delta(later: Ktime, earlier: Ktime) -> i64 { - (later - earlier).to_ms() -} - -impl core::ops::Sub for Ktime { - type Output = Ktime; - - #[inline] - fn sub(self, other: Ktime) -> Ktime { + pub fn now() -> Self { + // INVARIANT: The `ktime_get()` function returns a value in the range + // from 0 to `KTIME_MAX`. Self { - inner: self.inner - other.inner, + // SAFETY: It is always safe to call `ktime_get()` outside of NMI context. + inner: unsafe { bindings::ktime_get() }, + } + } + + /// Return the amount of time elapsed since the [`Instant`]. + #[inline] + pub fn elapsed(&self) -> Delta { + Self::now() - *self + } +} + +impl core::ops::Sub for Instant { + type Output = Delta; + + // By the type invariant, it never overflows. + #[inline] + fn sub(self, other: Instant) -> Delta { + Delta { + nanos: self.inner - other.inner, } } } From 679185904972421c570a1c337a8266835045012d Mon Sep 17 00:00:00 2001 From: FUJITA Tomonori Date: Thu, 24 Apr 2025 04:28:56 +0900 Subject: [PATCH 18/78] MAINTAINERS: rust: Add a new section for all of the time stuff Add a new section for all of the time stuff to MAINTAINERS file, with the existing hrtimer entry fold. Acked-by: John Stultz Reviewed-by: Boqun Feng Reviewed-by: Andreas Hindborg Signed-off-by: FUJITA Tomonori Link: https://lore.kernel.org/r/20250423192857.199712-7-fujita.tomonori@gmail.com [ Changed T: entry branch to `timekeeping-next` - Andreas ] Signed-off-by: Andreas Hindborg --- MAINTAINERS | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index fa1e04e87d1d..c2f539209ca0 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -10587,20 +10587,23 @@ F: kernel/time/timer_list.c F: kernel/time/timer_migration.* F: tools/testing/selftests/timers/ -HIGH-RESOLUTION TIMERS [RUST] +DELAY, SLEEP, TIMEKEEPING, TIMERS [RUST] M: Andreas Hindborg R: Boqun Feng +R: FUJITA Tomonori R: Frederic Weisbecker R: Lyude Paul R: Thomas Gleixner R: Anna-Maria Behnsen +R: John Stultz +R: Stephen Boyd L: rust-for-linux@vger.kernel.org S: Supported W: https://rust-for-linux.com B: https://github.com/Rust-for-Linux/linux/issues -T: git https://github.com/Rust-for-Linux/linux.git hrtimer-next -F: rust/kernel/time/hrtimer.rs -F: rust/kernel/time/hrtimer/ +T: git https://github.com/Rust-for-Linux/linux.git timekeeping-next +F: rust/kernel/time.rs +F: rust/kernel/time/ HIGH-SPEED SCC DRIVER FOR AX.25 L: linux-hams@vger.kernel.org From 1a4736c3d8394f5e64557a41b4b2b8d6dcd04622 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Wed, 23 Apr 2025 09:54:37 -0400 Subject: [PATCH 19/78] rust: types: add `ForeignOwnable::PointedTo` Allow implementors to specify the foreign pointer type; this exposes information about the pointed-to type such as its alignment. This requires the trait to be `unsafe` since it is now possible for implementors to break soundness by returning a misaligned pointer. Encoding the pointer type in the trait (and avoiding pointer casts) allows the compiler to check that implementors return the correct pointer type. This is preferable to directly encoding the alignment in the trait using a constant as the compiler would be unable to check it. Acked-by: Danilo Krummrich Signed-off-by: Tamir Duberstein Reviewed-by: Alice Ryhl Link: https://lore.kernel.org/r/20250423-rust-xarray-bindings-v19-1-83cdcf11c114@gmail.com Signed-off-by: Andreas Hindborg --- rust/kernel/alloc/kbox.rs | 38 +++++++++++++++++--------------- rust/kernel/miscdevice.rs | 10 ++++----- rust/kernel/pci.rs | 2 +- rust/kernel/platform.rs | 2 +- rust/kernel/sync/arc.rs | 21 ++++++++++-------- rust/kernel/types.rs | 46 ++++++++++++++++++++++++++------------- 6 files changed, 70 insertions(+), 49 deletions(-) diff --git a/rust/kernel/alloc/kbox.rs b/rust/kernel/alloc/kbox.rs index b77d32f3a58b..6aa88b01e84d 100644 --- a/rust/kernel/alloc/kbox.rs +++ b/rust/kernel/alloc/kbox.rs @@ -360,68 +360,70 @@ fn try_init(init: impl Init, flags: Flags) -> Result } } -impl ForeignOwnable for Box +// SAFETY: The `into_foreign` function returns a pointer that is well-aligned. +unsafe impl ForeignOwnable for Box where A: Allocator, { + type PointedTo = T; type Borrowed<'a> = &'a T; type BorrowedMut<'a> = &'a mut T; - fn into_foreign(self) -> *mut crate::ffi::c_void { - Box::into_raw(self).cast() + fn into_foreign(self) -> *mut Self::PointedTo { + Box::into_raw(self) } - unsafe fn from_foreign(ptr: *mut crate::ffi::c_void) -> Self { + unsafe fn from_foreign(ptr: *mut Self::PointedTo) -> Self { // SAFETY: The safety requirements of this function ensure that `ptr` comes from a previous // call to `Self::into_foreign`. - unsafe { Box::from_raw(ptr.cast()) } + unsafe { Box::from_raw(ptr) } } - unsafe fn borrow<'a>(ptr: *mut crate::ffi::c_void) -> &'a T { + unsafe fn borrow<'a>(ptr: *mut Self::PointedTo) -> &'a T { // SAFETY: The safety requirements of this method ensure that the object remains alive and // immutable for the duration of 'a. - unsafe { &*ptr.cast() } + unsafe { &*ptr } } - unsafe fn borrow_mut<'a>(ptr: *mut crate::ffi::c_void) -> &'a mut T { - let ptr = ptr.cast(); + unsafe fn borrow_mut<'a>(ptr: *mut Self::PointedTo) -> &'a mut T { // SAFETY: The safety requirements of this method ensure that the pointer is valid and that // nothing else will access the value for the duration of 'a. unsafe { &mut *ptr } } } -impl ForeignOwnable for Pin> +// SAFETY: The `into_foreign` function returns a pointer that is well-aligned. +unsafe impl ForeignOwnable for Pin> where A: Allocator, { + type PointedTo = T; type Borrowed<'a> = Pin<&'a T>; type BorrowedMut<'a> = Pin<&'a mut T>; - fn into_foreign(self) -> *mut crate::ffi::c_void { + fn into_foreign(self) -> *mut Self::PointedTo { // SAFETY: We are still treating the box as pinned. - Box::into_raw(unsafe { Pin::into_inner_unchecked(self) }).cast() + Box::into_raw(unsafe { Pin::into_inner_unchecked(self) }) } - unsafe fn from_foreign(ptr: *mut crate::ffi::c_void) -> Self { + unsafe fn from_foreign(ptr: *mut Self::PointedTo) -> Self { // SAFETY: The safety requirements of this function ensure that `ptr` comes from a previous // call to `Self::into_foreign`. - unsafe { Pin::new_unchecked(Box::from_raw(ptr.cast())) } + unsafe { Pin::new_unchecked(Box::from_raw(ptr)) } } - unsafe fn borrow<'a>(ptr: *mut crate::ffi::c_void) -> Pin<&'a T> { + unsafe fn borrow<'a>(ptr: *mut Self::PointedTo) -> Pin<&'a T> { // SAFETY: The safety requirements for this function ensure that the object is still alive, // so it is safe to dereference the raw pointer. // The safety requirements of `from_foreign` also ensure that the object remains alive for // the lifetime of the returned value. - let r = unsafe { &*ptr.cast() }; + let r = unsafe { &*ptr }; // SAFETY: This pointer originates from a `Pin>`. unsafe { Pin::new_unchecked(r) } } - unsafe fn borrow_mut<'a>(ptr: *mut crate::ffi::c_void) -> Pin<&'a mut T> { - let ptr = ptr.cast(); + unsafe fn borrow_mut<'a>(ptr: *mut Self::PointedTo) -> Pin<&'a mut T> { // SAFETY: The safety requirements for this function ensure that the object is still alive, // so it is safe to dereference the raw pointer. // The safety requirements of `from_foreign` also ensure that the object remains alive for diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs index fa9ecc42602a..b4c5f74de23d 100644 --- a/rust/kernel/miscdevice.rs +++ b/rust/kernel/miscdevice.rs @@ -200,7 +200,7 @@ impl MiscdeviceVTable { // type. // // SAFETY: The open call of a file can access the private data. - unsafe { (*raw_file).private_data = ptr.into_foreign() }; + unsafe { (*raw_file).private_data = ptr.into_foreign().cast() }; 0 } @@ -211,7 +211,7 @@ impl MiscdeviceVTable { /// must be associated with a `MiscDeviceRegistration`. unsafe extern "C" fn release(_inode: *mut bindings::inode, file: *mut bindings::file) -> c_int { // SAFETY: The release call of a file owns the private data. - let private = unsafe { (*file).private_data }; + let private = unsafe { (*file).private_data }.cast(); // SAFETY: The release call of a file owns the private data. let ptr = unsafe { ::from_foreign(private) }; @@ -228,7 +228,7 @@ impl MiscdeviceVTable { /// `file` must be a valid file that is associated with a `MiscDeviceRegistration`. unsafe extern "C" fn ioctl(file: *mut bindings::file, cmd: c_uint, arg: c_ulong) -> c_long { // SAFETY: The ioctl call of a file can access the private data. - let private = unsafe { (*file).private_data }; + let private = unsafe { (*file).private_data }.cast(); // SAFETY: Ioctl calls can borrow the private data of the file. let device = unsafe { ::borrow(private) }; @@ -253,7 +253,7 @@ impl MiscdeviceVTable { arg: c_ulong, ) -> c_long { // SAFETY: The compat ioctl call of a file can access the private data. - let private = unsafe { (*file).private_data }; + let private = unsafe { (*file).private_data }.cast(); // SAFETY: Ioctl calls can borrow the private data of the file. let device = unsafe { ::borrow(private) }; @@ -274,7 +274,7 @@ impl MiscdeviceVTable { /// - `seq_file` must be a valid `struct seq_file` that we can write to. unsafe extern "C" fn show_fdinfo(seq_file: *mut bindings::seq_file, file: *mut bindings::file) { // SAFETY: The release call of a file owns the private data. - let private = unsafe { (*file).private_data }; + let private = unsafe { (*file).private_data }.cast(); // SAFETY: Ioctl calls can borrow the private data of the file. let device = unsafe { ::borrow(private) }; // SAFETY: diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs index c97d6d470b28..3aeb1250c27f 100644 --- a/rust/kernel/pci.rs +++ b/rust/kernel/pci.rs @@ -89,7 +89,7 @@ extern "C" fn probe_callback( extern "C" fn remove_callback(pdev: *mut bindings::pci_dev) { // SAFETY: The PCI bus only ever calls the remove callback with a valid pointer to a // `struct pci_dev`. - let ptr = unsafe { bindings::pci_get_drvdata(pdev) }; + let ptr = unsafe { bindings::pci_get_drvdata(pdev) }.cast(); // SAFETY: `remove_callback` is only ever called after a successful call to // `probe_callback`, hence it's guaranteed that `ptr` points to a valid and initialized diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs index 4917cb34e2fe..fd4a494f30e8 100644 --- a/rust/kernel/platform.rs +++ b/rust/kernel/platform.rs @@ -80,7 +80,7 @@ extern "C" fn probe_callback(pdev: *mut bindings::platform_device) -> kernel::ff extern "C" fn remove_callback(pdev: *mut bindings::platform_device) { // SAFETY: `pdev` is a valid pointer to a `struct platform_device`. - let ptr = unsafe { bindings::platform_get_drvdata(pdev) }; + let ptr = unsafe { bindings::platform_get_drvdata(pdev) }.cast(); // SAFETY: `remove_callback` is only ever called after a successful call to // `probe_callback`, hence it's guaranteed that `ptr` points to a valid and initialized diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs index 8484c814609a..a42c164e577a 100644 --- a/rust/kernel/sync/arc.rs +++ b/rust/kernel/sync/arc.rs @@ -140,9 +140,10 @@ pub struct Arc { _p: PhantomData>, } +#[doc(hidden)] #[pin_data] #[repr(C)] -struct ArcInner { +pub struct ArcInner { refcount: Opaque, data: T, } @@ -371,18 +372,20 @@ pub fn into_unique_or_drop(self) -> Option>> { } } -impl ForeignOwnable for Arc { +// SAFETY: The `into_foreign` function returns a pointer that is well-aligned. +unsafe impl ForeignOwnable for Arc { + type PointedTo = ArcInner; type Borrowed<'a> = ArcBorrow<'a, T>; type BorrowedMut<'a> = Self::Borrowed<'a>; - fn into_foreign(self) -> *mut crate::ffi::c_void { - ManuallyDrop::new(self).ptr.as_ptr().cast() + fn into_foreign(self) -> *mut Self::PointedTo { + ManuallyDrop::new(self).ptr.as_ptr() } - unsafe fn from_foreign(ptr: *mut crate::ffi::c_void) -> Self { + unsafe fn from_foreign(ptr: *mut Self::PointedTo) -> Self { // SAFETY: The safety requirements of this function ensure that `ptr` comes from a previous // call to `Self::into_foreign`. - let inner = unsafe { NonNull::new_unchecked(ptr.cast::>()) }; + let inner = unsafe { NonNull::new_unchecked(ptr) }; // SAFETY: By the safety requirement of this function, we know that `ptr` came from // a previous call to `Arc::into_foreign`, which guarantees that `ptr` is valid and @@ -390,17 +393,17 @@ unsafe fn from_foreign(ptr: *mut crate::ffi::c_void) -> Self { unsafe { Self::from_inner(inner) } } - unsafe fn borrow<'a>(ptr: *mut crate::ffi::c_void) -> ArcBorrow<'a, T> { + unsafe fn borrow<'a>(ptr: *mut Self::PointedTo) -> ArcBorrow<'a, T> { // SAFETY: The safety requirements of this function ensure that `ptr` comes from a previous // call to `Self::into_foreign`. - let inner = unsafe { NonNull::new_unchecked(ptr.cast::>()) }; + let inner = unsafe { NonNull::new_unchecked(ptr) }; // SAFETY: The safety requirements of `from_foreign` ensure that the object remains alive // for the lifetime of the returned value. unsafe { ArcBorrow::new(inner) } } - unsafe fn borrow_mut<'a>(ptr: *mut crate::ffi::c_void) -> ArcBorrow<'a, T> { + unsafe fn borrow_mut<'a>(ptr: *mut Self::PointedTo) -> ArcBorrow<'a, T> { // SAFETY: The safety requirements for `borrow_mut` are a superset of the safety // requirements for `borrow`. unsafe { Self::borrow(ptr) } diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs index 9d0471afc964..86562e738eac 100644 --- a/rust/kernel/types.rs +++ b/rust/kernel/types.rs @@ -18,7 +18,19 @@ /// /// This trait is meant to be used in cases when Rust objects are stored in C objects and /// eventually "freed" back to Rust. -pub trait ForeignOwnable: Sized { +/// +/// # Safety +/// +/// Implementers must ensure that [`into_foreign`] returns a pointer which meets the alignment +/// requirements of [`PointedTo`]. +/// +/// [`into_foreign`]: Self::into_foreign +/// [`PointedTo`]: Self::PointedTo +pub unsafe trait ForeignOwnable: Sized { + /// Type used when the value is foreign-owned. In practical terms only defines the alignment of + /// the pointer. + type PointedTo; + /// Type used to immutably borrow a value that is currently foreign-owned. type Borrowed<'a>; @@ -27,16 +39,18 @@ pub trait ForeignOwnable: Sized { /// Converts a Rust-owned object to a foreign-owned one. /// - /// The foreign representation is a pointer to void. There are no guarantees for this pointer. - /// For example, it might be invalid, dangling or pointing to uninitialized memory. Using it in - /// any way except for [`from_foreign`], [`try_from_foreign`], [`borrow`], or [`borrow_mut`] can - /// result in undefined behavior. + /// # Guarantees + /// + /// The return value is guaranteed to be well-aligned, but there are no other guarantees for + /// this pointer. For example, it might be null, dangling, or point to uninitialized memory. + /// Using it in any way except for [`ForeignOwnable::from_foreign`], [`ForeignOwnable::borrow`], + /// [`ForeignOwnable::try_from_foreign`] can result in undefined behavior. /// /// [`from_foreign`]: Self::from_foreign /// [`try_from_foreign`]: Self::try_from_foreign /// [`borrow`]: Self::borrow /// [`borrow_mut`]: Self::borrow_mut - fn into_foreign(self) -> *mut crate::ffi::c_void; + fn into_foreign(self) -> *mut Self::PointedTo; /// Converts a foreign-owned object back to a Rust-owned one. /// @@ -46,7 +60,7 @@ pub trait ForeignOwnable: Sized { /// must not be passed to `from_foreign` more than once. /// /// [`into_foreign`]: Self::into_foreign - unsafe fn from_foreign(ptr: *mut crate::ffi::c_void) -> Self; + unsafe fn from_foreign(ptr: *mut Self::PointedTo) -> Self; /// Tries to convert a foreign-owned object back to a Rust-owned one. /// @@ -58,7 +72,7 @@ pub trait ForeignOwnable: Sized { /// `ptr` must either be null or satisfy the safety requirements for [`from_foreign`]. /// /// [`from_foreign`]: Self::from_foreign - unsafe fn try_from_foreign(ptr: *mut crate::ffi::c_void) -> Option { + unsafe fn try_from_foreign(ptr: *mut Self::PointedTo) -> Option { if ptr.is_null() { None } else { @@ -81,7 +95,7 @@ unsafe fn try_from_foreign(ptr: *mut crate::ffi::c_void) -> Option { /// /// [`into_foreign`]: Self::into_foreign /// [`from_foreign`]: Self::from_foreign - unsafe fn borrow<'a>(ptr: *mut crate::ffi::c_void) -> Self::Borrowed<'a>; + unsafe fn borrow<'a>(ptr: *mut Self::PointedTo) -> Self::Borrowed<'a>; /// Borrows a foreign-owned object mutably. /// @@ -109,21 +123,23 @@ unsafe fn try_from_foreign(ptr: *mut crate::ffi::c_void) -> Option { /// [`from_foreign`]: Self::from_foreign /// [`borrow`]: Self::borrow /// [`Arc`]: crate::sync::Arc - unsafe fn borrow_mut<'a>(ptr: *mut crate::ffi::c_void) -> Self::BorrowedMut<'a>; + unsafe fn borrow_mut<'a>(ptr: *mut Self::PointedTo) -> Self::BorrowedMut<'a>; } -impl ForeignOwnable for () { +// SAFETY: The `into_foreign` function returns a pointer that is dangling, but well-aligned. +unsafe impl ForeignOwnable for () { + type PointedTo = (); type Borrowed<'a> = (); type BorrowedMut<'a> = (); - fn into_foreign(self) -> *mut crate::ffi::c_void { + fn into_foreign(self) -> *mut Self::PointedTo { core::ptr::NonNull::dangling().as_ptr() } - unsafe fn from_foreign(_: *mut crate::ffi::c_void) -> Self {} + unsafe fn from_foreign(_: *mut Self::PointedTo) -> Self {} - unsafe fn borrow<'a>(_: *mut crate::ffi::c_void) -> Self::Borrowed<'a> {} - unsafe fn borrow_mut<'a>(_: *mut crate::ffi::c_void) -> Self::BorrowedMut<'a> {} + unsafe fn borrow<'a>(_: *mut Self::PointedTo) -> Self::Borrowed<'a> {} + unsafe fn borrow_mut<'a>(_: *mut Self::PointedTo) -> Self::BorrowedMut<'a> {} } /// Runs a cleanup function/closure when dropped. From 210b81578efbe5c5e7748e50d313e1a90b03df55 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Wed, 23 Apr 2025 09:54:38 -0400 Subject: [PATCH 20/78] rust: xarray: Add an abstraction for XArray MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `XArray` is an efficient sparse array of pointers. Add a Rust abstraction for this type. This implementation bounds the element type on `ForeignOwnable` and requires explicit locking for all operations. Future work may leverage RCU to enable lockless operation. Inspired-by: Maíra Canal Inspired-by: Asahi Lina Reviewed-by: Andreas Hindborg Reviewed-by: Alice Ryhl Signed-off-by: Tamir Duberstein Link: https://lore.kernel.org/r/20250423-rust-xarray-bindings-v19-2-83cdcf11c114@gmail.com Signed-off-by: Andreas Hindborg --- rust/bindings/bindings_helper.h | 6 + rust/helpers/helpers.c | 1 + rust/helpers/xarray.c | 28 ++++ rust/kernel/lib.rs | 1 + rust/kernel/xarray.rs | 275 ++++++++++++++++++++++++++++++++ 5 files changed, 311 insertions(+) create mode 100644 rust/helpers/xarray.c create mode 100644 rust/kernel/xarray.rs diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h index ab37e1d35c70..e0bcd130b494 100644 --- a/rust/bindings/bindings_helper.h +++ b/rust/bindings/bindings_helper.h @@ -37,6 +37,7 @@ #include #include #include +#include #include #if defined(CONFIG_DRM_PANIC_SCREEN_QR_CODE) @@ -55,3 +56,8 @@ const gfp_t RUST_CONST_HELPER___GFP_ZERO = __GFP_ZERO; const gfp_t RUST_CONST_HELPER___GFP_HIGHMEM = ___GFP_HIGHMEM; const gfp_t RUST_CONST_HELPER___GFP_NOWARN = ___GFP_NOWARN; const blk_features_t RUST_CONST_HELPER_BLK_FEAT_ROTATIONAL = BLK_FEAT_ROTATIONAL; + +const xa_mark_t RUST_CONST_HELPER_XA_PRESENT = XA_PRESENT; + +const gfp_t RUST_CONST_HELPER_XA_FLAGS_ALLOC = XA_FLAGS_ALLOC; +const gfp_t RUST_CONST_HELPER_XA_FLAGS_ALLOC1 = XA_FLAGS_ALLOC1; diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c index 1e7c84df7252..80785b1e7a63 100644 --- a/rust/helpers/helpers.c +++ b/rust/helpers/helpers.c @@ -38,3 +38,4 @@ #include "vmalloc.c" #include "wait.c" #include "workqueue.c" +#include "xarray.c" diff --git a/rust/helpers/xarray.c b/rust/helpers/xarray.c new file mode 100644 index 000000000000..60b299f11451 --- /dev/null +++ b/rust/helpers/xarray.c @@ -0,0 +1,28 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include + +int rust_helper_xa_err(void *entry) +{ + return xa_err(entry); +} + +void rust_helper_xa_init_flags(struct xarray *xa, gfp_t flags) +{ + return xa_init_flags(xa, flags); +} + +int rust_helper_xa_trylock(struct xarray *xa) +{ + return xa_trylock(xa); +} + +void rust_helper_xa_lock(struct xarray *xa) +{ + return xa_lock(xa); +} + +void rust_helper_xa_unlock(struct xarray *xa) +{ + return xa_unlock(xa); +} diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs index de07aadd1ff5..715fab6b1345 100644 --- a/rust/kernel/lib.rs +++ b/rust/kernel/lib.rs @@ -88,6 +88,7 @@ pub mod types; pub mod uaccess; pub mod workqueue; +pub mod xarray; #[doc(hidden)] pub use bindings; diff --git a/rust/kernel/xarray.rs b/rust/kernel/xarray.rs new file mode 100644 index 000000000000..75719e7bb491 --- /dev/null +++ b/rust/kernel/xarray.rs @@ -0,0 +1,275 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! XArray abstraction. +//! +//! C header: [`include/linux/xarray.h`](srctree/include/linux/xarray.h) + +use crate::{ + alloc, bindings, build_assert, + error::{Error, Result}, + types::{ForeignOwnable, NotThreadSafe, Opaque}, +}; +use core::{iter, marker::PhantomData, mem, pin::Pin, ptr::NonNull}; +use pin_init::{pin_data, pin_init, pinned_drop, PinInit}; + +/// An array which efficiently maps sparse integer indices to owned objects. +/// +/// This is similar to a [`crate::alloc::kvec::Vec>`], but more efficient when there are +/// holes in the index space, and can be efficiently grown. +/// +/// # Invariants +/// +/// `self.xa` is always an initialized and valid [`bindings::xarray`] whose entries are either +/// `XA_ZERO_ENTRY` or came from `T::into_foreign`. +/// +/// # Examples +/// +/// ```rust +/// use kernel::alloc::KBox; +/// use kernel::xarray::{AllocKind, XArray}; +/// +/// let xa = KBox::pin_init(XArray::new(AllocKind::Alloc1), GFP_KERNEL)?; +/// +/// let dead = KBox::new(0xdead, GFP_KERNEL)?; +/// let beef = KBox::new(0xbeef, GFP_KERNEL)?; +/// +/// let mut guard = xa.lock(); +/// +/// assert_eq!(guard.get(0), None); +/// +/// assert_eq!(guard.store(0, dead, GFP_KERNEL)?.as_deref(), None); +/// assert_eq!(guard.get(0).copied(), Some(0xdead)); +/// +/// *guard.get_mut(0).unwrap() = 0xffff; +/// assert_eq!(guard.get(0).copied(), Some(0xffff)); +/// +/// assert_eq!(guard.store(0, beef, GFP_KERNEL)?.as_deref().copied(), Some(0xffff)); +/// assert_eq!(guard.get(0).copied(), Some(0xbeef)); +/// +/// guard.remove(0); +/// assert_eq!(guard.get(0), None); +/// +/// # Ok::<(), Error>(()) +/// ``` +#[pin_data(PinnedDrop)] +pub struct XArray { + #[pin] + xa: Opaque, + _p: PhantomData, +} + +#[pinned_drop] +impl PinnedDrop for XArray { + fn drop(self: Pin<&mut Self>) { + self.iter().for_each(|ptr| { + let ptr = ptr.as_ptr(); + // SAFETY: `ptr` came from `T::into_foreign`. + // + // INVARIANT: we own the only reference to the array which is being dropped so the + // broken invariant is not observable on function exit. + drop(unsafe { T::from_foreign(ptr) }) + }); + + // SAFETY: `self.xa` is always valid by the type invariant. + unsafe { bindings::xa_destroy(self.xa.get()) }; + } +} + +/// Flags passed to [`XArray::new`] to configure the array's allocation tracking behavior. +pub enum AllocKind { + /// Consider the first element to be at index 0. + Alloc, + /// Consider the first element to be at index 1. + Alloc1, +} + +impl XArray { + /// Creates a new initializer for this type. + pub fn new(kind: AllocKind) -> impl PinInit { + let flags = match kind { + AllocKind::Alloc => bindings::XA_FLAGS_ALLOC, + AllocKind::Alloc1 => bindings::XA_FLAGS_ALLOC1, + }; + pin_init!(Self { + // SAFETY: `xa` is valid while the closure is called. + // + // INVARIANT: `xa` is initialized here to an empty, valid [`bindings::xarray`]. + xa <- Opaque::ffi_init(|xa| unsafe { + bindings::xa_init_flags(xa, flags) + }), + _p: PhantomData, + }) + } + + fn iter(&self) -> impl Iterator> + '_ { + let mut index = 0; + + // SAFETY: `self.xa` is always valid by the type invariant. + iter::once(unsafe { + bindings::xa_find(self.xa.get(), &mut index, usize::MAX, bindings::XA_PRESENT) + }) + .chain(iter::from_fn(move || { + // SAFETY: `self.xa` is always valid by the type invariant. + Some(unsafe { + bindings::xa_find_after(self.xa.get(), &mut index, usize::MAX, bindings::XA_PRESENT) + }) + })) + .map_while(|ptr| NonNull::new(ptr.cast())) + } + + /// Attempts to lock the [`XArray`] for exclusive access. + pub fn try_lock(&self) -> Option> { + // SAFETY: `self.xa` is always valid by the type invariant. + if (unsafe { bindings::xa_trylock(self.xa.get()) } != 0) { + Some(Guard { + xa: self, + _not_send: NotThreadSafe, + }) + } else { + None + } + } + + /// Locks the [`XArray`] for exclusive access. + pub fn lock(&self) -> Guard<'_, T> { + // SAFETY: `self.xa` is always valid by the type invariant. + unsafe { bindings::xa_lock(self.xa.get()) }; + + Guard { + xa: self, + _not_send: NotThreadSafe, + } + } +} + +/// A lock guard. +/// +/// The lock is unlocked when the guard goes out of scope. +#[must_use = "the lock unlocks immediately when the guard is unused"] +pub struct Guard<'a, T: ForeignOwnable> { + xa: &'a XArray, + _not_send: NotThreadSafe, +} + +impl Drop for Guard<'_, T> { + fn drop(&mut self) { + // SAFETY: + // - `self.xa.xa` is always valid by the type invariant. + // - The caller holds the lock, so it is safe to unlock it. + unsafe { bindings::xa_unlock(self.xa.xa.get()) }; + } +} + +/// The error returned by [`store`](Guard::store). +/// +/// Contains the underlying error and the value that was not stored. +pub struct StoreError { + /// The error that occurred. + pub error: Error, + /// The value that was not stored. + pub value: T, +} + +impl From> for Error { + fn from(value: StoreError) -> Self { + value.error + } +} + +impl<'a, T: ForeignOwnable> Guard<'a, T> { + fn load(&self, index: usize, f: F) -> Option + where + F: FnOnce(NonNull) -> U, + { + // SAFETY: `self.xa.xa` is always valid by the type invariant. + let ptr = unsafe { bindings::xa_load(self.xa.xa.get(), index) }; + let ptr = NonNull::new(ptr.cast())?; + Some(f(ptr)) + } + + /// Provides a reference to the element at the given index. + pub fn get(&self, index: usize) -> Option> { + self.load(index, |ptr| { + // SAFETY: `ptr` came from `T::into_foreign`. + unsafe { T::borrow(ptr.as_ptr()) } + }) + } + + /// Provides a mutable reference to the element at the given index. + pub fn get_mut(&mut self, index: usize) -> Option> { + self.load(index, |ptr| { + // SAFETY: `ptr` came from `T::into_foreign`. + unsafe { T::borrow_mut(ptr.as_ptr()) } + }) + } + + /// Removes and returns the element at the given index. + pub fn remove(&mut self, index: usize) -> Option { + // SAFETY: + // - `self.xa.xa` is always valid by the type invariant. + // - The caller holds the lock. + let ptr = unsafe { bindings::__xa_erase(self.xa.xa.get(), index) }.cast(); + // SAFETY: + // - `ptr` is either NULL or came from `T::into_foreign`. + // - `&mut self` guarantees that the lifetimes of [`T::Borrowed`] and [`T::BorrowedMut`] + // borrowed from `self` have ended. + unsafe { T::try_from_foreign(ptr) } + } + + /// Stores an element at the given index. + /// + /// May drop the lock if needed to allocate memory, and then reacquire it afterwards. + /// + /// On success, returns the element which was previously at the given index. + /// + /// On failure, returns the element which was attempted to be stored. + pub fn store( + &mut self, + index: usize, + value: T, + gfp: alloc::Flags, + ) -> Result, StoreError> { + build_assert!( + mem::align_of::() >= 4, + "pointers stored in XArray must be 4-byte aligned" + ); + let new = value.into_foreign(); + + let old = { + let new = new.cast(); + // SAFETY: + // - `self.xa.xa` is always valid by the type invariant. + // - The caller holds the lock. + // + // INVARIANT: `new` came from `T::into_foreign`. + unsafe { bindings::__xa_store(self.xa.xa.get(), index, new, gfp.as_raw()) } + }; + + // SAFETY: `__xa_store` returns the old entry at this index on success or `xa_err` if an + // error happened. + let errno = unsafe { bindings::xa_err(old) }; + if errno != 0 { + // SAFETY: `new` came from `T::into_foreign` and `__xa_store` does not take + // ownership of the value on error. + let value = unsafe { T::from_foreign(new) }; + Err(StoreError { + value, + error: Error::from_errno(errno), + }) + } else { + let old = old.cast(); + // SAFETY: `ptr` is either NULL or came from `T::into_foreign`. + // + // NB: `XA_ZERO_ENTRY` is never returned by functions belonging to the Normal XArray + // API; such entries present as `NULL`. + Ok(unsafe { T::try_from_foreign(old) }) + } + } +} + +// SAFETY: `XArray` has no shared mutable state so it is `Send` iff `T` is `Send`. +unsafe impl Send for XArray {} + +// SAFETY: `XArray` serialises the interior mutability it provides so it is `Sync` iff `T` is +// `Send`. +unsafe impl Sync for XArray {} From fa616196fbea12462107774fb6a1908c95f71cf0 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Wed, 23 Apr 2025 09:54:39 -0400 Subject: [PATCH 21/78] MAINTAINERS: add entry for Rust XArray API Add an entry for the Rust xarray abstractions. Acked-by: Andreas Hindborg Signed-off-by: Tamir Duberstein Link: https://lore.kernel.org/r/20250423-rust-xarray-bindings-v19-3-83cdcf11c114@gmail.com [ Changed T: entry branch to `xarray-rust` - Andreas ] Signed-off-by: Andreas Hindborg --- MAINTAINERS | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index fa1e04e87d1d..871721367c45 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -26310,6 +26310,17 @@ F: lib/test_xarray.c F: lib/xarray.c F: tools/testing/radix-tree +XARRAY API [RUST] +M: Tamir Duberstein +M: Andreas Hindborg +L: rust-for-linux@vger.kernel.org +S: Supported +W: https://rust-for-linux.com +B: https://github.com/Rust-for-Linux/linux/issues +C: https://rust-for-linux.zulipchat.com +T: git https://github.com/Rust-for-Linux/linux.git xarray-next +F: rust/kernel/xarray.rs + XBOX DVD IR REMOTE M: Benjamin Valentin S: Maintained From 90348980a305cc24a067cc6e606e1c318e277930 Mon Sep 17 00:00:00 2001 From: Benno Lossin Date: Mon, 21 Apr 2025 22:17:50 +0000 Subject: [PATCH 22/78] rust: pin-init: add `cast_[pin_]init` functions to change the initialized type These functions cast the given pointer from one type to another. They are particularly useful when initializing transparent wrapper types. Link: https://github.com/Rust-for-Linux/pin-init/pull/39/commits/80c03ddee41b154f1099fd8cc7c2bbd8c80af0ad Reviewed-by: Christian Schrefl Signed-off-by: Benno Lossin --- rust/pin-init/src/lib.rs | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/rust/pin-init/src/lib.rs b/rust/pin-init/src/lib.rs index 0806c689f693..a880c21d3f09 100644 --- a/rust/pin-init/src/lib.rs +++ b/rust/pin-init/src/lib.rs @@ -1216,6 +1216,38 @@ unsafe fn __pinned_init(self, slot: *mut T) -> Result<(), E> { __internal::InitClosure(f, PhantomData) } +/// Changes the to be initialized type. +/// +/// # Safety +/// +/// - `*mut U` must be castable to `*mut T` and any value of type `T` written through such a +/// pointer must result in a valid `U`. +#[expect(clippy::let_and_return)] +pub const unsafe fn cast_pin_init(init: impl PinInit) -> impl PinInit { + // SAFETY: initialization delegated to a valid initializer. Cast is valid by function safety + // requirements. + let res = unsafe { pin_init_from_closure(|ptr: *mut U| init.__pinned_init(ptr.cast::())) }; + // FIXME: remove the let statement once the nightly-MSRV allows it (1.78 otherwise encounters a + // cycle when computing the type returned by this function) + res +} + +/// Changes the to be initialized type. +/// +/// # Safety +/// +/// - `*mut U` must be castable to `*mut T` and any value of type `T` written through such a +/// pointer must result in a valid `U`. +#[expect(clippy::let_and_return)] +pub const unsafe fn cast_init(init: impl Init) -> impl Init { + // SAFETY: initialization delegated to a valid initializer. Cast is valid by function safety + // requirements. + let res = unsafe { init_from_closure(|ptr: *mut U| init.__init(ptr.cast::())) }; + // FIXME: remove the let statement once the nightly-MSRV allows it (1.78 otherwise encounters a + // cycle when computing the type returned by this function) + res +} + /// An initializer that leaves the memory uninitialized. /// /// The initializer is a no-op. The `slot` memory is not changed. From 2f7c73825f8f435ebdfb2cfa3b01cfa2b1c79041 Mon Sep 17 00:00:00 2001 From: Christian Schrefl Date: Mon, 21 Apr 2025 22:17:59 +0000 Subject: [PATCH 23/78] rust: pin-init: Add the `Wrapper` trait. This trait allows creating `PinInitializers` for wrapper or new-type structs with the inner value structurally pinned, when given the initializer for the inner value. Implement this trait for `UnsafeCell` and `MaybeUninit`. Signed-off-by: Christian Schrefl Link: https://github.com/Rust-for-Linux/pin-init/pull/37/commits/3ab4db083bd7b41a1bc23d937224f975d7400e50 [ Reworded commit message into imperative mode, fixed typo and fixed commit authorship. - Benno ] Signed-off-by: Benno Lossin --- rust/pin-init/src/lib.rs | 44 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/rust/pin-init/src/lib.rs b/rust/pin-init/src/lib.rs index a880c21d3f09..467ccc8bd616 100644 --- a/rust/pin-init/src/lib.rs +++ b/rust/pin-init/src/lib.rs @@ -1513,3 +1513,47 @@ unsafe impl<$first: Zeroable, $($t: Zeroable),*> Zeroable for ($first, $($t),*) } impl_tuple_zeroable!(A, B, C, D, E, F, G, H, I, J); + +/// This trait allows creating an instance of `Self` which contains exactly one +/// [structurally pinned value](https://doc.rust-lang.org/std/pin/index.html#projections-and-structural-pinning). +/// +/// This is useful when using wrapper `struct`s like [`UnsafeCell`] or with new-type `struct`s. +/// +/// # Examples +/// +/// ``` +/// # use core::cell::UnsafeCell; +/// # use pin_init::{pin_data, pin_init, Wrapper}; +/// +/// #[pin_data] +/// struct Foo {} +/// +/// #[pin_data] +/// struct Bar { +/// #[pin] +/// content: UnsafeCell +/// }; +/// +/// let foo_initializer = pin_init!(Foo{}); +/// let initializer = pin_init!(Bar { +/// content <- UnsafeCell::pin_init(foo_initializer) +/// }); +/// ``` +pub trait Wrapper { + /// Create an pin-initializer for a [`Self`] containing `T` form the `value_init` initializer. + fn pin_init(value_init: impl PinInit) -> impl PinInit; +} + +impl Wrapper for UnsafeCell { + fn pin_init(value_init: impl PinInit) -> impl PinInit { + // SAFETY: `UnsafeCell` has a compatible layout to `T`. + unsafe { cast_pin_init(value_init) } + } +} + +impl Wrapper for MaybeUninit { + fn pin_init(value_init: impl PinInit) -> impl PinInit { + // SAFETY: `MaybeUninit` has a compatible layout to `T`. + unsafe { cast_pin_init(value_init) } + } +} From b862aac8fd46601fa20226c9f5d6c6d308678b4d Mon Sep 17 00:00:00 2001 From: Christian Schrefl Date: Mon, 21 Apr 2025 22:18:06 +0000 Subject: [PATCH 24/78] rust: pin-init: Implement `Wrapper` for `UnsafePinned` behind feature flag. Add the `unsafe-pinned` feature which gates the `Wrapper` implementation of the `core::pin::UnsafePinned` struct. For now this is just a cargo feature, but once `core::pin::UnsafePinned` is stable a config flag can be added to allow the usage of this implementation in the linux kernel. Signed-off-by: Christian Schrefl Link: https://github.com/Rust-for-Linux/pin-init/pull/37/commits/99cb1934425357e780ea5b0628f66633123847b8 [ Fixed commit authorship. - Benno ] Signed-off-by: Benno Lossin --- rust/pin-init/src/lib.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/rust/pin-init/src/lib.rs b/rust/pin-init/src/lib.rs index 467ccc8bd616..745cf534d239 100644 --- a/rust/pin-init/src/lib.rs +++ b/rust/pin-init/src/lib.rs @@ -269,6 +269,10 @@ #![forbid(missing_docs, unsafe_op_in_unsafe_fn)] #![cfg_attr(not(feature = "std"), no_std)] #![cfg_attr(feature = "alloc", feature(allocator_api))] +#![cfg_attr( + all(feature = "unsafe-pinned", CONFIG_RUSTC_HAS_UNSAFE_PINNED), + feature(unsafe_pinned) +)] use core::{ cell::UnsafeCell, @@ -1557,3 +1561,11 @@ fn pin_init(value_init: impl PinInit) -> impl PinInit { unsafe { cast_pin_init(value_init) } } } + +#[cfg(all(feature = "unsafe-pinned", CONFIG_RUSTC_HAS_UNSAFE_PINNED))] +impl Wrapper for core::pin::UnsafePinned { + fn pin_init(init: impl PinInit) -> impl PinInit { + // SAFETY: `UnsafePinned` has a compatible layout to `T`. + unsafe { cast_pin_init(init) } + } +} From bc5f3e0e01a5f2d067ff4292d5a10093ae680f53 Mon Sep 17 00:00:00 2001 From: Christian Schrefl Date: Mon, 21 Apr 2025 22:18:13 +0000 Subject: [PATCH 25/78] rust: pin-init: Update Changelog and Readme Add Changelog entry for the `Wrapper` trait and document the `unsafe-pinned` feature in the Readme. Signed-off-by: Christian Schrefl Link: https://github.com/Rust-for-Linux/pin-init/pull/37/commits/986555f564645efb238e8092c6314388c859efe5 [ Fixed commit authorship. - Benno ] Signed-off-by: Benno Lossin --- rust/pin-init/README.md | 6 ++++++ rust/pin-init/src/lib.rs | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/rust/pin-init/README.md b/rust/pin-init/README.md index 24d0f0a3f8fb..1a03b200d4ce 100644 --- a/rust/pin-init/README.md +++ b/rust/pin-init/README.md @@ -40,6 +40,12 @@ However, using the crate on stable compilers is possible by disabling `alloc`. I will require the `std` feature, because stable compilers have neither `Box` nor `Arc` in no-std mode. +### Nightly needed for `unsafe-pinned` feature + +This feature enables the `Wrapper` implementation on the unstable `core::pin::UnsafePinned` type. +This requires the [`unsafe_pinned` unstable feature](https://github.com/rust-lang/rust/issues/125735) +and therefore a nightly compiler. Note that this feature is not enabled by default. + ## Overview To initialize a `struct` with an in-place constructor you will need two things: diff --git a/rust/pin-init/src/lib.rs b/rust/pin-init/src/lib.rs index 745cf534d239..1521500a46b1 100644 --- a/rust/pin-init/src/lib.rs +++ b/rust/pin-init/src/lib.rs @@ -32,6 +32,12 @@ //! will require the `std` feature, because stable compilers have neither `Box` nor `Arc` in no-std //! mode. //! +//! ## Nightly needed for `unsafe-pinned` feature +//! +//! This feature enables the `Wrapper` implementation on the unstable `core::pin::UnsafePinned` type. +//! This requires the [`unsafe_pinned` unstable feature](https://github.com/rust-lang/rust/issues/125735) +//! and therefore a nightly compiler. Note that this feature is not enabled by default. +//! //! # Overview //! //! To initialize a `struct` with an in-place constructor you will need two things: From c3815aa4bb5c6248e78785269357e87bfa4d0909 Mon Sep 17 00:00:00 2001 From: Christian Schrefl Date: Mon, 21 Apr 2025 22:18:23 +0000 Subject: [PATCH 26/78] rust: pin-init: Update the structural pinning link in readme. The previous link anchor was broken in rust 1.77, because the documentation was refactored in upstream rust. Change the link to refer to the new section in the rust documentation. Signed-off-by: Christian Schrefl Link: https://github.com/Rust-for-Linux/pin-init/pull/37/commits/a146142fe18cafa52f8c6da306ca2729d789cfbf [ Fixed commit authorship. - Benno ] Signed-off-by: Benno Lossin --- rust/pin-init/README.md | 2 +- rust/pin-init/src/lib.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/rust/pin-init/README.md b/rust/pin-init/README.md index 1a03b200d4ce..2d0cda961d45 100644 --- a/rust/pin-init/README.md +++ b/rust/pin-init/README.md @@ -222,7 +222,7 @@ the `kernel` crate. The [`sync`] module is a good starting point. [`sync`]: https://rust.docs.kernel.org/kernel/sync/index.html [pinning]: https://doc.rust-lang.org/std/pin/index.html -[structurally pinned fields]: https://doc.rust-lang.org/std/pin/index.html#pinning-is-structural-for-field +[structurally pinned fields]: https://doc.rust-lang.org/std/pin/index.html#projections-and-structural-pinning [stack]: https://docs.rs/pin-init/latest/pin_init/macro.stack_pin_init.html [`impl PinInit`]: https://docs.rs/pin-init/latest/pin_init/trait.PinInit.html [`impl PinInit`]: https://docs.rs/pin-init/latest/pin_init/trait.PinInit.html diff --git a/rust/pin-init/src/lib.rs b/rust/pin-init/src/lib.rs index 1521500a46b1..774f8ca033bc 100644 --- a/rust/pin-init/src/lib.rs +++ b/rust/pin-init/src/lib.rs @@ -247,7 +247,7 @@ //! [`sync`]: https://rust.docs.kernel.org/kernel/sync/index.html //! [pinning]: https://doc.rust-lang.org/std/pin/index.html //! [structurally pinned fields]: -//! https://doc.rust-lang.org/std/pin/index.html#pinning-is-structural-for-field +//! https://doc.rust-lang.org/std/pin/index.html#projections-and-structural-pinning //! [stack]: crate::stack_pin_init #![cfg_attr( kernel, From 983d13fc2cf12f0a753700d48be7d04155a1272c Mon Sep 17 00:00:00 2001 From: Benno Lossin Date: Mon, 21 Apr 2025 22:18:33 +0000 Subject: [PATCH 27/78] rust: pin-init: allow `pub` fields in `derive(Zeroable)` Add support for parsing `pub`, `pub(crate)` and `pub(super)` to the derive macro `Zeroable`. Link: https://github.com/Rust-for-Linux/pin-init/pull/42/commits/e8311e52ca57273e7ed6d099144384971677a0ba Signed-off-by: Benno Lossin --- rust/pin-init/src/macros.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/pin-init/src/macros.rs b/rust/pin-init/src/macros.rs index 361623324d5c..e4054fe3ed3d 100644 --- a/rust/pin-init/src/macros.rs +++ b/rust/pin-init/src/macros.rs @@ -1393,7 +1393,7 @@ macro_rules! __derive_zeroable { @body({ $( $(#[$($field_attr:tt)*])* - $field:ident : $field_ty:ty + $field_vis:vis $field:ident : $field_ty:ty ),* $(,)? }), ) => { From a313d41a2b515bbb76d56df490b731ff6d64e571 Mon Sep 17 00:00:00 2001 From: Benno Lossin Date: Mon, 21 Apr 2025 22:18:41 +0000 Subject: [PATCH 28/78] rust: pin-init: allow `Zeroable` derive macro to also be applied to unions Enabling the same behavior for unions as for structs is correct, but could be relaxed: the valid bit patterns for unions are the union of all valid bit patterns of their fields. So for a union to implement `Zeroable`, only a single field needs to implement `Zeroable`. This can be a future improvement, as it is currently only needed for unions where all fields implement `Zeroable`. There is no danger for mis-parsing with the two optional tokens (ie neither one or both tokens are parsed), as the compiler will already have rejected that before giving it as the input to the derive macro. Link: https://github.com/Rust-for-Linux/pin-init/pull/42/commits/5927b497ce522d82f6c082d5ba9235df57bfdb32 Signed-off-by: Benno Lossin --- rust/pin-init/src/macros.rs | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/rust/pin-init/src/macros.rs b/rust/pin-init/src/macros.rs index e4054fe3ed3d..332d7e08925b 100644 --- a/rust/pin-init/src/macros.rs +++ b/rust/pin-init/src/macros.rs @@ -1412,4 +1412,34 @@ fn ensure_zeroable<$($impl_generics)*>() } }; }; + (parse_input: + @sig( + $(#[$($struct_attr:tt)*])* + $vis:vis union $name:ident + $(where $($whr:tt)*)? + ), + @impl_generics($($impl_generics:tt)*), + @ty_generics($($ty_generics:tt)*), + @body({ + $( + $(#[$($field_attr:tt)*])* + $field_vis:vis $field:ident : $field_ty:ty + ),* $(,)? + }), + ) => { + // SAFETY: Every field type implements `Zeroable` and padding bytes may be zero. + #[automatically_derived] + unsafe impl<$($impl_generics)*> $crate::Zeroable for $name<$($ty_generics)*> + where + $($($whr)*)? + {} + const _: () = { + fn assert_zeroable() {} + fn ensure_zeroable<$($impl_generics)*>() + where $($($whr)*)? + { + $(assert_zeroable::<$field_ty>();)* + } + }; + }; } From 00fccd3ecc2129ee32fd181079eb643f497044c4 Mon Sep 17 00:00:00 2001 From: Benno Lossin Date: Mon, 21 Apr 2025 22:18:52 +0000 Subject: [PATCH 29/78] rust: pin-init: add `MaybeZeroable` derive macro This derive macro implements `Zeroable` for structs & unions precisely if all fields also implement `Zeroable` and does nothing otherwise. The plain `Zeroable` derive macro instead errors when it cannot derive `Zeroable` safely. The `MaybeZeroable` derive macro is useful in cases where manual checking is infeasible such as with the bindings crate. Move the zeroable generics parsing into a standalone function in order to avoid code duplication between the two derive macros. Link: https://github.com/Rust-for-Linux/pin-init/pull/42/commits/1165cdad1a391b923efaf30cf76bc61e38da022e Signed-off-by: Benno Lossin --- rust/pin-init/internal/src/lib.rs | 5 +++ rust/pin-init/internal/src/zeroable.rs | 27 +++++++++++- rust/pin-init/src/lib.rs | 30 +++++++++++++ rust/pin-init/src/macros.rs | 59 ++++++++++++++++++++++++++ 4 files changed, 120 insertions(+), 1 deletion(-) diff --git a/rust/pin-init/internal/src/lib.rs b/rust/pin-init/internal/src/lib.rs index 56aa9ecc1e1a..297b0129a5bf 100644 --- a/rust/pin-init/internal/src/lib.rs +++ b/rust/pin-init/internal/src/lib.rs @@ -47,3 +47,8 @@ pub fn pinned_drop(args: TokenStream, input: TokenStream) -> TokenStream { pub fn derive_zeroable(input: TokenStream) -> TokenStream { zeroable::derive(input.into()).into() } + +#[proc_macro_derive(MaybeZeroable)] +pub fn maybe_derive_zeroable(input: TokenStream) -> TokenStream { + zeroable::maybe_derive(input.into()).into() +} diff --git a/rust/pin-init/internal/src/zeroable.rs b/rust/pin-init/internal/src/zeroable.rs index acc94008c152..e0ed3998445c 100644 --- a/rust/pin-init/internal/src/zeroable.rs +++ b/rust/pin-init/internal/src/zeroable.rs @@ -6,7 +6,14 @@ use crate::helpers::{parse_generics, Generics}; use proc_macro::{TokenStream, TokenTree}; -pub(crate) fn derive(input: TokenStream) -> TokenStream { +pub(crate) fn parse_zeroable_derive_input( + input: TokenStream, +) -> ( + Vec, + Vec, + Vec, + Option, +) { let ( Generics { impl_generics, @@ -64,6 +71,11 @@ pub(crate) fn derive(input: TokenStream) -> TokenStream { if in_generic && !inserted { new_impl_generics.extend(quote! { : ::pin_init::Zeroable }); } + (rest, new_impl_generics, ty_generics, last) +} + +pub(crate) fn derive(input: TokenStream) -> TokenStream { + let (rest, new_impl_generics, ty_generics, last) = parse_zeroable_derive_input(input); quote! { ::pin_init::__derive_zeroable!( parse_input: @@ -74,3 +86,16 @@ pub(crate) fn derive(input: TokenStream) -> TokenStream { ); } } + +pub(crate) fn maybe_derive(input: TokenStream) -> TokenStream { + let (rest, new_impl_generics, ty_generics, last) = parse_zeroable_derive_input(input); + quote! { + ::pin_init::__maybe_derive_zeroable!( + parse_input: + @sig(#(#rest)*), + @impl_generics(#(#new_impl_generics)*), + @ty_generics(#(#ty_generics)*), + @body(#last), + ); + } +} diff --git a/rust/pin-init/src/lib.rs b/rust/pin-init/src/lib.rs index 774f8ca033bc..05a0cd6ad8f4 100644 --- a/rust/pin-init/src/lib.rs +++ b/rust/pin-init/src/lib.rs @@ -413,6 +413,36 @@ /// ``` pub use ::pin_init_internal::Zeroable; +/// Derives the [`Zeroable`] trait for the given struct if all fields implement [`Zeroable`]. +/// +/// Contrary to the derive macro named [`macro@Zeroable`], this one silently fails when a field +/// doesn't implement [`Zeroable`]. +/// +/// # Examples +/// +/// ``` +/// use pin_init::MaybeZeroable; +/// +/// // implmements `Zeroable` +/// #[derive(MaybeZeroable)] +/// pub struct DriverData { +/// id: i64, +/// buf_ptr: *mut u8, +/// len: usize, +/// } +/// +/// // does not implmement `Zeroable` +/// #[derive(MaybeZeroable)] +/// pub struct DriverData2 { +/// id: i64, +/// buf_ptr: *mut u8, +/// len: usize, +/// // this field doesn't implement `Zeroable` +/// other_data: &'static i32, +/// } +/// ``` +pub use ::pin_init_internal::MaybeZeroable; + /// Initialize and pin a type directly on the stack. /// /// # Examples diff --git a/rust/pin-init/src/macros.rs b/rust/pin-init/src/macros.rs index 332d7e08925b..935d77745d1d 100644 --- a/rust/pin-init/src/macros.rs +++ b/rust/pin-init/src/macros.rs @@ -1443,3 +1443,62 @@ fn ensure_zeroable<$($impl_generics)*>() }; }; } + +#[doc(hidden)] +#[macro_export] +macro_rules! __maybe_derive_zeroable { + (parse_input: + @sig( + $(#[$($struct_attr:tt)*])* + $vis:vis struct $name:ident + $(where $($whr:tt)*)? + ), + @impl_generics($($impl_generics:tt)*), + @ty_generics($($ty_generics:tt)*), + @body({ + $( + $(#[$($field_attr:tt)*])* + $field_vis:vis $field:ident : $field_ty:ty + ),* $(,)? + }), + ) => { + // SAFETY: Every field type implements `Zeroable` and padding bytes may be zero. + #[automatically_derived] + unsafe impl<$($impl_generics)*> $crate::Zeroable for $name<$($ty_generics)*> + where + $( + // the `for<'__dummy>` HRTB makes this not error without the `trivial_bounds` + // feature . + $field_ty: for<'__dummy> $crate::Zeroable, + )* + $($($whr)*)? + {} + }; + (parse_input: + @sig( + $(#[$($struct_attr:tt)*])* + $vis:vis union $name:ident + $(where $($whr:tt)*)? + ), + @impl_generics($($impl_generics:tt)*), + @ty_generics($($ty_generics:tt)*), + @body({ + $( + $(#[$($field_attr:tt)*])* + $field_vis:vis $field:ident : $field_ty:ty + ),* $(,)? + }), + ) => { + // SAFETY: Every field type implements `Zeroable` and padding bytes may be zero. + #[automatically_derived] + unsafe impl<$($impl_generics)*> $crate::Zeroable for $name<$($ty_generics)*> + where + $( + // the `for<'__dummy>` HRTB makes this not error without the `trivial_bounds` + // feature . + $field_ty: for<'__dummy> $crate::Zeroable, + )* + $($($whr)*)? + {} + }; +} From a919ba21594bfa1e67639785d409e1bdce332097 Mon Sep 17 00:00:00 2001 From: Benno Lossin Date: Sat, 26 Apr 2025 08:39:22 +0000 Subject: [PATCH 30/78] rust: pin-init: fix typos Correct two typos in the `Wrapper::pin_init` documentation. Link: https://github.com/Rust-for-Linux/pin-init/pull/48/commits/fd0bf5e244b685188dc642fc4a0bd3f042468fdb Reviewed-by: Christian Schrefl Signed-off-by: Benno Lossin --- rust/pin-init/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/pin-init/src/lib.rs b/rust/pin-init/src/lib.rs index 05a0cd6ad8f4..b5a295effd9c 100644 --- a/rust/pin-init/src/lib.rs +++ b/rust/pin-init/src/lib.rs @@ -1580,7 +1580,7 @@ unsafe impl<$first: Zeroable, $($t: Zeroable),*> Zeroable for ($first, $($t),*) /// }); /// ``` pub trait Wrapper { - /// Create an pin-initializer for a [`Self`] containing `T` form the `value_init` initializer. + /// Creates an pin-initializer for a [`Self`] containing `T` from the `value_init` initializer. fn pin_init(value_init: impl PinInit) -> impl PinInit; } From 9de1a293c8ece00d226b21a35751ec178be2a9fa Mon Sep 17 00:00:00 2001 From: Benno Lossin Date: Sat, 26 Apr 2025 08:39:28 +0000 Subject: [PATCH 31/78] rust: pin-init: improve documentation for `Zeroable` derive macros Specify that both `MaybeZeroable` and `Zeroable` work on `union`s. Add a doc example for a union. Also include an example with visibility on the field. Link: https://github.com/Rust-for-Linux/pin-init/pull/48/commits/ab0985a0e08df06c60a32ca5888f74adcc2c1cf3 Reviewed-by: Christian Schrefl Signed-off-by: Benno Lossin --- rust/pin-init/src/lib.rs | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/rust/pin-init/src/lib.rs b/rust/pin-init/src/lib.rs index b5a295effd9c..9ab34036e6bc 100644 --- a/rust/pin-init/src/lib.rs +++ b/rust/pin-init/src/lib.rs @@ -395,9 +395,10 @@ /// ``` pub use ::pin_init_internal::pinned_drop; -/// Derives the [`Zeroable`] trait for the given struct. +/// Derives the [`Zeroable`] trait for the given `struct` or `union`. /// -/// This can only be used for structs where every field implements the [`Zeroable`] trait. +/// This can only be used for `struct`s/`union`s where every field implements the [`Zeroable`] +/// trait. /// /// # Examples /// @@ -406,14 +407,25 @@ /// /// #[derive(Zeroable)] /// pub struct DriverData { -/// id: i64, +/// pub(crate) id: i64, /// buf_ptr: *mut u8, /// len: usize, /// } /// ``` +/// +/// ``` +/// use pin_init::Zeroable; +/// +/// #[derive(Zeroable)] +/// pub union SignCast { +/// signed: i64, +/// unsigned: u64, +/// } +/// ``` pub use ::pin_init_internal::Zeroable; -/// Derives the [`Zeroable`] trait for the given struct if all fields implement [`Zeroable`]. +/// Derives the [`Zeroable`] trait for the given `struct` or `union` if all fields implement +/// [`Zeroable`]. /// /// Contrary to the derive macro named [`macro@Zeroable`], this one silently fails when a field /// doesn't implement [`Zeroable`]. @@ -426,7 +438,7 @@ /// // implmements `Zeroable` /// #[derive(MaybeZeroable)] /// pub struct DriverData { -/// id: i64, +/// pub(crate) id: i64, /// buf_ptr: *mut u8, /// len: usize, /// } @@ -434,7 +446,7 @@ /// // does not implmement `Zeroable` /// #[derive(MaybeZeroable)] /// pub struct DriverData2 { -/// id: i64, +/// pub(crate) id: i64, /// buf_ptr: *mut u8, /// len: usize, /// // this field doesn't implement `Zeroable` From a1e4d5c9d708d7a0e7071015a120a4489404128f Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Fri, 2 May 2025 13:19:29 +0000 Subject: [PATCH 32/78] rust: alloc: add Vec::clear Our custom Vec type is missing the stdlib method `clear`, thus add it. It will be used in the miscdevice sample. Reviewed-by: Benno Lossin Reviewed-by: Tamir Duberstein Signed-off-by: Alice Ryhl Reviewed-by: Greg Kroah-Hartman Link: https://lore.kernel.org/r/20250502-vec-methods-v5-1-06d20ad9366f@google.com Signed-off-by: Danilo Krummrich --- rust/kernel/alloc/kvec.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/rust/kernel/alloc/kvec.rs b/rust/kernel/alloc/kvec.rs index 5798e2c890a2..412a2fe3ce79 100644 --- a/rust/kernel/alloc/kvec.rs +++ b/rust/kernel/alloc/kvec.rs @@ -413,6 +413,26 @@ pub fn into_raw_parts(self) -> (*mut T, usize, usize) { (ptr, len, capacity) } + /// Clears the vector, removing all values. + /// + /// Note that this method has no effect on the allocated capacity + /// of the vector. + /// + /// # Examples + /// + /// ``` + /// let mut v = kernel::kvec![1, 2, 3]?; + /// + /// v.clear(); + /// + /// assert!(v.is_empty()); + /// # Ok::<(), Error>(()) + /// ``` + #[inline] + pub fn clear(&mut self) { + self.truncate(0); + } + /// Ensures that the capacity exceeds the length by at least `additional` elements. /// /// # Examples From f2b4dd7093438e4884cb01a783212abfbc9cc40b Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Fri, 2 May 2025 13:19:30 +0000 Subject: [PATCH 33/78] rust: alloc: add Vec::pop This introduces a basic method that our custom Vec is missing. I expect that it will be used in many places, but at the time of writing, Rust Binder has six calls to Vec::pop. Signed-off-by: Alice Ryhl Reviewed-by: Greg Kroah-Hartman Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20250502-vec-methods-v5-2-06d20ad9366f@google.com Signed-off-by: Danilo Krummrich --- rust/kernel/alloc/kvec.rs | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/rust/kernel/alloc/kvec.rs b/rust/kernel/alloc/kvec.rs index 412a2fe3ce79..ebca0cfd31c6 100644 --- a/rust/kernel/alloc/kvec.rs +++ b/rust/kernel/alloc/kvec.rs @@ -320,6 +320,37 @@ pub fn push(&mut self, v: T, flags: Flags) -> Result<(), AllocError> { Ok(()) } + /// Removes the last element from a vector and returns it, or `None` if it is empty. + /// + /// # Examples + /// + /// ``` + /// let mut v = KVec::new(); + /// v.push(1, GFP_KERNEL)?; + /// v.push(2, GFP_KERNEL)?; + /// assert_eq!(&v, &[1, 2]); + /// + /// assert_eq!(v.pop(), Some(2)); + /// assert_eq!(v.pop(), Some(1)); + /// assert_eq!(v.pop(), None); + /// # Ok::<(), Error>(()) + /// ``` + pub fn pop(&mut self) -> Option { + if self.is_empty() { + return None; + } + + let removed: *mut T = { + // SAFETY: We just checked that the length is at least one. + let slice = unsafe { self.dec_len(1) }; + // SAFETY: The argument to `dec_len` was 1 so this returns a slice of length 1. + unsafe { slice.get_unchecked_mut(0) } + }; + + // SAFETY: The guarantees of `dec_len` allow us to take ownership of this value. + Some(unsafe { removed.read() }) + } + /// Creates a new [`Vec`] instance with at least the given capacity. /// /// # Examples From 9def0d0a2a1c62d7970f4ce5ad5557968c98f637 Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Fri, 2 May 2025 13:19:31 +0000 Subject: [PATCH 34/78] rust: alloc: add Vec::push_within_capacity This introduces a new method called `push_within_capacity` for appending to a vector without attempting to allocate if the capacity is full. Rust Binder will use this in various places to safely push to a vector while holding a spinlock. The implementation is moved to a push_within_capacity_unchecked method. This is preferred over having push() call push_within_capacity() followed by an unwrap_unchecked() for simpler unsafe. Panics in the kernel are best avoided when possible, so an error is returned if the vector does not have sufficient capacity. An error type is used rather than just returning Result<(),T> to make it more convenient for callers (i.e. they can use ? or unwrap). Signed-off-by: Alice Ryhl Reviewed-by: Greg Kroah-Hartman Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20250502-vec-methods-v5-3-06d20ad9366f@google.com [ Remove public visibility from `Vec::push_within_capacity_unchecked()`. - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/alloc/kvec.rs | 46 +++++++++++++++++++++++++++++--- rust/kernel/alloc/kvec/errors.rs | 23 ++++++++++++++++ 2 files changed, 65 insertions(+), 4 deletions(-) create mode 100644 rust/kernel/alloc/kvec/errors.rs diff --git a/rust/kernel/alloc/kvec.rs b/rust/kernel/alloc/kvec.rs index ebca0cfd31c6..64dfa9af5589 100644 --- a/rust/kernel/alloc/kvec.rs +++ b/rust/kernel/alloc/kvec.rs @@ -21,6 +21,9 @@ slice::SliceIndex, }; +mod errors; +pub use self::errors::PushError; + /// Create a [`KVec`] containing the arguments. /// /// New memory is allocated with `GFP_KERNEL`. @@ -307,17 +310,52 @@ pub fn spare_capacity_mut(&mut self) -> &mut [MaybeUninit] { /// ``` pub fn push(&mut self, v: T, flags: Flags) -> Result<(), AllocError> { self.reserve(1, flags)?; + // SAFETY: The call to `reserve` was successful, so the capacity is at least one greater + // than the length. + unsafe { self.push_within_capacity_unchecked(v) }; + Ok(()) + } + /// Appends an element to the back of the [`Vec`] instance without reallocating. + /// + /// Fails if the vector does not have capacity for the new element. + /// + /// # Examples + /// + /// ``` + /// let mut v = KVec::with_capacity(10, GFP_KERNEL)?; + /// for i in 0..10 { + /// v.push_within_capacity(i)?; + /// } + /// + /// assert!(v.push_within_capacity(10).is_err()); + /// # Ok::<(), Error>(()) + /// ``` + pub fn push_within_capacity(&mut self, v: T) -> Result<(), PushError> { + if self.len() < self.capacity() { + // SAFETY: The length is less than the capacity. + unsafe { self.push_within_capacity_unchecked(v) }; + Ok(()) + } else { + Err(PushError(v)) + } + } + + /// Appends an element to the back of the [`Vec`] instance without reallocating. + /// + /// # Safety + /// + /// The length must be less than the capacity. + unsafe fn push_within_capacity_unchecked(&mut self, v: T) { let spare = self.spare_capacity_mut(); - // SAFETY: The call to `reserve` was successful so the spare capacity is at least 1. + // SAFETY: By the safety requirements, `spare` is non-empty. unsafe { spare.get_unchecked_mut(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. + // by 1. We also know that the new length is <= capacity because the caller guarantees that + // the length is less than the capacity at the beginning of this function. unsafe { self.inc_len(1) }; - Ok(()) } /// Removes the last element from a vector and returns it, or `None` if it is empty. diff --git a/rust/kernel/alloc/kvec/errors.rs b/rust/kernel/alloc/kvec/errors.rs new file mode 100644 index 000000000000..84c96ec5007d --- /dev/null +++ b/rust/kernel/alloc/kvec/errors.rs @@ -0,0 +1,23 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! Errors for the [`Vec`] type. + +use core::fmt::{self, Debug, Formatter}; +use kernel::prelude::*; + +/// Error type for [`Vec::push_within_capacity`]. +pub struct PushError(pub T); + +impl Debug for PushError { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + write!(f, "Not enough capacity") + } +} + +impl From> for Error { + fn from(_: PushError) -> Error { + // Returning ENOMEM isn't appropriate because the system is not out of memory. The vector + // is just full and we are refusing to resize it. + EINVAL + } +} From 088bf14a886e1e746c961a862ebccbb76d7cbd4e Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Fri, 2 May 2025 13:19:32 +0000 Subject: [PATCH 35/78] rust: alloc: add Vec::drain_all This is like the stdlib method drain, except that it's hard-coded to use the entire vector's range. Rust Binder uses it in the range allocator to take ownership of everything in a vector in a case where reusing the vector is desirable. Implementing `DrainAll` in terms of `slice::IterMut` lets us reuse some nice optimizations in core for the case where T is a ZST. Signed-off-by: Alice Ryhl Reviewed-by: Greg Kroah-Hartman Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20250502-vec-methods-v5-4-06d20ad9366f@google.com Signed-off-by: Danilo Krummrich --- rust/kernel/alloc/kvec.rs | 59 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/rust/kernel/alloc/kvec.rs b/rust/kernel/alloc/kvec.rs index 64dfa9af5589..afaf22865342 100644 --- a/rust/kernel/alloc/kvec.rs +++ b/rust/kernel/alloc/kvec.rs @@ -586,6 +586,30 @@ pub fn truncate(&mut self, len: usize) { unsafe { ptr::drop_in_place(ptr) }; } } + + /// Takes ownership of all items in this vector without consuming the allocation. + /// + /// # Examples + /// + /// ``` + /// let mut v = kernel::kvec![0, 1, 2, 3]?; + /// + /// for (i, j) in v.drain_all().enumerate() { + /// assert_eq!(i, j); + /// } + /// + /// assert!(v.capacity() >= 4); + /// # Ok::<(), Error>(()) + /// ``` + pub fn drain_all(&mut self) -> DrainAll<'_, T> { + // SAFETY: This does not underflow the length. + let elems = unsafe { self.dec_len(self.len()) }; + // INVARIANT: The first `len` elements of the spare capacity are valid values, and as we + // just set the length to zero, we may transfer ownership to the `DrainAll` object. + DrainAll { + elements: elems.iter_mut(), + } + } } impl Vec { @@ -1073,3 +1097,38 @@ fn into_iter(self) -> Self::IntoIter { } } } + +/// An iterator that owns all items in a vector, but does not own its allocation. +/// +/// # Invariants +/// +/// Every `&mut T` returned by the iterator references a `T` that the iterator may take ownership +/// of. +pub struct DrainAll<'vec, T> { + elements: slice::IterMut<'vec, T>, +} + +impl<'vec, T> Iterator for DrainAll<'vec, T> { + type Item = T; + + fn next(&mut self) -> Option { + let elem: *mut T = self.elements.next()?; + // SAFETY: By the type invariants, we may take ownership of this value. + Some(unsafe { elem.read() }) + } + + fn size_hint(&self) -> (usize, Option) { + self.elements.size_hint() + } +} + +impl<'vec, T> Drop for DrainAll<'vec, T> { + fn drop(&mut self) { + if core::mem::needs_drop::() { + let iter = core::mem::take(&mut self.elements); + let ptr: *mut [T] = iter.into_slice(); + // SAFETY: By the type invariants, we own these values so we may destroy them. + unsafe { ptr::drop_in_place(ptr) }; + } + } +} From 9f140894e72735f034fdc0e963d0550ef03c6f44 Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Fri, 2 May 2025 13:19:33 +0000 Subject: [PATCH 36/78] rust: alloc: add Vec::retain This adds a common Vec method called `retain` that removes all elements that don't match a certain condition. Rust Binder uses it to find all processes that match a given pid. The stdlib retain method takes &T rather than &mut T and has a separate retain_mut for the &mut T case. However, this is considered an API mistake that can't be fixed now due to backwards compatibility. There's no reason for us to repeat that mistake. Signed-off-by: Alice Ryhl Reviewed-by: Greg Kroah-Hartman Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20250502-vec-methods-v5-5-06d20ad9366f@google.com Signed-off-by: Danilo Krummrich --- rust/kernel/alloc/kvec.rs | 72 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/rust/kernel/alloc/kvec.rs b/rust/kernel/alloc/kvec.rs index afaf22865342..8843dea0b377 100644 --- a/rust/kernel/alloc/kvec.rs +++ b/rust/kernel/alloc/kvec.rs @@ -610,6 +610,29 @@ pub fn drain_all(&mut self) -> DrainAll<'_, T> { elements: elems.iter_mut(), } } + + /// Removes all elements that don't match the provided closure. + /// + /// # Examples + /// + /// ``` + /// let mut v = kernel::kvec![1, 2, 3, 4]?; + /// v.retain(|i| *i % 2 == 0); + /// assert_eq!(v, [2, 4]); + /// # Ok::<(), Error>(()) + /// ``` + pub fn retain(&mut self, mut f: impl FnMut(&mut T) -> bool) { + let mut num_kept = 0; + let mut next_to_check = 0; + while let Some(to_check) = self.get_mut(next_to_check) { + if f(to_check) { + self.swap(num_kept, next_to_check); + num_kept += 1; + } + next_to_check += 1; + } + self.truncate(num_kept); + } } impl Vec { @@ -1132,3 +1155,52 @@ fn drop(&mut self) { } } } + +#[macros::kunit_tests(rust_kvec_kunit)] +mod tests { + use super::*; + use crate::prelude::*; + + #[test] + fn test_kvec_retain() { + /// Verify correctness for one specific function. + #[expect(clippy::needless_range_loop)] + fn verify(c: &[bool]) { + let mut vec1: KVec = KVec::with_capacity(c.len(), GFP_KERNEL).unwrap(); + let mut vec2: KVec = KVec::with_capacity(c.len(), GFP_KERNEL).unwrap(); + + for i in 0..c.len() { + vec1.push_within_capacity(i).unwrap(); + if c[i] { + vec2.push_within_capacity(i).unwrap(); + } + } + + vec1.retain(|i| c[*i]); + + assert_eq!(vec1, vec2); + } + + /// Add one to a binary integer represented as a boolean array. + fn add(value: &mut [bool]) { + let mut carry = true; + for v in value { + let new_v = carry != *v; + carry = carry && *v; + *v = new_v; + } + } + + // This boolean array represents a function from index to boolean. We check that `retain` + // behaves correctly for all possible boolean arrays of every possible length less than + // ten. + let mut func = KVec::with_capacity(10, GFP_KERNEL).unwrap(); + for len in 0..10 { + for _ in 0u32..1u32 << len { + verify(&func); + add(&mut func); + } + func.push_within_capacity(false).unwrap(); + } + } +} From 294a7ecbdf0a5d65c6df1287c5d56241e9331cf2 Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Fri, 2 May 2025 13:19:34 +0000 Subject: [PATCH 37/78] rust: alloc: add Vec::remove This is needed by Rust Binder in the range allocator, and by upcoming GPU drivers during firmware initialization. Panics in the kernel are best avoided when possible, so an error is returned if the index is out of bounds. An error type is used rather than just returning Option to let callers handle errors with ?. Signed-off-by: Alice Ryhl Reviewed-by: Greg Kroah-Hartman Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20250502-vec-methods-v5-6-06d20ad9366f@google.com [ Remove `# Panics` section; `Vec::remove() handles the error properly.` - Danilo ] Signed-off-by: Danilo Krummrich --- rust/kernel/alloc/kvec.rs | 38 +++++++++++++++++++++++++++++++- rust/kernel/alloc/kvec/errors.rs | 15 +++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/rust/kernel/alloc/kvec.rs b/rust/kernel/alloc/kvec.rs index 8843dea0b377..3f2617b08753 100644 --- a/rust/kernel/alloc/kvec.rs +++ b/rust/kernel/alloc/kvec.rs @@ -22,7 +22,7 @@ }; mod errors; -pub use self::errors::PushError; +pub use self::errors::{PushError, RemoveError}; /// Create a [`KVec`] containing the arguments. /// @@ -389,6 +389,42 @@ pub fn pop(&mut self) -> Option { Some(unsafe { removed.read() }) } + /// Removes the element at the given index. + /// + /// # Examples + /// + /// ``` + /// let mut v = kernel::kvec![1, 2, 3]?; + /// assert_eq!(v.remove(1)?, 2); + /// assert_eq!(v, [1, 3]); + /// # Ok::<(), Error>(()) + /// ``` + pub fn remove(&mut self, i: usize) -> Result { + let value = { + let value_ref = self.get(i).ok_or(RemoveError)?; + // INVARIANT: This breaks the invariants by invalidating the value at index `i`, but we + // restore the invariants below. + // SAFETY: The value at index `i` is valid, because otherwise we would have already + // failed with `RemoveError`. + unsafe { ptr::read(value_ref) } + }; + + // SAFETY: We checked that `i` is in-bounds. + let p = unsafe { self.as_mut_ptr().add(i) }; + + // INVARIANT: After this call, the invalid value is at the last slot, so the Vec invariants + // are restored after the below call to `dec_len(1)`. + // SAFETY: `p.add(1).add(self.len - i - 1)` is `i+1+len-i-1 == len` elements after the + // beginning of the vector, so this is in-bounds of the vector's allocation. + unsafe { ptr::copy(p.add(1), p, self.len - i - 1) }; + + // SAFETY: Since the check at the beginning of this call did not fail with `RemoveError`, + // the length is at least one. + unsafe { self.dec_len(1) }; + + Ok(value) + } + /// Creates a new [`Vec`] instance with at least the given capacity. /// /// # Examples diff --git a/rust/kernel/alloc/kvec/errors.rs b/rust/kernel/alloc/kvec/errors.rs index 84c96ec5007d..06fe696e8bc6 100644 --- a/rust/kernel/alloc/kvec/errors.rs +++ b/rust/kernel/alloc/kvec/errors.rs @@ -21,3 +21,18 @@ fn from(_: PushError) -> Error { EINVAL } } + +/// Error type for [`Vec::remove`]. +pub struct RemoveError; + +impl Debug for RemoveError { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + write!(f, "Index out of bounds") + } +} + +impl From for Error { + fn from(_: RemoveError) -> Error { + EINVAL + } +} From 771c5a7d9843643b035938624050e7769133b9cc Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Fri, 2 May 2025 13:19:35 +0000 Subject: [PATCH 38/78] rust: alloc: add Vec::insert_within_capacity This adds a variant of Vec::insert that does not allocate memory. This makes it safe to use this function while holding a spinlock. Rust Binder uses it for the range allocator fast path. Signed-off-by: Alice Ryhl Reviewed-by: Greg Kroah-Hartman Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20250502-vec-methods-v5-7-06d20ad9366f@google.com Signed-off-by: Danilo Krummrich --- rust/kernel/alloc/kvec.rs | 51 +++++++++++++++++++++++++++++++- rust/kernel/alloc/kvec/errors.rs | 23 ++++++++++++++ 2 files changed, 73 insertions(+), 1 deletion(-) diff --git a/rust/kernel/alloc/kvec.rs b/rust/kernel/alloc/kvec.rs index 3f2617b08753..1a0dd852a468 100644 --- a/rust/kernel/alloc/kvec.rs +++ b/rust/kernel/alloc/kvec.rs @@ -22,7 +22,7 @@ }; mod errors; -pub use self::errors::{PushError, RemoveError}; +pub use self::errors::{InsertError, PushError, RemoveError}; /// Create a [`KVec`] containing the arguments. /// @@ -358,6 +358,55 @@ unsafe fn push_within_capacity_unchecked(&mut self, v: T) { unsafe { self.inc_len(1) }; } + /// Inserts an element at the given index in the [`Vec`] instance. + /// + /// Fails if the vector does not have capacity for the new element. Panics if the index is out + /// of bounds. + /// + /// # Examples + /// + /// ``` + /// use kernel::alloc::kvec::InsertError; + /// + /// let mut v = KVec::with_capacity(5, GFP_KERNEL)?; + /// for i in 0..5 { + /// v.insert_within_capacity(0, i)?; + /// } + /// + /// assert!(matches!(v.insert_within_capacity(0, 5), Err(InsertError::OutOfCapacity(_)))); + /// assert!(matches!(v.insert_within_capacity(1000, 5), Err(InsertError::IndexOutOfBounds(_)))); + /// assert_eq!(v, [4, 3, 2, 1, 0]); + /// # Ok::<(), Error>(()) + /// ``` + pub fn insert_within_capacity( + &mut self, + index: usize, + element: T, + ) -> Result<(), InsertError> { + let len = self.len(); + if index > len { + return Err(InsertError::IndexOutOfBounds(element)); + } + + if len >= self.capacity() { + return Err(InsertError::OutOfCapacity(element)); + } + + // SAFETY: This is in bounds since `index <= len < capacity`. + let p = unsafe { self.as_mut_ptr().add(index) }; + // INVARIANT: This breaks the Vec invariants by making `index` contain an invalid element, + // but we restore the invariants below. + // SAFETY: Both the src and dst ranges end no later than one element after the length. + // Since the length is less than the capacity, both ranges are in bounds of the allocation. + unsafe { ptr::copy(p, p.add(1), len - index) }; + // INVARIANT: This restores the Vec invariants. + // SAFETY: The pointer is in-bounds of the allocation. + unsafe { ptr::write(p, element) }; + // SAFETY: Index `len` contains a valid element due to the above copy and write. + unsafe { self.inc_len(1) }; + Ok(()) + } + /// Removes the last element from a vector and returns it, or `None` if it is empty. /// /// # Examples diff --git a/rust/kernel/alloc/kvec/errors.rs b/rust/kernel/alloc/kvec/errors.rs index 06fe696e8bc6..348b8d27e102 100644 --- a/rust/kernel/alloc/kvec/errors.rs +++ b/rust/kernel/alloc/kvec/errors.rs @@ -36,3 +36,26 @@ fn from(_: RemoveError) -> Error { EINVAL } } + +/// Error type for [`Vec::insert_within_capacity`]. +pub enum InsertError { + /// The value could not be inserted because the index is out of bounds. + IndexOutOfBounds(T), + /// The value could not be inserted because the vector is out of capacity. + OutOfCapacity(T), +} + +impl Debug for InsertError { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + match self { + InsertError::IndexOutOfBounds(_) => write!(f, "Index out of bounds"), + InsertError::OutOfCapacity(_) => write!(f, "Not enough capacity"), + } + } +} + +impl From> for Error { + fn from(_: InsertError) -> Error { + EINVAL + } +} From 878620c5a93a24af10e64b116e66c2314e91a719 Mon Sep 17 00:00:00 2001 From: Kunwu Chan Date: Fri, 21 Mar 2025 16:01:24 +0800 Subject: [PATCH 39/78] rust: page: optimize rust symbol generation for Page When build the kernel using the llvm-18.1.3-rust-1.85.0-x86_64 with ARCH=arm64, the following symbols are generated: $nm vmlinux | grep ' _R'.*Page | rustfilt ffff8000805b6f98 T ::alloc_page ffff8000805b715c T ::fill_zero_raw ffff8000805b720c T ::copy_from_user_slice_raw ffff8000805b6fb4 T ::read_raw ffff8000805b7088 T ::write_raw ffff8000805b72fc T ::drop These Rust symbols(alloc_page and drop) are trivial wrappers around the C functions alloc_pages and __free_pages. It doesn't make sense to go through a trivial wrapper for these functions, so mark them inline. Link: https://github.com/Rust-for-Linux/linux/issues/1145 Suggested-by: Alice Ryhl Co-developed-by: Grace Deng Signed-off-by: Grace Deng Signed-off-by: Kunwu Chan Reviewed-by: Alice Ryhl Link: https://lore.kernel.org/r/20250321080124.484647-1-kunwu.chan@linux.dev [ Removed spurious colon in title. - Miguel ] Signed-off-by: Miguel Ojeda --- rust/kernel/page.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rust/kernel/page.rs b/rust/kernel/page.rs index f6126aca33a6..7c1b17246ed5 100644 --- a/rust/kernel/page.rs +++ b/rust/kernel/page.rs @@ -69,6 +69,7 @@ impl Page { /// let page = Page::alloc_page(GFP_KERNEL | __GFP_ZERO)?; /// # Ok::<(), kernel::alloc::AllocError>(()) /// ``` + #[inline] pub fn alloc_page(flags: Flags) -> Result { // SAFETY: Depending on the value of `gfp_flags`, this call may sleep. Other than that, it // is always safe to call this method. @@ -251,6 +252,7 @@ pub unsafe fn copy_from_user_slice_raw( } impl Drop for Page { + #[inline] fn drop(&mut self) { // SAFETY: By the type invariants, we have ownership of the page and can free it. unsafe { bindings::__free_pages(self.page.as_ptr(), 0) }; From 79d04e73ba481f8537691095cdd2cfd3acdb9c80 Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Wed, 16 Apr 2025 13:24:54 +0200 Subject: [PATCH 40/78] docs: rust: explain that `///` vs. `//` applies to private items too Sometimes kernel developers use `//` for documenting private items, since those do not get rendered at the moment. That is reasonable, but the intention behind `///` (and `//!`) vs. `//` is to convey the distinction between documentation and other kinds of comments, such as implementation details or TODOs. It also increases consistency with the public items and thus e.g. allows to change visibility of an item with less changes involved. It is not just useful for human readers, but also tooling. For instance, we may want to eventually generate documentation for private items (perhaps as a toggle in the HTML UI). On top of that, `rustdoc` lints as usual for those, too, so we may want to do it even if we do not use the result. Thus document this explicitly. Link: https://lore.kernel.org/rust-for-linux/CANiq72n_C7exSOMe5yf-7jKKnhSCv+a9QcD=OE2B_Q2UFBL3Xg@mail.gmail.com/ Link: https://github.com/Rust-for-Linux/linux/issues/1157 Reviewed-by: Alice Ryhl Reviewed-by: Christian Schrefl Reviewed-by: Viresh Kumar Link: https://lore.kernel.org/r/20250416112454.2503872-1-ojeda@kernel.org [ Fixed typo. - Miguel ] Signed-off-by: Miguel Ojeda --- Documentation/rust/coding-guidelines.rst | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Documentation/rust/coding-guidelines.rst b/Documentation/rust/coding-guidelines.rst index 27f2a7bb5a4a..6c6c51b4cf46 100644 --- a/Documentation/rust/coding-guidelines.rst +++ b/Documentation/rust/coding-guidelines.rst @@ -85,6 +85,18 @@ written after the documentation, e.g.: // ... } +This applies to both public and private items. This increases consistency with +public items, allows changes to visibility with less changes involved and will +allow us to potentially generate the documentation for private items as well. +In other words, if documentation is written for a private item, then ``///`` +should still be used. For instance: + +.. code-block:: rust + + /// My private function. + // TODO: ... + fn f() {} + One special kind of comments are the ``// SAFETY:`` comments. These must appear before every ``unsafe`` block, and they explain why the code inside the block is correct/sound, i.e. why it cannot trigger undefined behavior in any case, e.g.: From 0fa5f8c877cae959de7cf6c3dc054e23e7ebcd75 Mon Sep 17 00:00:00 2001 From: Altan Ozlu Date: Wed, 26 Mar 2025 20:25:36 +0000 Subject: [PATCH 41/78] rust: static_assert: add optional message Add an optional panic message to the `static_assert!` macro. The panic message doesn't support argument formatting, because the `assert!` macro only supports formatting in non-const contexts. Suggested-by: Miguel Ojeda Link: https://github.com/Rust-for-Linux/linux/issues/1149 Signed-off-by: Altan Ozlu Reviewed-by: Benno Lossin Reviewed-by: Trevor Gross Link: https://lore.kernel.org/r/20250326202520.1176162-2-altan@ozlu.eu Signed-off-by: Miguel Ojeda --- rust/kernel/static_assert.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/rust/kernel/static_assert.rs b/rust/kernel/static_assert.rs index 3115ee0ba8e9..d8120f838260 100644 --- a/rust/kernel/static_assert.rs +++ b/rust/kernel/static_assert.rs @@ -6,6 +6,10 @@ /// /// Similar to C11 [`_Static_assert`] and C++11 [`static_assert`]. /// +/// An optional panic message can be supplied after the expression. +/// Currently only a string literal without formatting is supported +/// due to constness limitations of the [`assert!`] macro. +/// /// The feature may be added to Rust in the future: see [RFC 2790]. /// /// [`_Static_assert`]: https://en.cppreference.com/w/c/language/_Static_assert @@ -25,10 +29,11 @@ /// x + 2 /// } /// static_assert!(f(40) == 42); +/// static_assert!(f(40) == 42, "f(x) must add 2 to the given input."); /// ``` #[macro_export] macro_rules! static_assert { - ($condition:expr) => { - const _: () = core::assert!($condition); + ($condition:expr $(,$arg:literal)?) => { + const _: () = core::assert!($condition $(,$arg)?); }; } From 7d8dee4689278e174900509b8c4604651159d8ee Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Tue, 29 Apr 2025 17:14:45 +0200 Subject: [PATCH 42/78] rust: uaccess: take advantage of the prelude and `Result`'s defaults The `kernel` prelude brings `Result` and the error codes; and the prelude itself is already available in the examples automatically. In addition, `Result` already defaults to `T = ()`. Thus simplify. Reviewed-by: Alice Ryhl Link: https://lore.kernel.org/r/20250429151445.438977-1-ojeda@kernel.org Signed-off-by: Miguel Ojeda --- rust/kernel/uaccess.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs index 80a9782b1c6e..7e4c953ba8a1 100644 --- a/rust/kernel/uaccess.rs +++ b/rust/kernel/uaccess.rs @@ -46,10 +46,9 @@ /// /// ```no_run /// use kernel::ffi::c_void; -/// use kernel::error::Result; /// use kernel::uaccess::{UserPtr, UserSlice}; /// -/// fn bytes_add_one(uptr: UserPtr, len: usize) -> Result<()> { +/// fn bytes_add_one(uptr: UserPtr, len: usize) -> Result { /// let (read, mut write) = UserSlice::new(uptr, len).reader_writer(); /// /// let mut buf = KVec::new(); @@ -68,7 +67,6 @@ /// /// ```no_run /// use kernel::ffi::c_void; -/// use kernel::error::{code::EINVAL, Result}; /// use kernel::uaccess::{UserPtr, UserSlice}; /// /// /// Returns whether the data in this region is valid. From b9b701fce49a448b1e046f7cda592fec2958e5cd Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Thu, 27 Mar 2025 22:13:02 +0100 Subject: [PATCH 43/78] rust: clarify the language unstable features in use We track the details of which Rust features we use at our usual "live list" [1] (and its sub-lists), but in light of a discussion in the LWN article [2], it would help to clarify it in the source code. In particular, we are very close to rely only on stable Rust language-wise -- essentially only two language features remain (including the `kernel` crate). Thus add some details in both the feature list of the `kernel` crate as well as the list of allowed features. This does not over every single feature, and there are quite a few non-language features that we use too. To have the full picture, please refer to [1]. Link: https://github.com/Rust-for-Linux/linux/issues/2 [1] Link: https://lwn.net/Articles/1015409/ [2] Suggested-by: Andreas Hindborg Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20250327211302.286313-1-ojeda@kernel.org [ Improved comments with suggestions from the list. - Miguel ] Signed-off-by: Miguel Ojeda --- rust/kernel/lib.rs | 28 +++++++++++++++++++++------- scripts/Makefile.build | 9 +++++++++ 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs index de07aadd1ff5..28007be98fba 100644 --- a/rust/kernel/lib.rs +++ b/rust/kernel/lib.rs @@ -12,20 +12,34 @@ //! do so first instead of bypassing this crate. #![no_std] -#![feature(arbitrary_self_types)] -#![cfg_attr(CONFIG_RUSTC_HAS_COERCE_POINTEE, feature(derive_coerce_pointee))] -#![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(coerce_unsized))] -#![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(dispatch_from_dyn))] -#![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(unsize))] +// +// Please see https://github.com/Rust-for-Linux/linux/issues/2 for details on +// the unstable features in use. +// +// Stable since Rust 1.79.0. #![feature(inline_const)] +// +// Stable since Rust 1.81.0. #![feature(lint_reasons)] -// Stable in Rust 1.82 +// +// Stable since Rust 1.82.0. #![feature(raw_ref_op)] -// Stable in Rust 1.83 +// +// Stable since Rust 1.83.0. #![feature(const_maybe_uninit_as_mut_ptr)] #![feature(const_mut_refs)] #![feature(const_ptr_write)] #![feature(const_refs_to_cell)] +// +// Expected to become stable. +#![feature(arbitrary_self_types)] +// +// `feature(derive_coerce_pointee)` is expected to become stable. Before Rust +// 1.84.0, it did not exist, so enable the predecessor features. +#![cfg_attr(CONFIG_RUSTC_HAS_COERCE_POINTEE, feature(derive_coerce_pointee))] +#![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(coerce_unsized))] +#![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(dispatch_from_dyn))] +#![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(unsize))] // Ensure conditional compilation based on the kernel configuration works; // otherwise we may silently break things like initcall handling. diff --git a/scripts/Makefile.build b/scripts/Makefile.build index 13dcd86e74ca..da2d720c964a 100644 --- a/scripts/Makefile.build +++ b/scripts/Makefile.build @@ -222,6 +222,15 @@ $(obj)/%.lst: $(obj)/%.c FORCE # Compile Rust sources (.rs) # --------------------------------------------------------------------------- +# The features in this list are the ones allowed for non-`rust/` code. +# +# - Stable since Rust 1.81.0: `feature(lint_reasons)`. +# - Stable since Rust 1.82.0: `feature(asm_const)`, `feature(raw_ref_op)`. +# - Stable since Rust 1.87.0: `feature(asm_goto)`. +# - Expected to become stable: `feature(arbitrary_self_types)`. +# +# Please see https://github.com/Rust-for-Linux/linux/issues/2 for details on +# the unstable features in use. rust_allowed_features := asm_const,asm_goto,arbitrary_self_types,lint_reasons,raw_ref_op # `--out-dir` is required to avoid temporaries being created by `rustc` in the From 86d990c7b699581918de2a379c6eebac7292940e Mon Sep 17 00:00:00 2001 From: Xizhe Yin Date: Mon, 7 Apr 2025 11:34:41 +0800 Subject: [PATCH 44/78] rust: convert raw URLs to Markdown autolinks in comments Some comments in Rust files use raw URLs (http://example.com) rather than Markdown autolinks . This inconsistency makes the documentation less uniform and harder to maintain. This patch converts all remaining raw URLs in Rust code comments to use the Markdown autolink format, maintaining consistency with the rest of the codebase which already uses this style. Suggested-by: Miguel Ojeda Link: https://github.com/Rust-for-Linux/linux/issues/1153 Signed-off-by: Xizhe Yin Link: https://lore.kernel.org/r/509F0B66E3C1575D+20250407033441.5567-1-xizheyin@smail.nju.edu.cn [ Used From form for Signed-off-by. Sorted tags. - Miguel ] Signed-off-by: Miguel Ojeda --- rust/kernel/alloc/kbox.rs | 2 +- rust/kernel/block/mq/gen_disk.rs | 2 +- rust/kernel/std_vendor.rs | 2 +- rust/kernel/sync/arc.rs | 2 +- scripts/generate_rust_target.rs | 4 ++-- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/rust/kernel/alloc/kbox.rs b/rust/kernel/alloc/kbox.rs index b77d32f3a58b..604d12c6f5bd 100644 --- a/rust/kernel/alloc/kbox.rs +++ b/rust/kernel/alloc/kbox.rs @@ -101,7 +101,7 @@ pub type KVBox = Box; // SAFETY: All zeros is equivalent to `None` (option layout optimization guarantee: -// https://doc.rust-lang.org/stable/std/option/index.html#representation). +// ). unsafe impl ZeroableOption for Box {} // SAFETY: `Box` is `Send` if `T` is `Send` because the `Box` owns a `T`. diff --git a/rust/kernel/block/mq/gen_disk.rs b/rust/kernel/block/mq/gen_disk.rs index 14806e1997fd..cd54cd64ea88 100644 --- a/rust/kernel/block/mq/gen_disk.rs +++ b/rust/kernel/block/mq/gen_disk.rs @@ -129,7 +129,7 @@ pub fn build( get_unique_id: None, // TODO: Set to THIS_MODULE. Waiting for const_refs_to_static feature to // be merged (unstable in rustc 1.78 which is staged for linux 6.10) - // https://github.com/rust-lang/rust/issues/119618 + // owner: core::ptr::null_mut(), pr_ops: core::ptr::null_mut(), free_disk: None, diff --git a/rust/kernel/std_vendor.rs b/rust/kernel/std_vendor.rs index 279bd353687a..abbab5050cc5 100644 --- a/rust/kernel/std_vendor.rs +++ b/rust/kernel/std_vendor.rs @@ -148,7 +148,7 @@ macro_rules! dbg { }; ($val:expr $(,)?) => { // Use of `match` here is intentional because it affects the lifetimes - // of temporaries - https://stackoverflow.com/a/48732525/1063961 + // of temporaries - match $val { tmp => { $crate::pr_info!("[{}:{}:{}] {} = {:#?}\n", diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs index 8484c814609a..350c380bb8d4 100644 --- a/rust/kernel/sync/arc.rs +++ b/rust/kernel/sync/arc.rs @@ -135,7 +135,7 @@ pub struct Arc { // meaningful with respect to dropck - but this may change in the future so this is left here // out of an abundance of caution. // - // See https://doc.rust-lang.org/nomicon/phantom-data.html#generic-parameters-and-drop-checking + // See // for more detail on the semantics of dropck in the presence of `PhantomData`. _p: PhantomData>, } diff --git a/scripts/generate_rust_target.rs b/scripts/generate_rust_target.rs index 8667d0ae3c82..39c82908ff3a 100644 --- a/scripts/generate_rust_target.rs +++ b/scripts/generate_rust_target.rs @@ -209,7 +209,7 @@ fn main() { // target feature of the same name plus the other two target features in // `clang/lib/Driver/ToolChains/Arch/X86.cpp`. These should be eventually enabled via // `-Ctarget-feature` when `rustc` starts recognizing them (or via a new dedicated - // flag); see https://github.com/rust-lang/rust/issues/116852. + // flag); see . features += ",+retpoline-external-thunk"; features += ",+retpoline-indirect-branches"; features += ",+retpoline-indirect-calls"; @@ -218,7 +218,7 @@ fn main() { // The kernel uses `-mharden-sls=all`, which Clang maps to both these target features in // `clang/lib/Driver/ToolChains/Arch/X86.cpp`. These should be eventually enabled via // `-Ctarget-feature` when `rustc` starts recognizing them (or via a new dedicated - // flag); see https://github.com/rust-lang/rust/issues/116851. + // flag); see . features += ",+harden-sls-ijmp"; features += ",+harden-sls-ret"; } From edc5e6e019c99b529b3d1f2801d5cce9924ae79b Mon Sep 17 00:00:00 2001 From: Andrew Ballance Date: Tue, 25 Mar 2025 08:33:52 -0500 Subject: [PATCH 45/78] rust: replace rustdoc references to alloc::format Replace alloc::format[1] in the pr_* and dev_* macros' doc comments with std::format[2] because they are identical but less likely to get confused with the kernel's alloc crate. And add a url link for the std::format! macro. Link: https://doc.rust-lang.org/alloc/macro.format.html [1] Link: https://doc.rust-lang.org/std/macro.format.html [2] Reviewed-by: Benno Lossin Signed-off-by: Andrew Ballance Link: https://lore.kernel.org/r/20250325133352.441425-1-andrewjballance@gmail.com [ Fixed typo and reworded slightly. - Miguel ] Signed-off-by: Miguel Ojeda --- rust/kernel/device.rs | 24 ++++++++++++++++-------- rust/kernel/print.rs | 27 ++++++++++++++++++--------- 2 files changed, 34 insertions(+), 17 deletions(-) diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs index 21b343a1dc4d..5c372cf27ed0 100644 --- a/rust/kernel/device.rs +++ b/rust/kernel/device.rs @@ -252,9 +252,10 @@ macro_rules! dev_printk { /// Equivalent to the kernel's `dev_emerg` macro. /// /// Mimics the interface of [`std::print!`]. More information about the syntax is available from -/// [`core::fmt`] and `alloc::format!`. +/// [`core::fmt`] and [`std::format!`]. /// /// [`std::print!`]: https://doc.rust-lang.org/std/macro.print.html +/// [`std::format!`]: https://doc.rust-lang.org/std/macro.format.html /// /// # Examples /// @@ -277,9 +278,10 @@ macro_rules! dev_emerg { /// Equivalent to the kernel's `dev_alert` macro. /// /// Mimics the interface of [`std::print!`]. More information about the syntax is available from -/// [`core::fmt`] and `alloc::format!`. +/// [`core::fmt`] and [`std::format!`]. /// /// [`std::print!`]: https://doc.rust-lang.org/std/macro.print.html +/// [`std::format!`]: https://doc.rust-lang.org/std/macro.format.html /// /// # Examples /// @@ -302,9 +304,10 @@ macro_rules! dev_alert { /// Equivalent to the kernel's `dev_crit` macro. /// /// Mimics the interface of [`std::print!`]. More information about the syntax is available from -/// [`core::fmt`] and `alloc::format!`. +/// [`core::fmt`] and [`std::format!`]. /// /// [`std::print!`]: https://doc.rust-lang.org/std/macro.print.html +/// [`std::format!`]: https://doc.rust-lang.org/std/macro.format.html /// /// # Examples /// @@ -327,9 +330,10 @@ macro_rules! dev_crit { /// Equivalent to the kernel's `dev_err` macro. /// /// Mimics the interface of [`std::print!`]. More information about the syntax is available from -/// [`core::fmt`] and `alloc::format!`. +/// [`core::fmt`] and [`std::format!`]. /// /// [`std::print!`]: https://doc.rust-lang.org/std/macro.print.html +/// [`std::format!`]: https://doc.rust-lang.org/std/macro.format.html /// /// # Examples /// @@ -352,9 +356,10 @@ macro_rules! dev_err { /// Equivalent to the kernel's `dev_warn` macro. /// /// Mimics the interface of [`std::print!`]. More information about the syntax is available from -/// [`core::fmt`] and `alloc::format!`. +/// [`core::fmt`] and [`std::format!`]. /// /// [`std::print!`]: https://doc.rust-lang.org/std/macro.print.html +/// [`std::format!`]: https://doc.rust-lang.org/std/macro.format.html /// /// # Examples /// @@ -377,9 +382,10 @@ macro_rules! dev_warn { /// Equivalent to the kernel's `dev_notice` macro. /// /// Mimics the interface of [`std::print!`]. More information about the syntax is available from -/// [`core::fmt`] and `alloc::format!`. +/// [`core::fmt`] and [`std::format!`]. /// /// [`std::print!`]: https://doc.rust-lang.org/std/macro.print.html +/// [`std::format!`]: https://doc.rust-lang.org/std/macro.format.html /// /// # Examples /// @@ -402,9 +408,10 @@ macro_rules! dev_notice { /// Equivalent to the kernel's `dev_info` macro. /// /// Mimics the interface of [`std::print!`]. More information about the syntax is available from -/// [`core::fmt`] and `alloc::format!`. +/// [`core::fmt`] and [`std::format!`]. /// /// [`std::print!`]: https://doc.rust-lang.org/std/macro.print.html +/// [`std::format!`]: https://doc.rust-lang.org/std/macro.format.html /// /// # Examples /// @@ -427,9 +434,10 @@ macro_rules! dev_info { /// Equivalent to the kernel's `dev_dbg` macro, except that it doesn't support dynamic debug yet. /// /// Mimics the interface of [`std::print!`]. More information about the syntax is available from -/// [`core::fmt`] and `alloc::format!`. +/// [`core::fmt`] and [`std::format!`]. /// /// [`std::print!`]: https://doc.rust-lang.org/std/macro.print.html +/// [`std::format!`]: https://doc.rust-lang.org/std/macro.format.html /// /// # Examples /// diff --git a/rust/kernel/print.rs b/rust/kernel/print.rs index cf4714242e14..9783d960a97a 100644 --- a/rust/kernel/print.rs +++ b/rust/kernel/print.rs @@ -198,10 +198,11 @@ macro_rules! print_macro ( /// Equivalent to the kernel's [`pr_emerg`] macro. /// /// Mimics the interface of [`std::print!`]. See [`core::fmt`] and -/// `alloc::format!` for information about the formatting syntax. +/// [`std::format!`] for information about the formatting syntax. /// /// [`pr_emerg`]: https://docs.kernel.org/core-api/printk-basics.html#c.pr_emerg /// [`std::print!`]: https://doc.rust-lang.org/std/macro.print.html +/// [`std::format!`]: https://doc.rust-lang.org/std/macro.format.html /// /// # Examples /// @@ -222,10 +223,11 @@ macro_rules! pr_emerg ( /// Equivalent to the kernel's [`pr_alert`] macro. /// /// Mimics the interface of [`std::print!`]. See [`core::fmt`] and -/// `alloc::format!` for information about the formatting syntax. +/// [`std::format!`] for information about the formatting syntax. /// /// [`pr_alert`]: https://docs.kernel.org/core-api/printk-basics.html#c.pr_alert /// [`std::print!`]: https://doc.rust-lang.org/std/macro.print.html +/// [`std::format!`]: https://doc.rust-lang.org/std/macro.format.html /// /// # Examples /// @@ -246,10 +248,11 @@ macro_rules! pr_alert ( /// Equivalent to the kernel's [`pr_crit`] macro. /// /// Mimics the interface of [`std::print!`]. See [`core::fmt`] and -/// `alloc::format!` for information about the formatting syntax. +/// [`std::format!`] for information about the formatting syntax. /// /// [`pr_crit`]: https://docs.kernel.org/core-api/printk-basics.html#c.pr_crit /// [`std::print!`]: https://doc.rust-lang.org/std/macro.print.html +/// [`std::format!`]: https://doc.rust-lang.org/std/macro.format.html /// /// # Examples /// @@ -270,10 +273,11 @@ macro_rules! pr_crit ( /// Equivalent to the kernel's [`pr_err`] macro. /// /// Mimics the interface of [`std::print!`]. See [`core::fmt`] and -/// `alloc::format!` for information about the formatting syntax. +/// [`std::format!`] for information about the formatting syntax. /// /// [`pr_err`]: https://docs.kernel.org/core-api/printk-basics.html#c.pr_err /// [`std::print!`]: https://doc.rust-lang.org/std/macro.print.html +/// [`std::format!`]: https://doc.rust-lang.org/std/macro.format.html /// /// # Examples /// @@ -294,10 +298,11 @@ macro_rules! pr_err ( /// Equivalent to the kernel's [`pr_warn`] macro. /// /// Mimics the interface of [`std::print!`]. See [`core::fmt`] and -/// `alloc::format!` for information about the formatting syntax. +/// [`std::format!`] for information about the formatting syntax. /// /// [`pr_warn`]: https://docs.kernel.org/core-api/printk-basics.html#c.pr_warn /// [`std::print!`]: https://doc.rust-lang.org/std/macro.print.html +/// [`std::format!`]: https://doc.rust-lang.org/std/macro.format.html /// /// # Examples /// @@ -318,10 +323,11 @@ macro_rules! pr_warn ( /// Equivalent to the kernel's [`pr_notice`] macro. /// /// Mimics the interface of [`std::print!`]. See [`core::fmt`] and -/// `alloc::format!` for information about the formatting syntax. +/// [`std::format!`] for information about the formatting syntax. /// /// [`pr_notice`]: https://docs.kernel.org/core-api/printk-basics.html#c.pr_notice /// [`std::print!`]: https://doc.rust-lang.org/std/macro.print.html +/// [`std::format!`]: https://doc.rust-lang.org/std/macro.format.html /// /// # Examples /// @@ -342,10 +348,11 @@ macro_rules! pr_notice ( /// Equivalent to the kernel's [`pr_info`] macro. /// /// Mimics the interface of [`std::print!`]. See [`core::fmt`] and -/// `alloc::format!` for information about the formatting syntax. +/// [`std::format!`] for information about the formatting syntax. /// /// [`pr_info`]: https://docs.kernel.org/core-api/printk-basics.html#c.pr_info /// [`std::print!`]: https://doc.rust-lang.org/std/macro.print.html +/// [`std::format!`]: https://doc.rust-lang.org/std/macro.format.html /// /// # Examples /// @@ -368,10 +375,11 @@ macro_rules! pr_info ( /// yet. /// /// Mimics the interface of [`std::print!`]. See [`core::fmt`] and -/// `alloc::format!` for information about the formatting syntax. +/// [`std::format!`] for information about the formatting syntax. /// /// [`pr_debug`]: https://docs.kernel.org/core-api/printk-basics.html#c.pr_debug /// [`std::print!`]: https://doc.rust-lang.org/std/macro.print.html +/// [`std::format!`]: https://doc.rust-lang.org/std/macro.format.html /// /// # Examples /// @@ -395,11 +403,12 @@ macro_rules! pr_debug ( /// Equivalent to the kernel's [`pr_cont`] macro. /// /// Mimics the interface of [`std::print!`]. See [`core::fmt`] and -/// `alloc::format!` for information about the formatting syntax. +/// [`std::format!`] for information about the formatting syntax. /// /// [`pr_info!`]: crate::pr_info! /// [`pr_cont`]: https://docs.kernel.org/core-api/printk-basics.html#c.pr_cont /// [`std::print!`]: https://doc.rust-lang.org/std/macro.print.html +/// [`std::format!`]: https://doc.rust-lang.org/std/macro.format.html /// /// # Examples /// From ea33f91f754cb982116a8447a30b549720af3e82 Mon Sep 17 00:00:00 2001 From: Benno Lossin Date: Mon, 19 May 2025 16:39:51 +0200 Subject: [PATCH 46/78] MAINTAINERS: mailmap: update Benno Lossin's email address Update Benno Lossin's email address to `@kernel.org`. Signed-off-by: Benno Lossin Link: https://lore.kernel.org/r/20250519143952.11412-1-lossin@kernel.org [ Fixed email in `.mailmap`. - Miguel ] Signed-off-by: Miguel Ojeda --- .mailmap | 1 + MAINTAINERS | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.mailmap b/.mailmap index 1c70e51c789d..b58e36b48a96 100644 --- a/.mailmap +++ b/.mailmap @@ -134,6 +134,7 @@ Ben Widawsky Benjamin Poirier Benjamin Tissoires Benjamin Tissoires +Benno Lossin Bingwu Zhang Bingwu Zhang Bjorn Andersson diff --git a/MAINTAINERS b/MAINTAINERS index b659fb27ab63..e58cafb019e6 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -21255,7 +21255,7 @@ M: Alex Gaynor R: Boqun Feng R: Gary Guo R: Björn Roy Baron -R: Benno Lossin +R: Benno Lossin R: Andreas Hindborg R: Alice Ryhl R: Trevor Gross @@ -21285,7 +21285,7 @@ F: rust/kernel/alloc.rs F: rust/kernel/alloc/ RUST [PIN-INIT] -M: Benno Lossin +M: Benno Lossin L: rust-for-linux@vger.kernel.org S: Maintained W: https://rust-for-linux.com/pin-init From ae8b3a83fb9de394f609035041cd7a668fda2ab3 Mon Sep 17 00:00:00 2001 From: Jihed Chaibi Date: Sat, 17 May 2025 02:26:04 +0200 Subject: [PATCH 47/78] rust: str: fix typo in comment Fix a typo ("then" to "than") in a comment. Signed-off-by: Jihed Chaibi Reviewed-by: Benno Lossin Fixes: fffed679eeea ("rust: str: add `Formatter` type") Link: https://lore.kernel.org/r/20250517002604.603223-1-jihed.chaibi.dev@gmail.com [ Reworded. - Miguel ] Signed-off-by: Miguel Ojeda --- rust/kernel/str.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs index 8768ab790580..98d5c74ec4f7 100644 --- a/rust/kernel/str.rs +++ b/rust/kernel/str.rs @@ -752,7 +752,7 @@ pub(crate) unsafe fn from_ptrs(pos: *mut u8, end: *mut u8) -> Self { /// for the lifetime of the returned [`RawFormatter`]. pub(crate) unsafe fn from_buffer(buf: *mut u8, len: usize) -> Self { let pos = buf as usize; - // INVARIANT: We ensure that `end` is never less then `buf`, and the safety requirements + // INVARIANT: We ensure that `end` is never less than `buf`, and the safety requirements // guarantees that the memory region is valid for writes. Self { pos, From bb941ea789f803cce766ca1e0f7c59a362aaf99a Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Tue, 20 May 2025 20:21:25 +0200 Subject: [PATCH 48/78] rust: remove unneeded Rust 1.87.0 `allow(clippy::ptr_eq)` For the Rust 1.87.0 release, Clippy was expected to warn with: error: use `core::ptr::eq` when comparing raw pointers --> rust/kernel/list.rs:438:12 | 438 | if self.first == item { | ^^^^^^^^^^^^^^^^^^ help: try: `core::ptr::eq(self.first, item)` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#ptr_eq = note: `-D clippy::ptr-eq` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::ptr_eq)]` However, a backport to relax a bit the `clippy::ptr_eq` finally landed, and thus Clippy did not warn by the time the release happened. Thus remove the `allow`s added back then, which were added just in case the backport did not land in time. See commit a39f30870927 ("rust: allow Rust 1.87.0's `clippy::ptr_eq` lint") for details. Link: https://github.com/rust-lang/rust/pull/140859 [1] Reviewed-by: Alice Ryhl Link: https://lore.kernel.org/r/20250520182125.806758-1-ojeda@kernel.org [ Reworded for clarity. - Miguel ] Signed-off-by: Miguel Ojeda --- rust/kernel/alloc/kvec.rs | 3 --- rust/kernel/list.rs | 3 --- 2 files changed, 6 deletions(-) diff --git a/rust/kernel/alloc/kvec.rs b/rust/kernel/alloc/kvec.rs index d9f5b79b26dc..1a0dd852a468 100644 --- a/rust/kernel/alloc/kvec.rs +++ b/rust/kernel/alloc/kvec.rs @@ -2,9 +2,6 @@ //! Implementation of [`Vec`]. -// May not be needed in Rust 1.87.0 (pending beta backport). -#![allow(clippy::ptr_eq)] - use super::{ allocator::{KVmalloc, Kmalloc, Vmalloc}, layout::ArrayLayout, diff --git a/rust/kernel/list.rs b/rust/kernel/list.rs index 2054682c5724..a335c3b1ff5e 100644 --- a/rust/kernel/list.rs +++ b/rust/kernel/list.rs @@ -4,9 +4,6 @@ //! A linked list implementation. -// May not be needed in Rust 1.87.0 (pending beta backport). -#![allow(clippy::ptr_eq)] - use crate::sync::ArcBorrow; use crate::types::Opaque; use core::iter::{DoubleEndedIterator, FusedIterator}; From 249c3a0e53acefc2b06d3b3e1fc28fb2081f878d Mon Sep 17 00:00:00 2001 From: FUJITA Tomonori Date: Sat, 8 Mar 2025 13:45:06 +0900 Subject: [PATCH 49/78] rust: module: place cleanup_module() in .exit.text section Place cleanup_module() in .exit.text section. Currently, cleanup_module() is likely placed in the .text section. It's inconsistent with the layout of C modules, where cleanup_module() is placed in .exit.text. [ Boqun asked for an example of how the section changed to be put in the log. Tomonori provided the following examples: C module: $ objdump -t ~/build/x86/drivers/block/loop.o|grep clean 0000000000000000 l O .exit.data 0000000000000008 __UNIQUE_ID___addressable_cleanup_module412 0000000000000000 g F .exit.text 000000000000009c cleanup_module Rust module without this patch: $ objdump -t ~/build/x86/samples/rust/rust_minimal.o|grep clean 00000000000002b0 g F .text 00000000000000c6 cleanup_module 0000000000000000 g O .exit.data 0000000000000008 _R...___UNIQUE_ID___addressable_cleanup_module Rust module with this patch: $ objdump -t ~/build/x86/samples/rust/rust_minimal.o|grep clean 0000000000000000 g F .exit.text 00000000000000c6 cleanup_module 0000000000000000 g O .exit.data 0000000000000008 _R...___UNIQUE_ID___addressable_cleanup_module - Miguel ] Signed-off-by: FUJITA Tomonori Acked-by: Jarkko Sakkinen Link: https://lore.kernel.org/r/20250308044506.14458-1-fujita.tomonori@gmail.com Signed-off-by: Miguel Ojeda --- rust/macros/module.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/rust/macros/module.rs b/rust/macros/module.rs index 2f66107847f7..44e5cb108cea 100644 --- a/rust/macros/module.rs +++ b/rust/macros/module.rs @@ -278,6 +278,7 @@ mod __module_init {{ #[cfg(MODULE)] #[doc(hidden)] #[no_mangle] + #[link_section = \".exit.text\"] pub extern \"C\" fn cleanup_module() {{ // SAFETY: // - This function is inaccessible to the outside due to the double From 28669b2f37e9b7e98b62d0be2e10a5bf31c2b16f Mon Sep 17 00:00:00 2001 From: I Hsin Cheng Date: Mon, 10 Mar 2025 15:38:52 +0800 Subject: [PATCH 50/78] rust: list: Use "List::is_empty()" to perform checking when possible "List::is_empty()" provides a straight forward convention to check whether a given "List" is empty or not. There're numerous places in the current implementation still use "self.first.is_null()" to perform the equivalent check, replace them with "List::is_empty()". Signed-off-by: I Hsin Cheng Link: https://lore.kernel.org/r/20250310073853.427954-1-richard120310@gmail.com Reviewed-by: Benno Lossin [ Rebased dropping the cases that do not apply anymore. - Miguel ] Signed-off-by: Miguel Ojeda --- rust/kernel/list.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rust/kernel/list.rs b/rust/kernel/list.rs index a335c3b1ff5e..7740c8f16cf6 100644 --- a/rust/kernel/list.rs +++ b/rust/kernel/list.rs @@ -319,7 +319,7 @@ pub fn push_front(&mut self, item: ListArc) { /// Removes the last item from this list. pub fn pop_back(&mut self) -> Option> { - if self.first.is_null() { + if self.is_empty() { return None; } @@ -331,7 +331,7 @@ pub fn pop_back(&mut self) -> Option> { /// Removes the first item from this list. pub fn pop_front(&mut self) -> Option> { - if self.first.is_null() { + if self.is_empty() { return None; } From 195746046c256ce5324772394c886ba798859fca Mon Sep 17 00:00:00 2001 From: I Hsin Cheng Date: Tue, 11 Mar 2025 21:33:57 +0800 Subject: [PATCH 51/78] rust: list: Add examples for linked list Add basic examples for the structure "List", which also serve as unit tests for basic list methods. It includes the following manipulations: * List creation * List emptiness check * List insertion through push_front(), push_back() * List item removal through pop_front(), pop_back() * Push one list to another through push_all_back() The method "remove()" doesn't have an example here because insertion with push_front() or push_back() will take the ownership of the item, which means we can't keep any valid reference to the node we want to remove, unless Cursor is used. The "remove" example through Cursor is already demonstrated with commit 52ae96f5187c ("rust: list: make the cursor point between elements"). Link: https://github.com/Rust-for-Linux/linux/issues/1121 Signed-off-by: I Hsin Cheng Reviewed-by: Alice Ryhl Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20250311133357.90322-1-richard120310@gmail.com [ Removed prelude import and spurious newlines. Formatted comments with the usual style. Reworded slightly. - Miguel ] Signed-off-by: Miguel Ojeda --- rust/kernel/list.rs | 108 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/rust/kernel/list.rs b/rust/kernel/list.rs index 7740c8f16cf6..c391c30b80f8 100644 --- a/rust/kernel/list.rs +++ b/rust/kernel/list.rs @@ -35,6 +35,114 @@ /// * All prev/next pointers in `ListLinks` fields of items in the list are valid and form a cycle. /// * For every item in the list, the list owns the associated [`ListArc`] reference and has /// exclusive access to the `ListLinks` field. +/// +/// # Examples +/// +/// ``` +/// use kernel::list::*; +/// +/// #[pin_data] +/// struct BasicItem { +/// value: i32, +/// #[pin] +/// links: ListLinks, +/// } +/// +/// impl BasicItem { +/// fn new(value: i32) -> Result> { +/// ListArc::pin_init(try_pin_init!(Self { +/// value, +/// links <- ListLinks::new(), +/// }), GFP_KERNEL) +/// } +/// } +/// +/// impl_has_list_links! { +/// impl HasListLinks<0> for BasicItem { self.links } +/// } +/// impl_list_arc_safe! { +/// impl ListArcSafe<0> for BasicItem { untracked; } +/// } +/// impl_list_item! { +/// impl ListItem<0> for BasicItem { using ListLinks; } +/// } +/// +/// // Create a new empty list. +/// let mut list = List::new(); +/// { +/// assert!(list.is_empty()); +/// } +/// +/// // Insert 3 elements using `push_back()`. +/// list.push_back(BasicItem::new(15)?); +/// list.push_back(BasicItem::new(10)?); +/// list.push_back(BasicItem::new(30)?); +/// +/// // Iterate over the list to verify the nodes were inserted correctly. +/// // [15, 10, 30] +/// { +/// let mut iter = list.iter(); +/// assert_eq!(iter.next().unwrap().value, 15); +/// assert_eq!(iter.next().unwrap().value, 10); +/// assert_eq!(iter.next().unwrap().value, 30); +/// assert!(iter.next().is_none()); +/// +/// // Verify the length of the list. +/// assert_eq!(list.iter().count(), 3); +/// } +/// +/// // Pop the items from the list using `pop_back()` and verify the content. +/// { +/// assert_eq!(list.pop_back().unwrap().value, 30); +/// assert_eq!(list.pop_back().unwrap().value, 10); +/// assert_eq!(list.pop_back().unwrap().value, 15); +/// } +/// +/// // Insert 3 elements using `push_front()`. +/// list.push_front(BasicItem::new(15)?); +/// list.push_front(BasicItem::new(10)?); +/// list.push_front(BasicItem::new(30)?); +/// +/// // Iterate over the list to verify the nodes were inserted correctly. +/// // [30, 10, 15] +/// { +/// let mut iter = list.iter(); +/// assert_eq!(iter.next().unwrap().value, 30); +/// assert_eq!(iter.next().unwrap().value, 10); +/// assert_eq!(iter.next().unwrap().value, 15); +/// assert!(iter.next().is_none()); +/// +/// // Verify the length of the list. +/// assert_eq!(list.iter().count(), 3); +/// } +/// +/// // Pop the items from the list using `pop_front()` and verify the content. +/// { +/// assert_eq!(list.pop_front().unwrap().value, 30); +/// assert_eq!(list.pop_front().unwrap().value, 10); +/// } +/// +/// // Push `list2` to `list` through `push_all_back()`. +/// // list: [15] +/// // list2: [25, 35] +/// { +/// let mut list2 = List::new(); +/// list2.push_back(BasicItem::new(25)?); +/// list2.push_back(BasicItem::new(35)?); +/// +/// list.push_all_back(&mut list2); +/// +/// // list: [15, 25, 35] +/// // list2: [] +/// let mut iter = list.iter(); +/// assert_eq!(iter.next().unwrap().value, 15); +/// assert_eq!(iter.next().unwrap().value, 25); +/// assert_eq!(iter.next().unwrap().value, 35); +/// assert!(iter.next().is_none()); +/// assert!(list2.is_empty()); +/// } +/// # Result::<(), Error>::Ok(()) +/// ``` pub struct List, const ID: u64 = 0> { first: *mut ListLinksFields, _ty: PhantomData>, From cbeaa41dfe26b72639141e87183cb23e00d4b0dd Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Tue, 20 May 2025 20:55:55 +0200 Subject: [PATCH 52/78] objtool/rust: relax slice condition to cover more `noreturn` Rust functions Developers are indeed hitting other of the `noreturn` slice symbols in Nova [1], thus relax the last check in the list so that we catch all of them, i.e. *_4core5slice5index22slice_index_order_fail *_4core5slice5index24slice_end_index_len_fail *_4core5slice5index26slice_start_index_len_fail *_4core5slice5index29slice_end_index_overflow_fail *_4core5slice5index31slice_start_index_overflow_fail These all exist since at least Rust 1.78.0, thus backport it too. See commit 56d680dd23c3 ("objtool/rust: list `noreturn` Rust functions") for more details. Cc: stable@vger.kernel.org # Needed in 6.12.y and later. Cc: John Hubbard Cc: Timur Tabi Cc: Kane York Cc: Josh Poimboeuf Cc: Peter Zijlstra Reported-by: Joel Fernandes Fixes: 56d680dd23c3 ("objtool/rust: list `noreturn` Rust functions") Closes: https://lore.kernel.org/rust-for-linux/20250513180757.GA1295002@joelnvbox/ [1] Tested-by: Joel Fernandes Link: https://lore.kernel.org/r/20250520185555.825242-1-ojeda@kernel.org Signed-off-by: Miguel Ojeda --- tools/objtool/check.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/objtool/check.c b/tools/objtool/check.c index b21b12ec88d9..f23bdda737aa 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -230,7 +230,8 @@ static bool is_rust_noreturn(const struct symbol *func) str_ends_with(func->name, "_7___rustc17rust_begin_unwind") || strstr(func->name, "_4core9panicking13assert_failed") || strstr(func->name, "_4core9panicking11panic_const24panic_const_") || - (strstr(func->name, "_4core5slice5index24slice_") && + (strstr(func->name, "_4core5slice5index") && + strstr(func->name, "slice_") && str_ends_with(func->name, "_fail")); } From 8cbc95f983bcec7e042266766ffe0d68980e4290 Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Tue, 25 Mar 2025 19:43:09 +0100 Subject: [PATCH 53/78] rust: workaround `bindgen` issue with forward references to `enum` types `bindgen` currently generates the wrong type for an `enum` when there is a forward reference to it. For instance: enum E; enum E { A }; generates: pub const E_A: E = 0; pub type E = i32; instead of the expected: pub const E_A: E = 0; pub type E = ffi::c_uint; The issue was reported to upstream `bindgen` [1]. Now, both GCC and Clang support silently these forward references to `enum` types, unless `-Wpedantic` is passed, and it turns out that some headers in the kernel depend on them. Thus, depending on how the headers are included, which in turn may depend on the kernel configuration or the architecture, we may get a different type on the Rust side for a given C `enum`. That can be quite confusing, to say the least, especially since developers may only notice issues when building for other architectures like in [2]. In particular, they may end up forcing a cast and adding an `#[allow(clippy::unnecessary_cast)]` like it was done in commit 94e05a66ea3e ("rust: hrtimer: allow timer restart from timer handler"), which isn't great. Instead, let's have a section at the top of our `bindings_helper.h` that `#include`s the headers with the affected types -- hopefully there are not many cases and there is a single ordering that covers all cases. This allows us to remove the cast and the `#[allow]`, thus keeping the correct code in the source files. When the issue gets resolved in upstream `bindgen` (and we update our minimum `bindgen` version), we can easily remove this section at the top. Link: https://github.com/rust-lang/rust-bindgen/issues/3179 [1] Link: https://lore.kernel.org/rust-for-linux/87tt7md1s6.fsf@kernel.org/ [2] Acked-by: Andreas Hindborg Link: https://lore.kernel.org/r/20250325184309.97170-1-ojeda@kernel.org [ Added extra paragraph on the comment to clarify that the workaround may not be possible in some cases. - Miguel ] Signed-off-by: Miguel Ojeda --- rust/bindings/bindings_helper.h | 22 ++++++++++++++++++++++ rust/kernel/time/hrtimer.rs | 6 ++---- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h index e0bcd130b494..5532bbfd96eb 100644 --- a/rust/bindings/bindings_helper.h +++ b/rust/bindings/bindings_helper.h @@ -6,6 +6,28 @@ * Sorted alphabetically. */ +/* + * First, avoid forward references to `enum` types. + * + * This workarounds a `bindgen` issue with them: + * . + * + * Without this, the generated Rust type may be the wrong one (`i32`) or + * the proper one (typically `c_uint`) depending on how the headers are + * included, which in turn may depend on the particular kernel configuration + * or the architecture. + * + * The alternative would be to use casts and likely an + * `#[allow(clippy::unnecessary_cast)]` in the Rust source files. Instead, + * this approach allows us to keep the correct code in the source files and + * simply remove this section when the issue is fixed upstream and we bump + * the minimum `bindgen` version. + * + * This workaround may not be possible in some cases, depending on how the C + * headers are set up. + */ +#include + #include #include #include diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs index 17824aa0c0f3..9df3dcd2fa39 100644 --- a/rust/kernel/time/hrtimer.rs +++ b/rust/kernel/time/hrtimer.rs @@ -400,11 +400,9 @@ unsafe fn start(this: *const Self, expires: Ktime) { #[repr(u32)] pub enum HrTimerRestart { /// Timer should not be restarted. - #[allow(clippy::unnecessary_cast)] - NoRestart = bindings::hrtimer_restart_HRTIMER_NORESTART as u32, + NoRestart = bindings::hrtimer_restart_HRTIMER_NORESTART, /// Timer should be restarted. - #[allow(clippy::unnecessary_cast)] - Restart = bindings::hrtimer_restart_HRTIMER_RESTART as u32, + Restart = bindings::hrtimer_restart_HRTIMER_RESTART, } impl HrTimerRestart { From 977c4308ee4270cf46e2c66b37de8e04670daa0c Mon Sep 17 00:00:00 2001 From: Rudraksha Gupta Date: Thu, 22 May 2025 05:02:31 -0700 Subject: [PATCH 54/78] rust: arm: fix unknown (to Clang) argument '-mno-fdpic' Currently rust on arm fails to compile due to '-mno-fdpic'. This flag disables a GCC feature that we don't want for kernel builds, so let's skip it as it doesn't apply to Clang. UPD include/generated/asm-offsets.h CALL scripts/checksyscalls.sh RUSTC L rust/core.o BINDGEN rust/bindings/bindings_generated.rs BINDGEN rust/bindings/bindings_helpers_generated.rs CC rust/helpers/helpers.o Unable to generate bindings: clang diagnosed error: error: unknown argument: '-mno-fdpic' make[2]: *** [rust/Makefile:369: rust/bindings/bindings_helpers_generated.rs] Error 1 make[2]: *** Deleting file 'rust/bindings/bindings_helpers_generated.rs' make[2]: *** Waiting for unfinished jobs.... Unable to generate bindings: clang diagnosed error: error: unknown argument: '-mno-fdpic' make[2]: *** [rust/Makefile:349: rust/bindings/bindings_generated.rs] Error 1 make[2]: *** Deleting file 'rust/bindings/bindings_generated.rs' make[1]: *** [/home/pmos/build/src/linux-next-next-20250521/Makefile:1285: prepare] Error 2 make: *** [Makefile:248: __sub-make] Error 2 [ Naresh provided the draft diff [1]. Ben explained [2]: FDPIC is only relevant with no-MMU targets, and then only for userspace. When configured for the arm-*-uclinuxfdpiceabi target, GCC enables FDPIC by default to facilitate compiling userspace programs. FDPIC is never used for the kernel, and we pass -mno-fdpic when building the kernel to override the default and make sure FDPIC is disabled. and [3]: -mno-fdpic disables a GCC feature that we don't want for kernel builds. clang does not support this feature, so it always behaves as though -mno-fdpic is passed. Therefore, it should be fine to mix the two, at least as far as FDPIC is concerned. [1] https://lore.kernel.org/rust-for-linux/CA+G9fYt4otQK4pHv8pJBW9e28yHSGCDncKquwuJiJ_1ou0pq0w@mail.gmail.com/ [2] https://lore.kernel.org/rust-for-linux/aAKrq2InExQk7f_k@dell-precision-5540/ [3] https://lore.kernel.org/rust-for-linux/aAo_F_UP1Gd4jHlZ@dell-precision-5540/ - Miguel ] Reported-by: Linux Kernel Functional Testing Closes: https://lore.kernel.org/all/CA+G9fYvOanQBYXKSg7C6EU30k8sTRC0JRPJXYu7wWK51w38QUQ@mail.gmail.com/ Suggested-by: Miguel Ojeda Acked-by: Naresh Kamboju Signed-off-by: Rudraksha Gupta Link: https://lore.kernel.org/r/20250522-rust-mno-fdpic-arm-fix-v2-1-a6f691d9c198@gmail.com [ Reworded title. - Miguel ] Signed-off-by: Miguel Ojeda --- rust/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/Makefile b/rust/Makefile index 3aca903a7d08..f207ba0ed466 100644 --- a/rust/Makefile +++ b/rust/Makefile @@ -273,7 +273,7 @@ bindgen_skip_c_flags := -mno-fp-ret-in-387 -mpreferred-stack-boundary=% \ -fzero-call-used-regs=% -fno-stack-clash-protection \ -fno-inline-functions-called-once -fsanitize=bounds-strict \ -fstrict-flex-arrays=% -fmin-function-alignment=% \ - -fzero-init-padding-bits=% \ + -fzero-init-padding-bits=% -mno-fdpic \ --param=% --param asan-% # Derived from `scripts/Makefile.clang`. From de7cd3e4d6387df6a5ae8c4c32ff0479ebe0efb5 Mon Sep 17 00:00:00 2001 From: Igor Korotin Date: Mon, 19 May 2025 17:45:53 +0100 Subject: [PATCH 55/78] rust: use absolute paths in macros referencing core and kernel Macros and auto-generated code should use absolute paths, `::core::...` and `::kernel::...`, for core and kernel references. This prevents issues where user-defined modules named `core` or `kernel` could be picked up instead of the `core` or `kernel` crates. Thus clean some references up. Suggested-by: Benno Lossin Closes: https://github.com/Rust-for-Linux/linux/issues/1150 Signed-off-by: Igor Korotin Reviewed-by: Benno Lossin Acked-by: Greg Kroah-Hartman Link: https://lore.kernel.org/r/20250519164615.3310844-1-igor.korotin.linux@gmail.com [ Applied `rustfmt`. Reworded slightly. - Miguel ] Signed-off-by: Miguel Ojeda --- rust/ffi.rs | 2 +- rust/kernel/device.rs | 2 +- rust/kernel/device_id.rs | 4 ++-- rust/kernel/kunit.rs | 8 ++++---- rust/kernel/static_assert.rs | 2 +- rust/kernel/str.rs | 4 ++-- rust/macros/kunit.rs | 22 +++++++++++----------- rust/macros/lib.rs | 6 +++--- rust/macros/module.rs | 31 ++++++++++++++++--------------- scripts/rustdoc_test_builder.rs | 8 +++++--- scripts/rustdoc_test_gen.rs | 16 ++++++++++------ 11 files changed, 56 insertions(+), 49 deletions(-) diff --git a/rust/ffi.rs b/rust/ffi.rs index 584f75b49862..d60aad792af4 100644 --- a/rust/ffi.rs +++ b/rust/ffi.rs @@ -17,7 +17,7 @@ macro_rules! alias { // Check size compatibility with `core`. const _: () = assert!( - core::mem::size_of::<$name>() == core::mem::size_of::() + ::core::mem::size_of::<$name>() == ::core::mem::size_of::<::core::ffi::$name>() ); )*} } diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs index 5c372cf27ed0..539888465cc6 100644 --- a/rust/kernel/device.rs +++ b/rust/kernel/device.rs @@ -240,7 +240,7 @@ impl DeviceContext for Normal {} macro_rules! dev_printk { ($method:ident, $dev:expr, $($f:tt)*) => { { - ($dev).$method(core::format_args!($($f)*)); + ($dev).$method(::core::format_args!($($f)*)); } } } diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs index e5859217a579..0a4eb56d98f2 100644 --- a/rust/kernel/device_id.rs +++ b/rust/kernel/device_id.rs @@ -159,7 +159,7 @@ macro_rules! module_device_table { "_", line!(), "_", stringify!($table_name)) ] - static $module_table_name: [core::mem::MaybeUninit; $table_name.raw_ids().size()] = - unsafe { core::mem::transmute_copy($table_name.raw_ids()) }; + static $module_table_name: [::core::mem::MaybeUninit; $table_name.raw_ids().size()] = + unsafe { ::core::mem::transmute_copy($table_name.raw_ids()) }; }; } diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs index 1604fb6a5b1b..81833a687b75 100644 --- a/rust/kernel/kunit.rs +++ b/rust/kernel/kunit.rs @@ -59,7 +59,7 @@ macro_rules! kunit_assert { } static FILE: &'static $crate::str::CStr = $crate::c_str!($file); - static LINE: i32 = core::line!() as i32 - $diff; + static LINE: i32 = ::core::line!() as i32 - $diff; static CONDITION: &'static $crate::str::CStr = $crate::c_str!(stringify!($condition)); // SAFETY: FFI call without safety requirements. @@ -130,11 +130,11 @@ unsafe impl Sync for UnaryAssert {} unsafe { $crate::bindings::__kunit_do_failed_assertion( kunit_test, - core::ptr::addr_of!(LOCATION.0), + ::core::ptr::addr_of!(LOCATION.0), $crate::bindings::kunit_assert_type_KUNIT_ASSERTION, - core::ptr::addr_of!(ASSERTION.0.assert), + ::core::ptr::addr_of!(ASSERTION.0.assert), Some($crate::bindings::kunit_unary_assert_format), - core::ptr::null(), + ::core::ptr::null(), ); } diff --git a/rust/kernel/static_assert.rs b/rust/kernel/static_assert.rs index d8120f838260..a57ba14315a0 100644 --- a/rust/kernel/static_assert.rs +++ b/rust/kernel/static_assert.rs @@ -34,6 +34,6 @@ #[macro_export] macro_rules! static_assert { ($condition:expr $(,$arg:literal)?) => { - const _: () = core::assert!($condition $(,$arg)?); + const _: () = ::core::assert!($condition $(,$arg)?); }; } diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs index 98d5c74ec4f7..c554b243ebcb 100644 --- a/rust/kernel/str.rs +++ b/rust/kernel/str.rs @@ -595,7 +595,7 @@ fn deref(&self) -> &str { macro_rules! format { ($($f:tt)*) => ({ - &*String::from_fmt(kernel::fmt!($($f)*)) + &*String::from_fmt(::kernel::fmt!($($f)*)) }) } @@ -944,5 +944,5 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { /// A convenience alias for [`core::format_args`]. #[macro_export] macro_rules! fmt { - ($($f:tt)*) => ( core::format_args!($($f)*) ) + ($($f:tt)*) => ( ::core::format_args!($($f)*) ) } diff --git a/rust/macros/kunit.rs b/rust/macros/kunit.rs index 99ccac82edde..b146c5894e92 100644 --- a/rust/macros/kunit.rs +++ b/rust/macros/kunit.rs @@ -85,28 +85,28 @@ pub(crate) fn kunit_tests(attr: TokenStream, ts: TokenStream) -> TokenStream { // Looks like: // // ``` - // unsafe extern "C" fn kunit_rust_wrapper_foo(_test: *mut kernel::bindings::kunit) { foo(); } - // unsafe extern "C" fn kunit_rust_wrapper_bar(_test: *mut kernel::bindings::kunit) { bar(); } + // unsafe extern "C" fn kunit_rust_wrapper_foo(_test: *mut ::kernel::bindings::kunit) { foo(); } + // unsafe extern "C" fn kunit_rust_wrapper_bar(_test: *mut ::kernel::bindings::kunit) { bar(); } // - // static mut TEST_CASES: [kernel::bindings::kunit_case; 3] = [ - // kernel::kunit::kunit_case(kernel::c_str!("foo"), kunit_rust_wrapper_foo), - // kernel::kunit::kunit_case(kernel::c_str!("bar"), kunit_rust_wrapper_bar), - // kernel::kunit::kunit_case_null(), + // static mut TEST_CASES: [::kernel::bindings::kunit_case; 3] = [ + // ::kernel::kunit::kunit_case(::kernel::c_str!("foo"), kunit_rust_wrapper_foo), + // ::kernel::kunit::kunit_case(::kernel::c_str!("bar"), kunit_rust_wrapper_bar), + // ::kernel::kunit::kunit_case_null(), // ]; // - // kernel::kunit_unsafe_test_suite!(kunit_test_suit_name, TEST_CASES); + // ::kernel::kunit_unsafe_test_suite!(kunit_test_suit_name, TEST_CASES); // ``` let mut kunit_macros = "".to_owned(); let mut test_cases = "".to_owned(); for test in &tests { let kunit_wrapper_fn_name = format!("kunit_rust_wrapper_{test}"); let kunit_wrapper = format!( - "unsafe extern \"C\" fn {kunit_wrapper_fn_name}(_test: *mut kernel::bindings::kunit) {{ {test}(); }}" + "unsafe extern \"C\" fn {kunit_wrapper_fn_name}(_test: *mut ::kernel::bindings::kunit) {{ {test}(); }}" ); writeln!(kunit_macros, "{kunit_wrapper}").unwrap(); writeln!( test_cases, - " kernel::kunit::kunit_case(kernel::c_str!(\"{test}\"), {kunit_wrapper_fn_name})," + " ::kernel::kunit::kunit_case(::kernel::c_str!(\"{test}\"), {kunit_wrapper_fn_name})," ) .unwrap(); } @@ -114,14 +114,14 @@ pub(crate) fn kunit_tests(attr: TokenStream, ts: TokenStream) -> TokenStream { writeln!(kunit_macros).unwrap(); writeln!( kunit_macros, - "static mut TEST_CASES: [kernel::bindings::kunit_case; {}] = [\n{test_cases} kernel::kunit::kunit_case_null(),\n];", + "static mut TEST_CASES: [::kernel::bindings::kunit_case; {}] = [\n{test_cases} ::kernel::kunit::kunit_case_null(),\n];", tests.len() + 1 ) .unwrap(); writeln!( kunit_macros, - "kernel::kunit_unsafe_test_suite!({attr}, TEST_CASES);" + "::kernel::kunit_unsafe_test_suite!({attr}, TEST_CASES);" ) .unwrap(); diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs index 9acaa68c974e..cb429eceff71 100644 --- a/rust/macros/lib.rs +++ b/rust/macros/lib.rs @@ -283,7 +283,7 @@ pub fn concat_idents(ts: TokenStream) -> TokenStream { /// # const binder_driver_return_protocol_BR_FAILED_REPLY: u32 = 14; /// macro_rules! pub_no_prefix { /// ($prefix:ident, $($newname:ident),+) => { -/// kernel::macros::paste! { +/// ::kernel::macros::paste! { /// $(pub(crate) const $newname: u32 = [<$prefix $newname>];)+ /// } /// }; @@ -340,7 +340,7 @@ pub fn concat_idents(ts: TokenStream) -> TokenStream { /// # const binder_driver_return_protocol_BR_FAILED_REPLY: u32 = 14; /// macro_rules! pub_no_prefix { /// ($prefix:ident, $($newname:ident),+) => { -/// kernel::macros::paste! { +/// ::kernel::macros::paste! { /// $(pub(crate) const fn [<$newname:lower:span>]() -> u32 { [<$prefix $newname:span>] })+ /// } /// }; @@ -375,7 +375,7 @@ pub fn concat_idents(ts: TokenStream) -> TokenStream { /// ``` /// macro_rules! create_numbered_fn { /// ($name:literal, $val:literal) => { -/// kernel::macros::paste! { +/// ::kernel::macros::paste! { /// fn []() -> u32 { $val } /// } /// }; diff --git a/rust/macros/module.rs b/rust/macros/module.rs index 44e5cb108cea..de9304498a97 100644 --- a/rust/macros/module.rs +++ b/rust/macros/module.rs @@ -215,24 +215,24 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream { // SAFETY: `__this_module` is constructed by the kernel at load time and will not be // freed until the module is unloaded. #[cfg(MODULE)] - static THIS_MODULE: kernel::ThisModule = unsafe {{ + static THIS_MODULE: ::kernel::ThisModule = unsafe {{ extern \"C\" {{ - static __this_module: kernel::types::Opaque; + static __this_module: ::kernel::types::Opaque<::kernel::bindings::module>; }} - kernel::ThisModule::from_ptr(__this_module.get()) + ::kernel::ThisModule::from_ptr(__this_module.get()) }}; #[cfg(not(MODULE))] - static THIS_MODULE: kernel::ThisModule = unsafe {{ - kernel::ThisModule::from_ptr(core::ptr::null_mut()) + static THIS_MODULE: ::kernel::ThisModule = unsafe {{ + ::kernel::ThisModule::from_ptr(::core::ptr::null_mut()) }}; /// The `LocalModule` type is the type of the module created by `module!`, /// `module_pci_driver!`, `module_platform_driver!`, etc. type LocalModule = {type_}; - impl kernel::ModuleMetadata for {type_} {{ - const NAME: &'static kernel::str::CStr = kernel::c_str!(\"{name}\"); + impl ::kernel::ModuleMetadata for {type_} {{ + const NAME: &'static ::kernel::str::CStr = ::kernel::c_str!(\"{name}\"); }} // Double nested modules, since then nobody can access the public items inside. @@ -250,8 +250,8 @@ mod __module_init {{ #[used] static __IS_RUST_MODULE: () = (); - static mut __MOD: core::mem::MaybeUninit<{type_}> = - core::mem::MaybeUninit::uninit(); + static mut __MOD: ::core::mem::MaybeUninit<{type_}> = + ::core::mem::MaybeUninit::uninit(); // Loadable modules need to export the `{{init,cleanup}}_module` identifiers. /// # Safety @@ -262,7 +262,7 @@ mod __module_init {{ #[doc(hidden)] #[no_mangle] #[link_section = \".init.text\"] - pub unsafe extern \"C\" fn init_module() -> kernel::ffi::c_int {{ + pub unsafe extern \"C\" fn init_module() -> ::kernel::ffi::c_int {{ // SAFETY: This function is inaccessible to the outside due to the double // module wrapping it. It is called exactly once by the C side via its // unique name. @@ -302,11 +302,12 @@ mod __module_init {{ #[doc(hidden)] #[link_section = \"{initcall_section}\"] #[used] - pub static __{name}_initcall: extern \"C\" fn() -> kernel::ffi::c_int = __{name}_init; + pub static __{name}_initcall: extern \"C\" fn() -> ::kernel::ffi::c_int = + __{name}_init; #[cfg(not(MODULE))] #[cfg(CONFIG_HAVE_ARCH_PREL32_RELOCATIONS)] - core::arch::global_asm!( + ::core::arch::global_asm!( r#\".section \"{initcall_section}\", \"a\" __{name}_initcall: .long __{name}_init - . @@ -317,7 +318,7 @@ mod __module_init {{ #[cfg(not(MODULE))] #[doc(hidden)] #[no_mangle] - pub extern \"C\" fn __{name}_init() -> kernel::ffi::c_int {{ + pub extern \"C\" fn __{name}_init() -> ::kernel::ffi::c_int {{ // SAFETY: This function is inaccessible to the outside due to the double // module wrapping it. It is called exactly once by the C side via its // placement above in the initcall section. @@ -340,9 +341,9 @@ mod __module_init {{ /// # Safety /// /// This function must only be called once. - unsafe fn __init() -> kernel::ffi::c_int {{ + unsafe fn __init() -> ::kernel::ffi::c_int {{ let initer = - <{type_} as kernel::InPlaceModule>::init(&super::super::THIS_MODULE); + <{type_} as ::kernel::InPlaceModule>::init(&super::super::THIS_MODULE); // SAFETY: No data race, since `__MOD` can only be accessed by this module // and there only `__init` and `__exit` access it. These functions are only // called once and `__exit` cannot be called before or during `__init`. diff --git a/scripts/rustdoc_test_builder.rs b/scripts/rustdoc_test_builder.rs index e5894652f12c..f7540bcf595a 100644 --- a/scripts/rustdoc_test_builder.rs +++ b/scripts/rustdoc_test_builder.rs @@ -28,7 +28,7 @@ fn main() { // // ``` // fn main() { #[allow(non_snake_case)] fn _doctest_main_rust_kernel_file_rs_28_0() { - // fn main() { #[allow(non_snake_case)] fn _doctest_main_rust_kernel_file_rs_37_0() -> Result<(), impl core::fmt::Debug> { + // fn main() { #[allow(non_snake_case)] fn _doctest_main_rust_kernel_file_rs_37_0() -> Result<(), impl ::core::fmt::Debug> { // ``` // // It should be unlikely that doctest code matches such lines (when code is formatted properly). @@ -49,8 +49,10 @@ fn main() { // Qualify `Result` to avoid the collision with our own `Result` coming from the prelude. let body = body.replace( - &format!("{rustdoc_function_name}() -> Result<(), impl core::fmt::Debug> {{"), - &format!("{rustdoc_function_name}() -> core::result::Result<(), impl core::fmt::Debug> {{"), + &format!("{rustdoc_function_name}() -> Result<(), impl ::core::fmt::Debug> {{"), + &format!( + "{rustdoc_function_name}() -> ::core::result::Result<(), impl ::core::fmt::Debug> {{" + ), ); // For tests that get generated with `Result`, like above, `rustdoc` generates an `unwrap()` on diff --git a/scripts/rustdoc_test_gen.rs b/scripts/rustdoc_test_gen.rs index ec8d70ac888b..1ca253594d38 100644 --- a/scripts/rustdoc_test_gen.rs +++ b/scripts/rustdoc_test_gen.rs @@ -167,12 +167,14 @@ fn main() { rust_tests, r#"/// Generated `{name}` KUnit test case from a Rust documentation test. #[no_mangle] -pub extern "C" fn {kunit_name}(__kunit_test: *mut kernel::bindings::kunit) {{ +pub extern "C" fn {kunit_name}(__kunit_test: *mut ::kernel::bindings::kunit) {{ /// Overrides the usual [`assert!`] macro with one that calls KUnit instead. #[allow(unused)] macro_rules! assert {{ ($cond:expr $(,)?) => {{{{ - kernel::kunit_assert!("{kunit_name}", "{real_path}", __DOCTEST_ANCHOR - {line}, $cond); + ::kernel::kunit_assert!( + "{kunit_name}", "{real_path}", __DOCTEST_ANCHOR - {line}, $cond + ); }}}} }} @@ -180,13 +182,15 @@ macro_rules! assert {{ #[allow(unused)] macro_rules! assert_eq {{ ($left:expr, $right:expr $(,)?) => {{{{ - kernel::kunit_assert_eq!("{kunit_name}", "{real_path}", __DOCTEST_ANCHOR - {line}, $left, $right); + ::kernel::kunit_assert_eq!( + "{kunit_name}", "{real_path}", __DOCTEST_ANCHOR - {line}, $left, $right + ); }}}} }} // Many tests need the prelude, so provide it by default. #[allow(unused)] - use kernel::prelude::*; + use ::kernel::prelude::*; // Unconditionally print the location of the original doctest (i.e. rather than the location in // the generated file) so that developers can easily map the test back to the source code. @@ -197,11 +201,11 @@ macro_rules! assert_eq {{ // This follows the syntax for declaring test metadata in the proposed KTAP v2 spec, which may // be used for the proposed KUnit test attributes API. Thus hopefully this will make migration // easier later on. - kernel::kunit::info(format_args!(" # {kunit_name}.location: {real_path}:{line}\n")); + ::kernel::kunit::info(format_args!(" # {kunit_name}.location: {real_path}:{line}\n")); /// The anchor where the test code body starts. #[allow(unused)] - static __DOCTEST_ANCHOR: i32 = core::line!() as i32 + {body_offset} + 1; + static __DOCTEST_ANCHOR: i32 = ::core::line!() as i32 + {body_offset} + 1; {{ {body} main(); From f7819f7988f9c7292315c522d65301ced5cfa7db Mon Sep 17 00:00:00 2001 From: Igor Korotin Date: Wed, 2 Apr 2025 17:00:47 +0100 Subject: [PATCH 56/78] docs: rust: quick-start: update Ubuntu instructions Split installation instructions for Ubuntu into 2 different sections: - For Ubuntu 25.04: this release provides easy-to-install Rust packages. - For Ubuntu 24.10 and below: these releases provide rust-1.80 and bindgen-0.65, which do not set their tools as defaults. The instructions for these versions have been updated to configure Rust tools properly. Signed-off-by: Igor Korotin Link: https://lore.kernel.org/r/20250402160047.1827500-1-igor.korotin@yahoo.com [ Dropped 24.10 -- it is soon out of support and their `bindgen` issue (reported as issue #2086639) was never patched anyway. Removed trailing spaces. Split into subheaders. Added `rustfmt` link. Removed spurious backquotes. Reworded contents slightly. - Miguel ] Signed-off-by: Miguel Ojeda --- Documentation/rust/quick-start.rst | 44 ++++++++++++++++++++++++++++-- 1 file changed, 41 insertions(+), 3 deletions(-) diff --git a/Documentation/rust/quick-start.rst b/Documentation/rust/quick-start.rst index 6d2607870ba4..155f7107329a 100644 --- a/Documentation/rust/quick-start.rst +++ b/Documentation/rust/quick-start.rst @@ -90,15 +90,53 @@ they should generally work out of the box, e.g.:: Ubuntu ****** -Ubuntu LTS and non-LTS (interim) releases provide recent Rust releases and thus -they should generally work out of the box, e.g.:: +25.04 +~~~~~ - apt install rustc-1.80 rust-1.80-src bindgen-0.65 rustfmt-1.80 rust-1.80-clippy +The latest Ubuntu releases provide recent Rust releases and thus they should +generally work out of the box, e.g.:: + + apt install rustc rust-src bindgen rustfmt rust-clippy + +In addition, ``RUST_LIB_SRC`` needs to be set, e.g.:: + + RUST_LIB_SRC=/usr/src/rustc-$(rustc --version | cut -d' ' -f2)/library + +For convenience, ``RUST_LIB_SRC`` can be exported to the global environment. + + +24.04 LTS and older +~~~~~~~~~~~~~~~~~~~ + +Though Ubuntu 24.04 LTS and older versions still provide recent Rust +releases, they require some additional configuration to be set, using +the versioned packages, e.g.:: + + apt install rustc-1.80 rust-1.80-src bindgen-0.65 rustfmt-1.80 \ + rust-1.80-clippy + ln -s /usr/lib/rust-1.80/bin/rustfmt /usr/bin/rustfmt-1.80 + ln -s /usr/lib/rust-1.80/bin/clippy-driver /usr/bin/clippy-driver-1.80 + +None of these packages set their tools as defaults; therefore they should be +specified explicitly, e.g.:: + + make LLVM=1 RUSTC=rustc-1.80 RUSTDOC=rustdoc-1.80 RUSTFMT=rustfmt-1.80 \ + CLIPPY_DRIVER=clippy-driver-1.80 BINDGEN=bindgen-0.65 + +Alternatively, modify the ``PATH`` variable to place the Rust 1.80 binaries +first and set ``bindgen`` as the default, e.g.:: + + PATH=/usr/lib/rust-1.80/bin:$PATH + update-alternatives --install /usr/bin/bindgen bindgen \ + /usr/bin/bindgen-0.65 100 + update-alternatives --set bindgen /usr/bin/bindgen-0.65 ``RUST_LIB_SRC`` needs to be set when using the versioned packages, e.g.:: RUST_LIB_SRC=/usr/src/rustc-$(rustc-1.80 --version | cut -d' ' -f2)/library +For convenience, ``RUST_LIB_SRC`` can be exported to the global environment. + In addition, ``bindgen-0.65`` is available in newer releases (24.04 LTS and 24.10), but it may not be available in older ones (20.04 LTS and 22.04 LTS), thus ``bindgen`` may need to be built manually (please see below). From 3d5bef5d47c371e17041cd6c84e9c08e54ea9e63 Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Sun, 13 Apr 2025 02:56:50 +0200 Subject: [PATCH 57/78] rust: add C FFI types to the prelude Rust kernel code is supposed to use the custom mapping of C FFI types, i.e. those from the `ffi` crate, rather than the ones coming from `core`. Thus, to minimize mistakes and to simplify the code everywhere, just provide them in the `kernel` prelude and ask in the Coding Guidelines to use them directly, i.e. as a single segment path. After this lands, we can start cleaning up the existing users. Ideally, we would use something like Clippy's `disallowed-types` to prevent the use of the `core` ones, but that one sees through aliases. Link: https://lore.kernel.org/rust-for-linux/CANiq72kc4gzfieD-FjuWfELRDXXD2vLgPv4wqk3nt4pjdPQ=qg@mail.gmail.com/ Reviewed-by: Danilo Krummrich Reviewed-by: Alice Ryhl Link: https://lore.kernel.org/r/20250413005650.1745894-1-ojeda@kernel.org [ Reworded content of the documentation to focus on how to use the aliases first. - Miguel ] Signed-off-by: Miguel Ojeda --- Documentation/rust/coding-guidelines.rst | 17 +++++++++++++++++ rust/kernel/prelude.rs | 5 +++++ 2 files changed, 22 insertions(+) diff --git a/Documentation/rust/coding-guidelines.rst b/Documentation/rust/coding-guidelines.rst index 6c6c51b4cf46..6ff9e754755d 100644 --- a/Documentation/rust/coding-guidelines.rst +++ b/Documentation/rust/coding-guidelines.rst @@ -203,6 +203,23 @@ or: /// [`struct mutex`]: srctree/include/linux/mutex.h +C FFI types +----------- + +Rust kernel code refers to C types, such as ``int``, using type aliases such as +``c_int``, which are readily available from the ``kernel`` prelude. Please do +not use the aliases from ``core::ffi`` -- they may not map to the correct types. + +These aliases should generally be referred directly by their identifier, i.e. +as a single segment path. For instance: + +.. code-block:: rust + + fn f(p: *const c_char) -> c_int { + // ... + } + + Naming ------ diff --git a/rust/kernel/prelude.rs b/rust/kernel/prelude.rs index baa774a351ce..f869b02f1f25 100644 --- a/rust/kernel/prelude.rs +++ b/rust/kernel/prelude.rs @@ -14,6 +14,11 @@ #[doc(no_inline)] pub use core::pin::Pin; +pub use ::ffi::{ + c_char, c_int, c_long, c_longlong, c_schar, c_short, c_uchar, c_uint, c_ulong, c_ulonglong, + c_ushort, c_void, +}; + pub use crate::alloc::{flags::*, Box, KBox, KVBox, KVVec, KVec, VBox, VVec, Vec}; #[doc(no_inline)] From 9f047636831a61ce0840929555245dd17695206a Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Mon, 24 Mar 2025 22:03:55 +0100 Subject: [PATCH 58/78] rust: platform: fix docs related to missing Markdown code spans Convert `TODO` from documentation to a normal comment, and put code in block. This was found using the Clippy `doc_markdown` lint, which we may want to enable. Fixes: 683a63befc73 ("rust: platform: add basic platform device / driver abstractions") Reviewed-by: Benno Lossin Acked-by: Greg Kroah-Hartman Link: https://lore.kernel.org/r/20250324210359.1199574-9-ojeda@kernel.org Signed-off-by: Miguel Ojeda --- rust/kernel/platform.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs index fd4a494f30e8..a02ae5b25b74 100644 --- a/rust/kernel/platform.rs +++ b/rust/kernel/platform.rs @@ -151,10 +151,11 @@ macro_rules! module_platform_driver { ///``` pub trait Driver: Send { /// The type holding driver private data about each device id supported by the driver. - /// - /// TODO: Use associated_type_defaults once stabilized: - /// - /// type IdInfo: 'static = (); + // TODO: Use associated_type_defaults once stabilized: + // + // ``` + // type IdInfo: 'static = (); + // ``` type IdInfo: 'static; /// The table of OF device ids supported by the driver. From 673ec360cfb099a5f44dabee0f0e6c9b282efa7e Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Mon, 24 Mar 2025 22:03:50 +0100 Subject: [PATCH 59/78] rust: alloc: add missing Markdown code spans Add missing Markdown code spans. This was found using the Clippy `doc_markdown` lint, which we may want to enable. Fixes: b6a006e21b82 ("rust: alloc: introduce allocation flags") Reviewed-by: Benno Lossin Acked-by: Danilo Krummrich Link: https://lore.kernel.org/r/20250324210359.1199574-4-ojeda@kernel.org Signed-off-by: Miguel Ojeda --- rust/kernel/alloc.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rust/kernel/alloc.rs b/rust/kernel/alloc.rs index fc9c9c41cd79..a2c49e5494d3 100644 --- a/rust/kernel/alloc.rs +++ b/rust/kernel/alloc.rs @@ -94,10 +94,10 @@ pub mod flags { /// /// A lower watermark is applied to allow access to "atomic reserves". The current /// implementation doesn't support NMI and few other strict non-preemptive contexts (e.g. - /// raw_spin_lock). The same applies to [`GFP_NOWAIT`]. + /// `raw_spin_lock`). The same applies to [`GFP_NOWAIT`]. pub const GFP_ATOMIC: Flags = Flags(bindings::GFP_ATOMIC); - /// Typical for kernel-internal allocations. The caller requires ZONE_NORMAL or a lower zone + /// Typical for kernel-internal allocations. The caller requires `ZONE_NORMAL` or a lower zone /// for direct access but can direct reclaim. pub const GFP_KERNEL: Flags = Flags(bindings::GFP_KERNEL); From abd21a163d4188c180cabb6747b1d94e3c0586b9 Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Mon, 24 Mar 2025 22:03:51 +0100 Subject: [PATCH 60/78] rust: alloc: add missing Markdown code span Add missing Markdown code span. This was found using the Clippy `doc_markdown` lint, which we may want to enable. Fixes: dd09538fb409 ("rust: alloc: implement `Cmalloc` in module allocator_test") Reviewed-by: Benno Lossin Acked-by: Danilo Krummrich Link: https://lore.kernel.org/r/20250324210359.1199574-5-ojeda@kernel.org Signed-off-by: Miguel Ojeda --- rust/kernel/alloc/allocator_test.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/kernel/alloc/allocator_test.rs b/rust/kernel/alloc/allocator_test.rs index c37d4c0c64e9..d19c06ef0498 100644 --- a/rust/kernel/alloc/allocator_test.rs +++ b/rust/kernel/alloc/allocator_test.rs @@ -4,7 +4,7 @@ //! of those types (e.g. `CString`) use kernel allocators for instantiation. //! //! In order to allow userspace test cases to make use of such types as well, implement the -//! `Cmalloc` allocator within the allocator_test module and type alias all kernel allocators to +//! `Cmalloc` allocator within the `allocator_test` module and type alias all kernel allocators to //! `Cmalloc`. The `Cmalloc` allocator uses libc's `realloc()` function as allocator backend. #![allow(missing_docs)] From 1dbaf8b1bafb8557904eb54b98bb323a3061dd2c Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Mon, 24 Mar 2025 22:03:54 +0100 Subject: [PATCH 61/78] rust: pci: fix docs related to missing Markdown code spans In particular: - Add missing Markdown code spans. - Improve title for `DeviceId`, adding a link to the struct in the C side, rather than referring to `bindings::`. - Convert `TODO` from documentation to a normal comment, and put code in block. This was found using the Clippy `doc_markdown` lint, which we may want to enable. Fixes: 1bd8b6b2c5d3 ("rust: pci: add basic PCI device / driver abstractions") Reviewed-by: Benno Lossin Acked-by: Danilo Krummrich Link: https://lore.kernel.org/r/20250324210359.1199574-8-ojeda@kernel.org [ Prefixed link text with `struct`. - Miguel ] Signed-off-by: Miguel Ojeda --- rust/kernel/pci.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs index 3aeb1250c27f..c17a32b76d74 100644 --- a/rust/kernel/pci.rs +++ b/rust/kernel/pci.rs @@ -118,7 +118,9 @@ macro_rules! module_pci_driver { }; } -/// Abstraction for bindings::pci_device_id. +/// Abstraction for the PCI device ID structure ([`struct pci_device_id`]). +/// +/// [`struct pci_device_id`]: https://docs.kernel.org/PCI/pci.html#c.pci_device_id #[repr(transparent)] #[derive(Clone, Copy)] pub struct DeviceId(bindings::pci_device_id); @@ -173,7 +175,7 @@ fn index(&self) -> usize { } } -/// IdTable type for PCI +/// `IdTable` type for PCI. pub type IdTable = &'static dyn kernel::device_id::IdTable; /// Create a PCI `IdTable` with its alias for modpost. @@ -224,10 +226,11 @@ macro_rules! pci_device_table { /// `Adapter` documentation for an example. pub trait Driver: Send { /// The type holding information about each device id supported by the driver. - /// - /// TODO: Use associated_type_defaults once stabilized: - /// - /// type IdInfo: 'static = (); + // TODO: Use `associated_type_defaults` once stabilized: + // + // ``` + // type IdInfo: 'static = (); + // ``` type IdInfo: 'static; /// The table of device ids supported by the driver. From f54c750333381bfeaa0e4a69b9563d4e4e21f1b3 Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Mon, 24 Mar 2025 22:03:56 +0100 Subject: [PATCH 62/78] rust: task: add missing Markdown code spans and intra-doc links Add missing Markdown code spans and also convert them into intra-doc links. This was found using the Clippy `doc_markdown` lint, which we may want to enable. Fixes: e0020ba6cbcb ("rust: add PidNamespace") Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20250324210359.1199574-10-ojeda@kernel.org Signed-off-by: Miguel Ojeda --- rust/kernel/task.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rust/kernel/task.rs b/rust/kernel/task.rs index 9e6f6854948d..9dce3705ff63 100644 --- a/rust/kernel/task.rs +++ b/rust/kernel/task.rs @@ -158,10 +158,10 @@ fn deref(&self) -> &Self::Target { } } - /// Returns a PidNamespace reference for the currently executing task's/thread's pid namespace. + /// Returns a [`PidNamespace`] reference for the currently executing task's/thread's pid namespace. /// /// This function can be used to create an unbounded lifetime by e.g., storing the returned - /// PidNamespace in a global variable which would be a bug. So the recommended way to get the + /// [`PidNamespace`] in a global variable which would be a bug. So the recommended way to get the /// current task's/thread's pid namespace is to use the [`current_pid_ns`] macro because it is /// safe. /// From df523db15a06c37cbabad8f477ae87d08c9081f2 Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Mon, 24 Mar 2025 22:03:52 +0100 Subject: [PATCH 63/78] rust: dma: add missing Markdown code span Add missing Markdown code span. This was found using the Clippy `doc_markdown` lint, which we may want to enable. Fixes: ad2907b4e308 ("rust: add dma coherent allocator abstraction") Reviewed-by: Benno Lossin Acked-by: Danilo Krummrich Link: https://lore.kernel.org/r/20250324210359.1199574-6-ojeda@kernel.org Signed-off-by: Miguel Ojeda --- rust/kernel/dma.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs index 8cdc76043ee7..35fd8a638473 100644 --- a/rust/kernel/dma.rs +++ b/rust/kernel/dma.rs @@ -94,7 +94,7 @@ pub mod attrs { pub const DMA_ATTR_ALLOC_SINGLE_PAGES: Attrs = Attrs(bindings::DMA_ATTR_ALLOC_SINGLE_PAGES); /// This tells the DMA-mapping subsystem to suppress allocation failure reports (similarly to - /// __GFP_NOWARN). + /// `__GFP_NOWARN`). pub const DMA_ATTR_NO_WARN: Attrs = Attrs(bindings::DMA_ATTR_NO_WARN); /// Used to indicate that the buffer is fully accessible at an elevated privilege level (and From f4daa80d6be7d3c55ca72a8e560afc4e21f886aa Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Sat, 17 May 2025 09:55:59 +0100 Subject: [PATCH 64/78] rust: compile libcore with edition 2024 for 1.87+ Rust 1.87 (released on 2025-05-15) compiles core library with edition 2024 instead of 2021 [1]. Ensure that the edition matches libcore's expectation to avoid potential breakage. [ J3m3 reported in Zulip [2] that the `rust-analyzer` target was broken after this patch -- indeed, we need to avoid `core-cfgs` since those are passed to the `rust-analyzer` target. So, instead, I tweaked the patch to create a new `core-edition` variable and explicitly mention the `--edition` flag instead of reusing `core-cfg`s. In addition, pass a new argument using this new variable to `generate_rust_analyzer.py` so that we set the right edition there. By the way, for future reference: the `filter-out` change is needed for Rust < 1.87, since otherwise we would skip the `--edition=2021` we just added, ending up with no edition flag, and thus the compiler would default to the 2015 one. [2] https://rust-for-linux.zulipchat.com/#narrow/channel/291565/topic/x/near/520206547 - Miguel ] Cc: stable@vger.kernel.org # Needed in 6.12.y and later (Rust is pinned in older LTSs). Link: https://github.com/rust-lang/rust/pull/138162 [1] Reported-by: est31 Closes: https://github.com/Rust-for-Linux/linux/issues/1163 Signed-off-by: Gary Guo Link: https://lore.kernel.org/r/20250517085600.2857460-1-gary@garyguo.net Signed-off-by: Miguel Ojeda --- rust/Makefile | 14 ++++++++------ scripts/generate_rust_analyzer.py | 13 ++++++++----- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/rust/Makefile b/rust/Makefile index f207ba0ed466..d62b58d0a55c 100644 --- a/rust/Makefile +++ b/rust/Makefile @@ -60,6 +60,8 @@ endif core-cfgs = \ --cfg no_fp_fmt_parse +core-edition := $(if $(call rustc-min-version,108700),2024,2021) + # `rustc` recognizes `--remap-path-prefix` since 1.26.0, but `rustdoc` only # since Rust 1.81.0. Moreover, `rustdoc` ICEs on out-of-tree builds since Rust # 1.82.0 (https://github.com/rust-lang/rust/issues/138520). Thus workaround both @@ -106,8 +108,8 @@ rustdoc-macros: $(src)/macros/lib.rs FORCE # Starting with Rust 1.82.0, skipping `-Wrustdoc::unescaped_backticks` should # not be needed -- see https://github.com/rust-lang/rust/pull/128307. -rustdoc-core: private skip_flags = -Wrustdoc::unescaped_backticks -rustdoc-core: private rustc_target_flags = $(core-cfgs) +rustdoc-core: private skip_flags = --edition=2021 -Wrustdoc::unescaped_backticks +rustdoc-core: private rustc_target_flags = --edition=$(core-edition) $(core-cfgs) rustdoc-core: $(RUST_LIB_SRC)/core/src/lib.rs FORCE +$(call if_changed,rustdoc) @@ -416,7 +418,7 @@ quiet_cmd_rustc_library = $(if $(skip_clippy),RUSTC,$(RUSTC_OR_CLIPPY_QUIET)) L cmd_rustc_library = \ OBJTREE=$(abspath $(objtree)) \ $(if $(skip_clippy),$(RUSTC),$(RUSTC_OR_CLIPPY)) \ - $(filter-out $(skip_flags),$(rust_flags) $(rustc_target_flags)) \ + $(filter-out $(skip_flags),$(rust_flags)) $(rustc_target_flags) \ --emit=dep-info=$(depfile) --emit=obj=$@ \ --emit=metadata=$(dir $@)$(patsubst %.o,lib%.rmeta,$(notdir $@)) \ --crate-type rlib -L$(objtree)/$(obj) \ @@ -427,7 +429,7 @@ quiet_cmd_rustc_library = $(if $(skip_clippy),RUSTC,$(RUSTC_OR_CLIPPY_QUIET)) L rust-analyzer: $(Q)MAKEFLAGS= $(srctree)/scripts/generate_rust_analyzer.py \ - --cfgs='core=$(core-cfgs)' \ + --cfgs='core=$(core-cfgs)' $(core-edition) \ $(realpath $(srctree)) $(realpath $(objtree)) \ $(rustc_sysroot) $(RUST_LIB_SRC) $(if $(KBUILD_EXTMOD),$(srcroot)) \ > rust-project.json @@ -483,9 +485,9 @@ $(obj)/helpers/helpers.o: $(src)/helpers/helpers.c $(recordmcount_source) FORCE $(obj)/exports.o: private skip_gendwarfksyms = 1 $(obj)/core.o: private skip_clippy = 1 -$(obj)/core.o: private skip_flags = -Wunreachable_pub +$(obj)/core.o: private skip_flags = --edition=2021 -Wunreachable_pub $(obj)/core.o: private rustc_objcopy = $(foreach sym,$(redirect-intrinsics),--redefine-sym $(sym)=__rust$(sym)) -$(obj)/core.o: private rustc_target_flags = $(core-cfgs) +$(obj)/core.o: private rustc_target_flags = --edition=$(core-edition) $(core-cfgs) $(obj)/core.o: $(RUST_LIB_SRC)/core/src/lib.rs \ $(wildcard $(objtree)/include/config/RUSTC_VERSION_TEXT) FORCE +$(call if_changed_rule,rustc_library) diff --git a/scripts/generate_rust_analyzer.py b/scripts/generate_rust_analyzer.py index fe663dd0c43b..7c3ea2b55041 100755 --- a/scripts/generate_rust_analyzer.py +++ b/scripts/generate_rust_analyzer.py @@ -19,7 +19,7 @@ def args_crates_cfgs(cfgs): return crates_cfgs -def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs): +def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs, core_edition): # Generate the configuration list. cfg = [] with open(objtree / "include" / "generated" / "rustc_cfg") as fd: @@ -35,7 +35,7 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs): crates_indexes = {} crates_cfgs = args_crates_cfgs(cfgs) - def append_crate(display_name, root_module, deps, cfg=[], is_workspace_member=True, is_proc_macro=False): + def append_crate(display_name, root_module, deps, cfg=[], is_workspace_member=True, is_proc_macro=False, edition="2021"): crate = { "display_name": display_name, "root_module": str(root_module), @@ -43,7 +43,7 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs): "is_proc_macro": is_proc_macro, "deps": [{"crate": crates_indexes[dep], "name": dep} for dep in deps], "cfg": cfg, - "edition": "2021", + "edition": edition, "env": { "RUST_MODFILE": "This is only for rust-analyzer" } @@ -61,6 +61,7 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs): display_name, deps, cfg=[], + edition="2021", ): append_crate( display_name, @@ -68,12 +69,13 @@ def generate_crates(srctree, objtree, sysroot_src, external_src, cfgs): deps, cfg, is_workspace_member=False, + edition=edition, ) # NB: sysroot crates reexport items from one another so setting up our transitive dependencies # here is important for ensuring that rust-analyzer can resolve symbols. The sources of truth # for this dependency graph are `(sysroot_src / crate / "Cargo.toml" for crate in crates)`. - append_sysroot_crate("core", [], cfg=crates_cfgs.get("core", [])) + append_sysroot_crate("core", [], cfg=crates_cfgs.get("core", []), edition=core_edition) append_sysroot_crate("alloc", ["core"]) append_sysroot_crate("std", ["alloc", "core"]) append_sysroot_crate("proc_macro", ["core", "std"]) @@ -177,6 +179,7 @@ def main(): parser = argparse.ArgumentParser() parser.add_argument('--verbose', '-v', action='store_true') parser.add_argument('--cfgs', action='append', default=[]) + parser.add_argument("core_edition") parser.add_argument("srctree", type=pathlib.Path) parser.add_argument("objtree", type=pathlib.Path) parser.add_argument("sysroot", type=pathlib.Path) @@ -193,7 +196,7 @@ def main(): assert args.sysroot in args.sysroot_src.parents rust_project = { - "crates": generate_crates(args.srctree, args.objtree, args.sysroot_src, args.exttree, args.cfgs), + "crates": generate_crates(args.srctree, args.objtree, args.sysroot_src, args.exttree, args.cfgs, args.core_edition), "sysroot": str(args.sysroot), } From eb71feaacaaca227ae8f91c8578cf831553c5ab5 Mon Sep 17 00:00:00 2001 From: Benno Lossin Date: Sun, 25 May 2025 19:34:45 +0200 Subject: [PATCH 65/78] rust: list: fix path of `assert_pinned!` Commit dbd5058ba60c ("rust: make pin-init its own crate") moved all items from pin-init into the pin-init crate, including the `assert_pinned!` macro. Thus fix the path of the sole user of the `assert_pinned!` macro. This occurrence was missed in the commit above, since it is in a macro rule that has no current users (although binder is a future user). Cc: stable@kernel.org Fixes: dbd5058ba60c ("rust: make pin-init its own crate") Signed-off-by: Benno Lossin Link: https://lore.kernel.org/r/20250525173450.853413-1-lossin@kernel.org [ Reworded slightly as discussed in the list. - Miguel ] Signed-off-by: Miguel Ojeda --- rust/kernel/list/arc.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/kernel/list/arc.rs b/rust/kernel/list/arc.rs index 13c50df37b89..a88a2dc65aa7 100644 --- a/rust/kernel/list/arc.rs +++ b/rust/kernel/list/arc.rs @@ -96,7 +96,7 @@ unsafe fn on_drop_list_arc(&self) {} } $($rest:tt)*) => { impl$(<$($generics)*>)? $crate::list::ListArcSafe<$num> for $t { unsafe fn on_create_list_arc_from_unique(self: ::core::pin::Pin<&mut Self>) { - $crate::assert_pinned!($t, $field, $fty, inline); + ::pin_init::assert_pinned!($t, $field, $fty, inline); // SAFETY: This field is structurally pinned as per the above assertion. let field = unsafe { From 4bf7b97eb390f0a0730572101e0ce3367d31a770 Mon Sep 17 00:00:00 2001 From: Patrick Miller Date: Wed, 2 Oct 2024 02:28:48 +0000 Subject: [PATCH 66/78] rust: make section names plural Clean Rust documentation section headers to use plural names. Suggested-by: Miguel Ojeda Link: https://github.com/Rust-for-Linux/linux/issues/1110 Signed-off-by: Patrick Miller Link: https://lore.kernel.org/r/20241002022749.390836-1-paddymills@proton.me [ Removed the `init` one that doesn't apply anymore and reworded slightly. - Miguel ] Signed-off-by: Miguel Ojeda --- rust/kernel/list/arc.rs | 2 +- rust/kernel/sync/arc.rs | 2 +- rust/macros/lib.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/rust/kernel/list/arc.rs b/rust/kernel/list/arc.rs index a88a2dc65aa7..8401ddcd178a 100644 --- a/rust/kernel/list/arc.rs +++ b/rust/kernel/list/arc.rs @@ -464,7 +464,7 @@ impl core::ops::DispatchFromDyn> for ListArc /// A utility for tracking whether a [`ListArc`] exists using an atomic. /// -/// # Invariant +/// # Invariants /// /// If the boolean is `false`, then there is no [`ListArc`] for this value. #[repr(transparent)] diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs index 356eef3afdae..c7af0aa48a0a 100644 --- a/rust/kernel/sync/arc.rs +++ b/rust/kernel/sync/arc.rs @@ -492,7 +492,7 @@ fn from(item: Pin>) -> Self { /// There are no mutable references to the underlying [`Arc`], and it remains valid for the /// lifetime of the [`ArcBorrow`] instance. /// -/// # Example +/// # Examples /// /// ``` /// use kernel::sync::{Arc, ArcBorrow}; diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs index cb429eceff71..b0b86fb9bfab 100644 --- a/rust/macros/lib.rs +++ b/rust/macros/lib.rs @@ -263,7 +263,7 @@ pub fn concat_idents(ts: TokenStream) -> TokenStream { /// literals (lifetimes and documentation strings are not supported). There is a difference in /// supported modifiers as well. /// -/// # Example +/// # Examples /// /// ``` /// # const binder_driver_return_protocol_BR_OK: u32 = 0; From 36174d16f3ec072f9e07b6c6d59ba91b2d52f9e2 Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Fri, 2 May 2025 23:51:26 +0200 Subject: [PATCH 67/78] rust: kunit: support KUnit-mapped `assert!` macros in `#[test]`s The KUnit `#[test]` support that landed recently is very basic and does not map the `assert*!` macros into KUnit like the doctests do, so they panic at the moment. Thus implement the custom mapping in a similar way to doctests, reusing the infrastructure there. In Rust 1.88.0, the `file()` method in `Span` may be stable [1]. However, it was changed recently (from `SourceFile`), so we need to do something different in previous versions. Thus create a helper for it and use it to get the path. With this, a failing test suite like: #[kunit_tests(my_test_suite)] mod tests { use super::*; #[test] fn my_first_test() { assert_eq!(42, 43); } #[test] fn my_second_test() { assert!(42 >= 43); } } will properly map back to KUnit, printing something like: [ 1.924325] KTAP version 1 [ 1.924421] # Subtest: my_test_suite [ 1.924506] # speed: normal [ 1.924525] 1..2 [ 1.926385] # my_first_test: ASSERTION FAILED at rust/kernel/lib.rs:251 [ 1.926385] Expected 42 == 43 to be true, but is false [ 1.928026] # my_first_test.speed: normal [ 1.928075] not ok 1 my_first_test [ 1.928723] # my_second_test: ASSERTION FAILED at rust/kernel/lib.rs:256 [ 1.928723] Expected 42 >= 43 to be true, but is false [ 1.929834] # my_second_test.speed: normal [ 1.929868] not ok 2 my_second_test [ 1.930032] # my_test_suite: pass:0 fail:2 skip:0 total:2 [ 1.930153] # Totals: pass:0 fail:2 skip:0 total Link: https://github.com/rust-lang/rust/pull/140514 [1] Reviewed-by: David Gow Acked-by: Danilo Krummrich Link: https://lore.kernel.org/r/20250502215133.1923676-2-ojeda@kernel.org [ Required `KUNIT=y` like for doctests. Used the `cfg_attr` from the TODO comment and clarified its comment now that the stabilization is in beta and thus quite likely stable in Rust 1.88.0. Simplified the `new_body` code by introducing a new variable. Added `#[allow(clippy::incompatible_msrv)]`. - Miguel ] Signed-off-by: Miguel Ojeda --- init/Kconfig | 3 +++ rust/Makefile | 3 ++- rust/kernel/kunit.rs | 1 - rust/macros/helpers.rs | 17 +++++++++++++++++ rust/macros/kunit.rs | 35 ++++++++++++++++++++++++++++++----- rust/macros/lib.rs | 5 +++++ 6 files changed, 57 insertions(+), 7 deletions(-) diff --git a/init/Kconfig b/init/Kconfig index 4cdd1049283c..fb90cc5c44a7 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -140,6 +140,9 @@ config LD_CAN_USE_KEEP_IN_OVERLAY config RUSTC_HAS_COERCE_POINTEE def_bool RUSTC_VERSION >= 108400 +config RUSTC_HAS_SPAN_FILE + def_bool RUSTC_VERSION >= 108800 + config RUSTC_HAS_UNNECESSARY_TRANSMUTES def_bool RUSTC_VERSION >= 108800 diff --git a/rust/Makefile b/rust/Makefile index d62b58d0a55c..63f3fb265883 100644 --- a/rust/Makefile +++ b/rust/Makefile @@ -404,7 +404,8 @@ quiet_cmd_rustc_procmacro = $(RUSTC_OR_CLIPPY_QUIET) P $@ -Clink-args='$(call escsq,$(KBUILD_PROCMACROLDFLAGS))' \ --emit=dep-info=$(depfile) --emit=link=$@ --extern proc_macro \ --crate-type proc-macro \ - --crate-name $(patsubst lib%.$(libmacros_extension),%,$(notdir $@)) $< + --crate-name $(patsubst lib%.$(libmacros_extension),%,$(notdir $@)) \ + @$(objtree)/include/generated/rustc_cfg $< # Procedural macros can only be used with the `rustc` that compiled it. $(obj)/$(libmacros_name): $(src)/macros/lib.rs FORCE diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs index 81833a687b75..78b4acb6595f 100644 --- a/rust/kernel/kunit.rs +++ b/rust/kernel/kunit.rs @@ -323,7 +323,6 @@ mod tests { #[test] fn rust_test_kunit_example_test() { - #![expect(clippy::eq_op)] assert_eq!(1 + 1, 2); } diff --git a/rust/macros/helpers.rs b/rust/macros/helpers.rs index a3ee27e29a6f..e2602be402c1 100644 --- a/rust/macros/helpers.rs +++ b/rust/macros/helpers.rs @@ -86,3 +86,20 @@ pub(crate) fn function_name(input: TokenStream) -> Option { } None } + +pub(crate) fn file() -> String { + #[cfg(not(CONFIG_RUSTC_HAS_SPAN_FILE))] + { + proc_macro::Span::call_site() + .source_file() + .path() + .to_string_lossy() + .into_owned() + } + + #[cfg(CONFIG_RUSTC_HAS_SPAN_FILE)] + #[allow(clippy::incompatible_msrv)] + { + proc_macro::Span::call_site().file() + } +} diff --git a/rust/macros/kunit.rs b/rust/macros/kunit.rs index b146c5894e92..971c2c3d849a 100644 --- a/rust/macros/kunit.rs +++ b/rust/macros/kunit.rs @@ -57,8 +57,8 @@ pub(crate) fn kunit_tests(attr: TokenStream, ts: TokenStream) -> TokenStream { } } - // Add `#[cfg(CONFIG_KUNIT)]` before the module declaration. - let config_kunit = "#[cfg(CONFIG_KUNIT)]".to_owned().parse().unwrap(); + // Add `#[cfg(CONFIG_KUNIT="y")]` before the module declaration. + let config_kunit = "#[cfg(CONFIG_KUNIT=\"y\")]".to_owned().parse().unwrap(); tokens.insert( 0, TokenTree::Group(Group::new(Delimiter::None, config_kunit)), @@ -98,6 +98,8 @@ pub(crate) fn kunit_tests(attr: TokenStream, ts: TokenStream) -> TokenStream { // ``` let mut kunit_macros = "".to_owned(); let mut test_cases = "".to_owned(); + let mut assert_macros = "".to_owned(); + let path = crate::helpers::file(); for test in &tests { let kunit_wrapper_fn_name = format!("kunit_rust_wrapper_{test}"); let kunit_wrapper = format!( @@ -109,6 +111,27 @@ pub(crate) fn kunit_tests(attr: TokenStream, ts: TokenStream) -> TokenStream { " ::kernel::kunit::kunit_case(::kernel::c_str!(\"{test}\"), {kunit_wrapper_fn_name})," ) .unwrap(); + writeln!( + assert_macros, + r#" +/// Overrides the usual [`assert!`] macro with one that calls KUnit instead. +#[allow(unused)] +macro_rules! assert {{ + ($cond:expr $(,)?) => {{{{ + kernel::kunit_assert!("{test}", "{path}", 0, $cond); + }}}} +}} + +/// Overrides the usual [`assert_eq!`] macro with one that calls KUnit instead. +#[allow(unused)] +macro_rules! assert_eq {{ + ($left:expr, $right:expr $(,)?) => {{{{ + kernel::kunit_assert_eq!("{test}", "{path}", 0, $left, $right); + }}}} +}} + "# + ) + .unwrap(); } writeln!(kunit_macros).unwrap(); @@ -147,10 +170,12 @@ pub(crate) fn kunit_tests(attr: TokenStream, ts: TokenStream) -> TokenStream { } } - let mut new_body = TokenStream::from_iter(new_body); - new_body.extend::(kunit_macros.parse().unwrap()); + let mut final_body = TokenStream::new(); + final_body.extend::(assert_macros.parse().unwrap()); + final_body.extend(new_body); + final_body.extend::(kunit_macros.parse().unwrap()); - tokens.push(TokenTree::Group(Group::new(Delimiter::Brace, new_body))); + tokens.push(TokenTree::Group(Group::new(Delimiter::Brace, final_body))); tokens.into_iter().collect() } diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs index b0b86fb9bfab..bb5df3b730fc 100644 --- a/rust/macros/lib.rs +++ b/rust/macros/lib.rs @@ -6,6 +6,11 @@ // and thus add a dependency on `include/config/RUSTC_VERSION_TEXT`, which is // touched by Kconfig when the version string from the compiler changes. +// Stable since Rust 1.88.0 under a different name, `proc_macro_span_file`, +// which was added in Rust 1.88.0. This is why `cfg_attr` is used here, i.e. +// to avoid depending on the full `proc_macro_span` on Rust >= 1.88.0. +#![cfg_attr(not(CONFIG_RUSTC_HAS_SPAN_FILE), feature(proc_macro_span))] + #[macro_use] mod quote; mod concat_idents; From 950b306c296ec1e90d2d76f1974d2de2375a3d82 Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Fri, 2 May 2025 23:51:27 +0200 Subject: [PATCH 68/78] rust: kunit: support checked `-> Result`s in KUnit `#[test]`s Currently, return values of KUnit `#[test]` functions are ignored. Thus introduce support for `-> Result` functions by checking their returned values. At the same time, require that test functions return `()` or `Result`, which should avoid mistakes, especially with non-`#[must_use]` types. Other types can be supported in the future if needed. With this, a failing test like: #[test] fn my_test() -> Result { f()?; Ok(()) } will output: [ 3.744214] KTAP version 1 [ 3.744287] # Subtest: my_test_suite [ 3.744378] # speed: normal [ 3.744399] 1..1 [ 3.745817] # my_test: ASSERTION FAILED at rust/kernel/lib.rs:321 [ 3.745817] Expected is_test_result_ok(my_test()) to be true, but is false [ 3.747152] # my_test.speed: normal [ 3.747199] not ok 1 my_test [ 3.747345] not ok 4 my_test_suite Reviewed-by: David Gow Acked-by: Danilo Krummrich Link: https://lore.kernel.org/r/20250502215133.1923676-3-ojeda@kernel.org [ Used `::kernel` for paths. - Miguel ] Signed-off-by: Miguel Ojeda --- rust/kernel/kunit.rs | 25 +++++++++++++++++++++++++ rust/macros/kunit.rs | 3 ++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs index 78b4acb6595f..355e9d56dada 100644 --- a/rust/kernel/kunit.rs +++ b/rust/kernel/kunit.rs @@ -164,6 +164,31 @@ macro_rules! kunit_assert_eq { }}; } +trait TestResult { + fn is_test_result_ok(&self) -> bool; +} + +impl TestResult for () { + fn is_test_result_ok(&self) -> bool { + true + } +} + +impl TestResult for Result { + fn is_test_result_ok(&self) -> bool { + self.is_ok() + } +} + +/// Returns whether a test result is to be considered OK. +/// +/// This will be `assert!`ed from the generated tests. +#[doc(hidden)] +#[expect(private_bounds)] +pub fn is_test_result_ok(t: impl TestResult) -> bool { + t.is_test_result_ok() +} + /// Represents an individual test case. /// /// The [`kunit_unsafe_test_suite!`] macro expects a NULL-terminated list of valid test cases. diff --git a/rust/macros/kunit.rs b/rust/macros/kunit.rs index 971c2c3d849a..81d18149a0cc 100644 --- a/rust/macros/kunit.rs +++ b/rust/macros/kunit.rs @@ -102,8 +102,9 @@ pub(crate) fn kunit_tests(attr: TokenStream, ts: TokenStream) -> TokenStream { let path = crate::helpers::file(); for test in &tests { let kunit_wrapper_fn_name = format!("kunit_rust_wrapper_{test}"); + // An extra `use` is used here to reduce the length of the message. let kunit_wrapper = format!( - "unsafe extern \"C\" fn {kunit_wrapper_fn_name}(_test: *mut ::kernel::bindings::kunit) {{ {test}(); }}" + "unsafe extern \"C\" fn {kunit_wrapper_fn_name}(_test: *mut ::kernel::bindings::kunit) {{ use ::kernel::kunit::is_test_result_ok; assert!(is_test_result_ok({test}())); }}", ); writeln!(kunit_macros, "{kunit_wrapper}").unwrap(); writeln!( From 897d1df6532f05814acd364af9055cd6628fd1b3 Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Fri, 2 May 2025 23:51:28 +0200 Subject: [PATCH 69/78] rust: add `kunit_tests` to the prelude It is convenient to have certain things in the `kernel` prelude, and means kernel developers will find it even easier to start writing tests. And, anyway, nobody should need to use this identifier for anything else. Thus add it to the prelude. Reviewed-by: David Gow Acked-by: Danilo Krummrich Link: https://lore.kernel.org/r/20250502215133.1923676-4-ojeda@kernel.org Signed-off-by: Miguel Ojeda --- rust/kernel/kunit.rs | 3 +-- rust/kernel/prelude.rs | 2 +- rust/macros/lib.rs | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs index 355e9d56dada..4b8cdcb21e77 100644 --- a/rust/kernel/kunit.rs +++ b/rust/kernel/kunit.rs @@ -6,6 +6,7 @@ //! //! Reference: +use crate::prelude::*; use core::{ffi::c_void, fmt}; /// Prints a KUnit error-level message. @@ -40,8 +41,6 @@ pub fn info(args: fmt::Arguments<'_>) { } } -use macros::kunit_tests; - /// Asserts that a boolean expression is `true` at runtime. /// /// Public but hidden since it should only be used from generated tests. diff --git a/rust/kernel/prelude.rs b/rust/kernel/prelude.rs index f869b02f1f25..2f30a398dddd 100644 --- a/rust/kernel/prelude.rs +++ b/rust/kernel/prelude.rs @@ -22,7 +22,7 @@ pub use crate::alloc::{flags::*, Box, KBox, KVBox, KVVec, KVec, VBox, VVec, Vec}; #[doc(no_inline)] -pub use macros::{export, module, vtable}; +pub use macros::{export, kunit_tests, module, vtable}; pub use pin_init::{init, pin_data, pin_init, pinned_drop, InPlaceWrite, Init, PinInit, Zeroable}; diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs index bb5df3b730fc..fa847cf3a9b5 100644 --- a/rust/macros/lib.rs +++ b/rust/macros/lib.rs @@ -407,7 +407,7 @@ pub fn paste(input: TokenStream) -> TokenStream { /// # Examples /// /// ```ignore -/// # use macros::kunit_tests; +/// # use kernel::prelude::*; /// #[kunit_tests(kunit_test_suit_name)] /// mod tests { /// #[test] From 028df914e5466a02326829427bb8e26a31a05545 Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Fri, 2 May 2025 23:51:29 +0200 Subject: [PATCH 70/78] rust: str: convert `rusttest` tests into KUnit In general, we should aim to test as much as possible within the actual kernel, and not in the build host. Thus convert these `rusttest` tests into KUnit tests. Reviewed-by: David Gow Acked-by: Danilo Krummrich Link: https://lore.kernel.org/r/20250502215133.1923676-5-ojeda@kernel.org Signed-off-by: Miguel Ojeda --- rust/kernel/str.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs index c554b243ebcb..9b5228025e2c 100644 --- a/rust/kernel/str.rs +++ b/rust/kernel/str.rs @@ -6,7 +6,7 @@ use core::fmt::{self, Write}; use core::ops::{self, Deref, DerefMut, Index}; -use crate::error::{code::*, Error}; +use crate::prelude::*; /// Byte string without UTF-8 validity guarantee. #[repr(transparent)] @@ -572,8 +572,7 @@ macro_rules! c_str { }}; } -#[cfg(test)] -#[expect(clippy::items_after_test_module)] +#[kunit_tests(rust_kernel_str)] mod tests { use super::*; @@ -622,11 +621,10 @@ fn test_cstr_to_str() { } #[test] - #[should_panic] - fn test_cstr_to_str_panic() { + fn test_cstr_to_str_invalid_utf8() { let bad_bytes = b"\xc3\x28\0"; let checked_cstr = CStr::from_bytes_with_nul(bad_bytes).unwrap(); - checked_cstr.to_str().unwrap(); + assert!(checked_cstr.to_str().is_err()); } #[test] From 1486554392e242da5cbe95092d8dfec887bb8cca Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Mon, 26 May 2025 20:01:42 +0200 Subject: [PATCH 71/78] rust: str: simplify KUnit tests `format!` macro Simplify the `format!` macro used in the tests by using `CString::try_from_fmt` and directly `unwrap()`ing. This will allow us to change both `unwrap()`s here in order to showcase the `?` operator support now that the tests are KUnit ones. Reviewed-by: David Gow Acked-by: Danilo Krummrich [ Split from the next commit as suggested by Tamir. - Miguel ] Signed-off-by: Miguel Ojeda --- rust/kernel/str.rs | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs index 9b5228025e2c..52a500742c1a 100644 --- a/rust/kernel/str.rs +++ b/rust/kernel/str.rs @@ -576,25 +576,9 @@ macro_rules! c_str { mod tests { use super::*; - struct String(CString); - - impl String { - fn from_fmt(args: fmt::Arguments<'_>) -> Self { - String(CString::try_from_fmt(args).unwrap()) - } - } - - impl Deref for String { - type Target = str; - - fn deref(&self) -> &str { - self.0.to_str().unwrap() - } - } - macro_rules! format { ($($f:tt)*) => ({ - &*String::from_fmt(::kernel::fmt!($($f)*)) + CString::try_from_fmt(::kernel::fmt!($($f)*)).unwrap().to_str().unwrap() }) } From 2d6c87d0d6a0c0acf6b4dd9eec9ed44a82886836 Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Fri, 2 May 2025 23:51:30 +0200 Subject: [PATCH 72/78] rust: str: take advantage of the `-> Result` support in KUnit `#[test]`'s Since now we have support for returning `-> Result`s, we can convert some of these tests to use the feature, and serve as a first user for it too. Thus convert them, which allows us to remove some `unwrap()`s. We keep the actual assertions we want to make as explicit ones with `assert*!`s. Reviewed-by: David Gow Acked-by: Danilo Krummrich Link: https://lore.kernel.org/r/20250502215133.1923676-6-ojeda@kernel.org [ Split the `CString` simplification into a new commit. - Miguel ] Signed-off-by: Miguel Ojeda --- rust/kernel/str.rs | 52 ++++++++++++++++++++++++++-------------------- 1 file changed, 30 insertions(+), 22 deletions(-) diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs index 52a500742c1a..a927db8e079c 100644 --- a/rust/kernel/str.rs +++ b/rust/kernel/str.rs @@ -578,7 +578,7 @@ mod tests { macro_rules! format { ($($f:tt)*) => ({ - CString::try_from_fmt(::kernel::fmt!($($f)*)).unwrap().to_str().unwrap() + CString::try_from_fmt(::kernel::fmt!($($f)*))?.to_str()? }) } @@ -597,66 +597,72 @@ macro_rules! format { \\xf0\\xf1\\xf2\\xf3\\xf4\\xf5\\xf6\\xf7\\xf8\\xf9\\xfa\\xfb\\xfc\\xfd\\xfe\\xff"; #[test] - fn test_cstr_to_str() { + fn test_cstr_to_str() -> Result { let good_bytes = b"\xf0\x9f\xa6\x80\0"; - let checked_cstr = CStr::from_bytes_with_nul(good_bytes).unwrap(); - let checked_str = checked_cstr.to_str().unwrap(); + let checked_cstr = CStr::from_bytes_with_nul(good_bytes)?; + let checked_str = checked_cstr.to_str()?; assert_eq!(checked_str, "🦀"); + Ok(()) } #[test] - fn test_cstr_to_str_invalid_utf8() { + fn test_cstr_to_str_invalid_utf8() -> Result { let bad_bytes = b"\xc3\x28\0"; - let checked_cstr = CStr::from_bytes_with_nul(bad_bytes).unwrap(); + let checked_cstr = CStr::from_bytes_with_nul(bad_bytes)?; assert!(checked_cstr.to_str().is_err()); + Ok(()) } #[test] - fn test_cstr_as_str_unchecked() { + fn test_cstr_as_str_unchecked() -> Result { let good_bytes = b"\xf0\x9f\x90\xA7\0"; - let checked_cstr = CStr::from_bytes_with_nul(good_bytes).unwrap(); + let checked_cstr = CStr::from_bytes_with_nul(good_bytes)?; // SAFETY: The contents come from a string literal which contains valid UTF-8. let unchecked_str = unsafe { checked_cstr.as_str_unchecked() }; assert_eq!(unchecked_str, "🐧"); + Ok(()) } #[test] - fn test_cstr_display() { - let hello_world = CStr::from_bytes_with_nul(b"hello, world!\0").unwrap(); + fn test_cstr_display() -> Result { + let hello_world = CStr::from_bytes_with_nul(b"hello, world!\0")?; assert_eq!(format!("{hello_world}"), "hello, world!"); - let non_printables = CStr::from_bytes_with_nul(b"\x01\x09\x0a\0").unwrap(); + let non_printables = CStr::from_bytes_with_nul(b"\x01\x09\x0a\0")?; assert_eq!(format!("{non_printables}"), "\\x01\\x09\\x0a"); - let non_ascii = CStr::from_bytes_with_nul(b"d\xe9j\xe0 vu\0").unwrap(); + let non_ascii = CStr::from_bytes_with_nul(b"d\xe9j\xe0 vu\0")?; assert_eq!(format!("{non_ascii}"), "d\\xe9j\\xe0 vu"); - let good_bytes = CStr::from_bytes_with_nul(b"\xf0\x9f\xa6\x80\0").unwrap(); + let good_bytes = CStr::from_bytes_with_nul(b"\xf0\x9f\xa6\x80\0")?; assert_eq!(format!("{good_bytes}"), "\\xf0\\x9f\\xa6\\x80"); + Ok(()) } #[test] - fn test_cstr_display_all_bytes() { + fn test_cstr_display_all_bytes() -> Result { let mut bytes: [u8; 256] = [0; 256]; // fill `bytes` with [1..=255] + [0] for i in u8::MIN..=u8::MAX { bytes[i as usize] = i.wrapping_add(1); } - let cstr = CStr::from_bytes_with_nul(&bytes).unwrap(); + let cstr = CStr::from_bytes_with_nul(&bytes)?; assert_eq!(format!("{cstr}"), ALL_ASCII_CHARS); + Ok(()) } #[test] - fn test_cstr_debug() { - let hello_world = CStr::from_bytes_with_nul(b"hello, world!\0").unwrap(); + fn test_cstr_debug() -> Result { + let hello_world = CStr::from_bytes_with_nul(b"hello, world!\0")?; assert_eq!(format!("{hello_world:?}"), "\"hello, world!\""); - let non_printables = CStr::from_bytes_with_nul(b"\x01\x09\x0a\0").unwrap(); + let non_printables = CStr::from_bytes_with_nul(b"\x01\x09\x0a\0")?; assert_eq!(format!("{non_printables:?}"), "\"\\x01\\x09\\x0a\""); - let non_ascii = CStr::from_bytes_with_nul(b"d\xe9j\xe0 vu\0").unwrap(); + let non_ascii = CStr::from_bytes_with_nul(b"d\xe9j\xe0 vu\0")?; assert_eq!(format!("{non_ascii:?}"), "\"d\\xe9j\\xe0 vu\""); - let good_bytes = CStr::from_bytes_with_nul(b"\xf0\x9f\xa6\x80\0").unwrap(); + let good_bytes = CStr::from_bytes_with_nul(b"\xf0\x9f\xa6\x80\0")?; assert_eq!(format!("{good_bytes:?}"), "\"\\xf0\\x9f\\xa6\\x80\""); + Ok(()) } #[test] - fn test_bstr_display() { + fn test_bstr_display() -> Result { let hello_world = BStr::from_bytes(b"hello, world!"); assert_eq!(format!("{hello_world}"), "hello, world!"); let escapes = BStr::from_bytes(b"_\t_\n_\r_\\_\'_\"_"); @@ -667,10 +673,11 @@ fn test_bstr_display() { assert_eq!(format!("{non_ascii}"), "d\\xe9j\\xe0 vu"); let good_bytes = BStr::from_bytes(b"\xf0\x9f\xa6\x80"); assert_eq!(format!("{good_bytes}"), "\\xf0\\x9f\\xa6\\x80"); + Ok(()) } #[test] - fn test_bstr_debug() { + fn test_bstr_debug() -> Result { let hello_world = BStr::from_bytes(b"hello, world!"); assert_eq!(format!("{hello_world:?}"), "\"hello, world!\""); let escapes = BStr::from_bytes(b"_\t_\n_\r_\\_\'_\"_"); @@ -681,6 +688,7 @@ fn test_bstr_debug() { assert_eq!(format!("{non_ascii:?}"), "\"d\\xe9j\\xe0 vu\""); let good_bytes = BStr::from_bytes(b"\xf0\x9f\xa6\x80"); assert_eq!(format!("{good_bytes:?}"), "\"\\xf0\\x9f\\xa6\\x80\""); + Ok(()) } } From 0a8d4eab17e6f94d83e1ee4c58c21e613934ac66 Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Fri, 2 May 2025 23:51:31 +0200 Subject: [PATCH 73/78] Documentation: rust: rename `#[test]`s to "`rusttest` host tests" Now that `rusttest`s are not really used much, clarify the section of the documentation that describes them. In addition, free the section name for the KUnit-based `#[test]`s that will be added afterwards. To do so, rename the section into `rusttest` host tests. Reviewed-by: David Gow Acked-by: Danilo Krummrich Link: https://lore.kernel.org/r/20250502215133.1923676-7-ojeda@kernel.org Signed-off-by: Miguel Ojeda --- Documentation/rust/testing.rst | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/Documentation/rust/testing.rst b/Documentation/rust/testing.rst index f692494f7b74..6337b83815ab 100644 --- a/Documentation/rust/testing.rst +++ b/Documentation/rust/testing.rst @@ -130,16 +130,17 @@ please see: https://rust.docs.kernel.org/kernel/error/type.Result.html#error-codes-in-c-and-rust -The ``#[test]`` tests ---------------------- +The ``rusttest`` host tests +--------------------------- -Additionally, there are the ``#[test]`` tests. These can be run using the -``rusttest`` Make target:: +These are userspace tests that can be built and run in the host (i.e. the one +that performs the kernel build) using the ``rusttest`` Make target:: make LLVM=1 rusttest -This requires the kernel ``.config``. It runs the ``#[test]`` tests on the host -(currently) and thus is fairly limited in what these tests can test. +This requires the kernel ``.config``. + +Currently, they are mostly used for testing the ``macros`` crate's examples. The Kselftests -------------- From a3b2347343e077e81d3c169f32c9b2cb1364f4cc Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Fri, 2 May 2025 23:51:32 +0200 Subject: [PATCH 74/78] Documentation: rust: testing: add docs on the new KUnit `#[test]` tests There was no documentation yet on the KUnit-based `#[test]`s. Thus add it now. It includes an explanation about the `assert*!` macros being mapped to KUnit and the support for `-> Result` introduced in these series. Reviewed-by: David Gow Acked-by: Danilo Krummrich Link: https://lore.kernel.org/r/20250502215133.1923676-8-ojeda@kernel.org Signed-off-by: Miguel Ojeda --- Documentation/rust/testing.rst | 71 ++++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/Documentation/rust/testing.rst b/Documentation/rust/testing.rst index 6337b83815ab..f43cb77bcc69 100644 --- a/Documentation/rust/testing.rst +++ b/Documentation/rust/testing.rst @@ -130,6 +130,77 @@ please see: https://rust.docs.kernel.org/kernel/error/type.Result.html#error-codes-in-c-and-rust +The ``#[test]`` tests +--------------------- + +Additionally, there are the ``#[test]`` tests. Like for documentation tests, +these are also fairly similar to what you would expect from userspace, and they +are also mapped to KUnit. + +These tests are introduced by the ``kunit_tests`` procedural macro, which takes +the name of the test suite as an argument. + +For instance, assume we want to test the function ``f`` from the documentation +tests section. We could write, in the same file where we have our function: + +.. code-block:: rust + + #[kunit_tests(rust_kernel_mymod)] + mod tests { + use super::*; + + #[test] + fn test_f() { + assert_eq!(f(10, 20), 30); + } + } + +And if we run it, the kernel log would look like:: + + KTAP version 1 + # Subtest: rust_kernel_mymod + # speed: normal + 1..1 + # test_f.speed: normal + ok 1 test_f + ok 1 rust_kernel_mymod + +Like documentation tests, the ``assert!`` and ``assert_eq!`` macros are mapped +back to KUnit and do not panic. Similarly, the +`? `_ +operator is supported, i.e. the test functions may return either nothing (i.e. +the unit type ``()``) or ``Result`` (i.e. any ``Result``). For instance: + +.. code-block:: rust + + #[kunit_tests(rust_kernel_mymod)] + mod tests { + use super::*; + + #[test] + fn test_g() -> Result { + let x = g()?; + assert_eq!(x, 30); + Ok(()) + } + } + +If we run the test and the call to ``g`` fails, then the kernel log would show:: + + KTAP version 1 + # Subtest: rust_kernel_mymod + # speed: normal + 1..1 + # test_g: ASSERTION FAILED at rust/kernel/lib.rs:335 + Expected is_test_result_ok(test_g()) to be true, but is false + # test_g.speed: normal + not ok 1 test_g + not ok 1 rust_kernel_mymod + +If a ``#[test]`` test could be useful as an example for the user, then please +use a documentation test instead. Even edge cases of an API, e.g. error or +boundary cases, can be interesting to show in examples. + The ``rusttest`` host tests --------------------------- From 74d6a606c2b39fd830d2d7a6363bc5688b4e4b56 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Wed, 9 Apr 2025 10:43:16 -0400 Subject: [PATCH 75/78] rust: retain pointer mut-ness in `container_of!` Avoid casting the input pointer to `*const _`, allowing the output pointer to be `*mut` if the input is `*mut`. This allows a number of `*const` to `*mut` conversions to be removed at the cost of slightly worse ergonomics when the macro is used with a reference rather than a pointer; the only example of this was in the macro's own doctest. Reviewed-by: Benno Lossin Reviewed-by: Alice Ryhl Signed-off-by: Tamir Duberstein Reviewed-by: Andreas Hindborg Link: https://lore.kernel.org/r/20250409-container-of-mutness-v1-1-64f472b94534@gmail.com Signed-off-by: Miguel Ojeda --- rust/kernel/lib.rs | 5 ++--- rust/kernel/rbtree.rs | 23 ++++++++++------------- 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs index 6e9287136cac..909d305d0be8 100644 --- a/rust/kernel/lib.rs +++ b/rust/kernel/lib.rs @@ -205,7 +205,7 @@ fn panic(info: &core::panic::PanicInfo<'_>) -> ! { /// } /// /// let test = Test { a: 10, b: 20 }; -/// let b_ptr = &test.b; +/// let b_ptr: *const _ = &test.b; /// // SAFETY: The pointer points at the `b` field of a `Test`, so the resulting pointer will be /// // in-bounds of the same allocation as `b_ptr`. /// let test_alias = unsafe { container_of!(b_ptr, Test, b) }; @@ -214,9 +214,8 @@ fn panic(info: &core::panic::PanicInfo<'_>) -> ! { #[macro_export] macro_rules! container_of { ($ptr:expr, $type:ty, $($f:tt)*) => {{ - let ptr = $ptr as *const _ as *const u8; let offset: usize = ::core::mem::offset_of!($type, $($f)*); - ptr.sub(offset) as *const $type + $ptr.byte_sub(offset).cast::<$type>() }} } diff --git a/rust/kernel/rbtree.rs b/rust/kernel/rbtree.rs index 5246b2c8a4ff..8d978c896747 100644 --- a/rust/kernel/rbtree.rs +++ b/rust/kernel/rbtree.rs @@ -424,7 +424,7 @@ pub fn cursor_lower_bound(&mut self, key: &K) -> Option> while !node.is_null() { // SAFETY: By the type invariant of `Self`, all non-null `rb_node` pointers stored in `self` // point to the links field of `Node` objects. - let this = unsafe { container_of!(node, Node, links) }.cast_mut(); + let this = unsafe { container_of!(node, Node, links) }; // SAFETY: `this` is a non-null node so it is valid by the type invariants. let this_key = unsafe { &(*this).key }; // SAFETY: `node` is a non-null node so it is valid by the type invariants. @@ -496,7 +496,7 @@ fn drop(&mut self) { // but it is not observable. The loop invariant is still maintained. // SAFETY: `this` is valid per the loop invariant. - unsafe { drop(KBox::from_raw(this.cast_mut())) }; + unsafe { drop(KBox::from_raw(this)) }; } } } @@ -761,7 +761,7 @@ pub fn remove_current(self) -> (Option, RBTreeNode) { let next = self.get_neighbor_raw(Direction::Next); // SAFETY: By the type invariant of `Self`, all non-null `rb_node` pointers stored in `self` // point to the links field of `Node` objects. - let this = unsafe { container_of!(self.current.as_ptr(), Node, links) }.cast_mut(); + let this = unsafe { container_of!(self.current.as_ptr(), Node, links) }; // SAFETY: `this` is valid by the type invariants as described above. let node = unsafe { KBox::from_raw(this) }; let node = RBTreeNode { node }; @@ -806,7 +806,7 @@ fn remove_neighbor(&mut self, direction: Direction) -> Option> unsafe { bindings::rb_erase(neighbor, addr_of_mut!(self.tree.root)) }; // SAFETY: By the type invariant of `Self`, all non-null `rb_node` pointers stored in `self` // point to the links field of `Node` objects. - let this = unsafe { container_of!(neighbor, Node, links) }.cast_mut(); + let this = unsafe { container_of!(neighbor, Node, links) }; // SAFETY: `this` is valid by the type invariants as described above. let node = unsafe { KBox::from_raw(this) }; return Some(RBTreeNode { node }); @@ -912,7 +912,7 @@ unsafe fn to_key_value_mut<'b>(node: NonNull) -> (&'b K, &'b unsafe fn to_key_value_raw<'b>(node: NonNull) -> (&'b K, *mut V) { // SAFETY: By the type invariant of `Self`, all non-null `rb_node` pointers stored in `self` // point to the links field of `Node` objects. - let this = unsafe { container_of!(node.as_ptr(), Node, links) }.cast_mut(); + let this = unsafe { container_of!(node.as_ptr(), Node, links) }; // SAFETY: The passed `node` is the current node or a non-null neighbor, // thus `this` is valid by the type invariants. let k = unsafe { &(*this).key }; @@ -1021,7 +1021,7 @@ fn next(&mut self) -> Option { // SAFETY: By the type invariant of `IterRaw`, `self.next` is a valid node in an `RBTree`, // and by the type invariant of `RBTree`, all nodes point to the links field of `Node` objects. - let cur = unsafe { container_of!(self.next, Node, links) }.cast_mut(); + let cur = unsafe { container_of!(self.next, Node, links) }; // SAFETY: `self.next` is a valid tree node by the type invariants. self.next = unsafe { bindings::rb_next(self.next) }; @@ -1216,7 +1216,7 @@ pub fn get_mut(&mut self) -> &mut V { // SAFETY: // - `self.node_links` is a valid pointer to a node in the tree. // - We have exclusive access to the underlying tree, and can thus give out a mutable reference. - unsafe { &mut (*(container_of!(self.node_links, Node, links).cast_mut())).value } + unsafe { &mut (*(container_of!(self.node_links, Node, links))).value } } /// Converts the entry into a mutable reference to its value. @@ -1226,7 +1226,7 @@ pub fn into_mut(self) -> &'a mut V { // SAFETY: // - `self.node_links` is a valid pointer to a node in the tree. // - This consumes the `&'a mut RBTree`, therefore it can give out a mutable reference that lives for `'a`. - unsafe { &mut (*(container_of!(self.node_links, Node, links).cast_mut())).value } + unsafe { &mut (*(container_of!(self.node_links, Node, links))).value } } /// Remove this entry from the [`RBTree`]. @@ -1239,9 +1239,7 @@ pub fn remove_node(self) -> RBTreeNode { RBTreeNode { // SAFETY: The node was a node in the tree, but we removed it, so we can convert it // back into a box. - node: unsafe { - KBox::from_raw(container_of!(self.node_links, Node, links).cast_mut()) - }, + node: unsafe { KBox::from_raw(container_of!(self.node_links, Node, links)) }, } } @@ -1272,8 +1270,7 @@ fn replace(self, node: RBTreeNode) -> RBTreeNode { // SAFETY: // - `self.node_ptr` produces a valid pointer to a node in the tree. // - Now that we removed this entry from the tree, we can convert the node to a box. - let old_node = - unsafe { KBox::from_raw(container_of!(self.node_links, Node, links).cast_mut()) }; + let old_node = unsafe { KBox::from_raw(container_of!(self.node_links, Node, links)) }; RBTreeNode { node: old_node } } From 1ce98bb2bb30713ec4374ef11ead0d7d3e856766 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Fri, 11 Apr 2025 10:08:27 -0400 Subject: [PATCH 76/78] rust: workqueue: remove HasWork::OFFSET Implement `HasWork::work_container_of` in `impl_has_work!`, narrowing the interface of `HasWork` and replacing pointer arithmetic with `container_of!`. Remove the provided implementation of `HasWork::get_work_offset` without replacement; an implementation is already generated in `impl_has_work!`. Remove the `Self: Sized` bound on `HasWork::work_container_of` which was apparently necessary to access `OFFSET` as `OFFSET` no longer exists. A similar API change was discussed on the hrtimer series[1]. Link: https://lore.kernel.org/all/20250224-hrtimer-v3-v6-12-rc2-v9-1-5bd3bf0ce6cc@kernel.org/ [1] Reviewed-by: Benno Lossin Reviewed-by: Alice Ryhl Tested-by: Alice Ryhl Reviewed-by: Andreas Hindborg Signed-off-by: Tamir Duberstein Acked-by: Tejun Heo Link: https://lore.kernel.org/r/20250411-no-offset-v3-1-c0b174640ec3@gmail.com Signed-off-by: Miguel Ojeda --- rust/kernel/workqueue.rs | 50 ++++++++++++++-------------------------- 1 file changed, 17 insertions(+), 33 deletions(-) diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs index f98bd02b838f..d092112d843f 100644 --- a/rust/kernel/workqueue.rs +++ b/rust/kernel/workqueue.rs @@ -429,51 +429,28 @@ pub unsafe fn raw_get(ptr: *const Self) -> *mut bindings::work_struct { /// /// # Safety /// -/// The [`OFFSET`] constant must be the offset of a field in `Self` of type [`Work`]. The -/// methods on this trait must have exactly the behavior that the definitions given below have. +/// The methods [`raw_get_work`] and [`work_container_of`] must return valid pointers and must be +/// true inverses of each other; that is, they must satisfy the following invariants: +/// - `work_container_of(raw_get_work(ptr)) == ptr` for any `ptr: *mut Self`. +/// - `raw_get_work(work_container_of(ptr)) == ptr` for any `ptr: *mut Work`. /// /// [`impl_has_work!`]: crate::impl_has_work -/// [`OFFSET`]: HasWork::OFFSET +/// [`raw_get_work`]: HasWork::raw_get_work +/// [`work_container_of`]: HasWork::work_container_of pub unsafe trait HasWork { - /// The offset of the [`Work`] field. - const OFFSET: usize; - - /// Returns the offset of the [`Work`] field. - /// - /// This method exists because the [`OFFSET`] constant cannot be accessed if the type is not - /// [`Sized`]. - /// - /// [`OFFSET`]: HasWork::OFFSET - #[inline] - fn get_work_offset(&self) -> usize { - Self::OFFSET - } - /// Returns a pointer to the [`Work`] field. /// /// # Safety /// /// The provided pointer must point at a valid struct of type `Self`. - #[inline] - unsafe fn raw_get_work(ptr: *mut Self) -> *mut Work { - // SAFETY: The caller promises that the pointer is valid. - unsafe { (ptr as *mut u8).add(Self::OFFSET) as *mut Work } - } + unsafe fn raw_get_work(ptr: *mut Self) -> *mut Work; /// Returns a pointer to the struct containing the [`Work`] field. /// /// # Safety /// /// The pointer must point at a [`Work`] field in a struct of type `Self`. - #[inline] - unsafe fn work_container_of(ptr: *mut Work) -> *mut Self - where - Self: Sized, - { - // SAFETY: The caller promises that the pointer points at a field of the right type in the - // right kind of struct. - unsafe { (ptr as *mut u8).sub(Self::OFFSET) as *mut Self } - } + unsafe fn work_container_of(ptr: *mut Work) -> *mut Self; } /// Used to safely implement the [`HasWork`] trait. @@ -504,8 +481,6 @@ macro_rules! impl_has_work { // SAFETY: The implementation of `raw_get_work` only compiles if the field has the right // type. unsafe impl$(<$($generics)+>)? $crate::workqueue::HasWork<$work_type $(, $id)?> for $self { - const OFFSET: usize = ::core::mem::offset_of!(Self, $field) as usize; - #[inline] unsafe fn raw_get_work(ptr: *mut Self) -> *mut $crate::workqueue::Work<$work_type $(, $id)?> { // SAFETY: The caller promises that the pointer is not dangling. @@ -513,6 +488,15 @@ unsafe fn raw_get_work(ptr: *mut Self) -> *mut $crate::workqueue::Work<$work_typ ::core::ptr::addr_of_mut!((*ptr).$field) } } + + #[inline] + unsafe fn work_container_of( + ptr: *mut $crate::workqueue::Work<$work_type $(, $id)?>, + ) -> *mut Self { + // SAFETY: The caller promises that the pointer points at a field of the right type + // in the right kind of struct. + unsafe { $crate::container_of!(ptr, Self, $field) } + } } )*}; } From b20fbbc08a363f28fd0f3ad5163b2b0ff7605ba5 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Thu, 29 May 2025 09:11:33 -0400 Subject: [PATCH 77/78] rust: check type of `$ptr` in `container_of!` Add a compile-time check that `*$ptr` is of the type of `$type->$($f)*`. Rename those placeholders for clarity. Given the incorrect usage: > diff --git a/rust/kernel/rbtree.rs b/rust/kernel/rbtree.rs > index 8d978c896747..6a7089149878 100644 > --- a/rust/kernel/rbtree.rs > +++ b/rust/kernel/rbtree.rs > @@ -329,7 +329,7 @@ fn raw_entry(&mut self, key: &K) -> RawEntry<'_, K, V> { > while !(*child_field_of_parent).is_null() { > let curr = *child_field_of_parent; > // SAFETY: All links fields we create are in a `Node`. > - let node = unsafe { container_of!(curr, Node, links) }; > + let node = unsafe { container_of!(curr, Node, key) }; > > // SAFETY: `node` is a non-null node so it is valid by the type invariants. > match key.cmp(unsafe { &(*node).key }) { this patch produces the compilation error: > error[E0308]: mismatched types > --> rust/kernel/lib.rs:220:45 > | > 220 | $crate::assert_same_type(field_ptr, (&raw const (*container_ptr).$($fields)*).cast_mut()); > | ------------------------ --------- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `*mut rb_node`, found `*mut K` > | | | > | | expected all arguments to be this `*mut bindings::rb_node` type because they need to match the type of this parameter > | arguments to this function are incorrect > | > ::: rust/kernel/rbtree.rs:270:6 > | > 270 | impl RBTree > | - found this type parameter > ... > 332 | let node = unsafe { container_of!(curr, Node, key) }; > | ------------------------------------ in this macro invocation > | > = note: expected raw pointer `*mut bindings::rb_node` > found raw pointer `*mut K` > note: function defined here > --> rust/kernel/lib.rs:227:8 > | > 227 | pub fn assert_same_type(_: T, _: T) {} > | ^^^^^^^^^^^^^^^^ - ---- ---- this parameter needs to match the `*mut bindings::rb_node` type of parameter #1 > | | | > | | parameter #2 needs to match the `*mut bindings::rb_node` type of this parameter > | parameter #1 and parameter #2 both reference this parameter `T` > = note: this error originates in the macro `container_of` (in Nightly builds, run with -Z macro-backtrace for more info) [ We decided to go with a variation of v1 [1] that became v4, since it seems like the obvious approach, the error messages seem good enough and the debug performance should be fine, given the kernel is always built with -O2. In the future, we may want to make the helper non-hidden, with proper documentation, for others to use. [1] https://lore.kernel.org/rust-for-linux/CANiq72kQWNfSV0KK6qs6oJt+aGdgY=hXg=wJcmK3zYcokY1LNw@mail.gmail.com/ - Miguel ] Suggested-by: Alice Ryhl Link: https://lore.kernel.org/all/CAH5fLgh6gmqGBhPMi2SKn7mCmMWfOSiS0WP5wBuGPYh9ZTAiww@mail.gmail.com/ Signed-off-by: Tamir Duberstein Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20250529-b4-container-of-type-check-v4-1-bf3a7ad73cec@gmail.com [ Added intra-doc link. - Miguel ] Signed-off-by: Miguel Ojeda --- rust/kernel/lib.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs index 909d305d0be8..7e227b52b4d8 100644 --- a/rust/kernel/lib.rs +++ b/rust/kernel/lib.rs @@ -213,12 +213,19 @@ fn panic(info: &core::panic::PanicInfo<'_>) -> ! { /// ``` #[macro_export] macro_rules! container_of { - ($ptr:expr, $type:ty, $($f:tt)*) => {{ - let offset: usize = ::core::mem::offset_of!($type, $($f)*); - $ptr.byte_sub(offset).cast::<$type>() + ($field_ptr:expr, $Container:ty, $($fields:tt)*) => {{ + let offset: usize = ::core::mem::offset_of!($Container, $($fields)*); + let field_ptr = $field_ptr; + let container_ptr = field_ptr.byte_sub(offset).cast::<$Container>(); + $crate::assert_same_type(field_ptr, (&raw const (*container_ptr).$($fields)*).cast_mut()); + container_ptr }} } +/// Helper for [`container_of!`]. +#[doc(hidden)] +pub fn assert_same_type(_: T, _: T) {} + /// Helper for `.rs.S` files. #[doc(hidden)] #[macro_export] From 7a17bbc1d952057898cb0739e60665908fbb8c72 Mon Sep 17 00:00:00 2001 From: Sylvan Smit Date: Thu, 29 May 2025 18:29:23 +0200 Subject: [PATCH 78/78] rust: list: Fix typo `much` in arc.rs Correct the typo (s/much/must) in the ListArc documentation. Reported-by: Miguel Ojeda Closes: https://github.com/Rust-for-Linux/linux/issues/1166 Fixes: a48026315cd7 ("rust: list: add tracking for ListArc") Signed-off-by: Sylvan Smit Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20250529162923.434978-1-sylvan@sylvansmit.com [ Changed tag to "Reported-by" and sorted. - Miguel ] Signed-off-by: Miguel Ojeda --- rust/kernel/list/arc.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/kernel/list/arc.rs b/rust/kernel/list/arc.rs index 8401ddcd178a..d92bcf665c89 100644 --- a/rust/kernel/list/arc.rs +++ b/rust/kernel/list/arc.rs @@ -74,7 +74,7 @@ pub unsafe trait TryNewListArc: ListArcSafe { /// /// * The `untracked` strategy does not actually keep track of whether a [`ListArc`] exists. When /// using this strategy, the only way to create a [`ListArc`] is using a [`UniqueArc`]. -/// * The `tracked_by` strategy defers the tracking to a field of the struct. The user much specify +/// * The `tracked_by` strategy defers the tracking to a field of the struct. The user must specify /// which field to defer the tracking to. The field must implement [`ListArcSafe`]. If the field /// implements [`TryNewListArc`], then the type will also implement [`TryNewListArc`]. ///