From c5fed8ce65493f71611280f225826e7bd5e49791 Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Sun, 24 Dec 2023 18:21:28 +0100 Subject: rust: upgrade to Rust 1.75.0 This is the next upgrade to the Rust toolchain, from 1.74.1 to 1.75.0 (i.e. the latest) [1]. See the upgrade policy [2] and the comments on the first upgrade in commit 3ed03f4da06e ("rust: upgrade to Rust 1.68.2"). # Unstable features The `const_maybe_uninit_zeroed` unstable feature [3] was stabilized in Rust 1.75.0, which we were using in the PHYLIB abstractions. The only unstable features allowed to be used outside the `kernel` crate are still `new_uninit,offset_of`, though other code to be upstreamed may increase the list. Please see [4] for details. # Other improvements Rust 1.75.0 stabilized `pointer_byte_offsets` [5] which we could potentially use as an alternative for `ptr_metadata` in the future. # Required changes For this upgrade, no changes were required (i.e. on our side). # `alloc` upgrade and reviewing The vast majority of changes are due to our `alloc` fork being upgraded at once. There are two kinds of changes to be aware of: the ones coming from upstream, which we should follow as closely as possible, and the updates needed in our added fallible APIs to keep them matching the newer infallible APIs coming from upstream. Instead of taking a look at the diff of this patch, an alternative approach is reviewing a diff of the changes between upstream `alloc` and the kernel's. This allows to easily inspect the kernel additions only, especially to check if the fallible methods we already have still match the infallible ones in the new version coming from upstream. Another approach is reviewing the changes introduced in the additions in the kernel fork between the two versions. This is useful to spot potentially unintended changes to our additions. To apply these approaches, one may follow steps similar to the following to generate a pair of patches that show the differences between upstream Rust and the kernel (for the subset of `alloc` we use) before and after applying this patch: # Get the difference with respect to the old version. git -C rust checkout $(linux/scripts/min-tool-version.sh rustc) git -C linux ls-tree -r --name-only HEAD -- rust/alloc | cut -d/ -f3- | grep -Fv README.md | xargs -IPATH cp rust/library/alloc/src/PATH linux/rust/alloc/PATH git -C linux diff --patch-with-stat --summary -R > old.patch git -C linux restore rust/alloc # Apply this patch. git -C linux am rust-upgrade.patch # Get the difference with respect to the new version. git -C rust checkout $(linux/scripts/min-tool-version.sh rustc) git -C linux ls-tree -r --name-only HEAD -- rust/alloc | cut -d/ -f3- | grep -Fv README.md | xargs -IPATH cp rust/library/alloc/src/PATH linux/rust/alloc/PATH git -C linux diff --patch-with-stat --summary -R > new.patch git -C linux restore rust/alloc Now one may check the `new.patch` to take a look at the additions (first approach) or at the difference between those two patches (second approach). For the latter, a side-by-side tool is recommended. Link: https://github.com/rust-lang/rust/blob/stable/RELEASES.md#version-1750-2023-12-28 [1] Link: https://rust-for-linux.com/rust-version-policy [2] Link: https://github.com/rust-lang/rust/issues/91850 [3] Link: https://github.com/Rust-for-Linux/linux/issues/2 [4] Link: https://github.com/rust-lang/rust/issues/96283 [5] Reviewed-by: Vincenzo Palazzo Reviewed-by: Martin Rodriguez Reboredo Tested-by: Boqun Feng Link: https://lore.kernel.org/r/20231224172128.271447-1-ojeda@kernel.org Signed-off-by: Miguel Ojeda --- rust/kernel/lib.rs | 1 - 1 file changed, 1 deletion(-) (limited to 'rust/kernel') diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs index 7ac39874aeac..cb2d024db51f 100644 --- a/rust/kernel/lib.rs +++ b/rust/kernel/lib.rs @@ -14,7 +14,6 @@ #![no_std] #![feature(allocator_api)] #![feature(coerce_unsized)] -#![feature(const_maybe_uninit_zeroed)] #![feature(dispatch_from_dyn)] #![feature(new_uninit)] #![feature(offset_of)] -- cgit v1.2.3-70-g09d2 From 6b1b2326b2bc3d6a90ec04815130d59ae57f3bc1 Mon Sep 17 00:00:00 2001 From: Charalampos Mitrodimas Date: Fri, 5 Jan 2024 01:29:30 +0000 Subject: rust: sync: `CondVar` rename "wait_list" to "wait_queue_head" Fields named "wait_list" usually are of type "struct list_head". To avoid confusion and because it is of type "Opaque" we are renaming "wait_list" to "wait_queue_head". Signed-off-by: Charalampos Mitrodimas Reviewed-by: Alice Ryhl Reviewed-by: Martin Rodriguez Reboredo Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20240105012930.1426214-1-charmitro@posteo.net Signed-off-by: Miguel Ojeda --- rust/kernel/sync/condvar.rs | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/sync/condvar.rs b/rust/kernel/sync/condvar.rs index f65e19d5a37c..90108cc6da0b 100644 --- a/rust/kernel/sync/condvar.rs +++ b/rust/kernel/sync/condvar.rs @@ -73,7 +73,7 @@ macro_rules! new_condvar { #[pin_data] pub struct CondVar { #[pin] - pub(crate) wait_list: Opaque, + pub(crate) wait_queue_head: Opaque, /// A condvar needs to be pinned because it contains a [`struct list_head`] that is /// self-referential, so it cannot be safely moved once it is initialised. @@ -96,7 +96,7 @@ impl CondVar { _pin: PhantomPinned, // SAFETY: `slot` is valid while the closure is called and both `name` and `key` have // static lifetimes so they live indefinitely. - wait_list <- Opaque::ffi_init(|slot| unsafe { + wait_queue_head <- Opaque::ffi_init(|slot| unsafe { bindings::__init_waitqueue_head(slot, name.as_char_ptr(), key.as_ptr()) }), }) @@ -108,16 +108,20 @@ impl CondVar { // SAFETY: `wait` points to valid memory. unsafe { bindings::init_wait(wait.get()) }; - // SAFETY: Both `wait` and `wait_list` point to valid memory. + // SAFETY: Both `wait` and `wait_queue_head` point to valid memory. unsafe { - bindings::prepare_to_wait_exclusive(self.wait_list.get(), wait.get(), wait_state as _) + bindings::prepare_to_wait_exclusive( + self.wait_queue_head.get(), + wait.get(), + wait_state as _, + ) }; // SAFETY: No arguments, switches to another thread. guard.do_unlocked(|| unsafe { bindings::schedule() }); - // SAFETY: Both `wait` and `wait_list` point to valid memory. - unsafe { bindings::finish_wait(self.wait_list.get(), wait.get()) }; + // SAFETY: Both `wait` and `wait_queue_head` point to valid memory. + unsafe { bindings::finish_wait(self.wait_queue_head.get(), wait.get()) }; } /// Releases the lock and waits for a notification in uninterruptible mode. @@ -144,10 +148,10 @@ impl CondVar { /// Calls the kernel function to notify the appropriate number of threads with the given flags. fn notify(&self, count: i32, flags: u32) { - // SAFETY: `wait_list` points to valid memory. + // SAFETY: `wait_queue_head` points to valid memory. unsafe { bindings::__wake_up( - self.wait_list.get(), + self.wait_queue_head.get(), bindings::TASK_NORMAL, count, flags as _, -- cgit v1.2.3-70-g09d2 From 3e6454177f3a76265cba4901d5caa4eb0c7b6447 Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Mon, 8 Jan 2024 14:49:57 +0000 Subject: rust: sync: add `CondVar::notify_sync` Wake up another thread synchronously. This method behaves like `notify_one`, except that it hints to the scheduler that the current thread is about to go to sleep, so it should schedule the target thread on the same CPU. This is used by Rust Binder as a performance optimization. When sending a transaction to a different process, we usually know which thread will handle it, so we can schedule that thread for execution next on this CPU for better cache locality. Reviewed-by: Benno Lossin Reviewed-by: Martin Rodriguez Reboredo Reviewed-by: Tiago Lam Reviewed-by: Boqun Feng Signed-off-by: Alice Ryhl Link: https://lore.kernel.org/r/20240108-rb-new-condvar-methods-v4-1-88e0c871cc05@google.com Signed-off-by: Miguel Ojeda --- rust/kernel/sync/condvar.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'rust/kernel') diff --git a/rust/kernel/sync/condvar.rs b/rust/kernel/sync/condvar.rs index 90108cc6da0b..30fb5cbfb730 100644 --- a/rust/kernel/sync/condvar.rs +++ b/rust/kernel/sync/condvar.rs @@ -159,6 +159,16 @@ impl CondVar { }; } + /// Calls the kernel function to notify one thread synchronously. + /// + /// This method behaves like `notify_one`, except that it hints to the scheduler that the + /// current thread is about to go to sleep, so it should schedule the target thread on the same + /// CPU. + pub fn notify_sync(&self) { + // SAFETY: `wait_queue_head` points to valid memory. + unsafe { bindings::__wake_up_sync(self.wait_queue_head.get(), bindings::TASK_NORMAL) }; + } + /// Wakes a single waiter up, if any. /// /// This is not 'sticky' in the sense that if no thread is waiting, the notification is lost -- cgit v1.2.3-70-g09d2 From 82e170874849c4c57dba6b4ee1a08fae1c0a5f02 Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Mon, 8 Jan 2024 14:49:58 +0000 Subject: rust: time: add msecs to jiffies conversion Defines type aliases and conversions for msecs and jiffies. This is used by Rust Binder for process freezing. There, we want to sleep until the freeze operation completes, but we want to be able to abort the process freezing if it doesn't complete within some timeout. The freeze timeout is supplied in msecs. Note that we need to convert to jiffies in Binder. It is not enough to introduce a variant of `CondVar::wait_timeout` that takes the timeout in msecs because we need to be able to restart the sleep with the remaining sleep duration if it is interrupted, and if the API takes msecs rather than jiffies, then that would require a conversion roundtrip jiffies-> msecs->jiffies that is best avoided. Suggested-by: Boqun Feng Reviewed-by: Boqun Feng Reviewed-by: Benno Lossin Reviewed-by: Martin Rodriguez Reboredo Reviewed-by: Tiago Lam Signed-off-by: Alice Ryhl Link: https://lore.kernel.org/r/20240108-rb-new-condvar-methods-v4-2-88e0c871cc05@google.com Signed-off-by: Miguel Ojeda --- rust/bindings/bindings_helper.h | 1 + rust/kernel/lib.rs | 1 + rust/kernel/time.rs | 20 ++++++++++++++++++++ 3 files changed, 22 insertions(+) create mode 100644 rust/kernel/time.rs (limited to 'rust/kernel') diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h index c0cb4b05b918..936651110c39 100644 --- a/rust/bindings/bindings_helper.h +++ b/rust/bindings/bindings_helper.h @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs index cb2d024db51f..b89ecf4e97a0 100644 --- a/rust/kernel/lib.rs +++ b/rust/kernel/lib.rs @@ -48,6 +48,7 @@ pub mod std_vendor; pub mod str; pub mod sync; pub mod task; +pub mod time; pub mod types; pub mod workqueue; diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs new file mode 100644 index 000000000000..25a896eed468 --- /dev/null +++ b/rust/kernel/time.rs @@ -0,0 +1,20 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! Time related primitives. +//! +//! 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. + +/// The time unit of Linux kernel. One jiffy equals (1/HZ) second. +pub type Jiffies = core::ffi::c_ulong; + +/// The millisecond time unit. +pub type Msecs = core::ffi::c_uint; + +/// Converts milliseconds to jiffies. +#[inline] +pub fn msecs_to_jiffies(msecs: Msecs) -> Jiffies { + // SAFETY: The `__msecs_to_jiffies` function is always safe to call no + // matter what the argument is. + unsafe { bindings::__msecs_to_jiffies(msecs) } +} -- cgit v1.2.3-70-g09d2 From e7b9b1ff1d49e2117f2d78fa5f11b0d006c169f7 Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Mon, 8 Jan 2024 14:49:59 +0000 Subject: rust: sync: add `CondVar::wait_timeout` Sleep on a condition variable with a timeout. This is used by Rust Binder for process freezing. There, we want to sleep until the freeze operation completes, but we want to be able to abort the process freezing if it doesn't complete within some timeout. Note that it is not enough to avoid jiffies by introducing a variant of `CondVar::wait_timeout` that takes the timeout in msecs because we need to be able to restart the sleep with the remaining sleep duration if it is interrupted, and if the API takes msecs rather than jiffies, then that would require a conversion roundtrip jiffies->msecs->jiffies that is best avoided. Reviewed-by: Martin Rodriguez Reboredo Reviewed-by: Tiago Lam Reviewed-by: Boqun Feng Signed-off-by: Alice Ryhl Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20240108-rb-new-condvar-methods-v4-3-88e0c871cc05@google.com [ Added `CondVarTimeoutResult` re-export and fixed typo. ] Signed-off-by: Miguel Ojeda --- rust/kernel/sync.rs | 2 +- rust/kernel/sync/condvar.rs | 60 ++++++++++++++++++++++++++++++++++++++++----- rust/kernel/sync/lock.rs | 4 +-- rust/kernel/task.rs | 5 +++- 4 files changed, 61 insertions(+), 10 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs index d219ee518eff..c1fb10fc64f4 100644 --- a/rust/kernel/sync.rs +++ b/rust/kernel/sync.rs @@ -13,7 +13,7 @@ pub mod lock; mod locked_by; pub use arc::{Arc, ArcBorrow, UniqueArc}; -pub use condvar::CondVar; +pub use condvar::{CondVar, CondVarTimeoutResult}; pub use lock::{mutex::Mutex, spinlock::SpinLock}; pub use locked_by::LockedBy; diff --git a/rust/kernel/sync/condvar.rs b/rust/kernel/sync/condvar.rs index 30fb5cbfb730..3f148d7040bb 100644 --- a/rust/kernel/sync/condvar.rs +++ b/rust/kernel/sync/condvar.rs @@ -6,7 +6,11 @@ //! variable. use super::{lock::Backend, lock::Guard, LockClassKey}; -use crate::{bindings, init::PinInit, pin_init, str::CStr, types::Opaque}; +use crate::{ + bindings, init::PinInit, pin_init, str::CStr, task::MAX_SCHEDULE_TIMEOUT, time::Jiffies, + types::Opaque, +}; +use core::ffi::c_long; use core::marker::PhantomPinned; use macros::pin_data; @@ -102,7 +106,12 @@ impl CondVar { }) } - fn wait_internal(&self, wait_state: u32, guard: &mut Guard<'_, T, B>) { + fn wait_internal( + &self, + wait_state: u32, + guard: &mut Guard<'_, T, B>, + timeout_in_jiffies: c_long, + ) -> c_long { let wait = Opaque::::uninit(); // SAFETY: `wait` points to valid memory. @@ -117,11 +126,13 @@ impl CondVar { ) }; - // SAFETY: No arguments, switches to another thread. - guard.do_unlocked(|| unsafe { bindings::schedule() }); + // SAFETY: Switches to another thread. The timeout can be any number. + let ret = guard.do_unlocked(|| unsafe { bindings::schedule_timeout(timeout_in_jiffies) }); // SAFETY: Both `wait` and `wait_queue_head` point to valid memory. unsafe { bindings::finish_wait(self.wait_queue_head.get(), wait.get()) }; + + ret } /// Releases the lock and waits for a notification in uninterruptible mode. @@ -131,7 +142,7 @@ impl CondVar { /// [`CondVar::notify_one`] or [`CondVar::notify_all`]. Note that it may also wake up /// spuriously. pub fn wait(&self, guard: &mut Guard<'_, T, B>) { - self.wait_internal(bindings::TASK_UNINTERRUPTIBLE, guard); + self.wait_internal(bindings::TASK_UNINTERRUPTIBLE, guard, MAX_SCHEDULE_TIMEOUT); } /// Releases the lock and waits for a notification in interruptible mode. @@ -142,10 +153,31 @@ impl CondVar { /// Returns whether there is a signal pending. #[must_use = "wait_interruptible returns if a signal is pending, so the caller must check the return value"] pub fn wait_interruptible(&self, guard: &mut Guard<'_, T, B>) -> bool { - self.wait_internal(bindings::TASK_INTERRUPTIBLE, guard); + self.wait_internal(bindings::TASK_INTERRUPTIBLE, guard, MAX_SCHEDULE_TIMEOUT); crate::current!().signal_pending() } + /// Releases the lock and waits for a notification in interruptible mode. + /// + /// Atomically releases the given lock (whose ownership is proven by the guard) and puts the + /// thread to sleep. It wakes up when notified by [`CondVar::notify_one`] or + /// [`CondVar::notify_all`], or when a timeout occurs, or when the thread receives a signal. + #[must_use = "wait_interruptible_timeout returns if a signal is pending, so the caller must check the return value"] + pub fn wait_interruptible_timeout( + &self, + guard: &mut Guard<'_, T, B>, + jiffies: Jiffies, + ) -> CondVarTimeoutResult { + let jiffies = jiffies.try_into().unwrap_or(MAX_SCHEDULE_TIMEOUT); + let res = self.wait_internal(bindings::TASK_INTERRUPTIBLE, guard, jiffies); + + match (res as Jiffies, crate::current!().signal_pending()) { + (jiffies, true) => CondVarTimeoutResult::Signal { jiffies }, + (0, false) => CondVarTimeoutResult::Timeout, + (jiffies, false) => CondVarTimeoutResult::Woken { jiffies }, + } + } + /// Calls the kernel function to notify the appropriate number of threads with the given flags. fn notify(&self, count: i32, flags: u32) { // SAFETY: `wait_queue_head` points to valid memory. @@ -185,3 +217,19 @@ impl CondVar { self.notify(0, 0); } } + +/// The return type of `wait_timeout`. +pub enum CondVarTimeoutResult { + /// The timeout was reached. + Timeout, + /// Somebody woke us up. + Woken { + /// Remaining sleep duration. + jiffies: Jiffies, + }, + /// A signal occurred. + Signal { + /// Remaining sleep duration. + jiffies: Jiffies, + }, +} diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs index f12a684bc957..149a5259d431 100644 --- a/rust/kernel/sync/lock.rs +++ b/rust/kernel/sync/lock.rs @@ -139,7 +139,7 @@ pub struct Guard<'a, T: ?Sized, B: Backend> { unsafe impl Sync for Guard<'_, T, B> {} impl Guard<'_, T, B> { - pub(crate) fn do_unlocked(&mut self, cb: impl FnOnce()) { + pub(crate) fn do_unlocked(&mut self, cb: impl FnOnce() -> U) -> U { // SAFETY: The caller owns the lock, so it is safe to unlock it. unsafe { B::unlock(self.lock.state.get(), &self.state) }; @@ -147,7 +147,7 @@ impl Guard<'_, T, B> { let _relock = ScopeGuard::new(|| unsafe { B::relock(self.lock.state.get(), &mut self.state) }); - cb(); + cb() } } diff --git a/rust/kernel/task.rs b/rust/kernel/task.rs index 9451932d5d86..49a8e10d68f5 100644 --- a/rust/kernel/task.rs +++ b/rust/kernel/task.rs @@ -5,7 +5,10 @@ //! C header: [`include/linux/sched.h`](srctree/include/linux/sched.h). use crate::{bindings, types::Opaque}; -use core::{marker::PhantomData, ops::Deref, ptr}; +use core::{ffi::c_long, marker::PhantomData, ops::Deref, ptr}; + +/// A sentinel value used for infinite timeouts. +pub const MAX_SCHEDULE_TIMEOUT: c_long = c_long::MAX; /// Returns the currently running task. #[macro_export] -- cgit v1.2.3-70-g09d2 From f090f0d0eea9666a96702b29bc9a64cbabee85c5 Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Mon, 8 Jan 2024 14:50:00 +0000 Subject: rust: sync: update integer types in CondVar Reduce the chances of compilation failures due to integer type mismatches in `CondVar`. When an integer is defined using a #define in C, bindgen doesn't know which integer type it is supposed to be, so it will just use `u32` by default (if it fits in an u32). Whenever the right type is something else, we insert a cast in Rust. However, this means that the code has a lot of extra casts, and sometimes the code will be missing casts if u32 happens to be correct on the developer's machine, even though the type might be something else on a different platform. This patch updates all uses of such constants in `rust/kernel/sync/condvar.rs` to use constants defined with the right type. This allows us to remove various unnecessary casts, while also future-proofing for the case where `unsigned int != u32` (even though that is unlikely to ever happen in the kernel). I wrote this patch at the suggestion of Benno in [1]. Link: https://lore.kernel.org/all/nAEg-6vbtX72ZY3oirDhrSEf06TBWmMiTt73EklMzEAzN4FD4mF3TPEyAOxBZgZtjzoiaBYtYr3s8sa9wp1uYH9vEWRf2M-Lf4I0BY9rAgk=@proton.me/ [1] Suggested-by: Benno Lossin Reviewed-by: Tiago Lam Reviewed-by: Boqun Feng Reviewed-by: Benno Lossin Reviewed-by: Martin Rodriguez Reboredo Signed-off-by: Alice Ryhl Link: https://lore.kernel.org/r/20240108-rb-new-condvar-methods-v4-4-88e0c871cc05@google.com [ Added note on the unlikeliness of `sizeof(int)` changing. ] Signed-off-by: Miguel Ojeda --- rust/kernel/sync/condvar.rs | 38 ++++++++++++++++++++------------------ rust/kernel/task.rs | 15 ++++++++++++++- 2 files changed, 34 insertions(+), 19 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/sync/condvar.rs b/rust/kernel/sync/condvar.rs index 3f148d7040bb..9f1d83589beb 100644 --- a/rust/kernel/sync/condvar.rs +++ b/rust/kernel/sync/condvar.rs @@ -7,11 +7,17 @@ use super::{lock::Backend, lock::Guard, LockClassKey}; use crate::{ - bindings, init::PinInit, pin_init, str::CStr, task::MAX_SCHEDULE_TIMEOUT, time::Jiffies, + bindings, + init::PinInit, + pin_init, + str::CStr, + task::{MAX_SCHEDULE_TIMEOUT, TASK_INTERRUPTIBLE, TASK_NORMAL, TASK_UNINTERRUPTIBLE}, + time::Jiffies, types::Opaque, }; -use core::ffi::c_long; +use core::ffi::{c_int, c_long}; use core::marker::PhantomPinned; +use core::ptr; use macros::pin_data; /// Creates a [`CondVar`] initialiser with the given name and a newly-created lock class. @@ -108,7 +114,7 @@ impl CondVar { fn wait_internal( &self, - wait_state: u32, + wait_state: c_int, guard: &mut Guard<'_, T, B>, timeout_in_jiffies: c_long, ) -> c_long { @@ -119,11 +125,7 @@ impl CondVar { // SAFETY: Both `wait` and `wait_queue_head` point to valid memory. unsafe { - bindings::prepare_to_wait_exclusive( - self.wait_queue_head.get(), - wait.get(), - wait_state as _, - ) + bindings::prepare_to_wait_exclusive(self.wait_queue_head.get(), wait.get(), wait_state) }; // SAFETY: Switches to another thread. The timeout can be any number. @@ -142,7 +144,7 @@ impl CondVar { /// [`CondVar::notify_one`] or [`CondVar::notify_all`]. Note that it may also wake up /// spuriously. pub fn wait(&self, guard: &mut Guard<'_, T, B>) { - self.wait_internal(bindings::TASK_UNINTERRUPTIBLE, guard, MAX_SCHEDULE_TIMEOUT); + self.wait_internal(TASK_UNINTERRUPTIBLE, guard, MAX_SCHEDULE_TIMEOUT); } /// Releases the lock and waits for a notification in interruptible mode. @@ -153,7 +155,7 @@ impl CondVar { /// Returns whether there is a signal pending. #[must_use = "wait_interruptible returns if a signal is pending, so the caller must check the return value"] pub fn wait_interruptible(&self, guard: &mut Guard<'_, T, B>) -> bool { - self.wait_internal(bindings::TASK_INTERRUPTIBLE, guard, MAX_SCHEDULE_TIMEOUT); + self.wait_internal(TASK_INTERRUPTIBLE, guard, MAX_SCHEDULE_TIMEOUT); crate::current!().signal_pending() } @@ -169,7 +171,7 @@ impl CondVar { jiffies: Jiffies, ) -> CondVarTimeoutResult { let jiffies = jiffies.try_into().unwrap_or(MAX_SCHEDULE_TIMEOUT); - let res = self.wait_internal(bindings::TASK_INTERRUPTIBLE, guard, jiffies); + let res = self.wait_internal(TASK_INTERRUPTIBLE, guard, jiffies); match (res as Jiffies, crate::current!().signal_pending()) { (jiffies, true) => CondVarTimeoutResult::Signal { jiffies }, @@ -178,15 +180,15 @@ impl CondVar { } } - /// Calls the kernel function to notify the appropriate number of threads with the given flags. - fn notify(&self, count: i32, flags: u32) { + /// Calls the kernel function to notify the appropriate number of threads. + fn notify(&self, count: c_int) { // SAFETY: `wait_queue_head` points to valid memory. unsafe { bindings::__wake_up( self.wait_queue_head.get(), - bindings::TASK_NORMAL, + TASK_NORMAL, count, - flags as _, + ptr::null_mut(), ) }; } @@ -198,7 +200,7 @@ impl CondVar { /// CPU. pub fn notify_sync(&self) { // SAFETY: `wait_queue_head` points to valid memory. - unsafe { bindings::__wake_up_sync(self.wait_queue_head.get(), bindings::TASK_NORMAL) }; + unsafe { bindings::__wake_up_sync(self.wait_queue_head.get(), TASK_NORMAL) }; } /// Wakes a single waiter up, if any. @@ -206,7 +208,7 @@ impl CondVar { /// This is not 'sticky' in the sense that if no thread is waiting, the notification is lost /// completely (as opposed to automatically waking up the next waiter). pub fn notify_one(&self) { - self.notify(1, 0); + self.notify(1); } /// Wakes all waiters up, if any. @@ -214,7 +216,7 @@ impl CondVar { /// This is not 'sticky' in the sense that if no thread is waiting, the notification is lost /// completely (as opposed to automatically waking up the next waiter). pub fn notify_all(&self) { - self.notify(0, 0); + self.notify(0); } } diff --git a/rust/kernel/task.rs b/rust/kernel/task.rs index 49a8e10d68f5..a3a4007db682 100644 --- a/rust/kernel/task.rs +++ b/rust/kernel/task.rs @@ -5,11 +5,24 @@ //! C header: [`include/linux/sched.h`](srctree/include/linux/sched.h). use crate::{bindings, types::Opaque}; -use core::{ffi::c_long, marker::PhantomData, ops::Deref, ptr}; +use core::{ + ffi::{c_int, c_long, c_uint}, + marker::PhantomData, + ops::Deref, + ptr, +}; /// A sentinel value used for infinite timeouts. pub const MAX_SCHEDULE_TIMEOUT: c_long = c_long::MAX; +/// Bitmask for tasks that are sleeping in an interruptible state. +pub const TASK_INTERRUPTIBLE: c_int = bindings::TASK_INTERRUPTIBLE as c_int; +/// Bitmask for tasks that are sleeping in an uninterruptible state. +pub const TASK_UNINTERRUPTIBLE: c_int = bindings::TASK_UNINTERRUPTIBLE as c_int; +/// Convenience constant for waking up tasks regardless of whether they are in interruptible or +/// uninterruptible sleep. +pub const TASK_NORMAL: c_uint = bindings::TASK_NORMAL as c_uint; + /// Returns the currently running task. #[macro_export] macro_rules! current { -- cgit v1.2.3-70-g09d2 From b6cda913bba42e1fdad82df41d906ff603319743 Mon Sep 17 00:00:00 2001 From: Valentin Obst Date: Wed, 31 Jan 2024 21:23:23 +0100 Subject: rust: kernel: fix multiple typos in documentation Fixes multiple trivial typos in documentation and comments of the kernel crate. allocator: - Fix a trivial list item alignment issue in the last SAFETY comment of `krealloc_aligned`. init: - Replace 'type' with 'trait' in the doc comments of the `PinInit` and `Init` traits. - Add colons before starting lists. - Add spaces between the type and equal sign to respect the code formatting rules in example code. - End a sentence with a full stop instead of a colon. ioctl: - Replace 'an' with 'a' where appropriate. str: - Replace 'Return' with 'Returns' in the doc comment of `bytes_written` as the text describes what the function does. sync/lock: - Fix a trivial list item alignment issue in the Safety section of the `Backend` trait's description. sync/lock/spinlock: - The code in this module operates on spinlocks, not mutexes. Thus, replace 'mutex' with 'spinlock' in the SAFETY comment of `unlock`. workqueue: - Replace "wont" with "won't" in the doc comment of `__enqueue`. Signed-off-by: Valentin Obst Reviewed-by: Trevor Gross Reviewed-by: Martin Rodriguez Reboredo Reviewed-by: Alice Ryhl Link: https://lore.kernel.org/r/20240131-doc-fixes-v3-v3-1-0c8af94ed7de@valentinobst.de Signed-off-by: Miguel Ojeda --- rust/kernel/allocator.rs | 2 +- rust/kernel/init.rs | 16 ++++++++-------- rust/kernel/ioctl.rs | 4 ++-- rust/kernel/str.rs | 2 +- rust/kernel/sync/lock.rs | 4 ++-- rust/kernel/sync/lock/spinlock.rs | 2 +- rust/kernel/workqueue.rs | 2 +- 7 files changed, 16 insertions(+), 16 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/allocator.rs b/rust/kernel/allocator.rs index 4b057e837358..01ad139e19bc 100644 --- a/rust/kernel/allocator.rs +++ b/rust/kernel/allocator.rs @@ -35,7 +35,7 @@ unsafe fn krealloc_aligned(ptr: *mut u8, new_layout: Layout, flags: bindings::gf // - `ptr` is either null or a pointer returned from a previous `k{re}alloc()` by the // function safety requirement. // - `size` is greater than 0 since it's either a `layout.size()` (which cannot be zero - // according to the function safety requirement) or a result from `next_power_of_two()`. + // according to the function safety requirement) or a result from `next_power_of_two()`. unsafe { bindings::krealloc(ptr as *const core::ffi::c_void, size, flags) as *mut u8 } } diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs index 65be9ae57b80..16a99984622c 100644 --- a/rust/kernel/init.rs +++ b/rust/kernel/init.rs @@ -751,10 +751,10 @@ macro_rules! try_init { /// /// # Safety /// -/// When implementing this type you will need to take great care. Also there are probably very few +/// When implementing this trait you will need to take great care. Also there are probably very few /// cases where a manual implementation is necessary. Use [`pin_init_from_closure`] where possible. /// -/// The [`PinInit::__pinned_init`] function +/// The [`PinInit::__pinned_init`] function: /// - returns `Ok(())` if it initialized every field of `slot`, /// - returns `Err(err)` if it encountered an error and then cleaned `slot`, this means: /// - `slot` can be deallocated without UB occurring, @@ -861,10 +861,10 @@ where /// /// # Safety /// -/// When implementing this type you will need to take great care. Also there are probably very few +/// When implementing this trait you will need to take great care. Also there are probably very few /// cases where a manual implementation is necessary. Use [`init_from_closure`] where possible. /// -/// The [`Init::__init`] function +/// The [`Init::__init`] function: /// - returns `Ok(())` if it initialized every field of `slot`, /// - returns `Err(err)` if it encountered an error and then cleaned `slot`, this means: /// - `slot` can be deallocated without UB occurring, @@ -1013,7 +1013,7 @@ pub fn uninit() -> impl Init, E> { /// /// ```rust /// use kernel::{error::Error, init::init_array_from_fn}; -/// let array: Box<[usize; 1_000]>= Box::init::(init_array_from_fn(|i| i)).unwrap(); +/// let array: Box<[usize; 1_000]> = Box::init::(init_array_from_fn(|i| i)).unwrap(); /// assert_eq!(array.len(), 1_000); /// ``` pub fn init_array_from_fn( @@ -1027,7 +1027,7 @@ where // Counts the number of initialized elements and when dropped drops that many elements from // `slot`. let mut init_count = ScopeGuard::new_with_data(0, |i| { - // We now free every element that has been initialized before: + // We now free every element that has been initialized before. // SAFETY: The loop initialized exactly the values from 0..i and since we // return `Err` below, the caller will consider the memory at `slot` as // uninitialized. @@ -1056,7 +1056,7 @@ where /// /// ```rust /// use kernel::{sync::{Arc, Mutex}, init::pin_init_array_from_fn, new_mutex}; -/// let array: Arc<[Mutex; 1_000]>= +/// let array: Arc<[Mutex; 1_000]> = /// Arc::pin_init(pin_init_array_from_fn(|i| new_mutex!(i))).unwrap(); /// assert_eq!(array.len(), 1_000); /// ``` @@ -1071,7 +1071,7 @@ where // Counts the number of initialized elements and when dropped drops that many elements from // `slot`. let mut init_count = ScopeGuard::new_with_data(0, |i| { - // We now free every element that has been initialized before: + // We now free every element that has been initialized before. // SAFETY: The loop initialized exactly the values from 0..i and since we // return `Err` below, the caller will consider the memory at `slot` as // uninitialized. diff --git a/rust/kernel/ioctl.rs b/rust/kernel/ioctl.rs index f1d42ab69972..59050e5f5a5a 100644 --- a/rust/kernel/ioctl.rs +++ b/rust/kernel/ioctl.rs @@ -28,13 +28,13 @@ pub const fn _IO(ty: u32, nr: u32) -> u32 { _IOC(uapi::_IOC_NONE, ty, nr, 0) } -/// Build an ioctl number for an read-only ioctl. +/// Build an ioctl number for a read-only ioctl. #[inline(always)] pub const fn _IOR(ty: u32, nr: u32) -> u32 { _IOC(uapi::_IOC_READ, ty, nr, core::mem::size_of::()) } -/// Build an ioctl number for an write-only ioctl. +/// Build an ioctl number for a write-only ioctl. #[inline(always)] pub const fn _IOW(ty: u32, nr: u32) -> u32 { _IOC(uapi::_IOC_WRITE, ty, nr, core::mem::size_of::()) diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs index 7d848b83add4..0a8569594fc3 100644 --- a/rust/kernel/str.rs +++ b/rust/kernel/str.rs @@ -449,7 +449,7 @@ impl RawFormatter { self.pos as _ } - /// Return the number of bytes written to the formatter. + /// Returns the number of bytes written to the formatter. pub(crate) fn bytes_written(&self) -> usize { self.pos - self.beg } diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs index 149a5259d431..072b8ef2a0fa 100644 --- a/rust/kernel/sync/lock.rs +++ b/rust/kernel/sync/lock.rs @@ -21,9 +21,9 @@ pub mod spinlock; /// # Safety /// /// - Implementers must ensure that only one thread/CPU may access the protected data once the lock -/// is owned, that is, between calls to `lock` and `unlock`. +/// is owned, that is, between calls to `lock` and `unlock`. /// - Implementers must also ensure that `relock` uses the same locking method as the original -/// lock operation. +/// lock operation. pub unsafe trait Backend { /// The state required by the lock. type State; diff --git a/rust/kernel/sync/lock/spinlock.rs b/rust/kernel/sync/lock/spinlock.rs index 068535ce1b29..e5e0bf621988 100644 --- a/rust/kernel/sync/lock/spinlock.rs +++ b/rust/kernel/sync/lock/spinlock.rs @@ -112,7 +112,7 @@ unsafe impl super::Backend for SpinLockBackend { unsafe fn unlock(ptr: *mut Self::State, _guard_state: &Self::GuardState) { // SAFETY: The safety requirements of this function ensure that `ptr` is valid and that the - // caller is the owner of the mutex. + // caller is the owner of the spinlock. unsafe { bindings::spin_unlock(ptr) } } } diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs index 498397877376..8775c34d12a5 100644 --- a/rust/kernel/workqueue.rs +++ b/rust/kernel/workqueue.rs @@ -253,7 +253,7 @@ impl WorkItem for ClosureWork { /// actual value of the id is not important as long as you use different ids for different fields /// of the same struct. (Fields of different structs need not use different ids.) /// -/// Note that the id is used only to select the right method to call during compilation. It wont be +/// Note that the id is used only to select the right method to call during compilation. It won't be /// part of the final executable. /// /// # Safety -- cgit v1.2.3-70-g09d2 From 69d5fbb0159673ea6737204f4d458a220e81a0c9 Mon Sep 17 00:00:00 2001 From: Valentin Obst Date: Wed, 31 Jan 2024 21:23:24 +0100 Subject: rust: error: improve unsafe code in example The `from_err_ptr` function is safe. There is no need for the call to it to be inside the unsafe block. Reword the SAFETY comment to provide a better justification of why the FFI call is safe. Signed-off-by: Valentin Obst Reviewed-by: Martin Rodriguez Reboredo Reviewed-by: Alice Ryhl Reviewed-by: Trevor Gross Link: https://lore.kernel.org/r/20240131-doc-fixes-v3-v3-2-0c8af94ed7de@valentinobst.de Signed-off-by: Miguel Ojeda --- rust/kernel/error.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs index 4f0c1edd63b7..4786d3ee1e92 100644 --- a/rust/kernel/error.rs +++ b/rust/kernel/error.rs @@ -264,13 +264,9 @@ pub fn to_result(err: core::ffi::c_int) -> Result { /// pdev: &mut PlatformDevice, /// index: u32, /// ) -> Result<*mut core::ffi::c_void> { -/// // SAFETY: FFI call. -/// unsafe { -/// from_err_ptr(bindings::devm_platform_ioremap_resource( -/// pdev.to_ptr(), -/// index, -/// )) -/// } +/// // SAFETY: `pdev` points to a valid platform device. There are no safety requirements +/// // on `index`. +/// from_err_ptr(unsafe { bindings::devm_platform_ioremap_resource(pdev.to_ptr(), index) }) /// } /// ``` // TODO: Remove `dead_code` marker once an in-kernel client is available. -- cgit v1.2.3-70-g09d2 From 16dca5d12ed114110bcdfac3ae00de9f125821c8 Mon Sep 17 00:00:00 2001 From: Valentin Obst Date: Wed, 31 Jan 2024 21:23:25 +0100 Subject: rust: ioctl: end top-level module docs with full stop Every other module ends its first line of documentation with a full stop. Adapt the only outlier. Signed-off-by: Valentin Obst Reviewed-by: Trevor Gross Reviewed-by: Martin Rodriguez Reboredo Reviewed-by: Alice Ryhl Link: https://lore.kernel.org/r/20240131-doc-fixes-v3-v3-3-0c8af94ed7de@valentinobst.de Signed-off-by: Miguel Ojeda --- rust/kernel/ioctl.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'rust/kernel') diff --git a/rust/kernel/ioctl.rs b/rust/kernel/ioctl.rs index 59050e5f5a5a..5987ad6d38a7 100644 --- a/rust/kernel/ioctl.rs +++ b/rust/kernel/ioctl.rs @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0 -//! ioctl() number definitions +//! ioctl() number definitions. //! //! C header: [`include/asm-generic/ioctl.h`](srctree/include/asm-generic/ioctl.h) -- cgit v1.2.3-70-g09d2 From ed8596532a667e1a25ef0293acbb19078b94f686 Mon Sep 17 00:00:00 2001 From: Valentin Obst Date: Wed, 31 Jan 2024 21:23:26 +0100 Subject: rust: kernel: add srctree-relative doclinks Convert existing references to C header files to make use of Commit bc2e7d5c298a ("rust: support `srctree`-relative links"). Signed-off-by: Valentin Obst Reviewed-by: Trevor Gross Reviewed-by: Martin Rodriguez Reboredo Reviewed-by: Alice Ryhl Link: https://lore.kernel.org/r/20240131-doc-fixes-v3-v3-4-0c8af94ed7de@valentinobst.de Signed-off-by: Miguel Ojeda --- rust/kernel/lib.rs | 2 +- rust/kernel/sync/condvar.rs | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) (limited to 'rust/kernel') diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs index b89ecf4e97a0..74581c958a6c 100644 --- a/rust/kernel/lib.rs +++ b/rust/kernel/lib.rs @@ -78,7 +78,7 @@ pub trait Module: Sized + Sync { /// Equivalent to `THIS_MODULE` in the C API. /// -/// C header: `include/linux/export.h` +/// C header: [`include/linux/export.h`](srctree/include/linux/export.h) pub struct ThisModule(*mut bindings::module); // SAFETY: `THIS_MODULE` may be used from all threads within a module. diff --git a/rust/kernel/sync/condvar.rs b/rust/kernel/sync/condvar.rs index 9f1d83589beb..fa1794972aa0 100644 --- a/rust/kernel/sync/condvar.rs +++ b/rust/kernel/sync/condvar.rs @@ -87,6 +87,8 @@ pub struct CondVar { /// A condvar needs to be pinned because it contains a [`struct list_head`] that is /// self-referential, so it cannot be safely moved once it is initialised. + /// + /// [`struct list_head`]: srctree/include/linux/types.h #[pin] _pin: PhantomPinned, } -- cgit v1.2.3-70-g09d2 From 8cfce47d7598bed0481c8d79437dd572705bb51e Mon Sep 17 00:00:00 2001 From: Valentin Obst Date: Wed, 31 Jan 2024 21:23:27 +0100 Subject: rust: str: use `NUL` instead of 0 in doc comments Throughout the module, bytes with the value zero are referred to as `NUL` bytes. Adapt the only two outliers. Signed-off-by: Valentin Obst Reviewed-by: Trevor Gross Reviewed-by: Martin Rodriguez Reboredo Reviewed-by: Alice Ryhl Link: https://lore.kernel.org/r/20240131-doc-fixes-v3-v3-5-0c8af94ed7de@valentinobst.de Signed-off-by: Miguel Ojeda --- rust/kernel/str.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs index 0a8569594fc3..843ffeec9b3e 100644 --- a/rust/kernel/str.rs +++ b/rust/kernel/str.rs @@ -149,13 +149,13 @@ impl CStr { self.0.as_ptr() as _ } - /// Convert the string to a byte slice without the trailing 0 byte. + /// Convert the string to a byte slice without the trailing `NUL` byte. #[inline] pub fn as_bytes(&self) -> &[u8] { &self.0[..self.len()] } - /// Convert the string to a byte slice containing the trailing 0 byte. + /// Convert the string to a byte slice containing the trailing `NUL` byte. #[inline] pub const fn as_bytes_with_nul(&self) -> &[u8] { &self.0 -- cgit v1.2.3-70-g09d2 From 4c62348d5b845c9acb82378faf5aefdcb06c38be Mon Sep 17 00:00:00 2001 From: Valentin Obst Date: Wed, 31 Jan 2024 21:23:28 +0100 Subject: rust: str: move SAFETY comment in front of unsafe block SAFETY comments should immediately precede the unsafe block they justify. Move assignment to `bar` past comment as it is safe. Signed-off-by: Valentin Obst Reviewed-by: Trevor Gross Reviewed-by: Martin Rodriguez Reboredo Reviewed-by: Alice Ryhl Link: https://lore.kernel.org/r/20240131-doc-fixes-v3-v3-6-0c8af94ed7de@valentinobst.de 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 843ffeec9b3e..fec5c4314758 100644 --- a/rust/kernel/str.rs +++ b/rust/kernel/str.rs @@ -191,9 +191,9 @@ impl CStr { /// ``` /// # use kernel::c_str; /// # use kernel::str::CStr; + /// let bar = c_str!("ツ"); /// // SAFETY: String literals are guaranteed to be valid UTF-8 /// // by the Rust compiler. - /// let bar = c_str!("ツ"); /// assert_eq!(unsafe { bar.as_str_unchecked() }, "ツ"); /// ``` #[inline] -- cgit v1.2.3-70-g09d2 From ebf2b8a75a05a1683596aa73c24c1b5bcd5132ae Mon Sep 17 00:00:00 2001 From: Valentin Obst Date: Wed, 31 Jan 2024 21:23:29 +0100 Subject: rust: kernel: unify spelling of refcount in docs Replace instances of 'ref-count[ed]' with 'refcount[ed]' to increase consistency within the Rust documentation. The latter form is used more widely in the rest of the kernel: ```console $ rg '(\*|//).*?\srefcount(|ed)[\s,.]' | wc -l 1605 $ rg '(\*|//).*?\sref-count(|ed)[\s,.]' | wc -l 43 ``` (numbers are for commit 052d534373b7 ("Merge tag 'exfat-for-6.8-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/linkinjeon/exfat")) Signed-off-by: Valentin Obst Reviewed-by: Trevor Gross Reviewed-by: Martin Rodriguez Reboredo Reviewed-by: Alice Ryhl Link: https://lore.kernel.org/r/20240131-doc-fixes-v3-v3-7-0c8af94ed7de@valentinobst.de [ Reworded to use the kernel's commit description style. ] Signed-off-by: Miguel Ojeda --- rust/kernel/sync/arc.rs | 8 ++++---- rust/kernel/task.rs | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs index 77cdbcf7bd2e..6c46b1affca5 100644 --- a/rust/kernel/sync/arc.rs +++ b/rust/kernel/sync/arc.rs @@ -56,7 +56,7 @@ mod std_vendor; /// b: u32, /// } /// -/// // Create a ref-counted instance of `Example`. +/// // Create a refcounted instance of `Example`. /// let obj = Arc::try_new(Example { a: 10, b: 20 })?; /// /// // Get a new pointer to `obj` and increment the refcount. @@ -510,7 +510,7 @@ impl Deref for ArcBorrow<'_, T> { /// # test().unwrap(); /// ``` /// -/// In the following example we first allocate memory for a ref-counted `Example` but we don't +/// In the following example we first allocate memory for a refcounted `Example` but we don't /// initialise it on allocation. We do initialise it later with a call to [`UniqueArc::write`], /// followed by a conversion to `Arc`. This is particularly useful when allocation happens /// in one context (e.g., sleepable) and initialisation in another (e.g., atomic): @@ -560,7 +560,7 @@ impl UniqueArc { /// Tries to allocate a new [`UniqueArc`] instance. pub fn try_new(value: T) -> Result { Ok(Self { - // INVARIANT: The newly-created object has a ref-count of 1. + // INVARIANT: The newly-created object has a refcount of 1. inner: Arc::try_new(value)?, }) } @@ -574,7 +574,7 @@ impl UniqueArc { data <- init::uninit::(), }? AllocError))?; Ok(UniqueArc { - // INVARIANT: The newly-created object has a ref-count of 1. + // INVARIANT: The newly-created object has a refcount of 1. // SAFETY: The pointer from the `Box` is valid. inner: unsafe { Arc::from_inner(Box::leak(inner).into()) }, }) diff --git a/rust/kernel/task.rs b/rust/kernel/task.rs index a3a4007db682..6726f1056066 100644 --- a/rust/kernel/task.rs +++ b/rust/kernel/task.rs @@ -39,7 +39,7 @@ macro_rules! current { /// /// All instances are valid tasks created by the C portion of the kernel. /// -/// Instances of this type are always ref-counted, that is, a call to `get_task_struct` ensures +/// Instances of this type are always refcounted, that is, a call to `get_task_struct` ensures /// that the allocation remains valid at least until the matching call to `put_task_struct`. /// /// # Examples @@ -163,7 +163,7 @@ impl Task { } } -// SAFETY: The type invariants guarantee that `Task` is always ref-counted. +// SAFETY: The type invariants guarantee that `Task` is always refcounted. unsafe impl crate::types::AlwaysRefCounted for Task { fn inc_ref(&self) { // SAFETY: The existence of a shared reference means that the refcount is nonzero. -- cgit v1.2.3-70-g09d2 From af8b18d7404bfb82aa07a09ad5b53d33ab2955d8 Mon Sep 17 00:00:00 2001 From: Valentin Obst Date: Wed, 31 Jan 2024 21:23:30 +0100 Subject: rust: kernel: mark code fragments in docs with backticks Fix places where comments include code fragments that are not enclosed in backticks. Signed-off-by: Valentin Obst Reviewed-by: Trevor Gross Reviewed-by: Martin Rodriguez Reboredo Reviewed-by: Alice Ryhl Link: https://lore.kernel.org/r/20240131-doc-fixes-v3-v3-8-0c8af94ed7de@valentinobst.de Signed-off-by: Miguel Ojeda --- rust/kernel/ioctl.rs | 2 +- rust/kernel/sync/lock.rs | 2 +- rust/kernel/task.rs | 2 +- rust/kernel/workqueue.rs | 9 +++++---- 4 files changed, 8 insertions(+), 7 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/ioctl.rs b/rust/kernel/ioctl.rs index 5987ad6d38a7..cfa7d080b531 100644 --- a/rust/kernel/ioctl.rs +++ b/rust/kernel/ioctl.rs @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0 -//! ioctl() number definitions. +//! `ioctl()` number definitions. //! //! C header: [`include/asm-generic/ioctl.h`](srctree/include/asm-generic/ioctl.h) diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs index 072b8ef2a0fa..10ccc0e39147 100644 --- a/rust/kernel/sync/lock.rs +++ b/rust/kernel/sync/lock.rs @@ -28,7 +28,7 @@ pub unsafe trait Backend { /// The state required by the lock. type State; - /// The state required to be kept between lock and unlock. + /// The state required to be kept between `lock` and `unlock`. type GuardState; /// Initialises the lock. diff --git a/rust/kernel/task.rs b/rust/kernel/task.rs index 6726f1056066..ca6e7e31d71c 100644 --- a/rust/kernel/task.rs +++ b/rust/kernel/task.rs @@ -132,7 +132,7 @@ impl Task { /// Returns the group leader of the given task. pub fn group_leader(&self) -> &Task { // SAFETY: By the type invariant, we know that `self.0` is a valid task. Valid tasks always - // have a valid group_leader. + // have a valid `group_leader`. let ptr = unsafe { *ptr::addr_of!((*self.0.get()).group_leader) }; // SAFETY: The lifetime of the returned task reference is tied to the lifetime of `self`, diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs index 8775c34d12a5..d900dc911149 100644 --- a/rust/kernel/workqueue.rs +++ b/rust/kernel/workqueue.rs @@ -168,7 +168,7 @@ impl Queue { /// # Safety /// /// The caller must ensure that the provided raw pointer is not dangling, that it points at a - /// valid workqueue, and that it remains valid until the end of 'a. + /// valid workqueue, and that it remains valid until the end of `'a`. pub unsafe fn from_raw<'a>(ptr: *const bindings::workqueue_struct) -> &'a Queue { // SAFETY: The `Queue` type is `#[repr(transparent)]`, so the pointer cast is valid. The // caller promises that the pointer is not dangling. @@ -414,8 +414,8 @@ 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 [`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. /// /// [`Work`]: Work /// [`impl_has_work!`]: crate::impl_has_work @@ -428,7 +428,8 @@ pub unsafe trait HasWork { /// Returns the offset of the [`Work`] field. /// - /// This method exists because the [`OFFSET`] constant cannot be accessed if the type is not Sized. + /// This method exists because the [`OFFSET`] constant cannot be accessed if the type is not + /// `Sized`. /// /// [`Work`]: Work /// [`OFFSET`]: HasWork::OFFSET -- cgit v1.2.3-70-g09d2 From 6269fadf351eafad6f890091eed69875125285b6 Mon Sep 17 00:00:00 2001 From: Valentin Obst Date: Wed, 31 Jan 2024 21:23:31 +0100 Subject: rust: kernel: add blank lines in front of code blocks Throughout the code base, blank lines are used before starting a code block. Adapt outliers to improve consistency within the kernel crate. Signed-off-by: Valentin Obst Reviewed-by: Trevor Gross Reviewed-by: Martin Rodriguez Reboredo Reviewed-by: Alice Ryhl Link: https://lore.kernel.org/r/20240131-doc-fixes-v3-v3-9-0c8af94ed7de@valentinobst.de Signed-off-by: Miguel Ojeda --- rust/kernel/types.rs | 3 +++ 1 file changed, 3 insertions(+) (limited to 'rust/kernel') diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs index fdb778e65d79..8aabe348b194 100644 --- a/rust/kernel/types.rs +++ b/rust/kernel/types.rs @@ -90,6 +90,7 @@ impl ForeignOwnable for () { /// /// In the example below, we have multiple exit paths and we want to log regardless of which one is /// taken: +/// /// ``` /// # use kernel::types::ScopeGuard; /// fn example1(arg: bool) { @@ -108,6 +109,7 @@ impl ForeignOwnable for () { /// /// In the example below, we want to log the same message on all early exits but a different one on /// the main exit path: +/// /// ``` /// # use kernel::types::ScopeGuard; /// fn example2(arg: bool) { @@ -129,6 +131,7 @@ impl ForeignOwnable for () { /// /// In the example below, we need a mutable object (the vector) to be accessible within the log /// function, so we wrap it in the [`ScopeGuard`]: +/// /// ``` /// # use kernel::types::ScopeGuard; /// fn example3(arg: bool) -> Result { -- cgit v1.2.3-70-g09d2 From 4c799d1dc89b5287f82c7d7bdc5039928980b1bd Mon Sep 17 00:00:00 2001 From: Valentin Obst Date: Wed, 31 Jan 2024 21:23:32 +0100 Subject: rust: kernel: add doclinks Add doclinks to existing documentation. Signed-off-by: Valentin Obst Reviewed-by: Trevor Gross Reviewed-by: Martin Rodriguez Reboredo Reviewed-by: Alice Ryhl Link: https://lore.kernel.org/r/20240131-doc-fixes-v3-v3-10-0c8af94ed7de@valentinobst.de Signed-off-by: Miguel Ojeda --- rust/kernel/sync/arc.rs | 6 +++--- rust/kernel/sync/lock.rs | 13 ++++++++++--- rust/kernel/workqueue.rs | 45 +++++++++++++++++++++++++++------------------ 3 files changed, 40 insertions(+), 24 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs index 6c46b1affca5..936bc549a082 100644 --- a/rust/kernel/sync/arc.rs +++ b/rust/kernel/sync/arc.rs @@ -365,12 +365,12 @@ impl From>> for Arc { /// A borrowed reference to an [`Arc`] instance. /// /// For cases when one doesn't ever need to increment the refcount on the allocation, it is simpler -/// to use just `&T`, which we can trivially get from an `Arc` instance. +/// to use just `&T`, which we can trivially get from an [`Arc`] instance. /// /// However, when one may need to increment the refcount, it is preferable to use an `ArcBorrow` /// over `&Arc` because the latter results in a double-indirection: a pointer (shared reference) -/// to a pointer (`Arc`) to the object (`T`). An [`ArcBorrow`] eliminates this double -/// indirection while still allowing one to increment the refcount and getting an `Arc` when/if +/// to a pointer ([`Arc`]) to the object (`T`). An [`ArcBorrow`] eliminates this double +/// indirection while still allowing one to increment the refcount and getting an [`Arc`] when/if /// needed. /// /// # Invariants diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs index 10ccc0e39147..5b5c8efe427a 100644 --- a/rust/kernel/sync/lock.rs +++ b/rust/kernel/sync/lock.rs @@ -21,14 +21,21 @@ pub mod spinlock; /// # Safety /// /// - Implementers must ensure that only one thread/CPU may access the protected data once the lock -/// is owned, that is, between calls to `lock` and `unlock`. -/// - Implementers must also ensure that `relock` uses the same locking method as the original +/// is owned, that is, between calls to [`lock`] and [`unlock`]. +/// - Implementers must also ensure that [`relock`] uses the same locking method as the original /// lock operation. +/// +/// [`lock`]: Backend::lock +/// [`unlock`]: Backend::unlock +/// [`relock`]: Backend::relock pub unsafe trait Backend { /// The state required by the lock. type State; - /// The state required to be kept between `lock` and `unlock`. + /// The state required to be kept between [`lock`] and [`unlock`]. + /// + /// [`lock`]: Backend::lock + /// [`unlock`]: Backend::unlock type GuardState; /// Initialises the lock. diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs index d900dc911149..ed3af3491b47 100644 --- a/rust/kernel/workqueue.rs +++ b/rust/kernel/workqueue.rs @@ -12,19 +12,19 @@ //! //! # The raw API //! -//! The raw API consists of the `RawWorkItem` trait, where the work item needs to provide an +//! The raw API consists of the [`RawWorkItem`] trait, where the work item needs to provide an //! arbitrary function that knows how to enqueue the work item. It should usually not be used //! directly, but if you want to, you can use it without using the pieces from the safe API. //! //! # The safe API //! -//! The safe API is used via the `Work` struct and `WorkItem` traits. Furthermore, it also includes -//! a trait called `WorkItemPointer`, which is usually not used directly by the user. +//! The safe API is used via the [`Work`] struct and [`WorkItem`] traits. Furthermore, it also +//! includes a trait called [`WorkItemPointer`], which is usually not used directly by the user. //! -//! * The `Work` struct is the Rust wrapper for the C `work_struct` type. -//! * The `WorkItem` trait is implemented for structs that can be enqueued to a workqueue. -//! * The `WorkItemPointer` trait is implemented for the pointer type that points at a something -//! that implements `WorkItem`. +//! * The [`Work`] struct is the Rust wrapper for the C `work_struct` type. +//! * The [`WorkItem`] trait is implemented for structs that can be enqueued to a workqueue. +//! * The [`WorkItemPointer`] trait is implemented for the pointer type that points at a something +//! that implements [`WorkItem`]. //! //! ## Example //! @@ -218,7 +218,9 @@ impl Queue { } } -/// A helper type used in `try_spawn`. +/// A helper type used in [`try_spawn`]. +/// +/// [`try_spawn`]: Queue::try_spawn #[pin_data] struct ClosureWork { #[pin] @@ -258,9 +260,11 @@ impl WorkItem for ClosureWork { /// /// # Safety /// -/// Implementers must ensure that any pointers passed to a `queue_work_on` closure by `__enqueue` +/// Implementers must ensure that any pointers passed to a `queue_work_on` closure by [`__enqueue`] /// remain valid for the duration specified in the guarantees section of the documentation for -/// `__enqueue`. +/// [`__enqueue`]. +/// +/// [`__enqueue`]: RawWorkItem::__enqueue pub unsafe trait RawWorkItem { /// The return type of [`Queue::enqueue`]. type EnqueueOutput; @@ -290,10 +294,11 @@ pub unsafe trait RawWorkItem { /// Defines the method that should be called directly when a work item is executed. /// -/// This trait is implemented by `Pin>` and `Arc`, and is mainly intended to be +/// This trait is implemented by `Pin>` and [`Arc`], and is mainly intended to be /// implemented for smart pointer types. For your own structs, you would implement [`WorkItem`] -/// instead. The `run` method on this trait will usually just perform the appropriate -/// `container_of` translation and then call into the `run` method from the [`WorkItem`] trait. +/// instead. The [`run`] method on this trait will usually just perform the appropriate +/// `container_of` translation and then call into the [`run`][WorkItem::run] method from the +/// [`WorkItem`] trait. /// /// This trait is used when the `work_struct` field is defined using the [`Work`] helper. /// @@ -309,8 +314,10 @@ pub unsafe trait WorkItemPointer: RawWorkItem { /// /// # Safety /// - /// The provided `work_struct` pointer must originate from a previous call to `__enqueue` where - /// the `queue_work_on` closure returned true, and the pointer must still be valid. + /// The provided `work_struct` pointer must originate from a previous call to [`__enqueue`] + /// where the `queue_work_on` closure returned true, and the pointer must still be valid. + /// + /// [`__enqueue`]: RawWorkItem::__enqueue unsafe extern "C" fn run(ptr: *mut bindings::work_struct); } @@ -328,12 +335,14 @@ pub trait WorkItem { /// Links for a work item. /// -/// This struct contains a function pointer to the `run` function from the [`WorkItemPointer`] +/// This struct contains a function pointer to the [`run`] function from the [`WorkItemPointer`] /// trait, and defines the linked list pointers necessary to enqueue a work item in a workqueue. /// /// Wraps the kernel's C `struct work_struct`. /// /// This is a helper type used to associate a `work_struct` with the [`WorkItem`] that uses it. +/// +/// [`run`]: WorkItemPointer::run #[repr(transparent)] pub struct Work { work: Opaque, @@ -409,7 +418,7 @@ impl Work { /// } /// ``` /// -/// Note that since the `Work` type is annotated with an id, you can have several `work_struct` +/// Note that since the [`Work`] type is annotated with an id, you can have several `work_struct` /// fields by using a different id for each one. /// /// # Safety @@ -429,7 +438,7 @@ pub unsafe trait HasWork { /// Returns the offset of the [`Work`] field. /// /// This method exists because the [`OFFSET`] constant cannot be accessed if the type is not - /// `Sized`. + /// [`Sized`]. /// /// [`Work`]: Work /// [`OFFSET`]: HasWork::OFFSET -- cgit v1.2.3-70-g09d2 From cd16c41fdefa014c719d9ad04cf88ef5e77edffa Mon Sep 17 00:00:00 2001 From: Valentin Obst Date: Wed, 31 Jan 2024 21:23:33 +0100 Subject: rust: kernel: remove unneeded doclink targets Remove explicit targets for doclinks in cases where rustdoc can determine the correct target by itself. The goal is to reduce unneeded verbosity in the source code. Signed-off-by: Valentin Obst Reviewed-by: Trevor Gross Reviewed-by: Martin Rodriguez Reboredo Reviewed-by: Alice Ryhl Link: https://lore.kernel.org/r/20240131-doc-fixes-v3-v3-11-0c8af94ed7de@valentinobst.de Signed-off-by: Miguel Ojeda --- rust/kernel/workqueue.rs | 10 ---------- 1 file changed, 10 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs index ed3af3491b47..73d6fa544ca6 100644 --- a/rust/kernel/workqueue.rs +++ b/rust/kernel/workqueue.rs @@ -426,13 +426,10 @@ impl Work { /// 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. /// -/// [`Work`]: Work /// [`impl_has_work!`]: crate::impl_has_work /// [`OFFSET`]: HasWork::OFFSET pub unsafe trait HasWork { /// The offset of the [`Work`] field. - /// - /// [`Work`]: Work const OFFSET: usize; /// Returns the offset of the [`Work`] field. @@ -440,7 +437,6 @@ pub unsafe trait HasWork { /// This method exists because the [`OFFSET`] constant cannot be accessed if the type is not /// [`Sized`]. /// - /// [`Work`]: Work /// [`OFFSET`]: HasWork::OFFSET #[inline] fn get_work_offset(&self) -> usize { @@ -452,8 +448,6 @@ pub unsafe trait HasWork { /// # Safety /// /// The provided pointer must point at a valid struct of type `Self`. - /// - /// [`Work`]: Work #[inline] unsafe fn raw_get_work(ptr: *mut Self) -> *mut Work { // SAFETY: The caller promises that the pointer is valid. @@ -465,8 +459,6 @@ pub unsafe trait HasWork { /// # Safety /// /// The pointer must point at a [`Work`] field in a struct of type `Self`. - /// - /// [`Work`]: Work #[inline] unsafe fn work_container_of(ptr: *mut Work) -> *mut Self where @@ -495,8 +487,6 @@ pub unsafe trait HasWork { /// impl HasWork for MyStruct { self.work_field } /// } /// ``` -/// -/// [`HasWork`]: HasWork #[macro_export] macro_rules! impl_has_work { ($(impl$(<$($implarg:ident),*>)? -- cgit v1.2.3-70-g09d2 From ed6d0bed343ee5448f3c7b4884adc54c15e63984 Mon Sep 17 00:00:00 2001 From: Valentin Obst Date: Wed, 31 Jan 2024 21:23:34 +0100 Subject: rust: locked_by: shorten doclink preview Increases readability by removing `super::` from the link preview text. Signed-off-by: Valentin Obst Reviewed-by: Trevor Gross Reviewed-by: Martin Rodriguez Reboredo Reviewed-by: Alice Ryhl Link: https://lore.kernel.org/r/20240131-doc-fixes-v3-v3-12-0c8af94ed7de@valentinobst.de Signed-off-by: Miguel Ojeda --- rust/kernel/sync/locked_by.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/sync/locked_by.rs b/rust/kernel/sync/locked_by.rs index b17ee5cd98f3..babc731bd5f6 100644 --- a/rust/kernel/sync/locked_by.rs +++ b/rust/kernel/sync/locked_by.rs @@ -9,14 +9,17 @@ use core::{cell::UnsafeCell, mem::size_of, ptr}; /// Allows access to some data to be serialised by a lock that does not wrap it. /// /// In most cases, data protected by a lock is wrapped by the appropriate lock type, e.g., -/// [`super::Mutex`] or [`super::SpinLock`]. [`LockedBy`] is meant for cases when this is not -/// possible. For example, if a container has a lock and some data in the contained elements needs +/// [`Mutex`] or [`SpinLock`]. [`LockedBy`] is meant for cases when this is not possible. +/// For example, if a container has a lock and some data in the contained elements needs /// to be protected by the same lock. /// /// [`LockedBy`] wraps the data in lieu of another locking primitive, and only allows access to it /// when the caller shows evidence that the 'external' lock is locked. It panics if the evidence /// refers to the wrong instance of the lock. /// +/// [`Mutex`]: super::Mutex +/// [`SpinLock`]: super::SpinLock +/// /// # Examples /// /// The following is an example for illustrative purposes: `InnerDirectory::bytes_used` is an -- cgit v1.2.3-70-g09d2 From e283ee239220908118d66eea46dd8bb6175767b2 Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Mon, 29 Jan 2024 14:58:37 +0000 Subject: rust: kernel: add reexports for macros Currently, all macros are reexported with #[macro_export] only, which means that to access `new_work!` from the workqueue, you need to import it from the path `kernel::new_work` instead of importing it from the workqueue module like all other items in the workqueue. By adding reexports of the macros, it becomes possible to import the macros from the correct modules. It's still possible to import the macros from the root, but I don't think we can do anything about that. There is no functional change. This is merely a code cleanliness improvement. Signed-off-by: Alice Ryhl Reviewed-by: Trevor Gross Reviewed-by: Martin Rodriguez Reboredo Tested-by: Boqun Feng Link: https://lore.kernel.org/r/20240129145837.1419880-1-aliceryhl@google.com [ Removed new `use kernel::prelude::*`s, reworded title. ] Signed-off-by: Miguel Ojeda --- rust/kernel/init.rs | 6 +++--- rust/kernel/sync.rs | 5 +++-- rust/kernel/sync/condvar.rs | 4 ++-- rust/kernel/sync/lock/mutex.rs | 3 ++- rust/kernel/sync/lock/spinlock.rs | 3 ++- rust/kernel/workqueue.rs | 14 ++++++-------- 6 files changed, 18 insertions(+), 17 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs index 16a99984622c..424257284d16 100644 --- a/rust/kernel/init.rs +++ b/rust/kernel/init.rs @@ -36,7 +36,7 @@ //! //! ```rust //! # #![allow(clippy::disallowed_names)] -//! use kernel::{prelude::*, sync::Mutex, new_mutex}; +//! use kernel::sync::{new_mutex, Mutex}; //! # use core::pin::Pin; //! #[pin_data] //! struct Foo { @@ -56,7 +56,7 @@ //! //! ```rust //! # #![allow(clippy::disallowed_names)] -//! # use kernel::{prelude::*, sync::Mutex, new_mutex}; +//! # use kernel::sync::{new_mutex, Mutex}; //! # use core::pin::Pin; //! # #[pin_data] //! # struct Foo { @@ -79,7 +79,7 @@ //! above method only works for types where you can access the fields. //! //! ```rust -//! # use kernel::{new_mutex, sync::{Arc, Mutex}}; +//! # use kernel::sync::{new_mutex, Arc, Mutex}; //! let mtx: Result>> = Arc::pin_init(new_mutex!(42, "example::mtx")); //! ``` //! diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs index c1fb10fc64f4..c983f63fd56e 100644 --- a/rust/kernel/sync.rs +++ b/rust/kernel/sync.rs @@ -13,8 +13,9 @@ pub mod lock; mod locked_by; pub use arc::{Arc, ArcBorrow, UniqueArc}; -pub use condvar::{CondVar, CondVarTimeoutResult}; -pub use lock::{mutex::Mutex, spinlock::SpinLock}; +pub use condvar::{new_condvar, CondVar, CondVarTimeoutResult}; +pub use lock::mutex::{new_mutex, Mutex}; +pub use lock::spinlock::{new_spinlock, SpinLock}; pub use locked_by::LockedBy; /// Represents a lockdep class. It's a wrapper around C's `lock_class_key`. diff --git a/rust/kernel/sync/condvar.rs b/rust/kernel/sync/condvar.rs index fa1794972aa0..0c3671caffeb 100644 --- a/rust/kernel/sync/condvar.rs +++ b/rust/kernel/sync/condvar.rs @@ -27,6 +27,7 @@ macro_rules! new_condvar { $crate::sync::CondVar::new($crate::optional_name!($($name)?), $crate::static_lock_class!()) }; } +pub use new_condvar; /// A conditional variable. /// @@ -44,8 +45,7 @@ macro_rules! new_condvar { /// The following is an example of using a condvar with a mutex: /// /// ``` -/// use kernel::sync::{CondVar, Mutex}; -/// use kernel::{new_condvar, new_mutex}; +/// use kernel::sync::{new_condvar, new_mutex, CondVar, Mutex}; /// /// #[pin_data] /// pub struct Example { diff --git a/rust/kernel/sync/lock/mutex.rs b/rust/kernel/sync/lock/mutex.rs index 8c524a3ec45a..ef4c4634d294 100644 --- a/rust/kernel/sync/lock/mutex.rs +++ b/rust/kernel/sync/lock/mutex.rs @@ -17,6 +17,7 @@ macro_rules! new_mutex { $inner, $crate::optional_name!($($name)?), $crate::static_lock_class!()) }; } +pub use new_mutex; /// A mutual exclusion primitive. /// @@ -35,7 +36,7 @@ macro_rules! new_mutex { /// contains an inner struct (`Inner`) that is protected by a mutex. /// /// ``` -/// use kernel::{init::InPlaceInit, init::PinInit, new_mutex, pin_init, sync::Mutex}; +/// use kernel::sync::{new_mutex, Mutex}; /// /// struct Inner { /// a: u32, diff --git a/rust/kernel/sync/lock/spinlock.rs b/rust/kernel/sync/lock/spinlock.rs index e5e0bf621988..0b22c635634f 100644 --- a/rust/kernel/sync/lock/spinlock.rs +++ b/rust/kernel/sync/lock/spinlock.rs @@ -17,6 +17,7 @@ macro_rules! new_spinlock { $inner, $crate::optional_name!($($name)?), $crate::static_lock_class!()) }; } +pub use new_spinlock; /// A spinlock. /// @@ -33,7 +34,7 @@ macro_rules! new_spinlock { /// contains an inner struct (`Inner`) that is protected by a spinlock. /// /// ``` -/// use kernel::{init::InPlaceInit, init::PinInit, new_spinlock, pin_init, sync::SpinLock}; +/// use kernel::sync::{new_spinlock, SpinLock}; /// /// struct Inner { /// a: u32, diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs index 73d6fa544ca6..544f0c51f1b7 100644 --- a/rust/kernel/workqueue.rs +++ b/rust/kernel/workqueue.rs @@ -35,8 +35,7 @@ //! ``` //! use kernel::prelude::*; //! use kernel::sync::Arc; -//! use kernel::workqueue::{self, Work, WorkItem}; -//! use kernel::{impl_has_work, new_work}; +//! use kernel::workqueue::{self, impl_has_work, new_work, Work, WorkItem}; //! //! #[pin_data] //! struct MyStruct { @@ -78,8 +77,7 @@ //! ``` //! use kernel::prelude::*; //! use kernel::sync::Arc; -//! use kernel::workqueue::{self, Work, WorkItem}; -//! use kernel::{impl_has_work, new_work}; +//! use kernel::workqueue::{self, impl_has_work, new_work, Work, WorkItem}; //! //! #[pin_data] //! struct MyStruct { @@ -147,6 +145,7 @@ macro_rules! new_work { $crate::workqueue::Work::new($crate::optional_name!($($name)?), $crate::static_lock_class!()) }; } +pub use new_work; /// A kernel work queue. /// @@ -405,9 +404,8 @@ impl Work { /// like this: /// /// ```no_run -/// use kernel::impl_has_work; /// use kernel::prelude::*; -/// use kernel::workqueue::Work; +/// use kernel::workqueue::{impl_has_work, Work}; /// /// struct MyWorkItem { /// work_field: Work, @@ -475,9 +473,8 @@ pub unsafe trait HasWork { /// # Examples /// /// ``` -/// use kernel::impl_has_work; /// use kernel::sync::Arc; -/// use kernel::workqueue::{self, Work}; +/// use kernel::workqueue::{self, impl_has_work, Work}; /// /// struct MyStruct { /// work_field: Work, @@ -509,6 +506,7 @@ macro_rules! impl_has_work { } )*}; } +pub use impl_has_work; impl_has_work! { impl HasWork for ClosureWork { self.work } -- cgit v1.2.3-70-g09d2 From 44f2e626cbf777f3035aef7fa379ce34bf2f57e8 Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Thu, 15 Feb 2024 10:46:01 +0000 Subject: rust: kernel: stop using ptr_metadata feature The `byte_sub` method was stabilized in Rust 1.75.0. By using that method, we no longer need the unstable `ptr_metadata` feature for implementing `Arc::from_raw`. This brings us one step closer towards not using unstable compiler features. Reviewed-by: Benno Lossin Reviewed-by: Martin Rodriguez Reboredo Reviewed-by: Trevor Gross Signed-off-by: Alice Ryhl Link: https://lore.kernel.org/r/20240215104601.1267763-1-aliceryhl@google.com [ Reworded title. ] Signed-off-by: Miguel Ojeda --- rust/kernel/lib.rs | 1 - rust/kernel/sync/arc.rs | 16 +++++++--------- 2 files changed, 7 insertions(+), 10 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs index 74581c958a6c..1e5f229b8263 100644 --- a/rust/kernel/lib.rs +++ b/rust/kernel/lib.rs @@ -17,7 +17,6 @@ #![feature(dispatch_from_dyn)] #![feature(new_uninit)] #![feature(offset_of)] -#![feature(ptr_metadata)] #![feature(receiver_trait)] #![feature(unsize)] diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs index 936bc549a082..7d4c4bf58388 100644 --- a/rust/kernel/sync/arc.rs +++ b/rust/kernel/sync/arc.rs @@ -30,7 +30,7 @@ use core::{ mem::{ManuallyDrop, MaybeUninit}, ops::{Deref, DerefMut}, pin::Pin, - ptr::{NonNull, Pointee}, + ptr::NonNull, }; use macros::pin_data; @@ -239,22 +239,20 @@ impl Arc { // binary, so its layout is not so large that it can trigger arithmetic overflow. let val_offset = unsafe { refcount_layout.extend(val_layout).unwrap_unchecked().1 }; - let metadata: ::Metadata = core::ptr::metadata(ptr); - // SAFETY: The metadata of `T` and `ArcInner` is the same because `ArcInner` is a struct - // with `T` as its last field. + // Pointer casts leave the metadata unchanged. This is okay because the metadata of `T` and + // `ArcInner` is the same since `ArcInner` is a struct with `T` as its last field. // // This is documented at: // . - let metadata: as Pointee>::Metadata = - unsafe { core::mem::transmute_copy(&metadata) }; + let ptr = ptr as *const ArcInner; + // SAFETY: The pointer is in-bounds of an allocation both before and after offsetting the // pointer, since it originates from a previous call to `Arc::into_raw` and is still valid. - let ptr = unsafe { (ptr as *mut u8).sub(val_offset) as *mut () }; - let ptr = core::ptr::from_raw_parts_mut(ptr, metadata); + let ptr = unsafe { ptr.byte_sub(val_offset) }; // SAFETY: By the safety requirements we know that `ptr` came from `Arc::into_raw`, so the // reference count held then will be owned by the new `Arc` object. - unsafe { Self::from_inner(NonNull::new_unchecked(ptr)) } + unsafe { Self::from_inner(NonNull::new_unchecked(ptr.cast_mut())) } } /// Returns an [`ArcBorrow`] from the given [`Arc`]. -- cgit v1.2.3-70-g09d2 From 5bc818419a551ab1fcdbff3c5d8b6043527d90dd Mon Sep 17 00:00:00 2001 From: Obei Sideg Date: Mon, 29 Jan 2024 06:45:27 +0000 Subject: rust: types: add `try_from_foreign()` method Currently `ForeignOwnable::from_foreign()` only works for non-null pointers for the existing `impl`s (e.g. `Box`, `Arc`). In turn, this means callers may write code like: ```rust // `p` is a pointer that may be null. if p.is_null() { None } else { unsafe { Some(Self::from_foreign(ptr)) } } ``` Add a `try_from_foreign()` method to the trait with a default implementation that returns `None` if `ptr` is null, otherwise `Some(from_foreign(ptr))`, so that it can be used by callers instead. Link: https://github.com/Rust-for-Linux/linux/issues/1057 Signed-off-by: Obei Sideg Reviewed-by: Alice Ryhl Reviewed-by: Trevor Gross Link: https://lore.kernel.org/r/0100018d53f737f8-80c1fe97-0019-40d7-ab69-b1b192785cd7-000000@email.amazonses.com [ Fixed intra-doc links, improved `SAFETY` comment and reworded commit. ] Signed-off-by: Miguel Ojeda --- rust/kernel/types.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) (limited to 'rust/kernel') diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs index 8aabe348b194..aa77bad9bce4 100644 --- a/rust/kernel/types.rs +++ b/rust/kernel/types.rs @@ -46,6 +46,25 @@ pub trait ForeignOwnable: Sized { /// Additionally, all instances (if any) of values returned by [`ForeignOwnable::borrow`] for /// this object must have been dropped. unsafe fn from_foreign(ptr: *const core::ffi::c_void) -> Self; + + /// Tries to convert a foreign-owned object back to a Rust-owned one. + /// + /// A convenience wrapper over [`ForeignOwnable::from_foreign`] that returns [`None`] if `ptr` + /// is null. + /// + /// # Safety + /// + /// `ptr` must either be null or satisfy the safety requirements for + /// [`ForeignOwnable::from_foreign`]. + unsafe fn try_from_foreign(ptr: *const core::ffi::c_void) -> Option { + if ptr.is_null() { + None + } else { + // SAFETY: Since `ptr` is not null here, then `ptr` satisfies the safety requirements + // of `from_foreign` given the safety requirements of this function. + unsafe { Some(Self::from_foreign(ptr)) } + } + } } impl ForeignOwnable for Box { -- cgit v1.2.3-70-g09d2 From 4951ddd51b816b5e4095cd3cd3bd46fb73b96a65 Mon Sep 17 00:00:00 2001 From: Yutaro Ohno Date: Thu, 8 Feb 2024 18:55:36 +0900 Subject: rust: str: implement `Display` and `Debug` for `BStr` Currently, `BStr` is just a type alias of `[u8]`, limiting its representation to a byte list rather than a character list, which is not ideal for printing and debugging. Implement `Display` and `Debug` traits for `BStr` to facilitate easier printing and debugging. Also, for this purpose, change `BStr` from a type alias of `[u8]` to a struct wrapper of `[u8]`. Co-developed-by: Virgile Andreani Signed-off-by: Virgile Andreani Signed-off-by: Yutaro Ohno Reviewed-by: Alice Ryhl Link: https://lore.kernel.org/r/ZcSlGMGP-e9HqybA@ohnotp [ Formatted code comment. ] Signed-off-by: Miguel Ojeda --- rust/kernel/str.rs | 185 +++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 178 insertions(+), 7 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs index fec5c4314758..925ced8fdc61 100644 --- a/rust/kernel/str.rs +++ b/rust/kernel/str.rs @@ -13,9 +13,102 @@ use crate::{ }; /// Byte string without UTF-8 validity guarantee. -/// -/// `BStr` is simply an alias to `[u8]`, but has a more evident semantical meaning. -pub type BStr = [u8]; +#[repr(transparent)] +pub struct BStr([u8]); + +impl BStr { + /// Returns the length of this string. + #[inline] + pub const fn len(&self) -> usize { + self.0.len() + } + + /// Returns `true` if the string is empty. + #[inline] + pub const fn is_empty(&self) -> bool { + self.len() == 0 + } + + /// Creates a [`BStr`] from a `[u8]`. + #[inline] + pub const fn from_bytes(bytes: &[u8]) -> &Self { + // SAFETY: `BStr` is transparent to `[u8]`. + unsafe { &*(bytes as *const [u8] as *const BStr) } + } +} + +impl fmt::Display for BStr { + /// Formats printable ASCII characters, escaping the rest. + /// + /// ``` + /// # use kernel::{fmt, b_str, str::{BStr, CString}}; + /// let ascii = b_str!("Hello, BStr!"); + /// let s = CString::try_from_fmt(fmt!("{}", ascii)).unwrap(); + /// assert_eq!(s.as_bytes(), "Hello, BStr!".as_bytes()); + /// + /// let non_ascii = b_str!("🦀"); + /// let s = CString::try_from_fmt(fmt!("{}", non_ascii)).unwrap(); + /// assert_eq!(s.as_bytes(), "\\xf0\\x9f\\xa6\\x80".as_bytes()); + /// ``` + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + for &b in &self.0 { + match b { + // Common escape codes. + b'\t' => f.write_str("\\t")?, + b'\n' => f.write_str("\\n")?, + b'\r' => f.write_str("\\r")?, + // Printable characters. + 0x20..=0x7e => f.write_char(b as char)?, + _ => write!(f, "\\x{:02x}", b)?, + } + } + Ok(()) + } +} + +impl fmt::Debug for BStr { + /// Formats printable ASCII characters with a double quote on either end, + /// escaping the rest. + /// + /// ``` + /// # use kernel::{fmt, b_str, str::{BStr, CString}}; + /// // Embedded double quotes are escaped. + /// let ascii = b_str!("Hello, \"BStr\"!"); + /// let s = CString::try_from_fmt(fmt!("{:?}", ascii)).unwrap(); + /// assert_eq!(s.as_bytes(), "\"Hello, \\\"BStr\\\"!\"".as_bytes()); + /// + /// let non_ascii = b_str!("😺"); + /// let s = CString::try_from_fmt(fmt!("{:?}", non_ascii)).unwrap(); + /// assert_eq!(s.as_bytes(), "\"\\xf0\\x9f\\x98\\xba\"".as_bytes()); + /// ``` + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_char('"')?; + for &b in &self.0 { + match b { + // Common escape codes. + b'\t' => f.write_str("\\t")?, + b'\n' => f.write_str("\\n")?, + b'\r' => f.write_str("\\r")?, + // String escape characters. + b'\"' => f.write_str("\\\"")?, + b'\\' => f.write_str("\\\\")?, + // Printable characters. + 0x20..=0x7e => f.write_char(b as char)?, + _ => write!(f, "\\x{:02x}", b)?, + } + } + f.write_char('"') + } +} + +impl Deref for BStr { + type Target = [u8]; + + #[inline] + fn deref(&self) -> &Self::Target { + &self.0 + } +} /// Creates a new [`BStr`] from a string literal. /// @@ -33,7 +126,7 @@ pub type BStr = [u8]; macro_rules! b_str { ($str:literal) => {{ const S: &'static str = $str; - const C: &'static $crate::str::BStr = S.as_bytes(); + const C: &'static $crate::str::BStr = $crate::str::BStr::from_bytes(S.as_bytes()); C }}; } @@ -271,7 +364,7 @@ impl fmt::Debug for CStr { impl AsRef for CStr { #[inline] fn as_ref(&self) -> &BStr { - self.as_bytes() + BStr::from_bytes(self.as_bytes()) } } @@ -280,7 +373,7 @@ impl Deref for CStr { #[inline] fn deref(&self) -> &Self::Target { - self.as_bytes() + self.as_ref() } } @@ -327,7 +420,7 @@ where #[inline] fn index(&self, index: Idx) -> &Self::Output { - &self.as_bytes()[index] + &self.as_ref()[index] } } @@ -357,6 +450,21 @@ macro_rules! c_str { #[cfg(test)] mod tests { use super::*; + use alloc::format; + + const ALL_ASCII_CHARS: &'static str = + "\\x01\\x02\\x03\\x04\\x05\\x06\\x07\\x08\\x09\\x0a\\x0b\\x0c\\x0d\\x0e\\x0f\ + \\x10\\x11\\x12\\x13\\x14\\x15\\x16\\x17\\x18\\x19\\x1a\\x1b\\x1c\\x1d\\x1e\\x1f \ + !\"#$%&'()*+,-./0123456789:;<=>?@\ + ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~\\x7f\ + \\x80\\x81\\x82\\x83\\x84\\x85\\x86\\x87\\x88\\x89\\x8a\\x8b\\x8c\\x8d\\x8e\\x8f\ + \\x90\\x91\\x92\\x93\\x94\\x95\\x96\\x97\\x98\\x99\\x9a\\x9b\\x9c\\x9d\\x9e\\x9f\ + \\xa0\\xa1\\xa2\\xa3\\xa4\\xa5\\xa6\\xa7\\xa8\\xa9\\xaa\\xab\\xac\\xad\\xae\\xaf\ + \\xb0\\xb1\\xb2\\xb3\\xb4\\xb5\\xb6\\xb7\\xb8\\xb9\\xba\\xbb\\xbc\\xbd\\xbe\\xbf\ + \\xc0\\xc1\\xc2\\xc3\\xc4\\xc5\\xc6\\xc7\\xc8\\xc9\\xca\\xcb\\xcc\\xcd\\xce\\xcf\ + \\xd0\\xd1\\xd2\\xd3\\xd4\\xd5\\xd6\\xd7\\xd8\\xd9\\xda\\xdb\\xdc\\xdd\\xde\\xdf\ + \\xe0\\xe1\\xe2\\xe3\\xe4\\xe5\\xe6\\xe7\\xe8\\xe9\\xea\\xeb\\xec\\xed\\xee\\xef\ + \\xf0\\xf1\\xf2\\xf3\\xf4\\xf5\\xf6\\xf7\\xf8\\xf9\\xfa\\xfb\\xfc\\xfd\\xfe\\xff"; #[test] fn test_cstr_to_str() { @@ -381,6 +489,69 @@ mod tests { let unchecked_str = unsafe { checked_cstr.as_str_unchecked() }; assert_eq!(unchecked_str, "🐧"); } + + #[test] + fn test_cstr_display() { + let hello_world = CStr::from_bytes_with_nul(b"hello, world!\0").unwrap(); + assert_eq!(format!("{}", hello_world), "hello, world!"); + let non_printables = CStr::from_bytes_with_nul(b"\x01\x09\x0a\0").unwrap(); + assert_eq!(format!("{}", non_printables), "\\x01\\x09\\x0a"); + let non_ascii = CStr::from_bytes_with_nul(b"d\xe9j\xe0 vu\0").unwrap(); + assert_eq!(format!("{}", non_ascii), "d\\xe9j\\xe0 vu"); + let good_bytes = CStr::from_bytes_with_nul(b"\xf0\x9f\xa6\x80\0").unwrap(); + assert_eq!(format!("{}", good_bytes), "\\xf0\\x9f\\xa6\\x80"); + } + + #[test] + fn test_cstr_display_all_bytes() { + 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(); + assert_eq!(format!("{}", cstr), ALL_ASCII_CHARS); + } + + #[test] + fn test_cstr_debug() { + let hello_world = CStr::from_bytes_with_nul(b"hello, world!\0").unwrap(); + assert_eq!(format!("{:?}", hello_world), "\"hello, world!\""); + let non_printables = CStr::from_bytes_with_nul(b"\x01\x09\x0a\0").unwrap(); + assert_eq!(format!("{:?}", non_printables), "\"\\x01\\x09\\x0a\""); + let non_ascii = CStr::from_bytes_with_nul(b"d\xe9j\xe0 vu\0").unwrap(); + assert_eq!(format!("{:?}", non_ascii), "\"d\\xe9j\\xe0 vu\""); + let good_bytes = CStr::from_bytes_with_nul(b"\xf0\x9f\xa6\x80\0").unwrap(); + assert_eq!(format!("{:?}", good_bytes), "\"\\xf0\\x9f\\xa6\\x80\""); + } + + #[test] + fn test_bstr_display() { + 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_\\_\'_\"_"); + assert_eq!(format!("{}", escapes), "_\\t_\\n_\\r_\\_'_\"_"); + let others = BStr::from_bytes(b"\x01"); + assert_eq!(format!("{}", others), "\\x01"); + let non_ascii = BStr::from_bytes(b"d\xe9j\xe0 vu"); + 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"); + } + + #[test] + fn test_bstr_debug() { + 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_\\_\'_\"_"); + assert_eq!(format!("{:?}", escapes), "\"_\\t_\\n_\\r_\\\\_'_\\\"_\""); + let others = BStr::from_bytes(b"\x01"); + assert_eq!(format!("{:?}", others), "\"\\x01\""); + let non_ascii = BStr::from_bytes(b"d\xe9j\xe0 vu"); + 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\""); + } } /// Allows formatting of [`fmt::Arguments`] into a raw buffer. -- cgit v1.2.3-70-g09d2 From e944171070b65521c0513c01c56174ec8fd7d249 Mon Sep 17 00:00:00 2001 From: Wedson Almeida Filho Date: Mon, 19 Feb 2024 11:48:08 +0000 Subject: rust: add `container_of!` macro This macro is used to obtain a pointer to an entire struct when given a pointer to a field in that struct. Signed-off-by: Wedson Almeida Filho Reviewed-by: Alice Ryhl Tested-by: Alice Ryhl Signed-off-by: Matt Gilbride Link: https://lore.kernel.org/r/20240219-b4-rbtree-v2-1-0b113aab330d@google.com Signed-off-by: Miguel Ojeda --- rust/kernel/lib.rs | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) (limited to 'rust/kernel') diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs index 1e5f229b8263..be68d5e567b1 100644 --- a/rust/kernel/lib.rs +++ b/rust/kernel/lib.rs @@ -101,3 +101,35 @@ fn panic(info: &core::panic::PanicInfo<'_>) -> ! { // SAFETY: FFI call. unsafe { bindings::BUG() }; } + +/// Produces a pointer to an object from a pointer to one of its fields. +/// +/// # Safety +/// +/// The pointer passed to this macro, and the pointer returned by this macro, must both be in +/// bounds of the same allocation. +/// +/// # Examples +/// +/// ``` +/// # use kernel::container_of; +/// struct Test { +/// a: u64, +/// b: u32, +/// } +/// +/// let test = Test { a: 10, b: 20 }; +/// let b_ptr = &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) }; +/// assert!(core::ptr::eq(&test, test_alias)); +/// ``` +#[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 + }} +} -- cgit v1.2.3-70-g09d2