* [PATCH 0/2] rust: Move unfit code out of `rust/kernel/types.rs`
@ 2024-09-18 22:51 Aliet Exposito Garcia
2024-09-18 22:51 ` [PATCH 1/2] rust: Move `FromBytes` and `AsBytes` traits to a new `transmute` module Aliet Exposito Garcia
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Aliet Exposito Garcia @ 2024-09-18 22:51 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho
Cc: Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, rust-for-linux,
Aliet Exposito Garcia
Hello all,
These are the patches for a good first issue that Benno Lossin oppened
on https://github.com/Rust-for-Linux/linux/issues/1117
According to the issue description, the file `rust/kernel/types.rs` is a
rather non-descriptive, general file that contains code that might be
better suited elsewhere:
- `ARef<T>` and `AlwaysRefCounted` should be in `rust/kernel/sync/aref.rs`
- The `FromBytes` and `AsBytes` traits and implementations should be in
`rust/kernel/transmute.rs`
Aliet Exposito Garcia (2):
rust: Move `FromBytes` and `AsBytes` traits to a new `transmute`
module
rust: Move `ARef<T>` and `AlwaysRefCounted` to new `sync::aref` module
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/lib.rs | 1 +
rust/kernel/sync.rs | 1 +
rust/kernel/sync/aref.rs | 152 ++++++++++++++++++++
rust/kernel/task.rs | 4 +-
rust/kernel/transmute.rs | 67 +++++++++
rust/kernel/types.rs | 216 +----------------------------
rust/kernel/uaccess.rs | 2 +-
12 files changed, 236 insertions(+), 229 deletions(-)
create mode 100644 rust/kernel/sync/aref.rs
create mode 100644 rust/kernel/transmute.rs
base-commit: a2f11547052001bd448ccec81dd1e68409078fbb
--
2.46.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] rust: Move `FromBytes` and `AsBytes` traits to a new `transmute` module
2024-09-18 22:51 [PATCH 0/2] rust: Move unfit code out of `rust/kernel/types.rs` Aliet Exposito Garcia
@ 2024-09-18 22:51 ` Aliet Exposito Garcia
2024-09-19 14:39 ` Benno Lossin
2024-09-20 9:20 ` Fiona Behrens
2024-09-18 22:51 ` [PATCH 2/2] rust: Move `ARef<T>` and `AlwaysRefCounted` to new `sync::aref` module Aliet Exposito Garcia
2024-10-05 19:48 ` [PATCH 0/2] rust: Move unfit code out of `rust/kernel/types.rs` Miguel Ojeda
2 siblings, 2 replies; 11+ messages in thread
From: Aliet Exposito Garcia @ 2024-09-18 22:51 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho
Cc: Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, rust-for-linux,
Aliet Exposito Garcia
This patch refactors the `FromBytes` and `AsBytes` traits from `types.rs`
into a new `transmute.rs` module.
The traits and their implementations remain unchanged.
- Adds `rust/kernel/transmute.rs` with the definitions of `FromBytes`
and `AsBytes`
- Removes the same trait definitions from `rust/kernel/types.rs`
- Updates `rust/kernel/uaccess.rs` to import `AsBytes` and `FromBytes`
from `transmute.rs`
Suggested-by: Benno Lossin <benno.lossin@proton.me>
Link: https://github.com/Rust-for-Linux/linux/issues/1117
Signed-off-by: Aliet Exposito Garcia <aliet.exposito@gmail.com>
---
rust/kernel/lib.rs | 1 +
rust/kernel/transmute.rs | 67 ++++++++++++++++++++++++++++++++++++++++
rust/kernel/types.rs | 64 --------------------------------------
rust/kernel/uaccess.rs | 2 +-
4 files changed, 69 insertions(+), 65 deletions(-)
create mode 100644 rust/kernel/transmute.rs
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index f10b06a78..5007ba914 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -52,6 +52,7 @@
pub mod sync;
pub mod task;
pub mod time;
+pub mod transmute;
pub mod types;
pub mod uaccess;
pub mod workqueue;
diff --git a/rust/kernel/transmute.rs b/rust/kernel/transmute.rs
new file mode 100644
index 000000000..89d74de94
--- /dev/null
+++ b/rust/kernel/transmute.rs
@@ -0,0 +1,67 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Traits for transmuting types.
+
+/// Types for which any bit pattern is valid.
+///
+/// Not all types are valid for all values. For example, a `bool` must be either zero or one, so
+/// reading arbitrary bytes into something that contains a `bool` is not okay.
+///
+/// It's okay for the type to have padding, as initializing those bytes has no effect.
+///
+/// # Safety
+///
+/// All bit-patterns must be valid for this type. This type must not have interior mutability.
+pub unsafe trait FromBytes {}
+
+// SAFETY: All bit patterns are acceptable values of the types below.
+unsafe impl FromBytes for u8 {}
+unsafe impl FromBytes for u16 {}
+unsafe impl FromBytes for u32 {}
+unsafe impl FromBytes for u64 {}
+unsafe impl FromBytes for usize {}
+unsafe impl FromBytes for i8 {}
+unsafe impl FromBytes for i16 {}
+unsafe impl FromBytes for i32 {}
+unsafe impl FromBytes for i64 {}
+unsafe impl FromBytes for isize {}
+// SAFETY: If all bit patterns are acceptable for individual values in an array, then all bit
+// patterns are also acceptable for arrays of that type.
+unsafe impl<T: FromBytes> FromBytes for [T] {}
+unsafe impl<T: FromBytes, const N: usize> FromBytes for [T; N] {}
+
+/// Types that can be viewed as an immutable slice of initialized bytes.
+///
+/// If a struct implements this trait, then it is okay to copy it byte-for-byte to userspace. This
+/// means that it should not have any padding, as padding bytes are uninitialized. Reading
+/// uninitialized memory is not just undefined behavior, it may even lead to leaking sensitive
+/// information on the stack to userspace.
+///
+/// The struct should also not hold kernel pointers, as kernel pointer addresses are also considered
+/// sensitive. However, leaking kernel pointers is not considered undefined behavior by Rust, so
+/// this is a correctness requirement, but not a safety requirement.
+///
+/// # Safety
+///
+/// Values of this type may not contain any uninitialized bytes. This type must not have interior
+/// mutability.
+pub unsafe trait AsBytes {}
+
+// SAFETY: Instances of the following types have no uninitialized portions.
+unsafe impl AsBytes for u8 {}
+unsafe impl AsBytes for u16 {}
+unsafe impl AsBytes for u32 {}
+unsafe impl AsBytes for u64 {}
+unsafe impl AsBytes for usize {}
+unsafe impl AsBytes for i8 {}
+unsafe impl AsBytes for i16 {}
+unsafe impl AsBytes for i32 {}
+unsafe impl AsBytes for i64 {}
+unsafe impl AsBytes for isize {}
+unsafe impl AsBytes for bool {}
+unsafe impl AsBytes for char {}
+unsafe impl AsBytes for str {}
+// SAFETY: If individual values in an array have no uninitialized portions, then the array itself
+// does not have any uninitialized portions either.
+unsafe impl<T: AsBytes> AsBytes for [T] {}
+unsafe impl<T: AsBytes, const N: usize> AsBytes for [T; N] {}
diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index 9e7ca0663..d086f8503 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -468,67 +468,3 @@ pub enum Either<L, R> {
/// Constructs an instance of [`Either`] containing a value of type `R`.
Right(R),
}
-
-/// Types for which any bit pattern is valid.
-///
-/// Not all types are valid for all values. For example, a `bool` must be either zero or one, so
-/// reading arbitrary bytes into something that contains a `bool` is not okay.
-///
-/// It's okay for the type to have padding, as initializing those bytes has no effect.
-///
-/// # Safety
-///
-/// All bit-patterns must be valid for this type. This type must not have interior mutability.
-pub unsafe trait FromBytes {}
-
-// SAFETY: All bit patterns are acceptable values of the types below.
-unsafe impl FromBytes for u8 {}
-unsafe impl FromBytes for u16 {}
-unsafe impl FromBytes for u32 {}
-unsafe impl FromBytes for u64 {}
-unsafe impl FromBytes for usize {}
-unsafe impl FromBytes for i8 {}
-unsafe impl FromBytes for i16 {}
-unsafe impl FromBytes for i32 {}
-unsafe impl FromBytes for i64 {}
-unsafe impl FromBytes for isize {}
-// SAFETY: If all bit patterns are acceptable for individual values in an array, then all bit
-// patterns are also acceptable for arrays of that type.
-unsafe impl<T: FromBytes> FromBytes for [T] {}
-unsafe impl<T: FromBytes, const N: usize> FromBytes for [T; N] {}
-
-/// Types that can be viewed as an immutable slice of initialized bytes.
-///
-/// If a struct implements this trait, then it is okay to copy it byte-for-byte to userspace. This
-/// means that it should not have any padding, as padding bytes are uninitialized. Reading
-/// uninitialized memory is not just undefined behavior, it may even lead to leaking sensitive
-/// information on the stack to userspace.
-///
-/// The struct should also not hold kernel pointers, as kernel pointer addresses are also considered
-/// sensitive. However, leaking kernel pointers is not considered undefined behavior by Rust, so
-/// this is a correctness requirement, but not a safety requirement.
-///
-/// # Safety
-///
-/// Values of this type may not contain any uninitialized bytes. This type must not have interior
-/// mutability.
-pub unsafe trait AsBytes {}
-
-// SAFETY: Instances of the following types have no uninitialized portions.
-unsafe impl AsBytes for u8 {}
-unsafe impl AsBytes for u16 {}
-unsafe impl AsBytes for u32 {}
-unsafe impl AsBytes for u64 {}
-unsafe impl AsBytes for usize {}
-unsafe impl AsBytes for i8 {}
-unsafe impl AsBytes for i16 {}
-unsafe impl AsBytes for i32 {}
-unsafe impl AsBytes for i64 {}
-unsafe impl AsBytes for isize {}
-unsafe impl AsBytes for bool {}
-unsafe impl AsBytes for char {}
-unsafe impl AsBytes for str {}
-// SAFETY: If individual values in an array have no uninitialized portions, then the array itself
-// does not have any uninitialized portions either.
-unsafe impl<T: AsBytes> AsBytes for [T] {}
-unsafe impl<T: AsBytes, const N: usize> AsBytes for [T; N] {}
diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs
index e9347cff9..0af243482 100644
--- a/rust/kernel/uaccess.rs
+++ b/rust/kernel/uaccess.rs
@@ -9,7 +9,7 @@
bindings,
error::Result,
prelude::*,
- types::{AsBytes, FromBytes},
+ transmute::{AsBytes, FromBytes},
};
use alloc::vec::Vec;
use core::ffi::{c_ulong, c_void};
--
2.46.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] rust: Move `ARef<T>` and `AlwaysRefCounted` to new `sync::aref` module
2024-09-18 22:51 [PATCH 0/2] rust: Move unfit code out of `rust/kernel/types.rs` Aliet Exposito Garcia
2024-09-18 22:51 ` [PATCH 1/2] rust: Move `FromBytes` and `AsBytes` traits to a new `transmute` module Aliet Exposito Garcia
@ 2024-09-18 22:51 ` Aliet Exposito Garcia
2024-09-19 14:39 ` Benno Lossin
2024-09-20 9:21 ` Fiona Behrens
2024-10-05 19:48 ` [PATCH 0/2] rust: Move unfit code out of `rust/kernel/types.rs` Miguel Ojeda
2 siblings, 2 replies; 11+ messages in thread
From: Aliet Exposito Garcia @ 2024-09-18 22:51 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho
Cc: Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, rust-for-linux,
Aliet Exposito Garcia
This patch refactors the `ARef` type and `AlwaysRefCounted` trait from
`types.rs` into a new `sync/aref.rs` module.
The type and trait definitions remain unchanged.
- Adds `rust/kernel/sync/aref.rs` with the definitions of `ARef` and
`AlwaysRefCounted`
- Removes the same type and trait definitions from `rust/kernel/types.rs`
- Updates relevant files to import `ARef` and `AlwaysRefCounted` from
`sync/aref.rs`
Suggested-by: Benno Lossin <benno.lossin@proton.me>
Link: https://github.com/Rust-for-Linux/linux/issues/1117
Signed-off-by: Aliet Exposito Garcia <aliet.exposito@gmail.com>
---
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 | 152 +++++++++++++++++++++++++++++
rust/kernel/task.rs | 4 +-
rust/kernel/types.rs | 152 +----------------------------
9 files changed, 167 insertions(+), 164 deletions(-)
create mode 100644 rust/kernel/sync/aref.rs
diff --git a/drivers/block/rnull.rs b/drivers/block/rnull.rs
index b0227cf9d..fd157d390 100644
--- a/drivers/block/rnull.rs
+++ b/drivers/block/rnull.rs
@@ -20,8 +20,7 @@
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 fb0f393c1..f9e32eecd 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<Request>`]
+//! IO requests are passed to the driver as [`kernel::sync::aref::ARef<Request>`]
//! 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::{Arc, Mutex, aref::{ARef, AlwaysRefCounted}},
+//! types::ForeignOwnable,
//! };
//!
//! struct MyBlkDevice;
diff --git a/rust/kernel/block/mq/operations.rs b/rust/kernel/block/mq/operations.rs
index 9ba7fdfeb..31f0f043f 100644
--- a/rust/kernel/block/mq/operations.rs
+++ b/rust/kernel/block/mq/operations.rs
@@ -9,7 +9,7 @@
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 a0e22827f..1e57cfef6 100644
--- a/rust/kernel/block/mq/request.rs
+++ b/rust/kernel/block/mq/request.rs
@@ -8,7 +8,8 @@
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 851018eef..21fb733fe 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 @@ pub unsafe fn as_ref<'a>(ptr: *mut bindings::device) -> &'a Self {
}
// 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 0ab20975a..0d0c44f10 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 000000000..cfea94cac
--- /dev/null
+++ b/rust/kernel/sync/aref.rs
@@ -0,0 +1,152 @@
+// 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<T>`].
+///
+/// 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<Self>);
+}
+
+/// 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<T: AlwaysRefCounted> {
+ ptr: NonNull<T>,
+ _p: PhantomData<T>,
+}
+
+// SAFETY: It is safe to send `ARef<T>` 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<T>` may ultimately access `T` using a
+// mutable reference, for example, when the reference count reaches zero and `T` is dropped.
+unsafe impl<T: AlwaysRefCounted + Sync + Send> Send for ARef<T> {}
+
+// SAFETY: It is safe to send `&ARef<T>` 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<T>` may clone it and get an
+// `ARef<T>` 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<T: AlwaysRefCounted + Sync + Send> Sync for ARef<T> {}
+
+impl<T: AlwaysRefCounted> ARef<T> {
+ /// 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<T>) -> 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 {}
+ ///
+ /// unsafe impl AlwaysRefCounted for Empty {
+ /// fn inc_ref(&self) {}
+ /// unsafe fn dec_ref(_obj: NonNull<Self>) {}
+ /// }
+ ///
+ /// let mut data = Empty {};
+ /// let ptr = NonNull::<Empty>::new(&mut data as *mut _).unwrap();
+ /// let data_ref: ARef<Empty> = unsafe { ARef::from_raw(ptr) };
+ /// let raw_ptr: NonNull<Empty> = ARef::into_raw(data_ref);
+ ///
+ /// assert_eq!(ptr, raw_ptr);
+ /// ```
+ pub fn into_raw(me: Self) -> NonNull<T> {
+ ManuallyDrop::new(me).ptr
+ }
+}
+
+impl<T: AlwaysRefCounted> Clone for ARef<T> {
+ fn clone(&self) -> Self {
+ self.inc_ref();
+ // SAFETY: We just incremented the refcount above.
+ unsafe { Self::from_raw(self.ptr) }
+ }
+}
+
+impl<T: AlwaysRefCounted> Deref for ARef<T> {
+ type Target = T;
+
+ fn deref(&self) -> &Self::Target {
+ // SAFETY: The type invariants guarantee that the object is valid.
+ unsafe { self.ptr.as_ref() }
+ }
+}
+
+impl<T: AlwaysRefCounted> From<&T> for ARef<T> {
+ fn from(b: &T) -> Self {
+ b.inc_ref();
+ // SAFETY: We just incremented the refcount above.
+ unsafe { Self::from_raw(NonNull::from(b)) }
+ }
+}
+
+impl<T: AlwaysRefCounted> Drop for ARef<T> {
+ 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 55dff7e08..0b358acea 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<Task>,
@@ -164,7 +164,7 @@ pub fn wake_up(&self) {
}
// 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 d086f8503..b9b27746c 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -6,11 +6,10 @@
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,153 +312,6 @@ pub const fn raw_get(this: *const Self) -> *mut T {
}
}
-/// 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<T>`].
-///
-/// 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<Self>);
-}
-
-/// 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<T: AlwaysRefCounted> {
- ptr: NonNull<T>,
- _p: PhantomData<T>,
-}
-
-// SAFETY: It is safe to send `ARef<T>` 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<T>` may ultimately access `T` using a
-// mutable reference, for example, when the reference count reaches zero and `T` is dropped.
-unsafe impl<T: AlwaysRefCounted + Sync + Send> Send for ARef<T> {}
-
-// SAFETY: It is safe to send `&ARef<T>` 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<T>` may clone it and get an
-// `ARef<T>` 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<T: AlwaysRefCounted + Sync + Send> Sync for ARef<T> {}
-
-impl<T: AlwaysRefCounted> ARef<T> {
- /// 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<T>) -> 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 {}
- ///
- /// unsafe impl AlwaysRefCounted for Empty {
- /// fn inc_ref(&self) {}
- /// unsafe fn dec_ref(_obj: NonNull<Self>) {}
- /// }
- ///
- /// let mut data = Empty {};
- /// let ptr = NonNull::<Empty>::new(&mut data as *mut _).unwrap();
- /// let data_ref: ARef<Empty> = unsafe { ARef::from_raw(ptr) };
- /// let raw_ptr: NonNull<Empty> = ARef::into_raw(data_ref);
- ///
- /// assert_eq!(ptr, raw_ptr);
- /// ```
- pub fn into_raw(me: Self) -> NonNull<T> {
- ManuallyDrop::new(me).ptr
- }
-}
-
-impl<T: AlwaysRefCounted> Clone for ARef<T> {
- fn clone(&self) -> Self {
- self.inc_ref();
- // SAFETY: We just incremented the refcount above.
- unsafe { Self::from_raw(self.ptr) }
- }
-}
-
-impl<T: AlwaysRefCounted> Deref for ARef<T> {
- type Target = T;
-
- fn deref(&self) -> &Self::Target {
- // SAFETY: The type invariants guarantee that the object is valid.
- unsafe { self.ptr.as_ref() }
- }
-}
-
-impl<T: AlwaysRefCounted> From<&T> for ARef<T> {
- fn from(b: &T) -> Self {
- b.inc_ref();
- // SAFETY: We just incremented the refcount above.
- unsafe { Self::from_raw(NonNull::from(b)) }
- }
-}
-
-impl<T: AlwaysRefCounted> Drop for ARef<T> {
- 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`.
pub enum Either<L, R> {
/// Constructs an instance of [`Either`] containing a value of type `L`.
--
2.46.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] rust: Move `FromBytes` and `AsBytes` traits to a new `transmute` module
2024-09-18 22:51 ` [PATCH 1/2] rust: Move `FromBytes` and `AsBytes` traits to a new `transmute` module Aliet Exposito Garcia
@ 2024-09-19 14:39 ` Benno Lossin
2024-09-20 9:20 ` Fiona Behrens
1 sibling, 0 replies; 11+ messages in thread
From: Benno Lossin @ 2024-09-19 14:39 UTC (permalink / raw)
To: Aliet Exposito Garcia, Miguel Ojeda, Alex Gaynor,
Wedson Almeida Filho
Cc: Boqun Feng, Gary Guo, Björn Roy Baron, Andreas Hindborg,
Alice Ryhl, Trevor Gross, rust-for-linux
On 19.09.24 00:51, Aliet Exposito Garcia wrote:
> This patch refactors the `FromBytes` and `AsBytes` traits from `types.rs`
> into a new `transmute.rs` module.
> The traits and their implementations remain unchanged.
> - Adds `rust/kernel/transmute.rs` with the definitions of `FromBytes`
> and `AsBytes`
> - Removes the same trait definitions from `rust/kernel/types.rs`
> - Updates `rust/kernel/uaccess.rs` to import `AsBytes` and `FromBytes`
> from `transmute.rs`
>
> Suggested-by: Benno Lossin <benno.lossin@proton.me>
>
> Link: https://github.com/Rust-for-Linux/linux/issues/1117
> Signed-off-by: Aliet Exposito Garcia <aliet.exposito@gmail.com>
> ---
> rust/kernel/lib.rs | 1 +
> rust/kernel/transmute.rs | 67 ++++++++++++++++++++++++++++++++++++++++
> rust/kernel/types.rs | 64 --------------------------------------
> rust/kernel/uaccess.rs | 2 +-
> 4 files changed, 69 insertions(+), 65 deletions(-)
> create mode 100644 rust/kernel/transmute.rs
Thanks for the patch!
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] rust: Move `ARef<T>` and `AlwaysRefCounted` to new `sync::aref` module
2024-09-18 22:51 ` [PATCH 2/2] rust: Move `ARef<T>` and `AlwaysRefCounted` to new `sync::aref` module Aliet Exposito Garcia
@ 2024-09-19 14:39 ` Benno Lossin
2024-09-20 9:21 ` Fiona Behrens
1 sibling, 0 replies; 11+ messages in thread
From: Benno Lossin @ 2024-09-19 14:39 UTC (permalink / raw)
To: Aliet Exposito Garcia, Miguel Ojeda, Alex Gaynor,
Wedson Almeida Filho
Cc: Boqun Feng, Gary Guo, Björn Roy Baron, Andreas Hindborg,
Alice Ryhl, Trevor Gross, rust-for-linux
On 19.09.24 00:51, Aliet Exposito Garcia wrote:
> This patch refactors the `ARef` type and `AlwaysRefCounted` trait from
> `types.rs` into a new `sync/aref.rs` module.
> The type and trait definitions remain unchanged.
> - Adds `rust/kernel/sync/aref.rs` with the definitions of `ARef` and
> `AlwaysRefCounted`
> - Removes the same type and trait definitions from `rust/kernel/types.rs`
> - Updates relevant files to import `ARef` and `AlwaysRefCounted` from
> `sync/aref.rs`
>
> Suggested-by: Benno Lossin <benno.lossin@proton.me>
>
> Link: https://github.com/Rust-for-Linux/linux/issues/1117
> Signed-off-by: Aliet Exposito Garcia <aliet.exposito@gmail.com>
> ---
> 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 | 152 +++++++++++++++++++++++++++++
> rust/kernel/task.rs | 4 +-
> rust/kernel/types.rs | 152 +----------------------------
> 9 files changed, 167 insertions(+), 164 deletions(-)
> create mode 100644 rust/kernel/sync/aref.rs
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] rust: Move `FromBytes` and `AsBytes` traits to a new `transmute` module
2024-09-18 22:51 ` [PATCH 1/2] rust: Move `FromBytes` and `AsBytes` traits to a new `transmute` module Aliet Exposito Garcia
2024-09-19 14:39 ` Benno Lossin
@ 2024-09-20 9:20 ` Fiona Behrens
1 sibling, 0 replies; 11+ messages in thread
From: Fiona Behrens @ 2024-09-20 9:20 UTC (permalink / raw)
To: Aliet Exposito Garcia
Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, rust-for-linux
On 19 Sep 2024, at 0:51, Aliet Exposito Garcia wrote:
> This patch refactors the `FromBytes` and `AsBytes` traits from `types.rs`
> into a new `transmute.rs` module.
> The traits and their implementations remain unchanged.
> - Adds `rust/kernel/transmute.rs` with the definitions of `FromBytes`
> and `AsBytes`
> - Removes the same trait definitions from `rust/kernel/types.rs`
> - Updates `rust/kernel/uaccess.rs` to import `AsBytes` and `FromBytes`
> from `transmute.rs`
>
> Suggested-by: Benno Lossin <benno.lossin@proton.me>
>
> Link: https://github.com/Rust-for-Linux/linux/issues/1117
> Signed-off-by: Aliet Exposito Garcia <aliet.exposito@gmail.com>
Reviewed-by: Fiona Behrens <me@kloenk.dev>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] rust: Move `ARef<T>` and `AlwaysRefCounted` to new `sync::aref` module
2024-09-18 22:51 ` [PATCH 2/2] rust: Move `ARef<T>` and `AlwaysRefCounted` to new `sync::aref` module Aliet Exposito Garcia
2024-09-19 14:39 ` Benno Lossin
@ 2024-09-20 9:21 ` Fiona Behrens
1 sibling, 0 replies; 11+ messages in thread
From: Fiona Behrens @ 2024-09-20 9:21 UTC (permalink / raw)
To: Aliet Exposito Garcia
Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, rust-for-linux
On 19 Sep 2024, at 0:51, Aliet Exposito Garcia wrote:
> This patch refactors the `ARef` type and `AlwaysRefCounted` trait from
> `types.rs` into a new `sync/aref.rs` module.
> The type and trait definitions remain unchanged.
> - Adds `rust/kernel/sync/aref.rs` with the definitions of `ARef` and
> `AlwaysRefCounted`
> - Removes the same type and trait definitions from `rust/kernel/types.rs`
> - Updates relevant files to import `ARef` and `AlwaysRefCounted` from
> `sync/aref.rs`
>
> Suggested-by: Benno Lossin <benno.lossin@proton.me>
>
> Link: https://github.com/Rust-for-Linux/linux/issues/1117
> Signed-off-by: Aliet Exposito Garcia <aliet.exposito@gmail.com>
Reviewed-by: Fiona Behrens <me@kloenk.dev>
Thanks,
Fiona
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] rust: Move unfit code out of `rust/kernel/types.rs`
2024-09-18 22:51 [PATCH 0/2] rust: Move unfit code out of `rust/kernel/types.rs` Aliet Exposito Garcia
2024-09-18 22:51 ` [PATCH 1/2] rust: Move `FromBytes` and `AsBytes` traits to a new `transmute` module Aliet Exposito Garcia
2024-09-18 22:51 ` [PATCH 2/2] rust: Move `ARef<T>` and `AlwaysRefCounted` to new `sync::aref` module Aliet Exposito Garcia
@ 2024-10-05 19:48 ` Miguel Ojeda
2024-10-06 19:29 ` Aliet Expósito
2024-10-14 12:13 ` Miguel Ojeda
2 siblings, 2 replies; 11+ messages in thread
From: Miguel Ojeda @ 2024-10-05 19:48 UTC (permalink / raw)
To: Aliet Exposito Garcia
Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, rust-for-linux
On Thu, Sep 19, 2024 at 12:51 AM Aliet Exposito Garcia
<aliet.exposito@gmail.com> wrote:
>
> Hello all,
> These are the patches for a good first issue that Benno Lossin oppened
> on https://github.com/Rust-for-Linux/linux/issues/1117
> According to the issue description, the file `rust/kernel/types.rs` is a
> rather non-descriptive, general file that contains code that might be
> better suited elsewhere:
> - `ARef<T>` and `AlwaysRefCounted` should be in `rust/kernel/sync/aref.rs`
> - The `FromBytes` and `AsBytes` traits and implementations should be in
> `rust/kernel/transmute.rs`
Applied to `rust-next` -- thanks everyone!
[ Rebased on top of the lints series and slightly reworded. - Miguel ]
[ Rebased on top of the lints series, slightly reworded and removed
unneeded `use AlwaysRefCounted` in example. - Miguel ]
Aliet: please double-check that you are happy with the rebase.
Cheers,
Miguel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] rust: Move unfit code out of `rust/kernel/types.rs`
2024-10-05 19:48 ` [PATCH 0/2] rust: Move unfit code out of `rust/kernel/types.rs` Miguel Ojeda
@ 2024-10-06 19:29 ` Aliet Expósito
2024-10-07 17:11 ` Miguel Ojeda
2024-10-14 12:13 ` Miguel Ojeda
1 sibling, 1 reply; 11+ messages in thread
From: Aliet Expósito @ 2024-10-06 19:29 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, rust-for-linux
On 5/10/24 15:48, Miguel Ojeda wrote:
> On Thu, Sep 19, 2024 at 12:51 AM Aliet Exposito Garcia
> <aliet.exposito@gmail.com> wrote:
>> Hello all,
>> These are the patches for a good first issue that Benno Lossin oppened
>> on https://github.com/Rust-for-Linux/linux/issues/1117
>> According to the issue description, the file `rust/kernel/types.rs` is a
>> rather non-descriptive, general file that contains code that might be
>> better suited elsewhere:
>> - `ARef<T>` and `AlwaysRefCounted` should be in `rust/kernel/sync/aref.rs`
>> - The `FromBytes` and `AsBytes` traits and implementations should be in
>> `rust/kernel/transmute.rs`
> Applied to `rust-next` -- thanks everyone!
>
> [ Rebased on top of the lints series and slightly reworded. - Miguel ]
>
> [ Rebased on top of the lints series, slightly reworded and removed
> unneeded `use AlwaysRefCounted` in example. - Miguel ]
>
> Aliet: please double-check that you are happy with the rebase.
Hello Miguel, yes, I have double-checked and I am happy with the rebase.
> Cheers,
> Miguel
I want to continue studying Rust so that little by little I can help in
the Rust-for-Linux subsystem.
Thank you very much to everyone; you have an excellent community, very
welcoming to beginners.
Best regards,
Aliet
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] rust: Move unfit code out of `rust/kernel/types.rs`
2024-10-06 19:29 ` Aliet Expósito
@ 2024-10-07 17:11 ` Miguel Ojeda
0 siblings, 0 replies; 11+ messages in thread
From: Miguel Ojeda @ 2024-10-07 17:11 UTC (permalink / raw)
To: Aliet Expósito
Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, rust-for-linux
On Sun, Oct 6, 2024 at 9:30 PM Aliet Expósito <aliet.exposito@gmail.com> wrote:
>
> Hello Miguel, yes, I have double-checked and I am happy with the rebase.
>
> I want to continue studying Rust so that little by little I can help in
> the Rust-for-Linux subsystem.
>
> Thank you very much to everyone; you have an excellent community, very
> welcoming to beginners.
Thanks for double-checking and the kind words -- messages like this
may make others decide to contribute! :)
Cheers,
Miguel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] rust: Move unfit code out of `rust/kernel/types.rs`
2024-10-05 19:48 ` [PATCH 0/2] rust: Move unfit code out of `rust/kernel/types.rs` Miguel Ojeda
2024-10-06 19:29 ` Aliet Expósito
@ 2024-10-14 12:13 ` Miguel Ojeda
1 sibling, 0 replies; 11+ messages in thread
From: Miguel Ojeda @ 2024-10-14 12:13 UTC (permalink / raw)
To: Aliet Exposito Garcia
Cc: Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, rust-for-linux
On Sat, Oct 5, 2024 at 9:48 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> Applied to `rust-next` -- thanks everyone!
>
> [ Rebased on top of the lints series and slightly reworded. - Miguel ]
>
> [ Rebased on top of the lints series, slightly reworded and removed
> unneeded `use AlwaysRefCounted` in example. - Miguel ]
The other day I dropped temporarily the second patch to keep
linux-next building and to simplify merging the VFS series. I will put
it back later on.
Cheers,
Miguel
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-10-14 12:14 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-18 22:51 [PATCH 0/2] rust: Move unfit code out of `rust/kernel/types.rs` Aliet Exposito Garcia
2024-09-18 22:51 ` [PATCH 1/2] rust: Move `FromBytes` and `AsBytes` traits to a new `transmute` module Aliet Exposito Garcia
2024-09-19 14:39 ` Benno Lossin
2024-09-20 9:20 ` Fiona Behrens
2024-09-18 22:51 ` [PATCH 2/2] rust: Move `ARef<T>` and `AlwaysRefCounted` to new `sync::aref` module Aliet Exposito Garcia
2024-09-19 14:39 ` Benno Lossin
2024-09-20 9:21 ` Fiona Behrens
2024-10-05 19:48 ` [PATCH 0/2] rust: Move unfit code out of `rust/kernel/types.rs` Miguel Ojeda
2024-10-06 19:29 ` Aliet Expósito
2024-10-07 17:11 ` Miguel Ojeda
2024-10-14 12:13 ` Miguel Ojeda
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).