From fb1bf1067de979c89ae33589e0466d6ce0dde204 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Sat, 15 Mar 2025 16:43:02 +0100 Subject: 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(+) (limited to 'rust/kernel') 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 @@ where #[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; } -- cgit v1.2.3 From 81e1c4dab5d0c508907722f18b028102454d52e6 Mon Sep 17 00:00:00 2001 From: Andrew Ballance Date: Sun, 16 Mar 2025 06:16:42 -0500 Subject: 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(+) (limited to 'rust/kernel') 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 @@ where 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 { -- cgit v1.2.3 From 1679b7159379d11100e4ab7d1de23c8cd7765aa1 Mon Sep 17 00:00:00 2001 From: Andrew Ballance Date: Sun, 16 Mar 2025 06:16:43 -0500 Subject: 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(+) (limited to 'rust/kernel') 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 @@ impl Vec { 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 -- cgit v1.2.3 From c3152988c047a7b6abb10d4dc5e24fafbabe8b7e Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Tue, 18 Mar 2025 10:52:42 -0400 Subject: 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(-) (limited to 'rust/kernel') 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 @@ where 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 -- cgit v1.2.3 From 85f8e98dbb0135d2bc1999c6015cd374fe2c69fa Mon Sep 17 00:00:00 2001 From: Alexandre Courbot Date: Sat, 12 Apr 2025 15:29:13 +0900 Subject: 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(-) (limited to 'rust/kernel') 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 @@ use pin_init::{InPlaceWrite, Init, PinInit, ZeroableOption}; /// 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. /// -- cgit v1.2.3 From 47a17a63f9e23f7e8f39d0965bcda8fee6c322f8 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Wed, 16 Apr 2025 13:15:40 -0400 Subject: 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(-) (limited to 'rust/kernel') 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 @@ where /// 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 @@ where 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) } { -- cgit v1.2.3 From dbb0b840a0cd2ebabbc94a0040e366c7f1a70f7b Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Wed, 16 Apr 2025 13:15:41 -0400 Subject: 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(+) (limited to 'rust/kernel') 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 @@ where 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] { -- cgit v1.2.3 From 1b04b466c873f62413bf65a05a558f036660aedc Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Wed, 16 Apr 2025 13:15:42 -0400 Subject: 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(-) (limited to 'rust/kernel') 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 @@ where /// # 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 @@ where /// # 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) }; } } -- cgit v1.2.3 From 88d5d6a38d5161228fbfe017eb94d777d5e8a0e4 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Wed, 16 Apr 2025 13:15:43 -0400 Subject: 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(-) (limited to 'rust/kernel') 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 @@ where 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 @@ where // 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 @@ impl Vec { // 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 @@ impl Vec { // 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 @@ impl CString { // 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 @@ impl UserSliceReader { // 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(()) } } -- cgit v1.2.3 From 1116f0c5ff3385658ceb8ae2c5c4cb05bd7836d7 Mon Sep 17 00:00:00 2001 From: FUJITA Tomonori Date: Thu, 24 Apr 2025 04:28:51 +0900 Subject: 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(-) (limited to 'rust/kernel') 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::HrTimer; 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::HasHrTimer; 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::HrTimer; 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 -- cgit v1.2.3 From 3caad57d29b5f64fa41cff0b12cc5d9144dacb04 Mon Sep 17 00:00:00 2001 From: FUJITA Tomonori Date: Thu, 24 Apr 2025 04:28:52 +0900 Subject: 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(-) (limited to 'rust/kernel') 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, } -- cgit v1.2.3 From fae0cdc12340ce402a4681dba0f357b05d167d00 Mon Sep 17 00:00:00 2001 From: FUJITA Tomonori Date: Thu, 24 Apr 2025 04:28:53 +0900 Subject: 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(+) (limited to 'rust/kernel') 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 @@ impl ClockId { 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 + } +} -- cgit v1.2.3 From ddc671506458849c1a1c882208bbffed033e770c Mon Sep 17 00:00:00 2001 From: FUJITA Tomonori Date: Thu, 24 Apr 2025 04:28:54 +0900 Subject: 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 | 77 +++++++++++++++++++++++++++-------------------------- 1 file changed, 39 insertions(+), 38 deletions(-) (limited to 'rust/kernel') 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 + pub fn now() -> Self { + // INVARIANT: The `ktime_get()` function returns a value in the range + // from 0 to `KTIME_MAX`. + Self { + // SAFETY: It is always safe to call `ktime_get()` outside of NMI context. + inner: unsafe { bindings::ktime_get() }, + } } - /// Returns the number of milliseconds. + /// Return the amount of time elapsed since the [`Instant`]. #[inline] - pub fn to_ms(self) -> i64 { - self.divns_constant::() + pub fn elapsed(&self) -> Delta { + Self::now() - *self } } -/// 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; +impl core::ops::Sub for Instant { + type Output = Delta; + // By the type invariant, it never overflows. #[inline] - fn sub(self, other: Ktime) -> Ktime { - Self { - inner: self.inner - other.inner, + fn sub(self, other: Instant) -> Delta { + Delta { + nanos: self.inner - other.inner, } } } -- cgit v1.2.3 From 1a4736c3d8394f5e64557a41b4b2b8d6dcd04622 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Wed, 23 Apr 2025 09:54:37 -0400 Subject: 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(-) (limited to 'rust/kernel') 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 @@ where } } -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 @@ impl Adapter { 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 @@ impl Adapter { 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 @@ impl Arc { } } -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 @@ impl ForeignOwnable for Arc { 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 @@ use pin_init::{PinInit, Zeroable}; /// /// 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 @@ pub trait ForeignOwnable: Sized { /// /// [`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 @@ pub trait ForeignOwnable: Sized { /// [`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. -- cgit v1.2.3 From 210b81578efbe5c5e7748e50d313e1a90b03df55 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Wed, 23 Apr 2025 09:54:38 -0400 Subject: 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/kernel/lib.rs | 1 + rust/kernel/xarray.rs | 275 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 276 insertions(+) create mode 100644 rust/kernel/xarray.rs (limited to 'rust/kernel') 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 transmute; 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 {} -- cgit v1.2.3 From a1e4d5c9d708d7a0e7071015a120a4489404128f Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Fri, 2 May 2025 13:19:29 +0000 Subject: 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(+) (limited to 'rust/kernel') 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 @@ where (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 -- cgit v1.2.3 From f2b4dd7093438e4884cb01a783212abfbc9cc40b Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Fri, 2 May 2025 13:19:30 +0000 Subject: 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(+) (limited to 'rust/kernel') 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 @@ where 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 -- cgit v1.2.3 From 9def0d0a2a1c62d7970f4ce5ad5557968c98f637 Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Fri, 2 May 2025 13:19:31 +0000 Subject: 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 (limited to 'rust/kernel') 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 @@ use core::{ 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 @@ where /// ``` 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 + } +} -- cgit v1.2.3 From 088bf14a886e1e746c961a862ebccbb76d7cbd4e Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Fri, 2 May 2025 13:19:32 +0000 Subject: 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(+) (limited to 'rust/kernel') 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 @@ where 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 @@ where } } } + +/// 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) }; + } + } +} -- cgit v1.2.3 From 9f140894e72735f034fdc0e963d0550ef03c6f44 Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Fri, 2 May 2025 13:19:33 +0000 Subject: 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(+) (limited to 'rust/kernel') 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 @@ where 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 @@ impl<'vec, T> Drop for DrainAll<'vec, T> { } } } + +#[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(); + } + } +} -- cgit v1.2.3 From 294a7ecbdf0a5d65c6df1287c5d56241e9331cf2 Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Fri, 2 May 2025 13:19:34 +0000 Subject: 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(-) (limited to 'rust/kernel') 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 @@ use core::{ }; mod errors; -pub use self::errors::PushError; +pub use self::errors::{PushError, RemoveError}; /// Create a [`KVec`] containing the arguments. /// @@ -389,6 +389,42 @@ where 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 @@ impl From> for 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 + } +} -- cgit v1.2.3 From 771c5a7d9843643b035938624050e7769133b9cc Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Fri, 2 May 2025 13:19:35 +0000 Subject: 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(-) (limited to 'rust/kernel') 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 @@ use core::{ }; 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 @@ where 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 @@ impl From for 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 + } +} -- cgit v1.2.3 From 878620c5a93a24af10e64b116e66c2314e91a719 Mon Sep 17 00:00:00 2001 From: Kunwu Chan Date: Fri, 21 Mar 2025 16:01:24 +0800 Subject: 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(+) (limited to 'rust/kernel') 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 @@ impl Page { } 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) }; -- cgit v1.2.3 From 0fa5f8c877cae959de7cf6c3dc054e23e7ebcd75 Mon Sep 17 00:00:00 2001 From: Altan Ozlu Date: Wed, 26 Mar 2025 20:25:36 +0000 Subject: 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(-) (limited to 'rust/kernel') 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)?); }; } -- cgit v1.2.3 From 7d8dee4689278e174900509b8c4604651159d8ee Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Tue, 29 Apr 2025 17:14:45 +0200 Subject: 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(-) (limited to 'rust/kernel') 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 @@ pub type UserPtr = usize; /// /// ```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 @@ pub type UserPtr = usize; /// /// ```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. -- cgit v1.2.3 From b9b701fce49a448b1e046f7cda592fec2958e5cd Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Thu, 27 Mar 2025 22:13:02 +0100 Subject: 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 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) (limited to 'rust/kernel') 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. -- cgit v1.2.3 From 86d990c7b699581918de2a379c6eebac7292940e Mon Sep 17 00:00:00 2001 From: Xizhe Yin Date: Mon, 7 Apr 2025 11:34:41 +0800 Subject: 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 +- 4 files changed, 4 insertions(+), 4 deletions(-) (limited to 'rust/kernel') 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 VBox = Box; 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 @@ impl GenDiskBuilder { 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>, } -- cgit v1.2.3 From edc5e6e019c99b529b3d1f2801d5cce9924ae79b Mon Sep 17 00:00:00 2001 From: Andrew Ballance Date: Tue, 25 Mar 2025 08:33:52 -0500 Subject: 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(-) (limited to 'rust/kernel') 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 /// -- cgit v1.2.3 From ae8b3a83fb9de394f609035041cd7a668fda2ab3 Mon Sep 17 00:00:00 2001 From: Jihed Chaibi Date: Sat, 17 May 2025 02:26:04 +0200 Subject: 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(-) (limited to 'rust/kernel') 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 @@ impl RawFormatter { /// 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, -- cgit v1.2.3 From 81e9edc1a8d657291409d70d93361d8277d226d8 Mon Sep 17 00:00:00 2001 From: Christian Schrefl Date: Sat, 17 May 2025 13:06:15 +0200 Subject: rust: miscdevice: fix typo in MiscDevice::ioctl documentation Fixes one small typo (`utilties` to `utilities`) in the documentation of `MiscDevice::ioctl`. Fixes: f893691e7426 ("rust: miscdevice: add base miscdevice abstraction") Signed-off-by: Christian Schrefl Reviewed-by: Benno Lossin Reviewed-by: Alice Ryhl Link: https://lore.kernel.org/r/20250517-rust_miscdevice_fix_typo-v1-1-8c30a6237ba9@gmail.com Signed-off-by: Greg Kroah-Hartman --- rust/kernel/miscdevice.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'rust/kernel') diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs index fa9ecc42602a..15d10e5c1db7 100644 --- a/rust/kernel/miscdevice.rs +++ b/rust/kernel/miscdevice.rs @@ -121,7 +121,7 @@ pub trait MiscDevice: Sized { /// Handler for ioctls. /// - /// The `cmd` argument is usually manipulated using the utilties in [`kernel::ioctl`]. + /// The `cmd` argument is usually manipulated using the utilities in [`kernel::ioctl`]. /// /// [`kernel::ioctl`]: mod@crate::ioctl fn ioctl( -- cgit v1.2.3 From bb941ea789f803cce766ca1e0f7c59a362aaf99a Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Tue, 20 May 2025 20:21:25 +0200 Subject: 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(-) (limited to 'rust/kernel') 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}; -- cgit v1.2.3 From 28669b2f37e9b7e98b62d0be2e10a5bf31c2b16f Mon Sep 17 00:00:00 2001 From: I Hsin Cheng Date: Mon, 10 Mar 2025 15:38:52 +0800 Subject: 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(-) (limited to 'rust/kernel') 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 @@ impl, const ID: u64> List { /// 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 @@ impl, const ID: u64> List { /// 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; } -- cgit v1.2.3 From 195746046c256ce5324772394c886ba798859fca Mon Sep 17 00:00:00 2001 From: I Hsin Cheng Date: Tue, 11 Mar 2025 21:33:57 +0800 Subject: 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(+) (limited to 'rust/kernel') 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 @@ pub use self::arc_field::{define_list_arc_field_getter, ListArcField}; /// * 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>, -- cgit v1.2.3 From 8cbc95f983bcec7e042266766ffe0d68980e4290 Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Tue, 25 Mar 2025 19:43:09 +0100 Subject: 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/kernel/time/hrtimer.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'rust/kernel') 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 @@ pub unsafe trait HasHrTimer { #[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 { -- cgit v1.2.3 From de7cd3e4d6387df6a5ae8c4c32ff0479ebe0efb5 Mon Sep 17 00:00:00 2001 From: Igor Korotin Date: Mon, 19 May 2025 17:45:53 +0100 Subject: 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/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 ++-- 5 files changed, 10 insertions(+), 10 deletions(-) (limited to 'rust/kernel') 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 @@ macro_rules! kunit_assert { 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 @@ mod tests { macro_rules! format { ($($f:tt)*) => ({ - &*String::from_fmt(kernel::fmt!($($f)*)) + &*String::from_fmt(::kernel::fmt!($($f)*)) }) } @@ -944,5 +944,5 @@ impl fmt::Debug for CString { /// A convenience alias for [`core::format_args`]. #[macro_export] macro_rules! fmt { - ($($f:tt)*) => ( core::format_args!($($f)*) ) + ($($f:tt)*) => ( ::core::format_args!($($f)*) ) } -- cgit v1.2.3 From 3d5bef5d47c371e17041cd6c84e9c08e54ea9e63 Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Sun, 13 Apr 2025 02:56:50 +0200 Subject: 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 --- rust/kernel/prelude.rs | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'rust/kernel') 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)] -- cgit v1.2.3 From 9f047636831a61ce0840929555245dd17695206a Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Mon, 24 Mar 2025 22:03:55 +0100 Subject: 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(-) (limited to 'rust/kernel') 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. -- cgit v1.2.3 From 673ec360cfb099a5f44dabee0f0e6c9b282efa7e Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Mon, 24 Mar 2025 22:03:50 +0100 Subject: 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(-) (limited to 'rust/kernel') 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); -- cgit v1.2.3 From abd21a163d4188c180cabb6747b1d94e3c0586b9 Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Mon, 24 Mar 2025 22:03:51 +0100 Subject: 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(-) (limited to 'rust/kernel') 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)] -- cgit v1.2.3 From 1dbaf8b1bafb8557904eb54b98bb323a3061dd2c Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Mon, 24 Mar 2025 22:03:54 +0100 Subject: 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(-) (limited to 'rust/kernel') 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 @@ unsafe impl RawDeviceId for DeviceId { } } -/// 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. -- cgit v1.2.3 From f54c750333381bfeaa0e4a69b9563d4e4e21f1b3 Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Mon, 24 Mar 2025 22:03:56 +0100 Subject: 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(-) (limited to 'rust/kernel') 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 @@ impl Task { } } - /// 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. /// -- cgit v1.2.3 From df523db15a06c37cbabad8f477ae87d08c9081f2 Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Mon, 24 Mar 2025 22:03:52 +0100 Subject: 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(-) (limited to 'rust/kernel') 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 -- cgit v1.2.3 From eb71feaacaaca227ae8f91c8578cf831553c5ab5 Mon Sep 17 00:00:00 2001 From: Benno Lossin Date: Sun, 25 May 2025 19:34:45 +0200 Subject: 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(-) (limited to 'rust/kernel') 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 @@ macro_rules! impl_list_arc_safe { } $($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 { -- cgit v1.2.3 From 4bf7b97eb390f0a0730572101e0ce3367d31a770 Mon Sep 17 00:00:00 2001 From: Patrick Miller Date: Wed, 2 Oct 2024 02:28:48 +0000 Subject: 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 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'rust/kernel') 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 @@ where /// 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 @@ impl From>> for Arc { /// 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}; -- cgit v1.2.3 From 36174d16f3ec072f9e07b6c6d59ba91b2d52f9e2 Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Fri, 2 May 2025 23:51:26 +0200 Subject: 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 --- rust/kernel/kunit.rs | 1 - 1 file changed, 1 deletion(-) (limited to 'rust/kernel') 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); } -- cgit v1.2.3 From 950b306c296ec1e90d2d76f1974d2de2375a3d82 Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Fri, 2 May 2025 23:51:27 +0200 Subject: 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 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) (limited to 'rust/kernel') 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. -- cgit v1.2.3 From 897d1df6532f05814acd364af9055cd6628fd1b3 Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Fri, 2 May 2025 23:51:28 +0200 Subject: 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 +- 2 files changed, 2 insertions(+), 3 deletions(-) (limited to 'rust/kernel') 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 ::ffi::{ 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}; -- cgit v1.2.3 From 028df914e5466a02326829427bb8e26a31a05545 Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Fri, 2 May 2025 23:51:29 +0200 Subject: 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(-) (limited to 'rust/kernel') 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 crate::alloc::{flags::*, AllocError, KVec}; 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 @@ mod tests { } #[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] -- cgit v1.2.3 From 1486554392e242da5cbe95092d8dfec887bb8cca Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Mon, 26 May 2025 20:01:42 +0200 Subject: 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(-) (limited to 'rust/kernel') 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() }) } -- cgit v1.2.3 From 2d6c87d0d6a0c0acf6b4dd9eec9ed44a82886836 Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Fri, 2 May 2025 23:51:30 +0200 Subject: 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(-) (limited to 'rust/kernel') 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 @@ mod tests { \\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 @@ mod tests { 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 @@ mod tests { 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(()) } } -- cgit v1.2.3 From 74d6a606c2b39fd830d2d7a6363bc5688b4e4b56 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Wed, 9 Apr 2025 10:43:16 -0400 Subject: 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(-) (limited to 'rust/kernel') 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 @@ where 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 @@ impl Drop for RBTree { // 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 @@ impl<'a, K, V> Cursor<'a, K, V> { 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 @@ impl<'a, K, V> Cursor<'a, K, V> { 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 @@ impl<'a, K, V> Cursor<'a, K, V> { 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 @@ impl Iterator for IterRaw { // 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 @@ impl<'a, K, V> OccupiedEntry<'a, K, 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 @@ impl<'a, K, V> OccupiedEntry<'a, K, 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 @@ impl<'a, K, V> OccupiedEntry<'a, K, V> { 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 @@ impl<'a, K, V> OccupiedEntry<'a, K, V> { // 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 } } -- cgit v1.2.3 From 1ce98bb2bb30713ec4374ef11ead0d7d3e856766 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Fri, 11 Apr 2025 10:08:27 -0400 Subject: 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(-) (limited to 'rust/kernel') 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 @@ impl Work { /// /// # 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 @@ macro_rules! impl_has_work { ::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) } + } } )*}; } -- cgit v1.2.3 From b20fbbc08a363f28fd0f3ad5163b2b0ff7605ba5 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Thu, 29 May 2025 09:11:33 -0400 Subject: 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(-) (limited to 'rust/kernel') 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] -- cgit v1.2.3 From 7a17bbc1d952057898cb0739e60665908fbb8c72 Mon Sep 17 00:00:00 2001 From: Sylvan Smit Date: Thu, 29 May 2025 18:29:23 +0200 Subject: 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(-) (limited to 'rust/kernel') 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`]. /// -- cgit v1.2.3 From 4823a58093c6dfa20df62b5c18da613621b9716e Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Tue, 10 Jun 2025 16:38:56 +0530 Subject: cpufreq: Convert `/// SAFETY` lines to `# Safety` sections Replace `/// SAFETY` comments in doc comments with proper `# Safety` sections, as per rustdoc conventions. Also mark the C FFI callbacks as `unsafe` to correctly reflect their safety requirements. Reported-by: Miguel Ojeda Closes: https://github.com/Rust-for-Linux/linux/issues/1169 Signed-off-by: Viresh Kumar --- rust/kernel/cpufreq.rs | 146 ++++++++++++++++++++++++++++++++++++------------- 1 file changed, 109 insertions(+), 37 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/cpufreq.rs b/rust/kernel/cpufreq.rs index b0a9c6182aec..9b995f18aac6 100644 --- a/rust/kernel/cpufreq.rs +++ b/rust/kernel/cpufreq.rs @@ -1055,8 +1055,11 @@ impl Registration { impl Registration { /// Driver's `init` callback. /// - /// SAFETY: Called from C. Inputs must be valid pointers. - extern "C" fn init_callback(ptr: *mut bindings::cpufreq_policy) -> kernel::ffi::c_int { + /// # Safety + /// + /// - This function may only be called from the cpufreq C infrastructure. + /// - The pointer arguments must be valid pointers. + unsafe extern "C" fn init_callback(ptr: *mut bindings::cpufreq_policy) -> kernel::ffi::c_int { from_result(|| { // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the // lifetime of `policy`. @@ -1070,8 +1073,11 @@ impl Registration { /// Driver's `exit` callback. /// - /// SAFETY: Called from C. Inputs must be valid pointers. - extern "C" fn exit_callback(ptr: *mut bindings::cpufreq_policy) { + /// # Safety + /// + /// - This function may only be called from the cpufreq C infrastructure. + /// - The pointer arguments must be valid pointers. + unsafe extern "C" fn exit_callback(ptr: *mut bindings::cpufreq_policy) { // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the // lifetime of `policy`. let policy = unsafe { Policy::from_raw_mut(ptr) }; @@ -1082,8 +1088,11 @@ impl Registration { /// Driver's `online` callback. /// - /// SAFETY: Called from C. Inputs must be valid pointers. - extern "C" fn online_callback(ptr: *mut bindings::cpufreq_policy) -> kernel::ffi::c_int { + /// # Safety + /// + /// - This function may only be called from the cpufreq C infrastructure. + /// - The pointer arguments must be valid pointers. + unsafe extern "C" fn online_callback(ptr: *mut bindings::cpufreq_policy) -> kernel::ffi::c_int { from_result(|| { // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the // lifetime of `policy`. @@ -1094,8 +1103,13 @@ impl Registration { /// Driver's `offline` callback. /// - /// SAFETY: Called from C. Inputs must be valid pointers. - extern "C" fn offline_callback(ptr: *mut bindings::cpufreq_policy) -> kernel::ffi::c_int { + /// # Safety + /// + /// - This function may only be called from the cpufreq C infrastructure. + /// - The pointer arguments must be valid pointers. + unsafe extern "C" fn offline_callback( + ptr: *mut bindings::cpufreq_policy, + ) -> kernel::ffi::c_int { from_result(|| { // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the // lifetime of `policy`. @@ -1106,8 +1120,13 @@ impl Registration { /// Driver's `suspend` callback. /// - /// SAFETY: Called from C. Inputs must be valid pointers. - extern "C" fn suspend_callback(ptr: *mut bindings::cpufreq_policy) -> kernel::ffi::c_int { + /// # Safety + /// + /// - This function may only be called from the cpufreq C infrastructure. + /// - The pointer arguments must be valid pointers. + unsafe extern "C" fn suspend_callback( + ptr: *mut bindings::cpufreq_policy, + ) -> kernel::ffi::c_int { from_result(|| { // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the // lifetime of `policy`. @@ -1118,8 +1137,11 @@ impl Registration { /// Driver's `resume` callback. /// - /// SAFETY: Called from C. Inputs must be valid pointers. - extern "C" fn resume_callback(ptr: *mut bindings::cpufreq_policy) -> kernel::ffi::c_int { + /// # Safety + /// + /// - This function may only be called from the cpufreq C infrastructure. + /// - The pointer arguments must be valid pointers. + unsafe extern "C" fn resume_callback(ptr: *mut bindings::cpufreq_policy) -> kernel::ffi::c_int { from_result(|| { // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the // lifetime of `policy`. @@ -1130,8 +1152,11 @@ impl Registration { /// Driver's `ready` callback. /// - /// SAFETY: Called from C. Inputs must be valid pointers. - extern "C" fn ready_callback(ptr: *mut bindings::cpufreq_policy) { + /// # Safety + /// + /// - This function may only be called from the cpufreq C infrastructure. + /// - The pointer arguments must be valid pointers. + unsafe extern "C" fn ready_callback(ptr: *mut bindings::cpufreq_policy) { // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the // lifetime of `policy`. let policy = unsafe { Policy::from_raw_mut(ptr) }; @@ -1140,8 +1165,13 @@ impl Registration { /// Driver's `verify` callback. /// - /// SAFETY: Called from C. Inputs must be valid pointers. - extern "C" fn verify_callback(ptr: *mut bindings::cpufreq_policy_data) -> kernel::ffi::c_int { + /// # Safety + /// + /// - This function may only be called from the cpufreq C infrastructure. + /// - The pointer arguments must be valid pointers. + unsafe extern "C" fn verify_callback( + ptr: *mut bindings::cpufreq_policy_data, + ) -> kernel::ffi::c_int { from_result(|| { // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the // lifetime of `policy`. @@ -1152,8 +1182,13 @@ impl Registration { /// Driver's `setpolicy` callback. /// - /// SAFETY: Called from C. Inputs must be valid pointers. - extern "C" fn setpolicy_callback(ptr: *mut bindings::cpufreq_policy) -> kernel::ffi::c_int { + /// # Safety + /// + /// - This function may only be called from the cpufreq C infrastructure. + /// - The pointer arguments must be valid pointers. + unsafe extern "C" fn setpolicy_callback( + ptr: *mut bindings::cpufreq_policy, + ) -> kernel::ffi::c_int { from_result(|| { // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the // lifetime of `policy`. @@ -1164,8 +1199,11 @@ impl Registration { /// Driver's `target` callback. /// - /// SAFETY: Called from C. Inputs must be valid pointers. - extern "C" fn target_callback( + /// # Safety + /// + /// - This function may only be called from the cpufreq C infrastructure. + /// - The pointer arguments must be valid pointers. + unsafe extern "C" fn target_callback( ptr: *mut bindings::cpufreq_policy, target_freq: u32, relation: u32, @@ -1180,8 +1218,11 @@ impl Registration { /// Driver's `target_index` callback. /// - /// SAFETY: Called from C. Inputs must be valid pointers. - extern "C" fn target_index_callback( + /// # Safety + /// + /// - This function may only be called from the cpufreq C infrastructure. + /// - The pointer arguments must be valid pointers. + unsafe extern "C" fn target_index_callback( ptr: *mut bindings::cpufreq_policy, index: u32, ) -> kernel::ffi::c_int { @@ -1200,8 +1241,11 @@ impl Registration { /// Driver's `fast_switch` callback. /// - /// SAFETY: Called from C. Inputs must be valid pointers. - extern "C" fn fast_switch_callback( + /// # Safety + /// + /// - This function may only be called from the cpufreq C infrastructure. + /// - The pointer arguments must be valid pointers. + unsafe extern "C" fn fast_switch_callback( ptr: *mut bindings::cpufreq_policy, target_freq: u32, ) -> kernel::ffi::c_uint { @@ -1212,7 +1256,11 @@ impl Registration { } /// Driver's `adjust_perf` callback. - extern "C" fn adjust_perf_callback( + /// + /// # Safety + /// + /// - This function may only be called from the cpufreq C infrastructure. + unsafe extern "C" fn adjust_perf_callback( cpu: u32, min_perf: usize, target_perf: usize, @@ -1225,8 +1273,11 @@ impl Registration { /// Driver's `get_intermediate` callback. /// - /// SAFETY: Called from C. Inputs must be valid pointers. - extern "C" fn get_intermediate_callback( + /// # Safety + /// + /// - This function may only be called from the cpufreq C infrastructure. + /// - The pointer arguments must be valid pointers. + unsafe extern "C" fn get_intermediate_callback( ptr: *mut bindings::cpufreq_policy, index: u32, ) -> kernel::ffi::c_uint { @@ -1243,8 +1294,11 @@ impl Registration { /// Driver's `target_intermediate` callback. /// - /// SAFETY: Called from C. Inputs must be valid pointers. - extern "C" fn target_intermediate_callback( + /// # Safety + /// + /// - This function may only be called from the cpufreq C infrastructure. + /// - The pointer arguments must be valid pointers. + unsafe extern "C" fn target_intermediate_callback( ptr: *mut bindings::cpufreq_policy, index: u32, ) -> kernel::ffi::c_int { @@ -1262,12 +1316,21 @@ impl Registration { } /// Driver's `get` callback. - extern "C" fn get_callback(cpu: u32) -> kernel::ffi::c_uint { + /// + /// # Safety + /// + /// - This function may only be called from the cpufreq C infrastructure. + unsafe extern "C" fn get_callback(cpu: u32) -> kernel::ffi::c_uint { PolicyCpu::from_cpu(cpu).map_or(0, |mut policy| T::get(&mut policy).map_or(0, |f| f)) } /// Driver's `update_limit` callback. - extern "C" fn update_limits_callback(ptr: *mut bindings::cpufreq_policy) { + /// + /// # Safety + /// + /// - This function may only be called from the cpufreq C infrastructure. + /// - The pointer arguments must be valid pointers. + unsafe extern "C" fn update_limits_callback(ptr: *mut bindings::cpufreq_policy) { // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the // lifetime of `policy`. let policy = unsafe { Policy::from_raw_mut(ptr) }; @@ -1276,8 +1339,11 @@ impl Registration { /// Driver's `bios_limit` callback. /// - /// SAFETY: Called from C. Inputs must be valid pointers. - extern "C" fn bios_limit_callback(cpu: i32, limit: *mut u32) -> kernel::ffi::c_int { + /// # Safety + /// + /// - This function may only be called from the cpufreq C infrastructure. + /// - The pointer arguments must be valid pointers. + unsafe extern "C" fn bios_limit_callback(cpu: i32, limit: *mut u32) -> kernel::ffi::c_int { from_result(|| { let mut policy = PolicyCpu::from_cpu(cpu as u32)?; @@ -1288,8 +1354,11 @@ impl Registration { /// Driver's `set_boost` callback. /// - /// SAFETY: Called from C. Inputs must be valid pointers. - extern "C" fn set_boost_callback( + /// # Safety + /// + /// - This function may only be called from the cpufreq C infrastructure. + /// - The pointer arguments must be valid pointers. + unsafe extern "C" fn set_boost_callback( ptr: *mut bindings::cpufreq_policy, state: i32, ) -> kernel::ffi::c_int { @@ -1303,8 +1372,11 @@ impl Registration { /// Driver's `register_em` callback. /// - /// SAFETY: Called from C. Inputs must be valid pointers. - extern "C" fn register_em_callback(ptr: *mut bindings::cpufreq_policy) { + /// # Safety + /// + /// - This function may only be called from the cpufreq C infrastructure. + /// - The pointer arguments must be valid pointers. + unsafe extern "C" fn register_em_callback(ptr: *mut bindings::cpufreq_policy) { // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the // lifetime of `policy`. let policy = unsafe { Policy::from_raw_mut(ptr) }; -- cgit v1.2.3 From 5b2d595efbfc9c46823bdb9ef11e1f9fa46adf9d Mon Sep 17 00:00:00 2001 From: FUJITA Tomonori Date: Fri, 6 Jun 2025 11:05:05 +0900 Subject: rust: time: Fix compile error in impl_has_hr_timer macro Fix a compile error in the `impl_has_hr_timer!` macro as follows: error[E0599]: no method named cast_mut found for raw pointer *mut Foo in the current scope The `container_of!` macro already returns a mutable pointer when used in a `*mut T` context so the `.cast_mut()` method is not available. [ We missed this one because there is no caller yet and it is a macro. - Miguel ] Fixes: 74d6a606c2b3 ("rust: retain pointer mut-ness in `container_of!`") Signed-off-by: FUJITA Tomonori Reviewed-by: Benno Lossin Acked-by: Andreas Hindborg Link: https://lore.kernel.org/r/20250606020505.3186533-1-fujita.tomonori@gmail.com Signed-off-by: Miguel Ojeda --- rust/kernel/time/hrtimer.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'rust/kernel') diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs index 9df3dcd2fa39..36e1290cd079 100644 --- a/rust/kernel/time/hrtimer.rs +++ b/rust/kernel/time/hrtimer.rs @@ -517,7 +517,7 @@ macro_rules! impl_has_hr_timer { ) -> *mut Self { // SAFETY: As per the safety requirement of this function, `ptr` // is pointing inside a `$timer_type`. - unsafe { ::kernel::container_of!(ptr, $timer_type, $field).cast_mut() } + unsafe { ::kernel::container_of!(ptr, $timer_type, $field) } } } } -- cgit v1.2.3 From ebf2e500e06f707654572bc7d8bc569a8caa51aa Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Mon, 9 Jun 2025 12:38:43 +0530 Subject: rust: cpu: Introduce CpuId abstraction This adds abstraction for representing a CPU identifier. Suggested-by: Boqun Feng Signed-off-by: Viresh Kumar Reviewed-by: Boqun Feng --- rust/kernel/cpu.rs | 110 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+) (limited to 'rust/kernel') diff --git a/rust/kernel/cpu.rs b/rust/kernel/cpu.rs index 10c5c3b25873..6a3aecb12468 100644 --- a/rust/kernel/cpu.rs +++ b/rust/kernel/cpu.rs @@ -6,6 +6,116 @@ use crate::{bindings, device::Device, error::Result, prelude::ENODEV}; +/// Returns the maximum number of possible CPUs in the current system configuration. +#[inline] +pub fn nr_cpu_ids() -> u32 { + #[cfg(any(NR_CPUS_1, CONFIG_FORCE_NR_CPUS))] + { + bindings::NR_CPUS + } + + #[cfg(not(any(NR_CPUS_1, CONFIG_FORCE_NR_CPUS)))] + // SAFETY: `nr_cpu_ids` is a valid global provided by the kernel. + unsafe { + bindings::nr_cpu_ids + } +} + +/// The CPU ID. +/// +/// Represents a CPU identifier as a wrapper around an [`u32`]. +/// +/// # Invariants +/// +/// The CPU ID lies within the range `[0, nr_cpu_ids())`. +/// +/// # Examples +/// +/// ``` +/// use kernel::cpu::CpuId; +/// +/// let cpu = 0; +/// +/// // SAFETY: 0 is always a valid CPU number. +/// let id = unsafe { CpuId::from_u32_unchecked(cpu) }; +/// +/// assert_eq!(id.as_u32(), cpu); +/// assert!(CpuId::from_i32(0).is_some()); +/// assert!(CpuId::from_i32(-1).is_none()); +/// ``` +#[derive(Copy, Clone, PartialEq, Eq, Debug)] +pub struct CpuId(u32); + +impl CpuId { + /// Creates a new [`CpuId`] from the given `id` without checking bounds. + /// + /// # Safety + /// + /// The caller must ensure that `id` is a valid CPU ID (i.e., `0 <= id < nr_cpu_ids()`). + #[inline] + pub unsafe fn from_i32_unchecked(id: i32) -> Self { + debug_assert!(id >= 0); + debug_assert!((id as u32) < nr_cpu_ids()); + + // INVARIANT: The function safety guarantees `id` is a valid CPU id. + Self(id as u32) + } + + /// Creates a new [`CpuId`] from the given `id`, checking that it is valid. + pub fn from_i32(id: i32) -> Option { + if id < 0 || id as u32 >= nr_cpu_ids() { + None + } else { + // INVARIANT: `id` has just been checked as a valid CPU ID. + Some(Self(id as u32)) + } + } + + /// Creates a new [`CpuId`] from the given `id` without checking bounds. + /// + /// # Safety + /// + /// The caller must ensure that `id` is a valid CPU ID (i.e., `0 <= id < nr_cpu_ids()`). + #[inline] + pub unsafe fn from_u32_unchecked(id: u32) -> Self { + debug_assert!(id < nr_cpu_ids()); + + // Ensure the `id` fits in an [`i32`] as it's also representable that way. + debug_assert!(id <= i32::MAX as u32); + + // INVARIANT: The function safety guarantees `id` is a valid CPU id. + Self(id) + } + + /// Creates a new [`CpuId`] from the given `id`, checking that it is valid. + pub fn from_u32(id: u32) -> Option { + if id >= nr_cpu_ids() { + None + } else { + // INVARIANT: `id` has just been checked as a valid CPU ID. + Some(Self(id)) + } + } + + /// Returns CPU number. + #[inline] + pub fn as_u32(&self) -> u32 { + self.0 + } +} + +impl From for u32 { + fn from(id: CpuId) -> Self { + id.as_u32() + } +} + +impl From for i32 { + fn from(id: CpuId) -> Self { + id.as_u32() as i32 + } +} + /// Creates a new instance of CPU's device. /// /// # Safety -- cgit v1.2.3 From 33db8c97b4cfa0328054fb755dfbcd6e7f3c7a9d Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Mon, 9 Jun 2025 14:26:54 +0530 Subject: rust: Use CpuId in place of raw CPU numbers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use the newly defined `CpuId` abstraction instead of raw CPU numbers. This also fixes a doctest failure for configurations where `nr_cpu_ids < 4`. The C `cpumask_{set|clear}_cpu()` APIs emit a warning when given an invalid CPU number — but only if `CONFIG_DEBUG_PER_CPU_MAPS=y` is set. Meanwhile, `cpumask_weight()` only considers CPUs up to `nr_cpu_ids`, which can cause inconsistencies: a CPU number greater than `nr_cpu_ids` may be set in the mask, yet the weight calculation won't reflect it. This leads to doctest failures when `nr_cpu_ids < 4`, as the test tries to set CPUs 2 and 3: rust_doctest_kernel_cpumask_rs_0.location: rust/kernel/cpumask.rs:180 rust_doctest_kernel_cpumask_rs_0: ASSERTION FAILED at rust/kernel/cpumask.rs:190 Fixes: 8961b8cb3099 ("rust: cpumask: Add initial abstractions") Reported-by: Miguel Ojeda Closes: https://lore.kernel.org/rust-for-linux/CANiq72k3ozKkLMinTLQwvkyg9K=BeRxs1oYZSKhJHY-veEyZdg@mail.gmail.com/ Reported-by: Andreas Hindborg Closes: https://lore.kernel.org/all/87qzzy3ric.fsf@kernel.org/ Suggested-by: Boqun Feng Signed-off-by: Viresh Kumar Reviewed-by: Boqun Feng --- rust/kernel/cpu.rs | 4 ++-- rust/kernel/cpufreq.rs | 27 ++++++++++++++++++-------- rust/kernel/cpumask.rs | 51 +++++++++++++++++++++++++++++++++++--------------- 3 files changed, 57 insertions(+), 25 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/cpu.rs b/rust/kernel/cpu.rs index 6a3aecb12468..abc780d7a8ec 100644 --- a/rust/kernel/cpu.rs +++ b/rust/kernel/cpu.rs @@ -127,9 +127,9 @@ impl From for i32 { /// Callers must ensure that the CPU device is not used after it has been unregistered. /// This can be achieved, for example, by registering a CPU hotplug notifier and removing /// any references to the CPU device within the notifier's callback. -pub unsafe fn from_cpu(cpu: u32) -> Result<&'static Device> { +pub unsafe fn from_cpu(cpu: CpuId) -> Result<&'static Device> { // SAFETY: It is safe to call `get_cpu_device()` for any CPU. - let ptr = unsafe { bindings::get_cpu_device(cpu) }; + let ptr = unsafe { bindings::get_cpu_device(u32::from(cpu)) }; if ptr.is_null() { return Err(ENODEV); } diff --git a/rust/kernel/cpufreq.rs b/rust/kernel/cpufreq.rs index 9b995f18aac6..11b03e9d7e89 100644 --- a/rust/kernel/cpufreq.rs +++ b/rust/kernel/cpufreq.rs @@ -10,6 +10,7 @@ use crate::{ clk::Hertz, + cpu::CpuId, cpumask, device::{Bound, Device}, devres::Devres, @@ -465,8 +466,9 @@ impl Policy { /// Returns the primary CPU for the [`Policy`]. #[inline] - pub fn cpu(&self) -> u32 { - self.as_ref().cpu + pub fn cpu(&self) -> CpuId { + // SAFETY: The C API guarantees that `cpu` refers to a valid CPU number. + unsafe { CpuId::from_u32_unchecked(self.as_ref().cpu) } } /// Returns the minimum frequency for the [`Policy`]. @@ -525,7 +527,7 @@ impl Policy { #[inline] pub fn generic_get(&self) -> Result { // SAFETY: By the type invariant, the pointer stored in `self` is valid. - Ok(unsafe { bindings::cpufreq_generic_get(self.cpu()) }) + Ok(unsafe { bindings::cpufreq_generic_get(u32::from(self.cpu())) }) } /// Provides a wrapper to the register with energy model using the OPP core. @@ -678,9 +680,9 @@ impl Policy { struct PolicyCpu<'a>(&'a mut Policy); impl<'a> PolicyCpu<'a> { - fn from_cpu(cpu: u32) -> Result { + fn from_cpu(cpu: CpuId) -> Result { // SAFETY: It is safe to call `cpufreq_cpu_get` for any valid CPU. - let ptr = from_err_ptr(unsafe { bindings::cpufreq_cpu_get(cpu) })?; + let ptr = from_err_ptr(unsafe { bindings::cpufreq_cpu_get(u32::from(cpu)) })?; Ok(Self( // SAFETY: The `ptr` is guaranteed to be valid and remains valid for the lifetime of @@ -1266,7 +1268,10 @@ impl Registration { target_perf: usize, capacity: usize, ) { - if let Ok(mut policy) = PolicyCpu::from_cpu(cpu) { + // SAFETY: The C API guarantees that `cpu` refers to a valid CPU number. + let cpu_id = unsafe { CpuId::from_u32_unchecked(cpu) }; + + if let Ok(mut policy) = PolicyCpu::from_cpu(cpu_id) { T::adjust_perf(&mut policy, min_perf, target_perf, capacity); } } @@ -1321,7 +1326,10 @@ impl Registration { /// /// - This function may only be called from the cpufreq C infrastructure. unsafe extern "C" fn get_callback(cpu: u32) -> kernel::ffi::c_uint { - PolicyCpu::from_cpu(cpu).map_or(0, |mut policy| T::get(&mut policy).map_or(0, |f| f)) + // SAFETY: The C API guarantees that `cpu` refers to a valid CPU number. + let cpu_id = unsafe { CpuId::from_u32_unchecked(cpu) }; + + PolicyCpu::from_cpu(cpu_id).map_or(0, |mut policy| T::get(&mut policy).map_or(0, |f| f)) } /// Driver's `update_limit` callback. @@ -1344,8 +1352,11 @@ impl Registration { /// - This function may only be called from the cpufreq C infrastructure. /// - The pointer arguments must be valid pointers. unsafe extern "C" fn bios_limit_callback(cpu: i32, limit: *mut u32) -> kernel::ffi::c_int { + // SAFETY: The C API guarantees that `cpu` refers to a valid CPU number. + let cpu_id = unsafe { CpuId::from_i32_unchecked(cpu) }; + from_result(|| { - let mut policy = PolicyCpu::from_cpu(cpu as u32)?; + let mut policy = PolicyCpu::from_cpu(cpu_id)?; // SAFETY: `limit` is guaranteed by the C code to be valid. T::bios_limit(&mut policy, &mut (unsafe { *limit })).map(|()| 0) diff --git a/rust/kernel/cpumask.rs b/rust/kernel/cpumask.rs index c90bfac9346a..19c607709b5f 100644 --- a/rust/kernel/cpumask.rs +++ b/rust/kernel/cpumask.rs @@ -6,6 +6,7 @@ use crate::{ alloc::{AllocError, Flags}, + cpu::CpuId, prelude::*, types::Opaque, }; @@ -35,9 +36,10 @@ use core::ops::{Deref, DerefMut}; /// /// ``` /// use kernel::bindings; +/// use kernel::cpu::CpuId; /// use kernel::cpumask::Cpumask; /// -/// fn set_clear_cpu(ptr: *mut bindings::cpumask, set_cpu: u32, clear_cpu: i32) { +/// fn set_clear_cpu(ptr: *mut bindings::cpumask, set_cpu: CpuId, clear_cpu: CpuId) { /// // SAFETY: The `ptr` is valid for writing and remains valid for the lifetime of the /// // returned reference. /// let mask = unsafe { Cpumask::as_mut_ref(ptr) }; @@ -90,9 +92,9 @@ impl Cpumask { /// This mismatches kernel naming convention and corresponds to the C /// function `__cpumask_set_cpu()`. #[inline] - pub fn set(&mut self, cpu: u32) { + pub fn set(&mut self, cpu: CpuId) { // SAFETY: By the type invariant, `self.as_raw` is a valid argument to `__cpumask_set_cpu`. - unsafe { bindings::__cpumask_set_cpu(cpu, self.as_raw()) }; + unsafe { bindings::__cpumask_set_cpu(u32::from(cpu), self.as_raw()) }; } /// Clear `cpu` in the cpumask. @@ -101,19 +103,19 @@ impl Cpumask { /// This mismatches kernel naming convention and corresponds to the C /// function `__cpumask_clear_cpu()`. #[inline] - pub fn clear(&mut self, cpu: i32) { + pub fn clear(&mut self, cpu: CpuId) { // SAFETY: By the type invariant, `self.as_raw` is a valid argument to // `__cpumask_clear_cpu`. - unsafe { bindings::__cpumask_clear_cpu(cpu, self.as_raw()) }; + unsafe { bindings::__cpumask_clear_cpu(i32::from(cpu), self.as_raw()) }; } /// Test `cpu` in the cpumask. /// /// Equivalent to the kernel's `cpumask_test_cpu` API. #[inline] - pub fn test(&self, cpu: i32) -> bool { + pub fn test(&self, cpu: CpuId) -> bool { // SAFETY: By the type invariant, `self.as_raw` is a valid argument to `cpumask_test_cpu`. - unsafe { bindings::cpumask_test_cpu(cpu, self.as_raw()) } + unsafe { bindings::cpumask_test_cpu(i32::from(cpu), self.as_raw()) } } /// Set all CPUs in the cpumask. @@ -178,21 +180,40 @@ impl Cpumask { /// The following example demonstrates how to create and update a [`CpumaskVar`]. /// /// ``` +/// use kernel::cpu::CpuId; /// use kernel::cpumask::CpumaskVar; /// /// let mut mask = CpumaskVar::new_zero(GFP_KERNEL).unwrap(); /// /// assert!(mask.empty()); -/// mask.set(2); -/// assert!(mask.test(2)); -/// mask.set(3); -/// assert!(mask.test(3)); -/// assert_eq!(mask.weight(), 2); +/// let mut count = 0; +/// +/// let cpu2 = CpuId::from_u32(2); +/// if let Some(cpu) = cpu2 { +/// mask.set(cpu); +/// assert!(mask.test(cpu)); +/// count += 1; +/// } +/// +/// let cpu3 = CpuId::from_u32(3); +/// if let Some(cpu) = cpu3 { +/// mask.set(cpu); +/// assert!(mask.test(cpu)); +/// count += 1; +/// } +/// +/// assert_eq!(mask.weight(), count); /// /// let mask2 = CpumaskVar::try_clone(&mask).unwrap(); -/// assert!(mask2.test(2)); -/// assert!(mask2.test(3)); -/// assert_eq!(mask2.weight(), 2); +/// +/// if let Some(cpu) = cpu2 { +/// assert!(mask2.test(cpu)); +/// } +/// +/// if let Some(cpu) = cpu3 { +/// assert!(mask2.test(cpu)); +/// } +/// assert_eq!(mask2.weight(), count); /// ``` pub struct CpumaskVar { #[cfg(CONFIG_CPUMASK_OFFSTACK)] -- cgit v1.2.3 From c7f005f70d22cd5613cac30bf6d34867189e36a9 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Mon, 9 Jun 2025 16:44:16 +0530 Subject: rust: cpu: Add CpuId::current() to retrieve current CPU ID Introduce `CpuId::current()`, a constructor that wraps the C function `raw_smp_processor_id()` to retrieve the current CPU identifier without guaranteeing stability. This function should be used only when the caller can ensure that the CPU ID won't change unexpectedly due to preemption or migration. Suggested-by: Boqun Feng Signed-off-by: Viresh Kumar Reviewed-by: Boqun Feng --- rust/kernel/cpu.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'rust/kernel') diff --git a/rust/kernel/cpu.rs b/rust/kernel/cpu.rs index abc780d7a8ec..b75403b0eb56 100644 --- a/rust/kernel/cpu.rs +++ b/rust/kernel/cpu.rs @@ -102,6 +102,17 @@ impl CpuId { pub fn as_u32(&self) -> u32 { self.0 } + + /// Returns the ID of the CPU the code is currently running on. + /// + /// The returned value is considered unstable because it may change + /// unexpectedly due to preemption or CPU migration. It should only be + /// used when the context ensures that the task remains on the same CPU + /// or the users could use a stale (yet valid) CPU ID. + pub fn current() -> Self { + // SAFETY: raw_smp_processor_id() always returns a valid CPU ID. + unsafe { Self::from_u32_unchecked(bindings::raw_smp_processor_id()) } + } } impl From for u32 { -- cgit v1.2.3 From 1b56e765bf8990f1f60e124926c11fc4ac63d752 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Thu, 12 Jun 2025 14:17:13 +0200 Subject: rust: completion: implement initial abstraction Implement a minimal abstraction for the completion synchronization primitive. This initial abstraction only adds complete_all() and wait_for_completion(), since that is what is required for the subsequent Devres patch. Cc: Ingo Molnar Cc: Peter Zijlstra Cc: Juri Lelli Cc: Vincent Guittot Cc: Dietmar Eggemann Cc: Steven Rostedt Cc: Ben Segall Cc: Mel Gorman Cc: Valentin Schneider Reviewed-by: Alice Ryhl Reviewed-by: Boqun Feng Reviewed-by: Benno Lossin Acked-by: Miguel Ojeda Link: https://lore.kernel.org/r/20250612121817.1621-2-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/sync.rs | 2 + rust/kernel/sync/completion.rs | 112 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 114 insertions(+) create mode 100644 rust/kernel/sync/completion.rs (limited to 'rust/kernel') diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs index 36a719015583..c23a12639924 100644 --- a/rust/kernel/sync.rs +++ b/rust/kernel/sync.rs @@ -10,6 +10,7 @@ use crate::types::Opaque; use pin_init; mod arc; +pub mod completion; mod condvar; pub mod lock; mod locked_by; @@ -17,6 +18,7 @@ pub mod poll; pub mod rcu; pub use arc::{Arc, ArcBorrow, UniqueArc}; +pub use completion::Completion; pub use condvar::{new_condvar, CondVar, CondVarTimeoutResult}; pub use lock::global::{global_lock, GlobalGuard, GlobalLock, GlobalLockBackend, GlobalLockedBy}; pub use lock::mutex::{new_mutex, Mutex, MutexGuard}; diff --git a/rust/kernel/sync/completion.rs b/rust/kernel/sync/completion.rs new file mode 100644 index 000000000000..c50012a940a3 --- /dev/null +++ b/rust/kernel/sync/completion.rs @@ -0,0 +1,112 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! Completion support. +//! +//! Reference: +//! +//! C header: [`include/linux/completion.h`](srctree/include/linux/completion.h) + +use crate::{bindings, prelude::*, types::Opaque}; + +/// Synchronization primitive to signal when a certain task has been completed. +/// +/// The [`Completion`] synchronization primitive signals when a certain task has been completed by +/// waking up other tasks that have been queued up to wait for the [`Completion`] to be completed. +/// +/// # Examples +/// +/// ``` +/// use kernel::sync::{Arc, Completion}; +/// use kernel::workqueue::{self, impl_has_work, new_work, Work, WorkItem}; +/// +/// #[pin_data] +/// struct MyTask { +/// #[pin] +/// work: Work, +/// #[pin] +/// done: Completion, +/// } +/// +/// impl_has_work! { +/// impl HasWork for MyTask { self.work } +/// } +/// +/// impl MyTask { +/// fn new() -> Result> { +/// let this = Arc::pin_init(pin_init!(MyTask { +/// work <- new_work!("MyTask::work"), +/// done <- Completion::new(), +/// }), GFP_KERNEL)?; +/// +/// let _ = workqueue::system().enqueue(this.clone()); +/// +/// Ok(this) +/// } +/// +/// fn wait_for_completion(&self) { +/// self.done.wait_for_completion(); +/// +/// pr_info!("Completion: task complete\n"); +/// } +/// } +/// +/// impl WorkItem for MyTask { +/// type Pointer = Arc; +/// +/// fn run(this: Arc) { +/// // process this task +/// this.done.complete_all(); +/// } +/// } +/// +/// let task = MyTask::new()?; +/// task.wait_for_completion(); +/// # Ok::<(), Error>(()) +/// ``` +#[pin_data] +pub struct Completion { + #[pin] + inner: Opaque, +} + +// SAFETY: `Completion` is safe to be send to any task. +unsafe impl Send for Completion {} + +// SAFETY: `Completion` is safe to be accessed concurrently. +unsafe impl Sync for Completion {} + +impl Completion { + /// Create an initializer for a new [`Completion`]. + pub fn new() -> impl PinInit { + pin_init!(Self { + inner <- Opaque::ffi_init(|slot: *mut bindings::completion| { + // SAFETY: `slot` is a valid pointer to an uninitialized `struct completion`. + unsafe { bindings::init_completion(slot) }; + }), + }) + } + + fn as_raw(&self) -> *mut bindings::completion { + self.inner.get() + } + + /// Signal all tasks waiting on this completion. + /// + /// This method wakes up all tasks waiting on this completion; after this operation the + /// completion is permanently done, i.e. signals all current and future waiters. + pub fn complete_all(&self) { + // SAFETY: `self.as_raw()` is a pointer to a valid `struct completion`. + unsafe { bindings::complete_all(self.as_raw()) }; + } + + /// Wait for completion of a task. + /// + /// This method waits for the completion of a task; it is not interruptible and there is no + /// timeout. + /// + /// See also [`Completion::complete_all`]. + pub fn wait_for_completion(&self) { + // SAFETY: `self.as_raw()` is a pointer to a valid `struct completion`. + unsafe { bindings::wait_for_completion(self.as_raw()) }; + } +} -- cgit v1.2.3 From 4b76fafb20dd4a2becb94949d78e86bc88006509 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Thu, 12 Jun 2025 14:17:14 +0200 Subject: rust: revocable: indicate whether `data` has been revoked already Return a boolean from Revocable::revoke() and Revocable::revoke_nosync() to indicate whether the data has been revoked already. Return true if the data hasn't been revoked yet (i.e. this call revoked the data), false otherwise. This is required by Devres in order to synchronize the completion of the revoke process. Reviewed-by: Benno Lossin Acked-by: Miguel Ojeda Link: https://lore.kernel.org/r/20250612121817.1621-3-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/revocable.rs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/revocable.rs b/rust/kernel/revocable.rs index db4aa46bb121..06a3cdfce344 100644 --- a/rust/kernel/revocable.rs +++ b/rust/kernel/revocable.rs @@ -154,8 +154,10 @@ impl Revocable { /// # Safety /// /// Callers must ensure that there are no more concurrent users of the revocable object. - unsafe fn revoke_internal(&self) { - if self.is_available.swap(false, Ordering::Relaxed) { + unsafe fn revoke_internal(&self) -> bool { + let revoke = self.is_available.swap(false, Ordering::Relaxed); + + if revoke { if SYNC { // SAFETY: Just an FFI call, there are no further requirements. unsafe { bindings::synchronize_rcu() }; @@ -165,6 +167,8 @@ impl Revocable { // `compare_exchange` above that takes `is_available` from `true` to `false`. unsafe { drop_in_place(self.data.get()) }; } + + revoke } /// Revokes access to and drops the wrapped object. @@ -172,10 +176,13 @@ impl Revocable { /// Access to the object is revoked immediately to new callers of [`Revocable::try_access`], /// expecting that there are no concurrent users of the object. /// + /// Returns `true` if `&self` has been revoked with this call, `false` if it was revoked + /// already. + /// /// # Safety /// /// Callers must ensure that there are no more concurrent users of the revocable object. - pub unsafe fn revoke_nosync(&self) { + pub unsafe fn revoke_nosync(&self) -> bool { // SAFETY: By the safety requirement of this function, the caller ensures that nobody is // accessing the data anymore and hence we don't have to wait for the grace period to // finish. @@ -189,7 +196,10 @@ impl Revocable { /// If there are concurrent users of the object (i.e., ones that called /// [`Revocable::try_access`] beforehand and still haven't dropped the returned guard), this /// function waits for the concurrent access to complete before dropping the wrapped object. - pub fn revoke(&self) { + /// + /// Returns `true` if `&self` has been revoked with this call, `false` if it was revoked + /// already. + pub fn revoke(&self) -> bool { // SAFETY: By passing `true` we ask `revoke_internal` to wait for the grace period to // finish. unsafe { self.revoke_internal::() } -- cgit v1.2.3 From f744201c6159fc7323c40936fd079525f7063598 Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Thu, 12 Jun 2025 14:17:15 +0200 Subject: rust: devres: fix race in Devres::drop() In Devres::drop() we first remove the devres action and then drop the wrapped device resource. The design goal is to give the owner of a Devres object control over when the device resource is dropped, but limit the overall scope to the corresponding device being bound to a driver. However, there's a race that was introduced with commit 8ff656643d30 ("rust: devres: remove action in `Devres::drop`"), but also has been (partially) present from the initial version on. In Devres::drop(), the devres action is removed successfully and subsequently the destructor of the wrapped device resource runs. However, there is no guarantee that the destructor of the wrapped device resource completes before the driver core is done unbinding the corresponding device. If in Devres::drop(), the devres action can't be removed, it means that the devres callback has been executed already, or is still running concurrently. In case of the latter, either Devres::drop() wins revoking the Revocable or the devres callback wins revoking the Revocable. If Devres::drop() wins, we (again) have no guarantee that the destructor of the wrapped device resource completes before the driver core is done unbinding the corresponding device. CPU0 CPU1 ------------------------------------------------------------------------ Devres::drop() { Devres::devres_callback() { self.data.revoke() { this.data.revoke() { is_available.swap() == true is_available.swap == false } } // [...] // device fully unbound drop_in_place() { // release device resource } } } Depending on the specific device resource, this can potentially lead to user-after-free bugs. In order to fix this, implement the following logic. In the devres callback, we're always good when we get to revoke the device resource ourselves, i.e. Revocable::revoke() returns true. If Revocable::revoke() returns false, it means that Devres::drop(), concurrently, already drops the device resource and we have to wait for Devres::drop() to signal that it finished dropping the device resource. Note that if we hit the case where we need to wait for the completion of Devres::drop() in the devres callback, it means that we're actually racing with a concurrent Devres::drop() call, which already started revoking the device resource for us. This is rather unlikely and means that the concurrent Devres::drop() already started doing our work and we just need to wait for it to complete it for us. Hence, there should not be any additional overhead from that. (Actually, for now it's even better if Devres::drop() does the work for us, since it can bypass the synchronize_rcu() call implied by Revocable::revoke(), but this goes away anyways once I get to implement the split devres callback approach, which allows us to first flip the atomics of all registered Devres objects of a certain device, execute a single synchronize_rcu() and then drop all revocable objects.) In Devres::drop() we try to revoke the device resource. If that is *not* successful, it means that the devres callback already did and we're good. Otherwise, we try to remove the devres action, which, if successful, means that we're good, since the device resource has just been revoked by us *before* we removed the devres action successfully. If the devres action could not be removed, it means that the devres callback must be running concurrently, hence we signal that the device resource has been revoked by us, using the completion. This makes it safe to drop a Devres object from any task and at any point of time, which is one of the design goals. Fixes: 76c01ded724b ("rust: add devres abstraction") Reported-by: Alice Ryhl Closes: https://lore.kernel.org/lkml/aD64YNuqbPPZHAa5@google.com/ Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20250612121817.1621-4-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/devres.rs | 37 +++++++++++++++++++++++++++++-------- 1 file changed, 29 insertions(+), 8 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs index 0f79a2ec9474..2f74bce5ed9d 100644 --- a/rust/kernel/devres.rs +++ b/rust/kernel/devres.rs @@ -13,7 +13,7 @@ use crate::{ ffi::c_void, prelude::*, revocable::Revocable, - sync::Arc, + sync::{Arc, Completion}, types::ARef, }; @@ -25,13 +25,17 @@ struct DevresInner { callback: unsafe extern "C" fn(*mut c_void), #[pin] data: Revocable, + #[pin] + revoke: Completion, } /// This abstraction is meant to be used by subsystems to containerize [`Device`] bound resources to /// manage their lifetime. /// /// [`Device`] bound resources should be freed when either the resource goes out of scope or the -/// [`Device`] is unbound respectively, depending on what happens first. +/// [`Device`] is unbound respectively, depending on what happens first. In any case, it is always +/// guaranteed that revoking the device resource is completed before the corresponding [`Device`] +/// is unbound. /// /// To achieve that [`Devres`] registers a devres callback on creation, which is called once the /// [`Device`] is unbound, revoking access to the encapsulated resource (see also [`Revocable`]). @@ -102,6 +106,7 @@ impl DevresInner { dev: dev.into(), callback: Self::devres_callback, data <- Revocable::new(data), + revoke <- Completion::new(), }), flags, )?; @@ -130,26 +135,28 @@ impl DevresInner { self as _ } - fn remove_action(this: &Arc) { + fn remove_action(this: &Arc) -> bool { // SAFETY: // - `self.inner.dev` is a valid `Device`, // - the `action` and `data` pointers are the exact same ones as given to devm_add_action() // previously, // - `self` is always valid, even if the action has been released already. - let ret = unsafe { + let success = unsafe { bindings::devm_remove_action_nowarn( this.dev.as_raw(), Some(this.callback), this.as_ptr() as _, ) - }; + } == 0; - if ret == 0 { + if success { // SAFETY: We leaked an `Arc` reference to devm_add_action() in `DevresInner::new`; if // devm_remove_action_nowarn() was successful we can (and have to) claim back ownership // of this reference. let _ = unsafe { Arc::from_raw(this.as_ptr()) }; } + + success } #[allow(clippy::missing_safety_doc)] @@ -161,7 +168,12 @@ impl DevresInner { // `DevresInner::new`. let inner = unsafe { Arc::from_raw(ptr) }; - inner.data.revoke(); + if !inner.data.revoke() { + // If `revoke()` returns false, it means that `Devres::drop` already started revoking + // `inner.data` for us. Hence we have to wait until `Devres::drop()` signals that it + // completed revoking `inner.data`. + inner.revoke.wait_for_completion(); + } } } @@ -232,6 +244,15 @@ impl Deref for Devres { impl Drop for Devres { fn drop(&mut self) { - DevresInner::remove_action(&self.0); + // SAFETY: When `drop` runs, it is guaranteed that nobody is accessing the revocable data + // anymore, hence it is safe not to wait for the grace period to finish. + if unsafe { self.revoke_nosync() } { + // We revoked `self.0.data` before the devres action did, hence try to remove it. + if !DevresInner::remove_action(&self.0) { + // We could not remove the devres action, which means that it now runs concurrently, + // hence signal that `self.0.data` has been revoked successfully. + self.0.revoke.complete_all(); + } + } } } -- cgit v1.2.3 From 20c96ed278e362ae4e324ed7d8c69fb48c508d3c Mon Sep 17 00:00:00 2001 From: Danilo Krummrich Date: Wed, 11 Jun 2025 19:48:25 +0200 Subject: rust: devres: do not dereference to the internal Revocable We can't expose direct access to the internal Revocable, since this allows users to directly revoke the internal Revocable without Devres having the chance to synchronize with the devres callback -- we have to guarantee that the internal Revocable has been fully revoked before the device is fully unbound. Hence, remove the corresponding Deref implementation and, instead, provide indirect accessors for the internal Revocable. Note that we can still support Devres::revoke() by implementing the required synchronization (which would be almost identical to the synchronization in Devres::drop()). Fixes: 76c01ded724b ("rust: add devres abstraction") Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20250611174827.380555-1-dakr@kernel.org Signed-off-by: Danilo Krummrich --- rust/kernel/devres.rs | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs index 2f74bce5ed9d..57502534d985 100644 --- a/rust/kernel/devres.rs +++ b/rust/kernel/devres.rs @@ -12,13 +12,11 @@ use crate::{ error::{Error, Result}, ffi::c_void, prelude::*, - revocable::Revocable, - sync::{Arc, Completion}, + revocable::{Revocable, RevocableGuard}, + sync::{rcu, Arc, Completion}, types::ARef, }; -use core::ops::Deref; - #[pin_data] struct DevresInner { dev: ARef, @@ -230,15 +228,22 @@ impl Devres { // SAFETY: `dev` being the same device as the device this `Devres` has been created for // proves that `self.0.data` hasn't been revoked and is guaranteed to not be revoked as // long as `dev` lives; `dev` lives at least as long as `self`. - Ok(unsafe { self.deref().access() }) + Ok(unsafe { self.0.data.access() }) } -} -impl Deref for Devres { - type Target = Revocable; + /// [`Devres`] accessor for [`Revocable::try_access`]. + pub fn try_access(&self) -> Option> { + self.0.data.try_access() + } + + /// [`Devres`] accessor for [`Revocable::try_access_with`]. + pub fn try_access_with R>(&self, f: F) -> Option { + self.0.data.try_access_with(f) + } - fn deref(&self) -> &Self::Target { - &self.0.data + /// [`Devres`] accessor for [`Revocable::try_access_with_guard`]. + pub fn try_access_with_guard<'a>(&'a self, guard: &'a rcu::Guard) -> Option<&'a T> { + self.0.data.try_access_with_guard(guard) } } @@ -246,7 +251,7 @@ impl Drop for Devres { fn drop(&mut self) { // SAFETY: When `drop` runs, it is guaranteed that nobody is accessing the revocable data // anymore, hence it is safe not to wait for the grace period to finish. - if unsafe { self.revoke_nosync() } { + if unsafe { self.0.data.revoke_nosync() } { // We revoked `self.0.data` before the devres action did, hence try to remove it. if !DevresInner::remove_action(&self.0) { // We could not remove the devres action, which means that it now runs concurrently, -- cgit v1.2.3