From 1f0433d07ab270d439088285882e903617c5d4d5 Mon Sep 17 00:00:00 2001 From: Aliet Exposito Garcia Date: Wed, 18 Sep 2024 18:51:15 -0400 Subject: rust: kernel: move `ARef` and `AlwaysRefCounted` to new `sync::aref` module Refactor the `ARef` type and `AlwaysRefCounted` trait from `types.rs` into a new `sync/aref.rs` module: - Add `rust/kernel/sync/aref.rs` with the definitions of `ARef` and `AlwaysRefCounted`. - Remove the same type and trait definitions from `rust/kernel/types.rs`. - Update relevant files to import `ARef` and `AlwaysRefCounted` from `sync/aref.rs`. The type and trait definitions remain unchanged. Suggested-by: Benno Lossin Link: https://github.com/Rust-for-Linux/linux/issues/1117 Signed-off-by: Aliet Exposito Garcia Reviewed-by: Fiona Behrens Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20240918225115.2309224-3-aliet.exposito@gmail.com [ Rebased on top of the lints series, slightly reworded and removed unneeded `use AlwaysRefCounted` in example. - Miguel ] Signed-off-by: Miguel Ojeda --- drivers/block/rnull.rs | 3 +- rust/kernel/block/mq.rs | 6 +- rust/kernel/block/mq/operations.rs | 2 +- rust/kernel/block/mq/request.rs | 3 +- rust/kernel/device.rs | 8 +- rust/kernel/sync.rs | 1 + rust/kernel/sync/aref.rs | 154 +++++++++++++++++++++++++++++++++++++ rust/kernel/task.rs | 4 +- rust/kernel/types.rs | 154 +------------------------------------ 9 files changed, 169 insertions(+), 166 deletions(-) create mode 100644 rust/kernel/sync/aref.rs diff --git a/drivers/block/rnull.rs b/drivers/block/rnull.rs index b0227cf9ddd3..fd157d390f75 100644 --- a/drivers/block/rnull.rs +++ b/drivers/block/rnull.rs @@ -20,8 +20,7 @@ use kernel::{ error::Result, new_mutex, pr_info, prelude::*, - sync::{Arc, Mutex}, - types::ARef, + sync::{aref::ARef, Arc, Mutex}, }; module! { diff --git a/rust/kernel/block/mq.rs b/rust/kernel/block/mq.rs index fb0f393c1cea..04843f2c0f82 100644 --- a/rust/kernel/block/mq.rs +++ b/rust/kernel/block/mq.rs @@ -20,7 +20,7 @@ //! The kernel will interface with the block device driver by calling the method //! implementations of the `Operations` trait. //! -//! IO requests are passed to the driver as [`kernel::types::ARef`] +//! IO requests are passed to the driver as [`kernel::sync::aref::ARef`] //! instances. The `Request` type is a wrapper around the C `struct request`. //! The driver must mark end of processing by calling one of the //! `Request::end`, methods. Failure to do so can lead to deadlock or timeout @@ -61,8 +61,8 @@ //! block::mq::*, //! new_mutex, //! prelude::*, -//! sync::{Arc, Mutex}, -//! types::{ARef, ForeignOwnable}, +//! sync::{aref::ARef, Arc, Mutex}, +//! types::ForeignOwnable, //! }; //! //! struct MyBlkDevice; diff --git a/rust/kernel/block/mq/operations.rs b/rust/kernel/block/mq/operations.rs index 9ba7fdfeb4b2..31f0f043fe7c 100644 --- a/rust/kernel/block/mq/operations.rs +++ b/rust/kernel/block/mq/operations.rs @@ -9,7 +9,7 @@ use crate::{ block::mq::request::RequestDataWrapper, block::mq::Request, error::{from_result, Result}, - types::ARef, + sync::aref::ARef, }; use core::{marker::PhantomData, sync::atomic::AtomicU64, sync::atomic::Ordering}; diff --git a/rust/kernel/block/mq/request.rs b/rust/kernel/block/mq/request.rs index a0e22827f3f4..1e57cfef63f1 100644 --- a/rust/kernel/block/mq/request.rs +++ b/rust/kernel/block/mq/request.rs @@ -8,7 +8,8 @@ use crate::{ bindings, block::mq::Operations, error::Result, - types::{ARef, AlwaysRefCounted, Opaque}, + sync::aref::{ARef, AlwaysRefCounted}, + types::Opaque, }; use core::{ marker::PhantomData, diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs index 851018eef885..21fb733feb63 100644 --- a/rust/kernel/device.rs +++ b/rust/kernel/device.rs @@ -4,10 +4,8 @@ //! //! C header: [`include/linux/device.h`](srctree/include/linux/device.h) -use crate::{ - bindings, - types::{ARef, Opaque}, -}; +use crate::{bindings, sync::aref::ARef, types::Opaque}; + use core::ptr; /// A reference-counted device. @@ -85,7 +83,7 @@ impl Device { } // SAFETY: Instances of `Device` are always reference-counted. -unsafe impl crate::types::AlwaysRefCounted for Device { +unsafe impl crate::sync::aref::AlwaysRefCounted for Device { fn inc_ref(&self) { // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero. unsafe { bindings::get_device(self.as_raw()) }; diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs index 0ab20975a3b5..0d0c44f10841 100644 --- a/rust/kernel/sync.rs +++ b/rust/kernel/sync.rs @@ -8,6 +8,7 @@ use crate::types::Opaque; mod arc; +pub mod aref; mod condvar; pub mod lock; mod locked_by; diff --git a/rust/kernel/sync/aref.rs b/rust/kernel/sync/aref.rs new file mode 100644 index 000000000000..331f55508046 --- /dev/null +++ b/rust/kernel/sync/aref.rs @@ -0,0 +1,154 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! Types and utilities for managing always-reference-counted objects. + +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::sync::aref::{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 as *mut _).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) }; + } +} diff --git a/rust/kernel/task.rs b/rust/kernel/task.rs index 55dff7e088bf..0b358aceace6 100644 --- a/rust/kernel/task.rs +++ b/rust/kernel/task.rs @@ -61,7 +61,7 @@ macro_rules! current { /// incremented when creating `State` and decremented when it is dropped: /// /// ``` -/// use kernel::{task::Task, types::ARef}; +/// use kernel::{task::Task, sync::aref::ARef}; /// /// struct State { /// creator: ARef, @@ -164,7 +164,7 @@ impl Task { } // SAFETY: The type invariants guarantee that `Task` is always refcounted. -unsafe impl crate::types::AlwaysRefCounted for Task { +unsafe impl crate::sync::aref::AlwaysRefCounted for Task { fn inc_ref(&self) { // SAFETY: The existence of a shared reference means that the refcount is nonzero. unsafe { bindings::get_task_struct(self.0.get()) }; diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs index 085e8076f078..5ea9126c8c93 100644 --- a/rust/kernel/types.rs +++ b/rust/kernel/types.rs @@ -6,11 +6,10 @@ use crate::init::{self, PinInit}; use alloc::boxed::Box; use core::{ cell::UnsafeCell, - marker::{PhantomData, PhantomPinned}, - mem::{ManuallyDrop, MaybeUninit}, + marker::PhantomPinned, + mem::MaybeUninit, ops::{Deref, DerefMut}, pin::Pin, - ptr::NonNull, }; /// Used to transfer ownership to and from foreign (non-Rust) languages. @@ -313,155 +312,6 @@ impl Opaque { } } -/// 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 as *mut _).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) }; - } -} - /// A sum type that always holds either a value of type `L` or `R`. /// /// # Examples -- cgit v1.2.3-58-ga151