From fcad9bbf9e1a7de6c53908954ba1b1a1ab11ef1e Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Sun, 15 Jun 2025 16:55:05 -0400 Subject: rust: enable `clippy::ptr_as_ptr` lint In Rust 1.51.0, Clippy introduced the `ptr_as_ptr` lint [1]: > Though `as` casts between raw pointers are not terrible, > `pointer::cast` is safer because it cannot accidentally change the > pointer's mutability, nor cast the pointer to other types like `usize`. There are a few classes of changes required: - Modules generated by bindgen are marked `#[allow(clippy::ptr_as_ptr)]`. - Inferred casts (` as _`) are replaced with `.cast()`. - Ascribed casts (` as *... T`) are replaced with `.cast::()`. - Multistep casts from references (` as *const _ as *const T`) are replaced with `core::ptr::from_ref(&x).cast()` with or without `::` according to the previous rules. The `core::ptr::from_ref` call is required because `(x as *const _).cast::()` results in inference failure. - Native literal C strings are replaced with `c_str!().as_char_ptr()`. - `*mut *mut T as _` is replaced with `let *mut *const T = (*mut *mut T)`.cast();` since pointer to pointer can be confusing. Apply these changes and enable the lint -- no functional change intended. Link: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_as_ptr [1] Reviewed-by: Benno Lossin Reviewed-by: Boqun Feng Signed-off-by: Tamir Duberstein Acked-by: Viresh Kumar Acked-by: Greg Kroah-Hartman Acked-by: Tejun Heo Acked-by: Danilo Krummrich Link: https://lore.kernel.org/r/20250615-ptr-as-ptr-v12-1-f43b024581e8@gmail.com [ Added `.cast()` for `opp`. - Miguel ] Signed-off-by: Miguel Ojeda --- rust/kernel/sync/poll.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'rust/kernel/sync') diff --git a/rust/kernel/sync/poll.rs b/rust/kernel/sync/poll.rs index d7e6e59e124b..339ab6097be7 100644 --- a/rust/kernel/sync/poll.rs +++ b/rust/kernel/sync/poll.rs @@ -73,7 +73,7 @@ impl PollTable { // be destroyed, the destructor must run. That destructor first removes all waiters, // and then waits for an rcu grace period. Therefore, `cv.wait_queue_head` is valid for // long enough. - unsafe { qproc(file.as_ptr() as _, cv.wait_queue_head.get(), self.0.get()) }; + unsafe { qproc(file.as_ptr().cast(), cv.wait_queue_head.get(), self.0.get()) }; } } } -- cgit v1.2.3 From 2009a2d5696944d85c34d75e691a6f3884e787c0 Mon Sep 17 00:00:00 2001 From: Alexandre Courbot Date: Mon, 16 Jun 2025 12:34:06 +0900 Subject: rust: sync: implement `Borrow` and `BorrowMut` for `Arc` types Implement `Borrow` and `BorrowMut` for `UniqueArc`, and `Borrow` for `Arc`. This allows these containers to be used in generic APIs asking for types implementing those traits. `T` and `&mut T` also implement those traits allowing users to use either owned, shared or borrowed values. `ForeignOwnable` makes a call to its own `borrow` method which must be disambiguated. Reviewed-by: Alice Ryhl Reviewed-by: Benno Lossin Signed-off-by: Alexandre Courbot Link: https://lore.kernel.org/r/20250616-borrow_impls-v4-2-36f9beb3fe6a@nvidia.com Signed-off-by: Miguel Ojeda --- rust/kernel/sync/arc.rs | 78 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 77 insertions(+), 1 deletion(-) (limited to 'rust/kernel/sync') diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs index c7af0aa48a0a..499175f637a7 100644 --- a/rust/kernel/sync/arc.rs +++ b/rust/kernel/sync/arc.rs @@ -25,6 +25,7 @@ use crate::{ }; use core::{ alloc::Layout, + borrow::{Borrow, BorrowMut}, fmt, marker::PhantomData, mem::{ManuallyDrop, MaybeUninit}, @@ -406,7 +407,7 @@ unsafe impl ForeignOwnable for Arc { 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) } + unsafe { ::borrow(ptr) } } } @@ -426,6 +427,31 @@ impl AsRef for Arc { } } +/// # Examples +/// +/// ``` +/// # use core::borrow::Borrow; +/// # use kernel::sync::Arc; +/// struct Foo>(B); +/// +/// // Owned instance. +/// let owned = Foo(1); +/// +/// // Shared instance. +/// let arc = Arc::new(1, GFP_KERNEL)?; +/// let shared = Foo(arc.clone()); +/// +/// let i = 1; +/// // Borrowed from `i`. +/// let borrowed = Foo(&i); +/// # Ok::<(), Error>(()) +/// ``` +impl Borrow for Arc { + fn borrow(&self) -> &T { + self.deref() + } +} + impl Clone for Arc { fn clone(&self) -> Self { // SAFETY: By the type invariant, there is necessarily a reference to the object, so it is @@ -834,6 +860,56 @@ impl DerefMut for UniqueArc { } } +/// # Examples +/// +/// ``` +/// # use core::borrow::Borrow; +/// # use kernel::sync::UniqueArc; +/// struct Foo>(B); +/// +/// // Owned instance. +/// let owned = Foo(1); +/// +/// // Owned instance using `UniqueArc`. +/// let arc = UniqueArc::new(1, GFP_KERNEL)?; +/// let shared = Foo(arc); +/// +/// let i = 1; +/// // Borrowed from `i`. +/// let borrowed = Foo(&i); +/// # Ok::<(), Error>(()) +/// ``` +impl Borrow for UniqueArc { + fn borrow(&self) -> &T { + self.deref() + } +} + +/// # Examples +/// +/// ``` +/// # use core::borrow::BorrowMut; +/// # use kernel::sync::UniqueArc; +/// struct Foo>(B); +/// +/// // Owned instance. +/// let owned = Foo(1); +/// +/// // Owned instance using `UniqueArc`. +/// let arc = UniqueArc::new(1, GFP_KERNEL)?; +/// let shared = Foo(arc); +/// +/// let mut i = 1; +/// // Borrowed from `i`. +/// let borrowed = Foo(&mut i); +/// # Ok::<(), Error>(()) +/// ``` +impl BorrowMut for UniqueArc { + fn borrow_mut(&mut self) -> &mut T { + self.deref_mut() + } +} + impl fmt::Display for UniqueArc { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { fmt::Display::fmt(self.deref(), f) -- cgit v1.2.3 From 12717ebeffcf3e34063dbc1e1b7f34924150c7c9 Mon Sep 17 00:00:00 2001 From: Andreas Hindborg Date: Thu, 12 Jun 2025 15:09:43 +0200 Subject: rust: types: add FOREIGN_ALIGN to ForeignOwnable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The current implementation of `ForeignOwnable` is leaking the type of the opaque pointer to consumers of the API. This allows consumers of the opaque pointer to rely on the information that can be extracted from the pointer type. To prevent this, change the API to the version suggested by Maira Canal (link below): Remove `ForeignOwnable::PointedTo` in favor of a constant, which specifies the alignment of the pointers returned by `into_foreign`. With this change, `ArcInner` no longer needs `pub` visibility, so change it to private. Suggested-by: Alice Ryhl Suggested-by: Maíra Canal Link: https://lore.kernel.org/r/20240309235927.168915-3-mcanal@igalia.com Acked-by: Danilo Krummrich Reviewed-by: Benno Lossin Signed-off-by: Andreas Hindborg Reviewed-by: Alice Ryhl Link: https://lore.kernel.org/r/20250612-pointed-to-v3-1-b009006d86a1@kernel.org Signed-off-by: Miguel Ojeda --- rust/kernel/sync/arc.rs | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) (limited to 'rust/kernel/sync') diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs index 499175f637a7..63a66761d0c7 100644 --- a/rust/kernel/sync/arc.rs +++ b/rust/kernel/sync/arc.rs @@ -19,6 +19,7 @@ use crate::{ alloc::{AllocError, Flags, KBox}, bindings, + ffi::c_void, init::InPlaceInit, try_init, types::{ForeignOwnable, Opaque}, @@ -141,10 +142,9 @@ pub struct Arc { _p: PhantomData>, } -#[doc(hidden)] #[pin_data] #[repr(C)] -pub struct ArcInner { +struct ArcInner { refcount: Opaque, data: T, } @@ -373,20 +373,22 @@ impl Arc { } } -// SAFETY: The `into_foreign` function returns a pointer that is well-aligned. +// SAFETY: The pointer returned by `into_foreign` comes from a well aligned +// pointer to `ArcInner`. unsafe impl ForeignOwnable for Arc { - type PointedTo = ArcInner; + const FOREIGN_ALIGN: usize = core::mem::align_of::>(); + type Borrowed<'a> = ArcBorrow<'a, T>; type BorrowedMut<'a> = Self::Borrowed<'a>; - fn into_foreign(self) -> *mut Self::PointedTo { - ManuallyDrop::new(self).ptr.as_ptr() + fn into_foreign(self) -> *mut c_void { + ManuallyDrop::new(self).ptr.as_ptr().cast() } - unsafe fn from_foreign(ptr: *mut Self::PointedTo) -> Self { + unsafe fn from_foreign(ptr: *mut c_void) -> 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) }; + let inner = unsafe { NonNull::new_unchecked(ptr.cast::>()) }; // 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 @@ -394,17 +396,17 @@ unsafe impl ForeignOwnable for Arc { unsafe { Self::from_inner(inner) } } - unsafe fn borrow<'a>(ptr: *mut Self::PointedTo) -> ArcBorrow<'a, T> { + unsafe fn borrow<'a>(ptr: *mut c_void) -> 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) }; + let inner = unsafe { NonNull::new_unchecked(ptr.cast::>()) }; // 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 Self::PointedTo) -> ArcBorrow<'a, T> { + unsafe fn borrow_mut<'a>(ptr: *mut c_void) -> ArcBorrow<'a, T> { // SAFETY: The safety requirements for `borrow_mut` are a superset of the safety // requirements for `borrow`. unsafe { ::borrow(ptr) } -- cgit v1.2.3 From 07dad44aa9a93b16af19e8609a10b241c352b440 Mon Sep 17 00:00:00 2001 From: Shankari Anand Date: Tue, 15 Jul 2025 16:34:23 +0530 Subject: rust: kernel: move ARef and AlwaysRefCounted to sync::aref Move the definitions of `ARef` and `AlwaysRefCounted` from `types.rs` to a new file `sync/aref.rs`. Define the corresponding `aref` module under `rust/kernel/sync.rs`. These types are better grouped in `sync`. To avoid breaking existing imports, they are re-exported from `types.rs`. Drop unused imports `mem::ManuallyDrop`, `ptr::NonNull` from `types.rs`, they are now only used in `sync/aref.rs`, where they are already imported. Suggested-by: Benno Lossin Link: https://github.com/Rust-for-Linux/linux/issues/1173 Signed-off-by: Shankari Anand Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20250715110423.334744-1-shankari.ak0208@gmail.com [ Added missing `///`. Changed module title. Reworded slightly. - Miguel ] Signed-off-by: Miguel Ojeda --- rust/kernel/sync/aref.rs | 154 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 154 insertions(+) create mode 100644 rust/kernel/sync/aref.rs (limited to 'rust/kernel/sync') diff --git a/rust/kernel/sync/aref.rs b/rust/kernel/sync/aref.rs new file mode 100644 index 000000000000..dbd77bb68617 --- /dev/null +++ b/rust/kernel/sync/aref.rs @@ -0,0 +1,154 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! Internal reference counting support. + +use core::{marker::PhantomData, mem::ManuallyDrop, ops::Deref, ptr::NonNull}; + +/// Types that are _always_ reference counted. +/// +/// It allows such types to define their own custom ref increment and decrement functions. +/// Additionally, it allows users to convert from a shared reference `&T` to an owned reference +/// [`ARef`]. +/// +/// This is usually implemented by wrappers to existing structures on the C side of the code. For +/// Rust code, the recommendation is to use [`Arc`](crate::sync::Arc) to create reference-counted +/// instances of a type. +/// +/// # Safety +/// +/// Implementers must ensure that increments to the reference count keep the object alive in memory +/// at least until matching decrements are performed. +/// +/// Implementers must also ensure that all instances are reference-counted. (Otherwise they +/// won't be able to honour the requirement that [`AlwaysRefCounted::inc_ref`] keep the object +/// alive.) +pub unsafe trait AlwaysRefCounted { + /// Increments the reference count on the object. + fn inc_ref(&self); + + /// Decrements the reference count on the object. + /// + /// Frees the object when the count reaches zero. + /// + /// # Safety + /// + /// Callers must ensure that there was a previous matching increment to the reference count, + /// and that the object is no longer used after its reference count is decremented (as it may + /// result in the object being freed), unless the caller owns another increment on the refcount + /// (e.g., it calls [`AlwaysRefCounted::inc_ref`] twice, then calls + /// [`AlwaysRefCounted::dec_ref`] once). + unsafe fn dec_ref(obj: NonNull); +} + +/// An owned reference to an always-reference-counted object. +/// +/// The object's reference count is automatically decremented when an instance of [`ARef`] is +/// dropped. It is also automatically incremented when a new instance is created via +/// [`ARef::clone`]. +/// +/// # Invariants +/// +/// The pointer stored in `ptr` is non-null and valid for the lifetime of the [`ARef`] instance. In +/// particular, the [`ARef`] instance owns an increment on the underlying object's reference count. +pub struct ARef { + ptr: NonNull, + _p: PhantomData, +} + +// SAFETY: It is safe to send `ARef` to another thread when the underlying `T` is `Sync` because +// it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs +// `T` to be `Send` because any thread that has an `ARef` may ultimately access `T` using a +// mutable reference, for example, when the reference count reaches zero and `T` is dropped. +unsafe impl Send for ARef {} + +// SAFETY: It is safe to send `&ARef` to another thread when the underlying `T` is `Sync` +// because it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, +// it needs `T` to be `Send` because any thread that has a `&ARef` may clone it and get an +// `ARef` on that thread, so the thread may ultimately access `T` using a mutable reference, for +// example, when the reference count reaches zero and `T` is dropped. +unsafe impl Sync for ARef {} + +impl ARef { + /// Creates a new instance of [`ARef`]. + /// + /// It takes over an increment of the reference count on the underlying object. + /// + /// # Safety + /// + /// Callers must ensure that the reference count was incremented at least once, and that they + /// are properly relinquishing one increment. That is, if there is only one increment, callers + /// must not use the underlying object anymore -- it is only safe to do so via the newly + /// created [`ARef`]. + pub unsafe fn from_raw(ptr: NonNull) -> Self { + // INVARIANT: The safety requirements guarantee that the new instance now owns the + // increment on the refcount. + Self { + ptr, + _p: PhantomData, + } + } + + /// Consumes the `ARef`, returning a raw pointer. + /// + /// This function does not change the refcount. After calling this function, the caller is + /// responsible for the refcount previously managed by the `ARef`. + /// + /// # Examples + /// + /// ``` + /// use core::ptr::NonNull; + /// use kernel::types::{ARef, AlwaysRefCounted}; + /// + /// struct Empty {} + /// + /// # // SAFETY: TODO. + /// unsafe impl AlwaysRefCounted for Empty { + /// fn inc_ref(&self) {} + /// unsafe fn dec_ref(_obj: NonNull) {} + /// } + /// + /// let mut data = Empty {}; + /// let ptr = NonNull::::new(&mut data).unwrap(); + /// # // SAFETY: TODO. + /// let data_ref: ARef = unsafe { ARef::from_raw(ptr) }; + /// let raw_ptr: NonNull = ARef::into_raw(data_ref); + /// + /// assert_eq!(ptr, raw_ptr); + /// ``` + pub fn into_raw(me: Self) -> NonNull { + ManuallyDrop::new(me).ptr + } +} + +impl Clone for ARef { + fn clone(&self) -> Self { + self.inc_ref(); + // SAFETY: We just incremented the refcount above. + unsafe { Self::from_raw(self.ptr) } + } +} + +impl Deref for ARef { + type Target = T; + + fn deref(&self) -> &Self::Target { + // SAFETY: The type invariants guarantee that the object is valid. + unsafe { self.ptr.as_ref() } + } +} + +impl From<&T> for ARef { + fn from(b: &T) -> Self { + b.inc_ref(); + // SAFETY: We just incremented the refcount above. + unsafe { Self::from_raw(NonNull::from(b)) } + } +} + +impl Drop for ARef { + fn drop(&mut self) { + // SAFETY: The type invariants guarantee that the `ARef` owns the reference we're about to + // decrement. + unsafe { T::dec_ref(self.ptr) }; + } +} -- cgit v1.2.3