AlwaysRefCounted will become a marker trait to indicate that it is allowed
to obtain an ARef from a `&`, which cannot be allowed for types which are
also Ownable.
Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
Suggested-by: Alice Ryhl <aliceryhl@google.com>
---
rust/kernel/block/mq/request.rs | 10 ++++++---
rust/kernel/cred.rs | 8 ++++++--
rust/kernel/device.rs | 8 ++++++--
rust/kernel/fs/file.rs | 10 ++++++---
rust/kernel/pid_namespace.rs | 8 ++++++--
rust/kernel/task.rs | 6 +++++-
rust/kernel/types.rs | 45 ++++++++++++++++++++++++-----------------
7 files changed, 64 insertions(+), 31 deletions(-)
diff --git a/rust/kernel/block/mq/request.rs b/rust/kernel/block/mq/request.rs
index 2f2bb5a04709cc90ae8971da166fc83bb53fb86b..f6bee0932499e554f88907861997f8b9d1ffd51a 100644
--- a/rust/kernel/block/mq/request.rs
+++ b/rust/kernel/block/mq/request.rs
@@ -9,7 +9,7 @@
block::mq::Operations,
error::Result,
sync::Refcount,
- types::{ARef, AlwaysRefCounted, Opaque},
+ types::{ARef, AlwaysRefCounted, Opaque, RefCounted},
};
use core::{
marker::PhantomData,
@@ -209,10 +209,10 @@ unsafe impl<T: Operations> Send for Request<T> {}
unsafe impl<T: Operations> Sync for Request<T> {}
// SAFETY: All instances of `Request<T>` are reference counted. This
-// implementation of `AlwaysRefCounted` ensure that increments to the ref count
+// implementation of `RefCounted` ensure that increments to the ref count
// keeps the object alive in memory at least until a matching reference count
// decrement is executed.
-unsafe impl<T: Operations> AlwaysRefCounted for Request<T> {
+unsafe impl<T: Operations> RefCounted for Request<T> {
fn inc_ref(&self) {
self.wrapper_ref().refcount().inc();
}
@@ -234,3 +234,7 @@ unsafe fn dec_ref(obj: core::ptr::NonNull<Self>) {
}
}
}
+
+// SAFETY: we currently do not implement `Ownable`, thus it is okay to can obtain an `ARef<Request>`
+// from a `&Request` (but this will change in the future).
+unsafe impl<T: Operations> AlwaysRefCounted for Request<T> {}
diff --git a/rust/kernel/cred.rs b/rust/kernel/cred.rs
index 81d67789b16f243e7832ff3b2e5e479a1ab2bf9e..e04d1021130eb1ec46fe48feb088959da7656d66 100644
--- a/rust/kernel/cred.rs
+++ b/rust/kernel/cred.rs
@@ -11,7 +11,7 @@
use crate::{
bindings,
task::Kuid,
- types::{AlwaysRefCounted, Opaque},
+ types::{AlwaysRefCounted, Opaque, RefCounted},
};
/// Wraps the kernel's `struct cred`.
@@ -71,7 +71,7 @@ pub fn euid(&self) -> Kuid {
}
// SAFETY: The type invariants guarantee that `Credential` is always ref-counted.
-unsafe impl AlwaysRefCounted for Credential {
+unsafe impl RefCounted for Credential {
fn inc_ref(&self) {
// SAFETY: The existence of a shared reference means that the refcount is nonzero.
unsafe { bindings::get_cred(self.0.get()) };
@@ -83,3 +83,7 @@ unsafe fn dec_ref(obj: core::ptr::NonNull<Credential>) {
unsafe { bindings::put_cred(obj.cast().as_ptr()) };
}
}
+
+// SAFETY: We do not implement `Ownable`, thus it is okay to can obtain an `ARef<Credential>` from a
+// `&Credential`.
+unsafe impl AlwaysRefCounted for Credential {}
diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index db2d9658ba47d9c492bc813ce3eb2ff29703ca31..189298518dc184405b1d62404b190d4c0b08b7ad 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -7,7 +7,7 @@
use crate::{
bindings,
str::CStr,
- types::{ARef, Opaque},
+ types::{ARef, AlwaysRefCounted, Opaque, RefCounted},
};
use core::{fmt, ptr};
@@ -190,7 +190,7 @@ pub fn property_present(&self, name: &CStr) -> bool {
}
// SAFETY: Instances of `Device` are always reference-counted.
-unsafe impl crate::types::AlwaysRefCounted for Device {
+unsafe impl RefCounted 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()) };
@@ -202,6 +202,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
}
}
+// SAFETY: We do not implement `Ownable`, thus it is okay to can obtain an `Device<Task>` from a
+// `&Device`.
+unsafe impl AlwaysRefCounted for Device {}
+
// SAFETY: As by the type invariant `Device` can be sent to any thread.
unsafe impl Send for Device {}
diff --git a/rust/kernel/fs/file.rs b/rust/kernel/fs/file.rs
index e03dbe14d62a566349c4100f2f78b17d4c79aab5..a7836cc754e7927b6addc3bd06cfe8c8119f1d9f 100644
--- a/rust/kernel/fs/file.rs
+++ b/rust/kernel/fs/file.rs
@@ -11,7 +11,7 @@
bindings,
cred::Credential,
error::{code::*, Error, Result},
- types::{ARef, AlwaysRefCounted, NotThreadSafe, Opaque},
+ types::{ARef, AlwaysRefCounted, NotThreadSafe, Opaque, RefCounted},
};
use core::ptr;
@@ -190,7 +190,7 @@ unsafe impl Sync for File {}
// SAFETY: The type invariants guarantee that `File` is always ref-counted. This implementation
// makes `ARef<File>` own a normal refcount.
-unsafe impl AlwaysRefCounted for File {
+unsafe impl RefCounted for File {
#[inline]
fn inc_ref(&self) {
// SAFETY: The existence of a shared reference means that the refcount is nonzero.
@@ -205,6 +205,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<File>) {
}
}
+// SAFETY: We do not implement `Ownable`, thus it is okay to can obtain an `ARef<File>` from a
+/// `&File`.
+unsafe impl AlwaysRefCounted for File {}
+
/// Wraps the kernel's `struct file`. Not thread safe.
///
/// This type represents a file that is not known to be safe to transfer across thread boundaries.
@@ -225,7 +229,7 @@ pub struct LocalFile {
// SAFETY: The type invariants guarantee that `LocalFile` is always ref-counted. This implementation
// makes `ARef<File>` own a normal refcount.
-unsafe impl AlwaysRefCounted for LocalFile {
+unsafe impl RefCounted for LocalFile {
#[inline]
fn inc_ref(&self) {
// SAFETY: The existence of a shared reference means that the refcount is nonzero.
diff --git a/rust/kernel/pid_namespace.rs b/rust/kernel/pid_namespace.rs
index 0e93808e4639b37dd77add5d79f64058dac7cb87..3e45e945b7509b9607266b2e0e6ef130e7a1ed39 100644
--- a/rust/kernel/pid_namespace.rs
+++ b/rust/kernel/pid_namespace.rs
@@ -9,7 +9,7 @@
use crate::{
bindings,
- types::{AlwaysRefCounted, Opaque},
+ types::{AlwaysRefCounted, RefCounted, Opaque},
};
use core::ptr;
@@ -44,7 +44,7 @@ pub unsafe fn from_ptr<'a>(ptr: *const bindings::pid_namespace) -> &'a Self {
}
// SAFETY: Instances of `PidNamespace` are always reference-counted.
-unsafe impl AlwaysRefCounted for PidNamespace {
+unsafe impl RefCounted for PidNamespace {
#[inline]
fn inc_ref(&self) {
// SAFETY: The existence of a shared reference means that the refcount is nonzero.
@@ -58,6 +58,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<PidNamespace>) {
}
}
+// SAFETY: We do not implement `Ownable`, thus it is okay to can obtain an `ARef<PidNamespace>`
+// from a `&PidNamespace`.
+unsafe impl AlwaysRefCounted for PidNamespace {}
+
// SAFETY:
// - `PidNamespace::dec_ref` can be called from any thread.
// - It is okay to send ownership of `PidNamespace` across thread boundaries.
diff --git a/rust/kernel/task.rs b/rust/kernel/task.rs
index 07bc22a7645c0c7d792a0a163dd55b8ff0fe5f92..1cc1b02cb7fada6703da8cad77549dee578fc08d 100644
--- a/rust/kernel/task.rs
+++ b/rust/kernel/task.rs
@@ -327,7 +327,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::types::RefCounted 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.as_ptr()) };
@@ -339,6 +339,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
}
}
+// SAFETY: We do not implement `Ownable`, thus it is okay to can obtain an `ARef<Task>` from a
+// `&Task`.
+unsafe impl crate::types::AlwaysRefCounted for Task {}
+
impl Kuid {
/// Get the current euid.
#[inline]
diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index 65f6d0721f5f23c8db79c6735dc7d5e1ac984ea7..5a96da714348cc2369969200e6070972226c00fe 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -402,11 +402,9 @@ pub const fn raw_get(this: *const Self) -> *mut T {
}
}
-/// Types that are _always_ reference counted.
+/// Types that are internally 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
@@ -418,9 +416,9 @@ pub const fn raw_get(this: *const Self) -> *mut T {
/// 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
+/// won't be able to honour the requirement that [`RefCounted::inc_ref`] keep the object
/// alive.)
-pub unsafe trait AlwaysRefCounted {
+pub unsafe trait RefCounted {
/// Increments the reference count on the object.
fn inc_ref(&self);
@@ -433,11 +431,22 @@ pub unsafe trait AlwaysRefCounted {
/// 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).
+ /// (e.g., it calls [`RefCounted::inc_ref`] twice, then calls
+ /// [`RefCounted::dec_ref`] once).
unsafe fn dec_ref(obj: NonNull<Self>);
}
+/// An extension to RefCounted, which declares that it is allowed to convert
+/// from a shared reference `&T` to an owned reference [`ARef<T>`].
+///
+/// # Safety
+///
+/// Implementers must ensure that no safety invariants are violated by upgrading an `&T`
+/// to an [`ARef<T>`]. In particular that implies [`AlwaysRefCounted`] and [`Ownable`]
+/// cannot be implemented for the same type, as this would allow to violate the uniqueness
+/// guarantee of [`Owned<T>`] by derefencing it into an `&T` and obtaining an [`ARef`] from that.
+pub unsafe trait AlwaysRefCounted: RefCounted {}
+
/// An owned reference to an always-reference-counted object.
///
/// The object's reference count is automatically decremented when an instance of [`ARef`] is
@@ -448,7 +457,7 @@ pub unsafe trait AlwaysRefCounted {
///
/// 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> {
+pub struct ARef<T: RefCounted> {
ptr: NonNull<T>,
_p: PhantomData<T>,
}
@@ -457,16 +466,16 @@ pub struct ARef<T: AlwaysRefCounted> {
// 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> {}
+unsafe impl<T: RefCounted + 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> {}
+unsafe impl<T: RefCounted + Sync + Send> Sync for ARef<T> {}
-impl<T: AlwaysRefCounted> ARef<T> {
+impl<T: RefCounted> ARef<T> {
/// Creates a new instance of [`ARef`].
///
/// It takes over an increment of the reference count on the underlying object.
@@ -495,12 +504,12 @@ pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
///
/// ```
/// use core::ptr::NonNull;
- /// use kernel::types::{ARef, AlwaysRefCounted};
+ /// use kernel::types::{ARef, RefCounted};
///
/// struct Empty {}
///
/// # // SAFETY: TODO.
- /// unsafe impl AlwaysRefCounted for Empty {
+ /// unsafe impl RefCounted for Empty {
/// fn inc_ref(&self) {}
/// unsafe fn dec_ref(_obj: NonNull<Self>) {}
/// }
@@ -518,7 +527,7 @@ pub fn into_raw(me: Self) -> NonNull<T> {
}
}
-impl<T: AlwaysRefCounted> Clone for ARef<T> {
+impl<T: RefCounted> Clone for ARef<T> {
fn clone(&self) -> Self {
self.inc_ref();
// SAFETY: We just incremented the refcount above.
@@ -526,7 +535,7 @@ fn clone(&self) -> Self {
}
}
-impl<T: AlwaysRefCounted> Deref for ARef<T> {
+impl<T: RefCounted> Deref for ARef<T> {
type Target = T;
fn deref(&self) -> &Self::Target {
@@ -543,10 +552,10 @@ fn from(b: &T) -> Self {
}
}
-impl<T: AlwaysRefCounted> Drop for ARef<T> {
+impl<T: RefCounted> Drop for ARef<T> {
fn drop(&mut self) {
- // SAFETY: The type invariants guarantee that the `ARef` owns the reference we're about to
- // decrement.
+ // SAFETY: The type invariants guarantee that the `ARef` owns the reference
+ // we're about to decrement.
unsafe { T::dec_ref(self.ptr) };
}
}
--
2.48.1On Thu, Mar 13, 2025 at 07:00:11AM +0000, Oliver Mangold wrote:
> AlwaysRefCounted will become a marker trait to indicate that it is allowed
> to obtain an ARef from a `&`, which cannot be allowed for types which are
> also Ownable.
>
This commit log doesn't explain why we need to do this.
> Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
> Suggested-by: Alice Ryhl <aliceryhl@google.com>
> ---
> rust/kernel/block/mq/request.rs | 10 ++++++---
> rust/kernel/cred.rs | 8 ++++++--
> rust/kernel/device.rs | 8 ++++++--
> rust/kernel/fs/file.rs | 10 ++++++---
> rust/kernel/pid_namespace.rs | 8 ++++++--
> rust/kernel/task.rs | 6 +++++-
> rust/kernel/types.rs | 45 ++++++++++++++++++++++++-----------------
> 7 files changed, 64 insertions(+), 31 deletions(-)
>
> diff --git a/rust/kernel/block/mq/request.rs b/rust/kernel/block/mq/request.rs
> index 2f2bb5a04709cc90ae8971da166fc83bb53fb86b..f6bee0932499e554f88907861997f8b9d1ffd51a 100644
> --- a/rust/kernel/block/mq/request.rs
> +++ b/rust/kernel/block/mq/request.rs
> @@ -9,7 +9,7 @@
> block::mq::Operations,
> error::Result,
> sync::Refcount,
> - types::{ARef, AlwaysRefCounted, Opaque},
> + types::{ARef, AlwaysRefCounted, Opaque, RefCounted},
> };
> use core::{
> marker::PhantomData,
> @@ -209,10 +209,10 @@ unsafe impl<T: Operations> Send for Request<T> {}
> unsafe impl<T: Operations> Sync for Request<T> {}
>
> // SAFETY: All instances of `Request<T>` are reference counted. This
> -// implementation of `AlwaysRefCounted` ensure that increments to the ref count
> +// implementation of `RefCounted` ensure that increments to the ref count
> // keeps the object alive in memory at least until a matching reference count
> // decrement is executed.
> -unsafe impl<T: Operations> AlwaysRefCounted for Request<T> {
> +unsafe impl<T: Operations> RefCounted for Request<T> {
> fn inc_ref(&self) {
> self.wrapper_ref().refcount().inc();
> }
> @@ -234,3 +234,7 @@ unsafe fn dec_ref(obj: core::ptr::NonNull<Self>) {
> }
> }
> }
> +
> +// SAFETY: we currently do not implement `Ownable`, thus it is okay to can obtain an `ARef<Request>`
s/we/We
> +// from a `&Request` (but this will change in the future).
> +unsafe impl<T: Operations> AlwaysRefCounted for Request<T> {}
> diff --git a/rust/kernel/cred.rs b/rust/kernel/cred.rs
> index 81d67789b16f243e7832ff3b2e5e479a1ab2bf9e..e04d1021130eb1ec46fe48feb088959da7656d66 100644
> --- a/rust/kernel/cred.rs
> +++ b/rust/kernel/cred.rs
> @@ -11,7 +11,7 @@
> use crate::{
> bindings,
> task::Kuid,
> - types::{AlwaysRefCounted, Opaque},
> + types::{AlwaysRefCounted, Opaque, RefCounted},
> };
>
> /// Wraps the kernel's `struct cred`.
> @@ -71,7 +71,7 @@ pub fn euid(&self) -> Kuid {
> }
>
> // SAFETY: The type invariants guarantee that `Credential` is always ref-counted.
> -unsafe impl AlwaysRefCounted for Credential {
> +unsafe impl RefCounted for Credential {
> fn inc_ref(&self) {
> // SAFETY: The existence of a shared reference means that the refcount is nonzero.
> unsafe { bindings::get_cred(self.0.get()) };
> @@ -83,3 +83,7 @@ unsafe fn dec_ref(obj: core::ptr::NonNull<Credential>) {
> unsafe { bindings::put_cred(obj.cast().as_ptr()) };
> }
> }
> +
> +// SAFETY: We do not implement `Ownable`, thus it is okay to can obtain an `ARef<Credential>` from a
> +// `&Credential`.
> +unsafe impl AlwaysRefCounted for Credential {}
> diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
> index db2d9658ba47d9c492bc813ce3eb2ff29703ca31..189298518dc184405b1d62404b190d4c0b08b7ad 100644
> --- a/rust/kernel/device.rs
> +++ b/rust/kernel/device.rs
> @@ -7,7 +7,7 @@
> use crate::{
> bindings,
> str::CStr,
> - types::{ARef, Opaque},
> + types::{ARef, AlwaysRefCounted, Opaque, RefCounted},
> };
> use core::{fmt, ptr};
>
> @@ -190,7 +190,7 @@ pub fn property_present(&self, name: &CStr) -> bool {
> }
>
> // SAFETY: Instances of `Device` are always reference-counted.
> -unsafe impl crate::types::AlwaysRefCounted for Device {
> +unsafe impl RefCounted 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()) };
> @@ -202,6 +202,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
> }
> }
>
> +// SAFETY: We do not implement `Ownable`, thus it is okay to can obtain an `Device<Task>` from a
> +// `&Device`.
> +unsafe impl AlwaysRefCounted for Device {}
> +
> // SAFETY: As by the type invariant `Device` can be sent to any thread.
> unsafe impl Send for Device {}
>
> diff --git a/rust/kernel/fs/file.rs b/rust/kernel/fs/file.rs
> index e03dbe14d62a566349c4100f2f78b17d4c79aab5..a7836cc754e7927b6addc3bd06cfe8c8119f1d9f 100644
> --- a/rust/kernel/fs/file.rs
> +++ b/rust/kernel/fs/file.rs
> @@ -11,7 +11,7 @@
> bindings,
> cred::Credential,
> error::{code::*, Error, Result},
> - types::{ARef, AlwaysRefCounted, NotThreadSafe, Opaque},
> + types::{ARef, AlwaysRefCounted, NotThreadSafe, Opaque, RefCounted},
> };
> use core::ptr;
>
> @@ -190,7 +190,7 @@ unsafe impl Sync for File {}
>
> // SAFETY: The type invariants guarantee that `File` is always ref-counted. This implementation
> // makes `ARef<File>` own a normal refcount.
> -unsafe impl AlwaysRefCounted for File {
> +unsafe impl RefCounted for File {
> #[inline]
> fn inc_ref(&self) {
> // SAFETY: The existence of a shared reference means that the refcount is nonzero.
> @@ -205,6 +205,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<File>) {
> }
> }
>
> +// SAFETY: We do not implement `Ownable`, thus it is okay to can obtain an `ARef<File>` from a
> +/// `&File`.
> +unsafe impl AlwaysRefCounted for File {}
> +
> /// Wraps the kernel's `struct file`. Not thread safe.
> ///
> /// This type represents a file that is not known to be safe to transfer across thread boundaries.
> @@ -225,7 +229,7 @@ pub struct LocalFile {
>
> // SAFETY: The type invariants guarantee that `LocalFile` is always ref-counted. This implementation
> // makes `ARef<File>` own a normal refcount.
> -unsafe impl AlwaysRefCounted for LocalFile {
> +unsafe impl RefCounted for LocalFile {
> #[inline]
> fn inc_ref(&self) {
> // SAFETY: The existence of a shared reference means that the refcount is nonzero.
> diff --git a/rust/kernel/pid_namespace.rs b/rust/kernel/pid_namespace.rs
> index 0e93808e4639b37dd77add5d79f64058dac7cb87..3e45e945b7509b9607266b2e0e6ef130e7a1ed39 100644
> --- a/rust/kernel/pid_namespace.rs
> +++ b/rust/kernel/pid_namespace.rs
> @@ -9,7 +9,7 @@
>
> use crate::{
> bindings,
> - types::{AlwaysRefCounted, Opaque},
> + types::{AlwaysRefCounted, RefCounted, Opaque},
> };
> use core::ptr;
>
> @@ -44,7 +44,7 @@ pub unsafe fn from_ptr<'a>(ptr: *const bindings::pid_namespace) -> &'a Self {
> }
>
> // SAFETY: Instances of `PidNamespace` are always reference-counted.
> -unsafe impl AlwaysRefCounted for PidNamespace {
> +unsafe impl RefCounted for PidNamespace {
> #[inline]
> fn inc_ref(&self) {
> // SAFETY: The existence of a shared reference means that the refcount is nonzero.
> @@ -58,6 +58,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<PidNamespace>) {
> }
> }
>
> +// SAFETY: We do not implement `Ownable`, thus it is okay to can obtain an `ARef<PidNamespace>`
> +// from a `&PidNamespace`.
> +unsafe impl AlwaysRefCounted for PidNamespace {}
> +
> // SAFETY:
> // - `PidNamespace::dec_ref` can be called from any thread.
> // - It is okay to send ownership of `PidNamespace` across thread boundaries.
> diff --git a/rust/kernel/task.rs b/rust/kernel/task.rs
> index 07bc22a7645c0c7d792a0a163dd55b8ff0fe5f92..1cc1b02cb7fada6703da8cad77549dee578fc08d 100644
> --- a/rust/kernel/task.rs
> +++ b/rust/kernel/task.rs
> @@ -327,7 +327,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::types::RefCounted 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.as_ptr()) };
> @@ -339,6 +339,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
> }
> }
>
> +// SAFETY: We do not implement `Ownable`, thus it is okay to can obtain an `ARef<Task>` from a
> +// `&Task`.
> +unsafe impl crate::types::AlwaysRefCounted for Task {}
> +
> impl Kuid {
> /// Get the current euid.
> #[inline]
> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index 65f6d0721f5f23c8db79c6735dc7d5e1ac984ea7..5a96da714348cc2369969200e6070972226c00fe 100644
> --- a/rust/kernel/types.rs
> +++ b/rust/kernel/types.rs
> @@ -402,11 +402,9 @@ pub const fn raw_get(this: *const Self) -> *mut T {
> }
> }
>
> -/// Types that are _always_ reference counted.
> +/// Types that are internally 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
> @@ -418,9 +416,9 @@ pub const fn raw_get(this: *const Self) -> *mut T {
> /// 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
> +/// won't be able to honour the requirement that [`RefCounted::inc_ref`] keep the object
> /// alive.)
> -pub unsafe trait AlwaysRefCounted {
> +pub unsafe trait RefCounted {
> /// Increments the reference count on the object.
> fn inc_ref(&self);
>
> @@ -433,11 +431,22 @@ pub unsafe trait AlwaysRefCounted {
> /// 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).
> + /// (e.g., it calls [`RefCounted::inc_ref`] twice, then calls
> + /// [`RefCounted::dec_ref`] once).
> unsafe fn dec_ref(obj: NonNull<Self>);
> }
>
> +/// An extension to RefCounted, which declares that it is allowed to convert
> +/// from a shared reference `&T` to an owned reference [`ARef<T>`].
> +///
> +/// # Safety
> +///
> +/// Implementers must ensure that no safety invariants are violated by upgrading an `&T`
> +/// to an [`ARef<T>`]. In particular that implies [`AlwaysRefCounted`] and [`Ownable`]
> +/// cannot be implemented for the same type, as this would allow to violate the uniqueness
> +/// guarantee of [`Owned<T>`] by derefencing it into an `&T` and obtaining an [`ARef`] from that.
> +pub unsafe trait AlwaysRefCounted: RefCounted {}
> +
> /// An owned reference to an always-reference-counted object.
> ///
> /// The object's reference count is automatically decremented when an instance of [`ARef`] is
> @@ -448,7 +457,7 @@ pub unsafe trait AlwaysRefCounted {
> ///
> /// 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> {
> +pub struct ARef<T: RefCounted> {
> ptr: NonNull<T>,
> _p: PhantomData<T>,
> }
> @@ -457,16 +466,16 @@ pub struct ARef<T: AlwaysRefCounted> {
> // 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> {}
> +unsafe impl<T: RefCounted + 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> {}
> +unsafe impl<T: RefCounted + Sync + Send> Sync for ARef<T> {}
>
> -impl<T: AlwaysRefCounted> ARef<T> {
> +impl<T: RefCounted> ARef<T> {
> /// Creates a new instance of [`ARef`].
> ///
> /// It takes over an increment of the reference count on the underlying object.
> @@ -495,12 +504,12 @@ pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
> ///
> /// ```
> /// use core::ptr::NonNull;
> - /// use kernel::types::{ARef, AlwaysRefCounted};
> + /// use kernel::types::{ARef, RefCounted};
> ///
> /// struct Empty {}
> ///
> /// # // SAFETY: TODO.
While you at it, could you also finish this todo? ;-) Thanks! I believe
this is just using a ZST to simulate a RefCounted type, therefore it's
safe to assume `Empty` is ref-counted.
Regards,
Boqun
> - /// unsafe impl AlwaysRefCounted for Empty {
> + /// unsafe impl RefCounted for Empty {
> /// fn inc_ref(&self) {}
> /// unsafe fn dec_ref(_obj: NonNull<Self>) {}
> /// }
> @@ -518,7 +527,7 @@ pub fn into_raw(me: Self) -> NonNull<T> {
> }
> }
>
> -impl<T: AlwaysRefCounted> Clone for ARef<T> {
> +impl<T: RefCounted> Clone for ARef<T> {
> fn clone(&self) -> Self {
> self.inc_ref();
> // SAFETY: We just incremented the refcount above.
> @@ -526,7 +535,7 @@ fn clone(&self) -> Self {
> }
> }
>
> -impl<T: AlwaysRefCounted> Deref for ARef<T> {
> +impl<T: RefCounted> Deref for ARef<T> {
> type Target = T;
>
> fn deref(&self) -> &Self::Target {
> @@ -543,10 +552,10 @@ fn from(b: &T) -> Self {
> }
> }
>
> -impl<T: AlwaysRefCounted> Drop for ARef<T> {
> +impl<T: RefCounted> Drop for ARef<T> {
> fn drop(&mut self) {
> - // SAFETY: The type invariants guarantee that the `ARef` owns the reference we're about to
> - // decrement.
> + // SAFETY: The type invariants guarantee that the `ARef` owns the reference
> + // we're about to decrement.
> unsafe { T::dec_ref(self.ptr) };
> }
> }
>
> --
> 2.48.1
>
>
On 250321 0920, Boqun Feng wrote: > On Thu, Mar 13, 2025 at 07:00:11AM +0000, Oliver Mangold wrote: > > AlwaysRefCounted will become a marker trait to indicate that it is allowed > > to obtain an ARef from a `&`, which cannot be allowed for types which are > > also Ownable. > > > > This commit log doesn't explain why we need to do this. Hi Boqun, what exactly do you have in mind? It mentions that it is to allow types that are both Ownable and RefCounted. That the next step is to introduce OwnableRefCounted? That we want to safely convert from ARef to Owned? > > + > > +// SAFETY: we currently do not implement `Ownable`, thus it is okay to can obtain an `ARef<Request>` > > s/we/We > Missed that one. Thanks. Best regards, Oliver
© 2016 - 2025 Red Hat, Inc.