[PATCH v10 0/5] New trait OwnableRefCounted for ARef<->Owned conversion.

[PATCH v10 0/5] New trait OwnableRefCounted for ARef<->Owned conversion.

[Oliver Mangold]

This allows to convert between ARef<T> and Owned<T> by
implementing the new trait OwnedRefCounted.

This way we will have a shared/unique reference counting scheme
for types with built-in refcounts in analogy to Arc/UniqueArc.

Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
---
Changes in v10:
- Moved kernel/ownable.rs to kernel/types/ownable.rs
- Fixes in documentation / comments as suggested by Andreas Hindborg
- Added Reviewed-by comment for Andreas Hindborg
- Fix rustfmt of pid_namespace.rs
- Link to v9: https://lore.kernel.org/r/20250325-unique-ref-v9-0-e91618c1de26@pm.me

Changes in v9:
- Rebase onto v6.14-rc7
- Move Ownable/OwnedRefCounted/Ownable, etc., into separate module
- Documentation fixes to Ownable/OwnableMut/OwnableRefCounted
- Add missing SAFETY documentation to ARef example
- Link to v8: https://lore.kernel.org/r/20250313-unique-ref-v8-0-3082ffc67a31@pm.me

Changes in v8:
- Fix Co-developed-by and Suggested-by tags as suggested by Miguel and Boqun
- Some small documentation fixes in Owned/Ownable patch
- removing redundant trait constraint on DerefMut for Owned as suggested by Boqun Feng
- make SimpleOwnedRefCounted no longer implement RefCounted as suggested by Boqun Feng
- documentation for RefCounted as suggested by Boqun Feng
- Link to v7: https://lore.kernel.org/r/20250310-unique-ref-v7-0-4caddb78aa05@pm.me

Changes in v7:
- Squash patch to make Owned::from_raw/into_raw public into parent
- Added Signed-off-by to other people's commits
- Link to v6: https://lore.kernel.org/r/20250310-unique-ref-v6-0-1ff53558617e@pm.me

Changes in v6:
- Changed comments/formatting as suggested by Miguel Ojeda
- Included and used new config flag RUSTC_HAS_DO_NOT_RECOMMEND,
  thus no changes to types.rs will be needed when the attribute
  becomes available.
- Fixed commit message for Owned patch.
- Link to v5: https://lore.kernel.org/r/20250307-unique-ref-v5-0-bffeb633277e@pm.me

Changes in v5:
- Rebase the whole thing on top of the Ownable/Owned traits by Asahi Lina.
- Rename AlwaysRefCounted to RefCounted and make AlwaysRefCounted a
  marker trait instead to allow to obtain an ARef<T> from an &T,
  which (as Alice pointed out) is unsound when combined with UniqueRef/Owned.
- Change the Trait design and naming to implement this feature,
  UniqueRef/UniqueRefCounted is dropped in favor of Ownable/Owned and
  OwnableRefCounted is used to provide the functions to convert
  between Owned and ARef.
- Link to v4: https://lore.kernel.org/r/20250305-unique-ref-v4-1-a8fdef7b1c2c@pm.me

Changes in v4:
- Just a minor change in naming by request from Andreas Hindborg,
  try_shared_to_unique() -> try_from_shared(),
  unique_to_shared() -> into_shared(),
  which is more in line with standard Rust naming conventions.
- Link to v3: https://lore.kernel.org/r/Z8Wuud2UQX6Yukyr@mango

---
Asahi Lina (1):
      rust: types: Add Ownable/Owned types

Miguel Ojeda (1):
      rust: kbuild: provide `RUSTC_HAS_DO_NOT_RECOMMEND` symbol

Oliver Mangold (3):
      rust: Rename AlwaysRefCounted to RefCounted
      rust: Add missing SAFETY documentation for ARef example
      rust: Add OwnableRefCounted and SimpleOwnableRefCounted

 init/Kconfig                    |   3 +
 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/pci.rs              |   6 +-
 rust/kernel/pid_namespace.rs    |   8 +-
 rust/kernel/platform.rs         |   6 +-
 rust/kernel/task.rs             |   6 +-
 rust/kernel/types.rs            |  56 ++++---
 rust/kernel/types/ownable.rs    | 361 ++++++++++++++++++++++++++++++++++++++++
 11 files changed, 448 insertions(+), 34 deletions(-)
---
base-commit: 8ffd015db85fea3e15a77027fda6c02ced4d2444
change-id: 20250305-unique-ref-29fcd675f9e9

Best regards,
-- 
Oliver Mangold <oliver.mangold@pm.me>

[PATCH v10 1/5] rust: types: Add Ownable/Owned types

[Oliver Mangold]

From: Asahi Lina <lina@asahilina.net>

By analogy to AlwaysRefCounted and ARef, an Ownable type is a (typically
C FFI) type that *may* be owned by Rust, but need not be. Unlike
AlwaysRefCounted, this mechanism expects the reference to be unique
within Rust, and does not allow cloning.

Conceptually, this is similar to a KBox<T>, except that it delegates
resource management to the T instead of using a generic allocator.

Link: https://lore.kernel.org/all/20250202-rust-page-v1-1-e3170d7fe55e@asahilina.net/
Signed-off-by: Asahi Lina <lina@asahilina.net>
[ om:
  - split code into separate file and `pub use` it from types.rs
  - make from_raw() and into_raw() public
  - fixes to documentation
]
Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
---
 rust/kernel/types.rs         |   3 ++
 rust/kernel/types/ownable.rs | 117 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 120 insertions(+)

diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index 9d0471afc9648f2973235488b441eb109069adb1..5d8a99dcba4bf733107635bf3f0c15840ec33e4c 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -11,6 +11,9 @@
 };
 use pin_init::{PinInit, Zeroable};
 
+pub mod ownable;
+pub use ownable::{Ownable, OwnableMut, Owned};
+
 /// Used to transfer ownership to and from foreign (non-Rust) languages.
 ///
 /// Ownership is transferred from Rust to a foreign language by calling [`Self::into_foreign`] and
diff --git a/rust/kernel/types/ownable.rs b/rust/kernel/types/ownable.rs
new file mode 100644
index 0000000000000000000000000000000000000000..52e7a69019f1e2bbbe3cf715651b67a5a5c7c13d
--- /dev/null
+++ b/rust/kernel/types/ownable.rs
@@ -0,0 +1,117 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Owned reference types.
+
+use core::{
+    marker::PhantomData,
+    mem::ManuallyDrop,
+    ops::{Deref, DerefMut},
+    ptr::NonNull,
+};
+
+/// Types that may be owned by Rust code or borrowed, but have a lifetime managed by C code.
+///
+/// It allows such types to define their own custom destructor function to be called when
+/// a Rust-owned reference is dropped.
+///
+/// This is usually implemented by wrappers to existing structures on the C side of the code.
+///
+/// # Safety
+///
+/// Implementers must ensure that:
+/// - Any objects owned by Rust as [`Owned<T>`] stay alive while that owned reference exists (i.e.
+///   until the [`release()`](Ownable::release) trait method is called).
+/// - That the C code follows the usual mutable reference requirements. That is, the kernel will
+///   never mutate the [`Ownable`] (excluding internal mutability that follows the usual rules)
+///   while Rust owns it.
+pub unsafe trait Ownable {
+    /// Releases the object (frees it or returns it to foreign ownership).
+    ///
+    /// # Safety
+    ///
+    /// Callers must ensure that the object is no longer referenced after this call.
+    unsafe fn release(this: NonNull<Self>);
+}
+
+/// A subtrait of Ownable that asserts that an [`Owned<T>`] or `&mut Owned<T>` Rust reference
+/// may be dereferenced into a `&mut T`.
+///
+/// # Safety
+///
+/// Implementers must ensure that access to a `&mut T` is safe, implying that it is okay to call
+/// [`core::mem::swap`] on the `Ownable`. This excludes pinned types (meaning: most kernel types).
+pub unsafe trait OwnableMut: Ownable {}
+
+/// An owned reference to an ownable kernel object.
+///
+/// The object is automatically freed or released when an instance of [`Owned`] is
+/// dropped.
+///
+/// # Invariants
+///
+/// The pointer stored in `ptr` is valid for the lifetime of the [`Owned`] instance.
+pub struct Owned<T: Ownable> {
+    ptr: NonNull<T>,
+    _p: PhantomData<T>,
+}
+
+// SAFETY: It is safe to send `Owned<T>` to another thread when the underlying `T` is `Send` because
+// it effectively means sending a `&mut T` (which is safe because `T` is `Send`).
+unsafe impl<T: Ownable + Send> Send for Owned<T> {}
+
+// SAFETY: It is safe to send `&Owned<T>` to another thread when the underlying `T` is `Sync`
+// because it effectively means sharing `&T` (which is safe because `T` is `Sync`).
+unsafe impl<T: Ownable + Sync> Sync for Owned<T> {}
+
+impl<T: Ownable> Owned<T> {
+    /// Creates a new instance of [`Owned`].
+    ///
+    /// It takes over ownership of the underlying object.
+    ///
+    /// # Safety
+    ///
+    /// Callers must ensure that the underlying object is acquired and can be considered owned by
+    /// Rust.
+    pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
+        // INVARIANT: The safety requirements guarantee that the new instance now owns the
+        // reference.
+        Self {
+            ptr,
+            _p: PhantomData,
+        }
+    }
+
+    /// Consumes the [`Owned`], returning a raw pointer.
+    ///
+    /// This function does not actually relinquish ownership of the object.
+    /// After calling this function, the caller is responsible for ownership previously managed
+    /// by the [`Owned`].
+    pub fn into_raw(me: Self) -> NonNull<T> {
+        ManuallyDrop::new(me).ptr
+    }
+}
+
+impl<T: Ownable> Deref for Owned<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: OwnableMut> DerefMut for Owned<T> {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        // SAFETY: The type invariants guarantee that the object is valid,
+        // and that we can safely return a mutable reference to it.
+        unsafe { self.ptr.as_mut() }
+    }
+}
+
+impl<T: Ownable> Drop for Owned<T> {
+    fn drop(&mut self) {
+        // SAFETY: The type invariants guarantee that the `Owned` owns the object we're about to
+        // release.
+        unsafe { T::release(self.ptr) };
+    }
+}

-- 
2.49.0

Re: [PATCH v10 1/5] rust: types: Add Ownable/Owned types

[Andreas Hindborg]

"Oliver Mangold" <oliver.mangold@pm.me> writes:

> From: Asahi Lina <lina@asahilina.net>
>
> By analogy to AlwaysRefCounted and ARef, an Ownable type is a (typically
> C FFI) type that *may* be owned by Rust, but need not be. Unlike
> AlwaysRefCounted, this mechanism expects the reference to be unique
> within Rust, and does not allow cloning.
>
> Conceptually, this is similar to a KBox<T>, except that it delegates
> resource management to the T instead of using a generic allocator.
>
> Link: https://lore.kernel.org/all/20250202-rust-page-v1-1-e3170d7fe55e@asahilina.net/
> Signed-off-by: Asahi Lina <lina@asahilina.net>
> [ om:
>   - split code into separate file and `pub use` it from types.rs
>   - make from_raw() and into_raw() public
>   - fixes to documentation
> ]
> Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
> Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
> ---
>  rust/kernel/types.rs         |   3 ++
>  rust/kernel/types/ownable.rs | 117 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 120 insertions(+)
>
> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index 9d0471afc9648f2973235488b441eb109069adb1..5d8a99dcba4bf733107635bf3f0c15840ec33e4c 100644
> --- a/rust/kernel/types.rs
> +++ b/rust/kernel/types.rs
> @@ -11,6 +11,9 @@
>  };
>  use pin_init::{PinInit, Zeroable};
>
> +pub mod ownable;
> +pub use ownable::{Ownable, OwnableMut, Owned};
> +
>  /// Used to transfer ownership to and from foreign (non-Rust) languages.
>  ///
>  /// Ownership is transferred from Rust to a foreign language by calling [`Self::into_foreign`] and
> diff --git a/rust/kernel/types/ownable.rs b/rust/kernel/types/ownable.rs
> new file mode 100644
> index 0000000000000000000000000000000000000000..52e7a69019f1e2bbbe3cf715651b67a5a5c7c13d
> --- /dev/null
> +++ b/rust/kernel/types/ownable.rs
> @@ -0,0 +1,117 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Owned reference types.
> +
> +use core::{
> +    marker::PhantomData,
> +    mem::ManuallyDrop,
> +    ops::{Deref, DerefMut},
> +    ptr::NonNull,
> +};
> +
> +/// Types that may be owned by Rust code or borrowed, but have a lifetime managed by C code.
> +///
> +/// It allows such types to define their own custom destructor function to be called when
> +/// a Rust-owned reference is dropped.
> +///
> +/// This is usually implemented by wrappers to existing structures on the C side of the code.
> +///
> +/// # Safety
> +///
> +/// Implementers must ensure that:
> +/// - Any objects owned by Rust as [`Owned<T>`] stay alive while that owned reference exists (i.e.
> +///   until the [`release()`](Ownable::release) trait method is called).
> +/// - That the C code follows the usual mutable reference requirements. That is, the kernel will
> +///   never mutate the [`Ownable`] (excluding internal mutability that follows the usual rules)
> +///   while Rust owns it.
> +pub unsafe trait Ownable {
> +    /// Releases the object (frees it or returns it to foreign ownership).
> +    ///
> +    /// # Safety
> +    ///
> +    /// Callers must ensure that the object is no longer referenced after this call.
> +    unsafe fn release(this: NonNull<Self>);
> +}
> +
> +/// A subtrait of Ownable that asserts that an [`Owned<T>`] or `&mut Owned<T>` Rust reference
> +/// may be dereferenced into a `&mut T`.
> +///
> +/// # Safety
> +///
> +/// Implementers must ensure that access to a `&mut T` is safe, implying that it is okay to call
> +/// [`core::mem::swap`] on the `Ownable`. This excludes pinned types (meaning: most kernel types).
> +pub unsafe trait OwnableMut: Ownable {}
> +
> +/// An owned reference to an ownable kernel object.
> +///
> +/// The object is automatically freed or released when an instance of [`Owned`] is
> +/// dropped.
> +///
> +/// # Invariants
> +///
> +/// The pointer stored in `ptr` is valid for the lifetime of the [`Owned`] instance.
> +pub struct Owned<T: Ownable> {
> +    ptr: NonNull<T>,
> +    _p: PhantomData<T>,
> +}
> +
> +// SAFETY: It is safe to send `Owned<T>` to another thread when the underlying `T` is `Send` because
> +// it effectively means sending a `&mut T` (which is safe because `T` is `Send`).
> +unsafe impl<T: Ownable + Send> Send for Owned<T> {}
> +
> +// SAFETY: It is safe to send `&Owned<T>` to another thread when the underlying `T` is `Sync`
> +// because it effectively means sharing `&T` (which is safe because `T` is `Sync`).
> +unsafe impl<T: Ownable + Sync> Sync for Owned<T> {}
> +
> +impl<T: Ownable> Owned<T> {
> +    /// Creates a new instance of [`Owned`].
> +    ///
> +    /// It takes over ownership of the underlying object.
> +    ///
> +    /// # Safety
> +    ///
> +    /// Callers must ensure that the underlying object is acquired and can be considered owned by
> +    /// Rust.


This part "the underlying object is acquired" is unclear to me. How about:

  Callers must ensure that *ownership of* the underlying object has been
  acquired. That is, the object can be considered owned by the caller.


Best regards,
Andreas Hindborg

Re: [PATCH v10 1/5] rust: types: Add Ownable/Owned types

[Oliver Mangold]

On 250502 1157, Andreas Hindborg wrote:
> > +
> > +impl<T: Ownable> Owned<T> {
> > +    /// Creates a new instance of [`Owned`].
> > +    ///
> > +    /// It takes over ownership of the underlying object.
> > +    ///
> > +    /// # Safety
> > +    ///
> > +    /// Callers must ensure that the underlying object is acquired and can be considered owned by
> > +    /// Rust.
> 
> 
> This part "the underlying object is acquired" is unclear to me. How about:
> 
>   Callers must ensure that *ownership of* the underlying object has been
>   acquired. That is, the object can be considered owned by the caller.
> 
> 

Yes, made me think about the phrasing, too. But the main point is, that the
object must be considered to be owned by the `Owned<T>` after the function
call, no?

So maybe:

   Callers must ensure that ownership of the underlying object can be
   transfered to the `Owned<T>` and must consider it to be transfered
   after the function call. This usually implies that the object
   most not be accessed through `ptr` anymore.

Re: [PATCH v10 1/5] rust: types: Add Ownable/Owned types

[Andreas Hindborg]

"Oliver Mangold" <oliver.mangold@pm.me> writes:

> On 250502 1157, Andreas Hindborg wrote:
>> > +
>> > +impl<T: Ownable> Owned<T> {
>> > +    /// Creates a new instance of [`Owned`].
>> > +    ///
>> > +    /// It takes over ownership of the underlying object.
>> > +    ///
>> > +    /// # Safety
>> > +    ///
>> > +    /// Callers must ensure that the underlying object is acquired and can be considered owned by
>> > +    /// Rust.
>>
>>
>> This part "the underlying object is acquired" is unclear to me. How about:
>>
>>   Callers must ensure that *ownership of* the underlying object has been
>>   acquired. That is, the object can be considered owned by the caller.
>>
>>
>
> Yes, made me think about the phrasing, too. But the main point is, that the
> object must be considered to be owned by the `Owned<T>` after the function
> call, no?
>
> So maybe:
>
>    Callers must ensure that ownership of the underlying object can be
>    transfered to the `Owned<T>` and must consider it to be transfered
>    after the function call. This usually implies that the object
>    most not be accessed through `ptr` anymore.

Sounds good to me 👍


Best regards,
Andreas Hindborg

Re: [PATCH v10 1/5] rust: types: Add Ownable/Owned types

[Alice Ryhl]

On Fri, May 02, 2025 at 09:02:29AM +0000, Oliver Mangold wrote:
> From: Asahi Lina <lina@asahilina.net>
> 
> By analogy to AlwaysRefCounted and ARef, an Ownable type is a (typically
> C FFI) type that *may* be owned by Rust, but need not be. Unlike
> AlwaysRefCounted, this mechanism expects the reference to be unique
> within Rust, and does not allow cloning.
> 
> Conceptually, this is similar to a KBox<T>, except that it delegates
> resource management to the T instead of using a generic allocator.
> 
> Link: https://lore.kernel.org/all/20250202-rust-page-v1-1-e3170d7fe55e@asahilina.net/
> Signed-off-by: Asahi Lina <lina@asahilina.net>
> [ om:
>   - split code into separate file and `pub use` it from types.rs
>   - make from_raw() and into_raw() public
>   - fixes to documentation
> ]
> Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
> Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
> ---
>  rust/kernel/types.rs         |   3 ++
>  rust/kernel/types/ownable.rs | 117 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 120 insertions(+)
> 
> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index 9d0471afc9648f2973235488b441eb109069adb1..5d8a99dcba4bf733107635bf3f0c15840ec33e4c 100644
> --- a/rust/kernel/types.rs
> +++ b/rust/kernel/types.rs
> @@ -11,6 +11,9 @@
>  };
>  use pin_init::{PinInit, Zeroable};
>  
> +pub mod ownable;
> +pub use ownable::{Ownable, OwnableMut, Owned};
> +
>  /// Used to transfer ownership to and from foreign (non-Rust) languages.
>  ///
>  /// Ownership is transferred from Rust to a foreign language by calling [`Self::into_foreign`] and
> diff --git a/rust/kernel/types/ownable.rs b/rust/kernel/types/ownable.rs
> new file mode 100644
> index 0000000000000000000000000000000000000000..52e7a69019f1e2bbbe3cf715651b67a5a5c7c13d
> --- /dev/null
> +++ b/rust/kernel/types/ownable.rs
> @@ -0,0 +1,117 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Owned reference types.
> +
> +use core::{
> +    marker::PhantomData,
> +    mem::ManuallyDrop,
> +    ops::{Deref, DerefMut},
> +    ptr::NonNull,
> +};
> +
> +/// Types that may be owned by Rust code or borrowed, but have a lifetime managed by C code.
> +///
> +/// It allows such types to define their own custom destructor function to be called when
> +/// a Rust-owned reference is dropped.
> +///
> +/// This is usually implemented by wrappers to existing structures on the C side of the code.
> +///
> +/// # Safety
> +///
> +/// Implementers must ensure that:
> +/// - Any objects owned by Rust as [`Owned<T>`] stay alive while that owned reference exists (i.e.
> +///   until the [`release()`](Ownable::release) trait method is called).
> +/// - That the C code follows the usual mutable reference requirements. That is, the kernel will
> +///   never mutate the [`Ownable`] (excluding internal mutability that follows the usual rules)
> +///   while Rust owns it.

This seems too strong? Or does the exception mean to say that this does
not apply to anything containing `Opaque`? By far most structs using
this will use Opaque, so maybe directly mention Opaque instead?

That C code follows the usual aliasing rules. That is, unless the value
is wrapped in `Opaque` (or `UnsafeCell`), then the value must not be
modified in any way while Rust owns it, unless that modification happens
inside a `&mut T` method on the value.

> +pub unsafe trait Ownable {
> +    /// Releases the object (frees it or returns it to foreign ownership).
> +    ///
> +    /// # Safety
> +    ///
> +    /// Callers must ensure that the object is no longer referenced after this call.
> +    unsafe fn release(this: NonNull<Self>);
> +}
> +
> +/// A subtrait of Ownable that asserts that an [`Owned<T>`] or `&mut Owned<T>` Rust reference
> +/// may be dereferenced into a `&mut T`.
> +///
> +/// # Safety
> +///
> +/// Implementers must ensure that access to a `&mut T` is safe, implying that it is okay to call
> +/// [`core::mem::swap`] on the `Ownable`. This excludes pinned types (meaning: most kernel types).
> +pub unsafe trait OwnableMut: Ownable {}
> +
> +/// An owned reference to an ownable kernel object.
> +///
> +/// The object is automatically freed or released when an instance of [`Owned`] is
> +/// dropped.
> +///
> +/// # Invariants
> +///
> +/// The pointer stored in `ptr` is valid for the lifetime of the [`Owned`] instance.

This should probably talk about ownership.

> +pub struct Owned<T: Ownable> {
> +    ptr: NonNull<T>,
> +    _p: PhantomData<T>,
> +}

Alice

Re: [PATCH v10 1/5] rust: types: Add Ownable/Owned types

[Andreas Hindborg]

"Alice Ryhl" <aliceryhl@google.com> writes:

> On Fri, May 02, 2025 at 09:02:29AM +0000, Oliver Mangold wrote:
>> From: Asahi Lina <lina@asahilina.net>
>>
>> By analogy to AlwaysRefCounted and ARef, an Ownable type is a (typically
>> C FFI) type that *may* be owned by Rust, but need not be. Unlike
>> AlwaysRefCounted, this mechanism expects the reference to be unique
>> within Rust, and does not allow cloning.
>>
>> Conceptually, this is similar to a KBox<T>, except that it delegates
>> resource management to the T instead of using a generic allocator.
>>
>> Link: https://lore.kernel.org/all/20250202-rust-page-v1-1-e3170d7fe55e@asahilina.net/
>> Signed-off-by: Asahi Lina <lina@asahilina.net>
>> [ om:
>>   - split code into separate file and `pub use` it from types.rs
>>   - make from_raw() and into_raw() public
>>   - fixes to documentation
>> ]
>> Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
>> Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
>> ---
>>  rust/kernel/types.rs         |   3 ++
>>  rust/kernel/types/ownable.rs | 117 +++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 120 insertions(+)
>>
>> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
>> index 9d0471afc9648f2973235488b441eb109069adb1..5d8a99dcba4bf733107635bf3f0c15840ec33e4c 100644
>> --- a/rust/kernel/types.rs
>> +++ b/rust/kernel/types.rs
>> @@ -11,6 +11,9 @@
>>  };
>>  use pin_init::{PinInit, Zeroable};
>>
>> +pub mod ownable;
>> +pub use ownable::{Ownable, OwnableMut, Owned};
>> +
>>  /// Used to transfer ownership to and from foreign (non-Rust) languages.
>>  ///
>>  /// Ownership is transferred from Rust to a foreign language by calling [`Self::into_foreign`] and
>> diff --git a/rust/kernel/types/ownable.rs b/rust/kernel/types/ownable.rs
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..52e7a69019f1e2bbbe3cf715651b67a5a5c7c13d
>> --- /dev/null
>> +++ b/rust/kernel/types/ownable.rs
>> @@ -0,0 +1,117 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +//! Owned reference types.
>> +
>> +use core::{
>> +    marker::PhantomData,
>> +    mem::ManuallyDrop,
>> +    ops::{Deref, DerefMut},
>> +    ptr::NonNull,
>> +};
>> +
>> +/// Types that may be owned by Rust code or borrowed, but have a lifetime managed by C code.
>> +///
>> +/// It allows such types to define their own custom destructor function to be called when
>> +/// a Rust-owned reference is dropped.
>> +///
>> +/// This is usually implemented by wrappers to existing structures on the C side of the code.
>> +///
>> +/// # Safety
>> +///
>> +/// Implementers must ensure that:
>> +/// - Any objects owned by Rust as [`Owned<T>`] stay alive while that owned reference exists (i.e.
>> +///   until the [`release()`](Ownable::release) trait method is called).
>> +/// - That the C code follows the usual mutable reference requirements. That is, the kernel will
>> +///   never mutate the [`Ownable`] (excluding internal mutability that follows the usual rules)
>> +///   while Rust owns it.
>
> This seems too strong? Or does the exception mean to say that this does
> not apply to anything containing `Opaque`? By far most structs using
> this will use Opaque, so maybe directly mention Opaque instead?

`Opaque` is covered by "(excluding internal mutability that follows the usual rules)".

>
> That C code follows the usual aliasing rules. That is, unless the value
> is wrapped in `Opaque` (or `UnsafeCell`), then the value must not be
> modified in any way while Rust owns it, unless that modification happens
> inside a `&mut T` method on the value.
>
>> +pub unsafe trait Ownable {
>> +    /// Releases the object (frees it or returns it to foreign ownership).
>> +    ///
>> +    /// # Safety
>> +    ///
>> +    /// Callers must ensure that the object is no longer referenced after this call.
>> +    unsafe fn release(this: NonNull<Self>);
>> +}
>> +
>> +/// A subtrait of Ownable that asserts that an [`Owned<T>`] or `&mut Owned<T>` Rust reference
>> +/// may be dereferenced into a `&mut T`.
>> +///
>> +/// # Safety
>> +///
>> +/// Implementers must ensure that access to a `&mut T` is safe, implying that it is okay to call
>> +/// [`core::mem::swap`] on the `Ownable`. This excludes pinned types (meaning: most kernel types).
>> +pub unsafe trait OwnableMut: Ownable {}
>> +
>> +/// An owned reference to an ownable kernel object.
>> +///
>> +/// The object is automatically freed or released when an instance of [`Owned`] is
>> +/// dropped.
>> +///
>> +/// # Invariants
>> +///
>> +/// The pointer stored in `ptr` is valid for the lifetime of the [`Owned`] instance.
>
> This should probably talk about ownership.

How about

  The pointee of `ptr` can be considered owned by the [`Owned`] instance.


Best regards,
Andreas Hindborg

Re: [PATCH v10 1/5] rust: types: Add Ownable/Owned types

[Alice Ryhl]

On Tue, May 06, 2025 at 01:20:04PM +0200, Andreas Hindborg wrote:
> "Alice Ryhl" <aliceryhl@google.com> writes:
> 
> > On Fri, May 02, 2025 at 09:02:29AM +0000, Oliver Mangold wrote:
> >> From: Asahi Lina <lina@asahilina.net>
> >>
> >> By analogy to AlwaysRefCounted and ARef, an Ownable type is a (typically
> >> C FFI) type that *may* be owned by Rust, but need not be. Unlike
> >> AlwaysRefCounted, this mechanism expects the reference to be unique
> >> within Rust, and does not allow cloning.
> >>
> >> Conceptually, this is similar to a KBox<T>, except that it delegates
> >> resource management to the T instead of using a generic allocator.
> >>
> >> Link: https://lore.kernel.org/all/20250202-rust-page-v1-1-e3170d7fe55e@asahilina.net/
> >> Signed-off-by: Asahi Lina <lina@asahilina.net>
> >> [ om:
> >>   - split code into separate file and `pub use` it from types.rs
> >>   - make from_raw() and into_raw() public
> >>   - fixes to documentation
> >> ]
> >> Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
> >> Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
> >> ---
> >>  rust/kernel/types.rs         |   3 ++
> >>  rust/kernel/types/ownable.rs | 117 +++++++++++++++++++++++++++++++++++++++++++
> >>  2 files changed, 120 insertions(+)
> >>
> >> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> >> index 9d0471afc9648f2973235488b441eb109069adb1..5d8a99dcba4bf733107635bf3f0c15840ec33e4c 100644
> >> --- a/rust/kernel/types.rs
> >> +++ b/rust/kernel/types.rs
> >> @@ -11,6 +11,9 @@
> >>  };
> >>  use pin_init::{PinInit, Zeroable};
> >>
> >> +pub mod ownable;
> >> +pub use ownable::{Ownable, OwnableMut, Owned};
> >> +
> >>  /// Used to transfer ownership to and from foreign (non-Rust) languages.
> >>  ///
> >>  /// Ownership is transferred from Rust to a foreign language by calling [`Self::into_foreign`] and
> >> diff --git a/rust/kernel/types/ownable.rs b/rust/kernel/types/ownable.rs
> >> new file mode 100644
> >> index 0000000000000000000000000000000000000000..52e7a69019f1e2bbbe3cf715651b67a5a5c7c13d
> >> --- /dev/null
> >> +++ b/rust/kernel/types/ownable.rs
> >> @@ -0,0 +1,117 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +
> >> +//! Owned reference types.
> >> +
> >> +use core::{
> >> +    marker::PhantomData,
> >> +    mem::ManuallyDrop,
> >> +    ops::{Deref, DerefMut},
> >> +    ptr::NonNull,
> >> +};
> >> +
> >> +/// Types that may be owned by Rust code or borrowed, but have a lifetime managed by C code.
> >> +///
> >> +/// It allows such types to define their own custom destructor function to be called when
> >> +/// a Rust-owned reference is dropped.
> >> +///
> >> +/// This is usually implemented by wrappers to existing structures on the C side of the code.
> >> +///
> >> +/// # Safety
> >> +///
> >> +/// Implementers must ensure that:
> >> +/// - Any objects owned by Rust as [`Owned<T>`] stay alive while that owned reference exists (i.e.
> >> +///   until the [`release()`](Ownable::release) trait method is called).
> >> +/// - That the C code follows the usual mutable reference requirements. That is, the kernel will
> >> +///   never mutate the [`Ownable`] (excluding internal mutability that follows the usual rules)
> >> +///   while Rust owns it.
> >
> > This seems too strong? Or does the exception mean to say that this does
> > not apply to anything containing `Opaque`? By far most structs using
> > this will use Opaque, so maybe directly mention Opaque instead?
> 
> `Opaque` is covered by "(excluding internal mutability that follows the usual rules)".

I guess. But I think it would still be good to rephrase to mention
Opaque for ease of understanding.

> > That C code follows the usual aliasing rules. That is, unless the value
> > is wrapped in `Opaque` (or `UnsafeCell`), then the value must not be
> > modified in any way while Rust owns it, unless that modification happens
> > inside a `&mut T` method on the value.
> >
> >> +pub unsafe trait Ownable {
> >> +    /// Releases the object (frees it or returns it to foreign ownership).
> >> +    ///
> >> +    /// # Safety
> >> +    ///
> >> +    /// Callers must ensure that the object is no longer referenced after this call.
> >> +    unsafe fn release(this: NonNull<Self>);
> >> +}
> >> +
> >> +/// A subtrait of Ownable that asserts that an [`Owned<T>`] or `&mut Owned<T>` Rust reference
> >> +/// may be dereferenced into a `&mut T`.
> >> +///
> >> +/// # Safety
> >> +///
> >> +/// Implementers must ensure that access to a `&mut T` is safe, implying that it is okay to call
> >> +/// [`core::mem::swap`] on the `Ownable`. This excludes pinned types (meaning: most kernel types).
> >> +pub unsafe trait OwnableMut: Ownable {}
> >> +
> >> +/// An owned reference to an ownable kernel object.
> >> +///
> >> +/// The object is automatically freed or released when an instance of [`Owned`] is
> >> +/// dropped.
> >> +///
> >> +/// # Invariants
> >> +///
> >> +/// The pointer stored in `ptr` is valid for the lifetime of the [`Owned`] instance.
> >
> > This should probably talk about ownership.
> 
> How about
> 
>   The pointee of `ptr` can be considered owned by the [`Owned`] instance.

Sure.

Alice

Re: [PATCH v10 1/5] rust: types: Add Ownable/Owned types

[Andreas Hindborg]

"Oliver Mangold" <oliver.mangold@pm.me> writes:

> From: Asahi Lina <lina@asahilina.net>
>
> By analogy to AlwaysRefCounted and ARef, an Ownable type is a (typically
> C FFI) type that *may* be owned by Rust, but need not be. Unlike
> AlwaysRefCounted, this mechanism expects the reference to be unique
> within Rust, and does not allow cloning.
>
> Conceptually, this is similar to a KBox<T>, except that it delegates
> resource management to the T instead of using a generic allocator.
>
> Link: https://lore.kernel.org/all/20250202-rust-page-v1-1-e3170d7fe55e@asahilina.net/
> Signed-off-by: Asahi Lina <lina@asahilina.net>
> [ om:
>   - split code into separate file and `pub use` it from types.rs
>   - make from_raw() and into_raw() public
>   - fixes to documentation
> ]
> Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
> Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
> ---
>  rust/kernel/types.rs         |   3 ++
>  rust/kernel/types/ownable.rs | 117 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 120 insertions(+)
>
> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index 9d0471afc9648f2973235488b441eb109069adb1..5d8a99dcba4bf733107635bf3f0c15840ec33e4c 100644
> --- a/rust/kernel/types.rs
> +++ b/rust/kernel/types.rs
> @@ -11,6 +11,9 @@
>  };
>  use pin_init::{PinInit, Zeroable};
>
> +pub mod ownable;
> +pub use ownable::{Ownable, OwnableMut, Owned};
> +
>  /// Used to transfer ownership to and from foreign (non-Rust) languages.
>  ///
>  /// Ownership is transferred from Rust to a foreign language by calling [`Self::into_foreign`] and
> diff --git a/rust/kernel/types/ownable.rs b/rust/kernel/types/ownable.rs
> new file mode 100644
> index 0000000000000000000000000000000000000000..52e7a69019f1e2bbbe3cf715651b67a5a5c7c13d
> --- /dev/null
> +++ b/rust/kernel/types/ownable.rs
> @@ -0,0 +1,117 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Owned reference types.
> +
> +use core::{
> +    marker::PhantomData,
> +    mem::ManuallyDrop,
> +    ops::{Deref, DerefMut},
> +    ptr::NonNull,
> +};
> +
> +/// Types that may be owned by Rust code or borrowed, but have a lifetime managed by C code.
> +///
> +/// It allows such types to define their own custom destructor function to be called when
> +/// a Rust-owned reference is dropped.
> +///
> +/// This is usually implemented by wrappers to existing structures on the C side of the code.
> +///
> +/// # Safety
> +///
> +/// Implementers must ensure that:
> +/// - Any objects owned by Rust as [`Owned<T>`] stay alive while that owned reference exists (i.e.
> +///   until the [`release()`](Ownable::release) trait method is called).
> +/// - That the C code follows the usual mutable reference requirements. That is, the kernel will
> +///   never mutate the [`Ownable`] (excluding internal mutability that follows the usual rules)
> +///   while Rust owns it.
> +pub unsafe trait Ownable {
> +    /// Releases the object (frees it or returns it to foreign ownership).
> +    ///
> +    /// # Safety
> +    ///
> +    /// Callers must ensure that the object is no longer referenced after this call.
> +    unsafe fn release(this: NonNull<Self>);

We should probably add as a safety requirement that `this` points to a
valid `Self`.


Best regards,
Andreas Hindborg

Re: [PATCH v10 1/5] rust: types: Add Ownable/Owned types

[Benno Lossin]

On Fri May 2, 2025 at 11:02 AM CEST, Oliver Mangold wrote:
> +/// Types that may be owned by Rust code or borrowed, but have a lifetime managed by C code.
> +///
> +/// It allows such types to define their own custom destructor function to be called when
> +/// a Rust-owned reference is dropped.
> +///
> +/// This is usually implemented by wrappers to existing structures on the C side of the code.

The docs should mention `AlwaysRefCounted` and when to use it instead of
this trait. We should probably also backlink from `AlwaysRefCounted` to
`Ownable`.

> +///
> +/// # Safety
> +///
> +/// Implementers must ensure that:
> +/// - Any objects owned by Rust as [`Owned<T>`] stay alive while that owned reference exists (i.e.
> +///   until the [`release()`](Ownable::release) trait method is called).

I don't immediately understand what this means. How about "Any value of
type `Self` needs to be stored as [`Owned<Self>`]."? And then ask in
`Owned::from_raw` for a pointer that is valid indefinitely (or at least
until `release` is called).

> +/// - That the C code follows the usual mutable reference requirements. That is, the kernel will
> +///   never mutate the [`Ownable`] (excluding internal mutability that follows the usual rules)
> +///   while Rust owns it.

I feel like this requirement is better put on the `Owned::from_raw`
function.

> +pub unsafe trait Ownable {
> +    /// Releases the object (frees it or returns it to foreign ownership).
> +    ///
> +    /// # Safety
> +    ///
> +    /// Callers must ensure that the object is no longer referenced after this call.
> +    unsafe fn release(this: NonNull<Self>);
> +}
> +
> +/// A subtrait of Ownable that asserts that an [`Owned<T>`] or `&mut Owned<T>` Rust reference
> +/// may be dereferenced into a `&mut T`.

The "A subtrait of Ownable that asserts" sounds a bit clumsy to me, how
about "Type where [`Owned<Self>`] derefs to `&mut Self`."?

> +///
> +/// # Safety
> +///
> +/// Implementers must ensure that access to a `&mut T` is safe, implying that it is okay to call
> +/// [`core::mem::swap`] on the `Ownable`. This excludes pinned types (meaning: most kernel types).

I don't like that we put this requirement here, since it's actually
something that should be asserted by `Owned::from_raw`.
The reason for that is that anyone can call `Owned::from_raw` with a
pointer pointing to `Self` and there is no safety requirement on that
function that ensures the correctness of the `DerefMut` impl.

> +pub unsafe trait OwnableMut: Ownable {}

I don't like the name, but at the same time I also have no good
suggestion :( I'll think some more about it.

---
Cheers,
Benno

Re: [PATCH v10 1/5] rust: types: Add Ownable/Owned types

[Oliver Mangold]

On 250514 1132, Benno Lossin wrote:
> On Fri May 2, 2025 at 11:02 AM CEST, Oliver Mangold wrote:
> > +/// Types that may be owned by Rust code or borrowed, but have a lifetime managed by C code.
> > +///
> > +/// It allows such types to define their own custom destructor function to be called when
> > +/// a Rust-owned reference is dropped.
> > +///
> > +/// This is usually implemented by wrappers to existing structures on the C side of the code.
> 
> The docs should mention `AlwaysRefCounted` and when to use it instead of
> this trait. We should probably also backlink from `AlwaysRefCounted` to
> `Ownable`.

Will do.

> > +///
> > +/// # Safety
> > +///
> > +/// Implementers must ensure that:
> > +/// - Any objects owned by Rust as [`Owned<T>`] stay alive while that owned reference exists (i.e.
> > +///   until the [`release()`](Ownable::release) trait method is called).
> 
> I don't immediately understand what this means. How about "Any value of
> type `Self` needs to be stored as [`Owned<Self>`]."?

Let me think. The safety requirements here talk about safety of
implementing the trait.  But if you have a `Self` which is not wrapped, you
still cannot create an `Owned<Self>` in safe code. It's different from an
`AlwaysRefCounted`, where an `ARef<Self>` can be created from a `&Self`.

> And then ask in
> `Owned::from_raw` for a pointer that is valid indefinitely (or at least
> until `release` is called).

So, hmm, I think one could even move this safety requirement to `Owned::from_raw()`.

> > +/// - That the C code follows the usual mutable reference requirements. That is, the kernel will
> > +///   never mutate the [`Ownable`] (excluding internal mutability that follows the usual rules)
> > +///   while Rust owns it.
> 
> I feel like this requirement is better put on the `Owned::from_raw`
> function.

Together with the above, this would leave to safety requirements for `Ownable.
Make `Ownable` a safe trait, then? Instead of safety requirements just add an invariant:

    # Invariant
    
    An `Owned<Self>` represents a unique reference to a `Self`, thus holding
    an `Owned<Self>` or `&mut Owned<Self>` allows one to assume that the object
    is not accessed concurrently from elsewhere.

Not sure what is best. Would that make sense?

> > +pub unsafe trait Ownable {
> > +    /// Releases the object (frees it or returns it to foreign ownership).
> > +    ///
> > +    /// # Safety
> > +    ///
> > +    /// Callers must ensure that the object is no longer referenced after this call.
> > +    unsafe fn release(this: NonNull<Self>);
> > +}
> > +
> > +/// A subtrait of Ownable that asserts that an [`Owned<T>`] or `&mut Owned<T>` Rust reference
> > +/// may be dereferenced into a `&mut T`.
> 
> The "A subtrait of Ownable that asserts" sounds a bit clumsy to me, how
> about "Type where [`Owned<Self>`] derefs to `&mut Self`."?

That's okay with me.

> > +///
> > +/// # Safety
> > +///
> > +/// Implementers must ensure that access to a `&mut T` is safe, implying that it is okay to call
> > +/// [`core::mem::swap`] on the `Ownable`. This excludes pinned types (meaning: most kernel types).
> 
> I don't like that we put this requirement here, since it's actually
> something that should be asserted by `Owned::from_raw`.
> The reason for that is that anyone can call `Owned::from_raw` with a
> pointer pointing to `Self` and there is no safety requirement on that
> function that ensures the correctness of the `DerefMut` impl.
> 
> > +pub unsafe trait OwnableMut: Ownable {}
> 
> I don't like the name, but at the same time I also have no good
> suggestion :( I'll think some more about it.
> 
> ---
> Cheers,
> Benno

Best regards,

Oliver

Re: [PATCH v10 1/5] rust: types: Add Ownable/Owned types

[Benno Lossin]

On Tue Jun 17, 2025 at 11:58 AM CEST, Oliver Mangold wrote:
> On 250514 1132, Benno Lossin wrote:
>> On Fri May 2, 2025 at 11:02 AM CEST, Oliver Mangold wrote:
>> > +///
>> > +/// # Safety
>> > +///
>> > +/// Implementers must ensure that:
>> > +/// - Any objects owned by Rust as [`Owned<T>`] stay alive while that owned reference exists (i.e.
>> > +///   until the [`release()`](Ownable::release) trait method is called).
>> 
>> I don't immediately understand what this means. How about "Any value of
>> type `Self` needs to be stored as [`Owned<Self>`]."?
>
> Let me think. The safety requirements here talk about safety of
> implementing the trait.  But if you have a `Self` which is not wrapped, you
> still cannot create an `Owned<Self>` in safe code. It's different from an
> `AlwaysRefCounted`, where an `ARef<Self>` can be created from a `&Self`.

That might be true, but AFAIK this trait is designed to be used for
stuff that has a `create_foo` and `destroy_foo` function in C returning
and taking a raw pointer to `foo` respectively. So creating it on the
stack doesn't make sense.

If we do want to make this trait more general, then we can do so, but
this is my current understanding.

>> And then ask in
>> `Owned::from_raw` for a pointer that is valid indefinitely (or at least
>> until `release` is called).
>
> So, hmm, I think one could even move this safety requirement to `Owned::from_raw()`.
>
>> > +/// - That the C code follows the usual mutable reference requirements. That is, the kernel will
>> > +///   never mutate the [`Ownable`] (excluding internal mutability that follows the usual rules)
>> > +///   while Rust owns it.
>> 
>> I feel like this requirement is better put on the `Owned::from_raw`
>> function.
>
> Together with the above, this would leave to safety requirements for `Ownable.
> Make `Ownable` a safe trait, then? Instead of safety requirements just add an invariant:
>
>     # Invariant
>     
>     An `Owned<Self>` represents a unique reference to a `Self`, thus holding
>     an `Owned<Self>` or `&mut Owned<Self>` allows one to assume that the object
>     is not accessed concurrently from elsewhere.
>
> Not sure what is best. Would that make sense?

Making it safe makes sense, when we can move all requirements to
`Owned::from_raw`. I don't think the invariants section makes sense, how
would the trait have any influence in that when `Owned::from_raw`
already guarantees it?

---
Cheers,
Benno

Re: [PATCH v10 1/5] rust: types: Add Ownable/Owned types

[Oliver Mangold]

On 250618 2322, Benno Lossin wrote:
> On Tue Jun 17, 2025 at 11:58 AM CEST, Oliver Mangold wrote:
> > On 250514 1132, Benno Lossin wrote:
> >> On Fri May 2, 2025 at 11:02 AM CEST, Oliver Mangold wrote:
> >> > +///
> >> > +/// # Safety
> >> > +///
> >> > +/// Implementers must ensure that:
> >> > +/// - Any objects owned by Rust as [`Owned<T>`] stay alive while that owned reference exists (i.e.
> >> > +///   until the [`release()`](Ownable::release) trait method is called).
> >>
> >> I don't immediately understand what this means. How about "Any value of
> >> type `Self` needs to be stored as [`Owned<Self>`]."?
> >
> > Let me think. The safety requirements here talk about safety of
> > implementing the trait.  But if you have a `Self` which is not wrapped, you
> > still cannot create an `Owned<Self>` in safe code. It's different from an
> > `AlwaysRefCounted`, where an `ARef<Self>` can be created from a `&Self`.
> 
> That might be true, but AFAIK this trait is designed to be used for
> stuff that has a `create_foo` and `destroy_foo` function in C returning
> and taking a raw pointer to `foo` respectively. So creating it on the
> stack doesn't make sense.

I didn't mean creating one on the stack, but keeping it in a raw pointer or
`NonNull<T>`, not bothering to wrap in in an `Owned<T>`. But doesn't
matter. In any case in v11 (which predates your answer), I moved this
requirement to `Owned::from_raw()`, as, you asked below, which should be
okay as that function is the only way to create an `Owned<T>`. But I can
add the "needs to be stored as `Owned<Self>`" requirement, if you think it
is important.


> If we do want to make this trait more general, then we can do so, but
> this is my current understanding.
> 
> >> And then ask in
> >> `Owned::from_raw` for a pointer that is valid indefinitely (or at least
> >> until `release` is called).
> >
> > So, hmm, I think one could even move this safety requirement to `Owned::from_raw()`.
> >
> >> > +/// - That the C code follows the usual mutable reference requirements. That is, the kernel will
> >> > +///   never mutate the [`Ownable`] (excluding internal mutability that follows the usual rules)
> >> > +///   while Rust owns it.
> >>
> >> I feel like this requirement is better put on the `Owned::from_raw`
> >> function.
> >
> > Together with the above, this would leave to safety requirements for `Ownable.
> > Make `Ownable` a safe trait, then? Instead of safety requirements just add an invariant:
> >
> >     # Invariant
> >
> >     An `Owned<Self>` represents a unique reference to a `Self`, thus holding
> >     an `Owned<Self>` or `&mut Owned<Self>` allows one to assume that the object
> >     is not accessed concurrently from elsewhere.
> >
> > Not sure what is best. Would that make sense?
> 
> Making it safe makes sense, when we can move all requirements to
> `Owned::from_raw`. I don't think the invariants section makes sense, how
> would the trait have any influence in that when `Owned::from_raw`
> already guarantees it?

I think you are right on that. Let's not do that.

Best,

Oliver

Re: [PATCH v10 1/5] rust: types: Add Ownable/Owned types

[Benno Lossin]

On Fri Jun 20, 2025 at 9:01 AM CEST, Oliver Mangold wrote:
> On 250618 2322, Benno Lossin wrote:
>> On Tue Jun 17, 2025 at 11:58 AM CEST, Oliver Mangold wrote:
>> > On 250514 1132, Benno Lossin wrote:
>> >> On Fri May 2, 2025 at 11:02 AM CEST, Oliver Mangold wrote:
>> >> > +///
>> >> > +/// # Safety
>> >> > +///
>> >> > +/// Implementers must ensure that:
>> >> > +/// - Any objects owned by Rust as [`Owned<T>`] stay alive while that owned reference exists (i.e.
>> >> > +///   until the [`release()`](Ownable::release) trait method is called).
>> >>
>> >> I don't immediately understand what this means. How about "Any value of
>> >> type `Self` needs to be stored as [`Owned<Self>`]."?
>> >
>> > Let me think. The safety requirements here talk about safety of
>> > implementing the trait.  But if you have a `Self` which is not wrapped, you
>> > still cannot create an `Owned<Self>` in safe code. It's different from an
>> > `AlwaysRefCounted`, where an `ARef<Self>` can be created from a `&Self`.
>> 
>> That might be true, but AFAIK this trait is designed to be used for
>> stuff that has a `create_foo` and `destroy_foo` function in C returning
>> and taking a raw pointer to `foo` respectively. So creating it on the
>> stack doesn't make sense.
>
> I didn't mean creating one on the stack, but keeping it in a raw pointer or
> `NonNull<T>`, not bothering to wrap in in an `Owned<T>`. But doesn't
> matter. In any case in v11 (which predates your answer), I moved this
> requirement to `Owned::from_raw()`, as, you asked below, which should be
> okay as that function is the only way to create an `Owned<T>`. But I can
> add the "needs to be stored as `Owned<Self>`" requirement, if you think it
> is important.

I'm not so sure, it depends on what we want `Owned<T>` to be. When I
take a look at v11, I might be able to figure it out.

---
Cheers,
Benno

Re: [PATCH v10 1/5] rust: types: Add Ownable/Owned types

[Oliver Mangold]

On 250514 1132, Benno Lossin wrote:
> On Fri May 2, 2025 at 11:02 AM CEST, Oliver Mangold wrote:
> 
> > +/// - That the C code follows the usual mutable reference requirements. That is, the kernel will
> > +///   never mutate the [`Ownable`] (excluding internal mutability that follows the usual rules)
> > +///   while Rust owns it.
> 
> I feel like this requirement is better put on the `Owned::from_raw`
> function.

Thinking about it some more, the problem I see here is that if the type
implements `OwnableMut` this requirement changes from "never mutate" to
"never access at all".

The safety requirements between `Ownable`, `OwnableMut`, `RefCounted`,
`OwnableRefCounted` and `AlwaysRefCounted` are interacting, but I agree
that, when looking at it a certain way, `Owned::from_raw()` is the place
where one would expect these to be. I'm not sure anymore what is best here
:/

> > +pub unsafe trait OwnableMut: Ownable {}
> 
> I don't like the name, but at the same time I also have no good
> suggestion :( I'll think some more about it.

There was already a bit of discussion about it. I had my own implementation of this
where I used the names `UniqueRefCounted` and `UniqueRef`, but after discovering
this version from Asahi Lina, I took it as it was, keeping the name.

No one else came up with different suggestions so far, so maybe we should just leave it
at `Owned`/`Ownable`?

Best,

Oliver

Re: [PATCH v10 1/5] rust: types: Add Ownable/Owned types

[Benno Lossin]

On Wed Jun 18, 2025 at 11:34 AM CEST, Oliver Mangold wrote:
> On 250514 1132, Benno Lossin wrote:
>> On Fri May 2, 2025 at 11:02 AM CEST, Oliver Mangold wrote:
>> 
>> > +/// - That the C code follows the usual mutable reference requirements. That is, the kernel will
>> > +///   never mutate the [`Ownable`] (excluding internal mutability that follows the usual rules)
>> > +///   while Rust owns it.
>> 
>> I feel like this requirement is better put on the `Owned::from_raw`
>> function.
>
> Thinking about it some more, the problem I see here is that if the type
> implements `OwnableMut` this requirement changes from "never mutate" to
> "never access at all".
>
> The safety requirements between `Ownable`, `OwnableMut`, `RefCounted`,
> `OwnableRefCounted` and `AlwaysRefCounted` are interacting, but I agree
> that, when looking at it a certain way, `Owned::from_raw()` is the place
> where one would expect these to be. I'm not sure anymore what is best here
> :/

I still think `Owned::from_raw` is the correct place to put this.

>
>> > +pub unsafe trait OwnableMut: Ownable {}
>> 
>> I don't like the name, but at the same time I also have no good
>> suggestion :( I'll think some more about it.
>
> There was already a bit of discussion about it. I had my own implementation of this
> where I used the names `UniqueRefCounted` and `UniqueRef`, but after discovering
> this version from Asahi Lina, I took it as it was, keeping the name.
>
> No one else came up with different suggestions so far, so maybe we should just leave it
> at `Owned`/`Ownable`?

I'm just hung up on the `Mut` part... Haven't come up with a good
replacement yet.

---
Cheers,
Benno

Re: [PATCH v10 1/5] rust: types: Add Ownable/Owned types

[Andreas Hindborg]

"Benno Lossin" <lossin@kernel.org> writes:

> On Wed Jun 18, 2025 at 11:34 AM CEST, Oliver Mangold wrote:
>> On 250514 1132, Benno Lossin wrote:
>>> On Fri May 2, 2025 at 11:02 AM CEST, Oliver Mangold wrote:
>>>
>>> > +/// - That the C code follows the usual mutable reference requirements. That is, the kernel will
>>> > +///   never mutate the [`Ownable`] (excluding internal mutability that follows the usual rules)
>>> > +///   while Rust owns it.
>>>
>>> I feel like this requirement is better put on the `Owned::from_raw`
>>> function.
>>
>> Thinking about it some more, the problem I see here is that if the type
>> implements `OwnableMut` this requirement changes from "never mutate" to
>> "never access at all".
>>
>> The safety requirements between `Ownable`, `OwnableMut`, `RefCounted`,
>> `OwnableRefCounted` and `AlwaysRefCounted` are interacting, but I agree
>> that, when looking at it a certain way, `Owned::from_raw()` is the place
>> where one would expect these to be. I'm not sure anymore what is best here
>> :/
>
> I still think `Owned::from_raw` is the correct place to put this.
>
>>
>>> > +pub unsafe trait OwnableMut: Ownable {}
>>>
>>> I don't like the name, but at the same time I also have no good
>>> suggestion :( I'll think some more about it.
>>
>> There was already a bit of discussion about it. I had my own implementation of this
>> where I used the names `UniqueRefCounted` and `UniqueRef`, but after discovering
>> this version from Asahi Lina, I took it as it was, keeping the name.
>>
>> No one else came up with different suggestions so far, so maybe we should just leave it
>> at `Owned`/`Ownable`?
>
> I'm just hung up on the `Mut` part... Haven't come up with a good
> replacement yet.

What do you dislike about the xxxxMut pattern?


Best regards,
Andreas Hindborg

Re: [PATCH v10 1/5] rust: types: Add Ownable/Owned types

[Benno Lossin]

On Thu Jun 19, 2025 at 11:33 AM CEST, Andreas Hindborg wrote:
> "Benno Lossin" <lossin@kernel.org> writes:
>> On Wed Jun 18, 2025 at 11:34 AM CEST, Oliver Mangold wrote:
>>> On 250514 1132, Benno Lossin wrote:
>>>> On Fri May 2, 2025 at 11:02 AM CEST, Oliver Mangold wrote:
>>>> > +pub unsafe trait OwnableMut: Ownable {}
>>>>
>>>> I don't like the name, but at the same time I also have no good
>>>> suggestion :( I'll think some more about it.
>>>
>>> There was already a bit of discussion about it. I had my own implementation of this
>>> where I used the names `UniqueRefCounted` and `UniqueRef`, but after discovering
>>> this version from Asahi Lina, I took it as it was, keeping the name.
>>>
>>> No one else came up with different suggestions so far, so maybe we should just leave it
>>> at `Owned`/`Ownable`?
>>
>> I'm just hung up on the `Mut` part... Haven't come up with a good
>> replacement yet.
>
> What do you dislike about the xxxxMut pattern?

Uh, I have re-read the docs & don't remember what originally I didn't
like about the name, so let's keep it :)

---
Cheers,
Benno

[PATCH v10 2/5] rust: Rename AlwaysRefCounted to RefCounted

[Oliver Mangold]

AlwaysRefCounted will become a marker trait to indicate that it is allowed
to obtain an ARef<T> from a `&T`, 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/pci.rs              |  6 +++++-
 rust/kernel/pid_namespace.rs    |  8 ++++++--
 rust/kernel/platform.rs         |  6 +++++-
 rust/kernel/task.rs             |  6 +++++-
 rust/kernel/types.rs            | 41 ++++++++++++++++++++++++-----------------
 9 files changed, 71 insertions(+), 32 deletions(-)

diff --git a/rust/kernel/block/mq/request.rs b/rust/kernel/block/mq/request.rs
index 4a5b7ec914efa598c65881b07c4ece59214fd7e7..5ec5dbc052df964f10403d65f9bf3dd847a41cf4 100644
--- a/rust/kernel/block/mq/request.rs
+++ b/rust/kernel/block/mq/request.rs
@@ -8,7 +8,7 @@
     bindings,
     block::mq::Operations,
     error::Result,
-    types::{ARef, AlwaysRefCounted, Opaque},
+    types::{ARef, AlwaysRefCounted, Opaque, RefCounted},
 };
 use core::{
     marker::PhantomData,
@@ -227,10 +227,10 @@ fn atomic_relaxed_op_unless(target: &AtomicU64, op: impl Fn(u64) -> u64, pred: u
 }
 
 // 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) {
         let refcount = &self.wrapper_ref().refcount();
 
@@ -260,3 +260,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 2599f01e8b285f2106aefd27c315ae2aff25293c..87fa2808050dd8a2838a0f5c21cd7f567ba6b534 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`.
@@ -74,7 +74,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 {
     #[inline]
     fn inc_ref(&self) {
         // SAFETY: The existence of a shared reference means that the refcount is nonzero.
@@ -88,3 +88,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 21b343a1dc4d2b4ba75c3886ba954be53ada88c2..0b77faa4cc972aeff5809939a5dbc430a396018b 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 13a0e44cd1aa812eb404d452c076f90a37275b5b..76032e6abb3f0f0ab1f6536ecff06e2c24255d00 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/pci.rs b/rust/kernel/pci.rs
index c97d6d470b282219de255450d0c1980aefcf12c0..4c1fc28a1b5526376377098139ea8c405b83ee25 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -443,7 +443,7 @@ fn from(dev: &Device<device::Core>) -> Self {
 }
 
 // SAFETY: Instances of `Device` are always reference-counted.
-unsafe impl crate::types::AlwaysRefCounted for Device {
+unsafe impl crate::types::RefCounted for Device {
     fn inc_ref(&self) {
         // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
         unsafe { bindings::pci_dev_get(self.as_raw()) };
@@ -455,6 +455,10 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
     }
 }
 
+// SAFETY: We do not implement `Ownable`, thus it is okay to can obtain an `ARef<Device>` from a
+// `&Device`.
+unsafe impl crate::types::AlwaysRefCounted for Device {}
+
 impl AsRef<device::Device> for Device {
     fn as_ref(&self) -> &device::Device {
         // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid
diff --git a/rust/kernel/pid_namespace.rs b/rust/kernel/pid_namespace.rs
index 0e93808e4639b37dd77add5d79f64058dac7cb87..3d8391113c1be150c5eb2ce354737bb1e4d63a80 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, Opaque, RefCounted},
 };
 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/platform.rs b/rust/kernel/platform.rs
index 4917cb34e2fe8027d3d861e90de51de85f006735..49add3ae88a6841f02a1e8a8f0043672f15cb088 100644
--- a/rust/kernel/platform.rs
+++ b/rust/kernel/platform.rs
@@ -211,7 +211,7 @@ fn from(dev: &Device<device::Core>) -> Self {
 }
 
 // SAFETY: Instances of `Device` are always reference-counted.
-unsafe impl crate::types::AlwaysRefCounted for Device {
+unsafe impl crate::types::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_ref().as_raw()) };
@@ -223,6 +223,10 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
     }
 }
 
+// SAFETY: We do not implement `Ownable`, thus it is okay to can obtain an `ARef<Device>` from a
+// `&Device`.
+unsafe impl crate::types::AlwaysRefCounted for Device {}
+
 impl AsRef<device::Device> for Device {
     fn as_ref(&self) -> &device::Device {
         // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid
diff --git a/rust/kernel/task.rs b/rust/kernel/task.rs
index 9e6f6854948d9ef9bb203a3548c9b082df8280e2..58086491ac85a219f795ecb473e9f6dd21bda773 100644
--- a/rust/kernel/task.rs
+++ b/rust/kernel/task.rs
@@ -329,7 +329,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()) };
@@ -341,6 +341,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 5d8a99dcba4bf733107635bf3f0c15840ec33e4c..d7fa8934c545f46a646ca900ab8957a04b0ad34d 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -394,11 +394,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
@@ -410,9 +408,8 @@ 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
-/// alive.)
-pub unsafe trait AlwaysRefCounted {
+/// won't be able to honour the requirement that [`RefCounted::inc_ref`] keep the object alive.)
+pub unsafe trait RefCounted {
     /// Increments the reference count on the object.
     fn inc_ref(&self);
 
@@ -425,11 +422,21 @@ 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
@@ -440,7 +447,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>,
 }
@@ -449,16 +456,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.
@@ -487,12 +494,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>) {}
     /// }
@@ -510,7 +517,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.
@@ -518,7 +525,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 {
@@ -535,7 +542,7 @@ 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.

-- 
2.49.0

Re: [PATCH v10 2/5] rust: Rename AlwaysRefCounted to RefCounted

[Andreas Hindborg]

"Oliver Mangold" <oliver.mangold@pm.me> writes:

> AlwaysRefCounted will become a marker trait to indicate that it is allowed
> to obtain an ARef<T> from a `&T`, 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>


I would suggest using `code spans` when you mention code items
(`ARef<T>` and `&T`) the commit message.


Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>


Best regards,
Andreas Hindborg

Re: [PATCH v10 2/5] rust: Rename AlwaysRefCounted to RefCounted

[Alice Ryhl]

On Fri, May 02, 2025 at 09:02:37AM +0000, Oliver Mangold wrote:
> AlwaysRefCounted will become a marker trait to indicate that it is allowed
> to obtain an ARef<T> from a `&T`, 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>

>  // 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.

It looks like "keeps" now fits on the previous line. I would reflow all
text in this patch.

> -/// 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
> @@ -410,9 +408,8 @@ 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
> -/// alive.)
> -pub unsafe trait AlwaysRefCounted {
> +/// won't be able to honour the requirement that [`RefCounted::inc_ref`] keep the object alive.)
> +pub unsafe trait RefCounted {
>      /// Increments the reference count on the object.
>      fn inc_ref(&self);
>  
> @@ -425,11 +422,21 @@ 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 {}

Adding a new trait should not happen in a commit called "rename X to Y".
Consider renaming this patch to "split AlwaysRefCounted into two traits"
or similar.

Alice

Re: [PATCH v10 2/5] rust: Rename AlwaysRefCounted to RefCounted

[Oliver Mangold]

On 250502 1132, Alice Ryhl wrote:
> On Fri, May 02, 2025 at 09:02:37AM +0000, Oliver Mangold wrote:
> > AlwaysRefCounted will become a marker trait to indicate that it is allowed
> > to obtain an ARef<T> from a `&T`, 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>
> 
> >  // 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.
> 
> It looks like "keeps" now fits on the previous line. I would reflow all
> text in this patch.

Say, this means you have a tool to do that automatically for you? I'm doing
it by hand currently.

> > +/// 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 {}
> 
> Adding a new trait should not happen in a commit called "rename X to Y".
> Consider renaming this patch to "split AlwaysRefCounted into two traits"
> or similar.

Sure. Will do.

Oliver

Re: [PATCH v10 2/5] rust: Rename AlwaysRefCounted to RefCounted

[Alice Ryhl]

On Mon, Jun 16, 2025 at 1:56 PM Oliver Mangold <oliver.mangold@pm.me> wrote:
>
> On 250502 1132, Alice Ryhl wrote:
> > On Fri, May 02, 2025 at 09:02:37AM +0000, Oliver Mangold wrote:
> > > AlwaysRefCounted will become a marker trait to indicate that it is allowed
> > > to obtain an ARef<T> from a `&T`, 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>
> >
> > >  // 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.
> >
> > It looks like "keeps" now fits on the previous line. I would reflow all
> > text in this patch.
>
> Say, this means you have a tool to do that automatically for you? I'm doing
> it by hand currently.

In vim you can do it by selecting the text and typing gq.

Alice

[PATCH v10 3/5] rust: Add missing SAFETY documentation for ARef example

[Oliver Mangold]

SAFETY comment in rustdoc example was just 'TODO'. Fixed.

Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
---
 rust/kernel/types.rs | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index d7fa8934c545f46a646ca900ab8957a04b0ad34d..33d2b4e4a87b991c6d934f4e8d2c6c71a15b1bcb 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -498,7 +498,9 @@ pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
     ///
     /// struct Empty {}
     ///
-    /// # // SAFETY: TODO.
+    /// // SAFETY: The `RefCounted` implementation for `Empty` does not count references
+    /// // and never frees the underlying object. Thus we can act as having a
+    /// // refcount on the object that we pass to the newly created `ARef`.
     /// unsafe impl RefCounted for Empty {
     ///     fn inc_ref(&self) {}
     ///     unsafe fn dec_ref(_obj: NonNull<Self>) {}
@@ -506,7 +508,7 @@ pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
     ///
     /// let mut data = Empty {};
     /// let ptr = NonNull::<Empty>::new(&mut data).unwrap();
-    /// # // SAFETY: TODO.
+    /// // SAFETY: We keep `data` around longer than the `ARef`.
     /// let data_ref: ARef<Empty> = unsafe { ARef::from_raw(ptr) };
     /// let raw_ptr: NonNull<Empty> = ARef::into_raw(data_ref);
     ///

-- 
2.49.0

Re: [PATCH v10 3/5] rust: Add missing SAFETY documentation for ARef example

[Andreas Hindborg]

"Oliver Mangold" <oliver.mangold@pm.me> writes:

> SAFETY comment in rustdoc example was just 'TODO'. Fixed.
>
> Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
> ---
>  rust/kernel/types.rs | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index d7fa8934c545f46a646ca900ab8957a04b0ad34d..33d2b4e4a87b991c6d934f4e8d2c6c71a15b1bcb 100644
> --- a/rust/kernel/types.rs
> +++ b/rust/kernel/types.rs
> @@ -498,7 +498,9 @@ pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
>      ///
>      /// struct Empty {}
>      ///
> -    /// # // SAFETY: TODO.
> +    /// // SAFETY: The `RefCounted` implementation for `Empty` does not count references
> +    /// // and never frees the underlying object. Thus we can act as having a
> +    /// // refcount on the object that we pass to the newly created `ARef`.
>      /// unsafe impl RefCounted for Empty {
>      ///     fn inc_ref(&self) {}
>      ///     unsafe fn dec_ref(_obj: NonNull<Self>) {}
> @@ -506,7 +508,7 @@ pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
>      ///
>      /// let mut data = Empty {};
>      /// let ptr = NonNull::<Empty>::new(&mut data).unwrap();
> -    /// # // SAFETY: TODO.
> +    /// // SAFETY: We keep `data` around longer than the `ARef`.

I still think this applies:

>> How about:
>> 
>>   The `RefCounted` implementation for `Empty` does not count references
>>   and never frees the underlying object. Thus we can act as having a
>>   refcount on the object that we pass to the newly created `ARef`.
>> 

>      /// let data_ref: ARef<Empty> = unsafe { ARef::from_raw(ptr) };
>      /// let raw_ptr: NonNull<Empty> = ARef::into_raw(data_ref);
>      ///


Best regards,
Andreas Hindborg

Re: [PATCH v10 3/5] rust: Add missing SAFETY documentation for ARef example

[Oliver Mangold]

On 250502 1241, Andreas Hindborg wrote:
> >
> > diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> > index d7fa8934c545f46a646ca900ab8957a04b0ad34d..33d2b4e4a87b991c6d934f4e8d2c6c71a15b1bcb 100644
> > --- a/rust/kernel/types.rs
> > +++ b/rust/kernel/types.rs
> > @@ -498,7 +498,9 @@ pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
> >      ///
> >      /// struct Empty {}
> >      ///
> > -    /// # // SAFETY: TODO.
> > +    /// // SAFETY: The `RefCounted` implementation for `Empty` does not count references
> > +    /// // and never frees the underlying object. Thus we can act as having a
> > +    /// // refcount on the object that we pass to the newly created `ARef`.
> >      /// unsafe impl RefCounted for Empty {
> >      ///     fn inc_ref(&self) {}
> >      ///     unsafe fn dec_ref(_obj: NonNull<Self>) {}
> > @@ -506,7 +508,7 @@ pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
> >      ///
> >      /// let mut data = Empty {};
> >      /// let ptr = NonNull::<Empty>::new(&mut data).unwrap();
> > -    /// # // SAFETY: TODO.
> > +    /// // SAFETY: We keep `data` around longer than the `ARef`.
> 
> I still think this applies:
> 
> >> How about:
> >>
> >>   The `RefCounted` implementation for `Empty` does not count references
> >>   and never frees the underlying object. Thus we can act as having a
> >>   refcount on the object that we pass to the newly created `ARef`.
> >>

Hi Andreas,

I agree. Sorry, I just messed up the fix. Your wording landed in the
previous to-be-fixed unsafe comment, as you can see.

Happens when you are too much in a hurry and didn't touch the patch for
too long :/

I will fix it in the next version.

Best,

Oliver

Re: [PATCH v10 3/5] rust: Add missing SAFETY documentation for ARef example

[Andreas Hindborg]

"Oliver Mangold" <oliver.mangold@pm.me> writes:

> On 250502 1241, Andreas Hindborg wrote:
>> >
>> > diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
>> > index d7fa8934c545f46a646ca900ab8957a04b0ad34d..33d2b4e4a87b991c6d934f4e8d2c6c71a15b1bcb 100644
>> > --- a/rust/kernel/types.rs
>> > +++ b/rust/kernel/types.rs
>> > @@ -498,7 +498,9 @@ pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
>> >      ///
>> >      /// struct Empty {}
>> >      ///
>> > -    /// # // SAFETY: TODO.
>> > +    /// // SAFETY: The `RefCounted` implementation for `Empty` does not count references
>> > +    /// // and never frees the underlying object. Thus we can act as having a
>> > +    /// // refcount on the object that we pass to the newly created `ARef`.
>> >      /// unsafe impl RefCounted for Empty {
>> >      ///     fn inc_ref(&self) {}
>> >      ///     unsafe fn dec_ref(_obj: NonNull<Self>) {}
>> > @@ -506,7 +508,7 @@ pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
>> >      ///
>> >      /// let mut data = Empty {};
>> >      /// let ptr = NonNull::<Empty>::new(&mut data).unwrap();
>> > -    /// # // SAFETY: TODO.
>> > +    /// // SAFETY: We keep `data` around longer than the `ARef`.
>>
>> I still think this applies:
>>
>> >> How about:
>> >>
>> >>   The `RefCounted` implementation for `Empty` does not count references
>> >>   and never frees the underlying object. Thus we can act as having a
>> >>   refcount on the object that we pass to the newly created `ARef`.
>> >>
>
> Hi Andreas,
>
> I agree. Sorry, I just messed up the fix. Your wording landed in the
> previous to-be-fixed unsafe comment, as you can see.
>
> Happens when you are too much in a hurry and didn't touch the patch for
> too long :/
>
> I will fix it in the next version.

Cool! Also check my response to v9: https://lore.kernel.org/all/87cycrz1pa.fsf@kernel.org


Best regards,
Andreas Hindborg

[PATCH v10 4/5] rust: kbuild: provide `RUSTC_HAS_DO_NOT_RECOMMEND` symbol

[Oliver Mangold]

From: Miguel Ojeda <ojeda@kernel.org>

Rust 1.85.0 (current stable version) stabilized [1]
`#[diagnostic::do_not_recommend]` [2].

In order to use it across all supported Rust versions, introduce a new
Kconfig symbol for it.

This allows to perform conditional compilation based on it, e.g. on the
use site to enable the attribute:

    #[cfg_attr(RUSTC_HAS_DO_NOT_RECOMMEND, diagnostic::do_not_recommend)]
    impl A for i32 {}

An alternative would have been to `allow` the following warning:

    #![allow(unknown_or_malformed_diagnostic_attributes)]

However, that would lose the checking for typos across all versions,
which we do not want to lose.

One can also use the Kconfig symbol to allow the warning in older
compilers instead, to avoid repeating the `cfg_attr` line above in all
use sites:

    #![cfg_attr(
        not(RUSTC_HAS_DO_NOT_RECOMMEND),
        expect(unknown_or_malformed_diagnostic_attributes)
    )]

That still loses the checking for typos in older versions, but we still
keep it in newer ones, thus we should still catch mistakes eventually.

In this case we can promote it to `expect` as shown above, so that we do
not forget to remove these lines if we stop using the attribute somewhere.

Link: https://github.com/rust-lang/rust/pull/132056 [1]
Link: https://doc.rust-lang.org/reference/attributes/diagnostics.html#the-diagnosticdo_not_recommend-attribute [2]
Link: https://lore.kernel.org/rust-for-linux/CANiq72mYfhuRWkjomb1vOMMPOaxvdS6qjfVLAwxUw6ecdqyh2A@mail.gmail.com/
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 init/Kconfig | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/init/Kconfig b/init/Kconfig
index dd2ea3b9a799205daa4c1f0c694a9027e344c690..556e274bba6c015cf482e22472e9c24d5a2a7ca5 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -140,6 +140,9 @@ config LD_CAN_USE_KEEP_IN_OVERLAY
 config RUSTC_HAS_COERCE_POINTEE
 	def_bool RUSTC_VERSION >= 108400
 
+config RUSTC_HAS_DO_NOT_RECOMMEND
+	def_bool RUSTC_VERSION >= 108500
+
 config PAHOLE_VERSION
 	int
 	default $(shell,$(srctree)/scripts/pahole-version.sh $(PAHOLE))

-- 
2.49.0

[PATCH v10 5/5] rust: Add OwnableRefCounted and SimpleOwnableRefCounted

[Oliver Mangold]

Types implementing one of these traits can safely convert between an
ARef<T> and an Owned<T>.

This is useful for types which generally are accessed through an ARef
but have methods which can only safely be called when the reference is
unique, like e.g. `block::mq::Request::end_ok()`.

Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 rust/kernel/types.rs         |   8 +-
 rust/kernel/types/ownable.rs | 244 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 251 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index 33d2b4e4a87b991c6d934f4e8d2c6c71a15b1bcb..3a58905599eb9acb0e701c97bd92d3b93b9515cf 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -12,7 +12,7 @@
 use pin_init::{PinInit, Zeroable};
 
 pub mod ownable;
-pub use ownable::{Ownable, OwnableMut, Owned};
+pub use ownable::{Ownable, OwnableMut, OwnableRefCounted, Owned, SimpleOwnableRefCounted};
 
 /// Used to transfer ownership to and from foreign (non-Rust) languages.
 ///
@@ -544,6 +544,12 @@ fn from(b: &T) -> Self {
     }
 }
 
+impl<T: OwnableRefCounted> From<Owned<T>> for ARef<T> {
+    fn from(b: Owned<T>) -> Self {
+        T::into_shared(b)
+    }
+}
+
 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
diff --git a/rust/kernel/types/ownable.rs b/rust/kernel/types/ownable.rs
index 52e7a69019f1e2bbbe3cf715651b67a5a5c7c13d..b79459d07870ea4fa4f5df0c7565ac72d65e2c53 100644
--- a/rust/kernel/types/ownable.rs
+++ b/rust/kernel/types/ownable.rs
@@ -2,6 +2,7 @@
 
 //! Owned reference types.
 
+use crate::types::{ARef, RefCounted};
 use core::{
     marker::PhantomData,
     mem::ManuallyDrop,
@@ -115,3 +116,246 @@ fn drop(&mut self) {
         unsafe { T::release(self.ptr) };
     }
 }
+
+/// A trait for objects that can be wrapped in either one of the reference types [`Owned`] and
+/// [`ARef`].
+///
+/// # Safety
+///
+/// Implementers must ensure that:
+///
+/// - Both the safety requirements for [`Ownable`] and [`RefCounted`] are fulfilled.
+/// - [`try_from_shared()`](OwnableRefCounted::into_shared) only returns an [`Owned`] if exactly
+///   one [`ARef`] exists.
+/// - [`into_shared()`](OwnableRefCounted::into_shared) set the reference count to the value which
+///   the returned [`ARef`] expects for an object with a single reference
+///   in existence. This implies that if [`into_shared()`](OwnableRefCounted::into_shared) is left
+///   on the default implementation, which just rewraps the underlying object, the reference count
+///   needs not to be modified when converting a [`Owned`] to an [`ARef`].
+///
+/// # Examples
+///
+/// A minimal example implementation of [`OwnableRefCounted`], [`Ownable`] and its usage with
+/// [`ARef`] and [`Owned`] looks like this:
+///
+/// ```
+/// # #![expect(clippy::disallowed_names)]
+/// use core::cell::Cell;
+/// use core::ptr::NonNull;
+/// use kernel::alloc::{flags, kbox::KBox, AllocError};
+/// use kernel::types::{
+///     ARef, RefCounted, Owned, Ownable, OwnableRefCounted,
+/// };
+///
+/// struct Foo {
+///     refcount: Cell<usize>,
+/// }
+///
+/// impl Foo {
+///     fn new() -> Result<Owned<Self>, AllocError> {
+///         // Use a `KBox` to handle the actual allocation.
+///         let result = KBox::new(
+///             Foo {
+///                 refcount: Cell::new(1),
+///             },
+///             flags::GFP_KERNEL,
+///         )?;
+///         let result = NonNull::new(KBox::into_raw(result))
+///             .expect("Raw pointer to newly allocation KBox is null, this should never happen.");
+///         // SAFETY: We just allocated the `Foo`, thus it is valid.
+///         Ok(unsafe { Owned::from_raw(result) })
+///     }
+/// }
+///
+/// // SAFETY: We increment and decrement each time the respective function is called and only free
+/// // the `Foo` when the refcount reaches zero.
+/// unsafe impl RefCounted for Foo {
+///     fn inc_ref(&self) {
+///         self.refcount.replace(self.refcount.get() + 1);
+///     }
+///
+///     unsafe fn dec_ref(this: NonNull<Self>) {
+///         // SAFETY: The underlying object is always valid when the function is called.
+///         let refcount = unsafe { &this.as_ref().refcount };
+///         let new_refcount = refcount.get() - 1;
+///         if new_refcount == 0 {
+///             // The `Foo` will be dropped when `KBox` goes out of scope.
+///             // SAFETY: The `Box<Foo>` is still alive as the old refcount is 1.
+///             unsafe { KBox::from_raw(this.as_ptr()) };
+///         } else {
+///             refcount.replace(new_refcount);
+///         }
+///     }
+/// }
+///
+/// // SAFETY: We only convert into an `Owned` when the refcount is 1.
+/// unsafe impl OwnableRefCounted for Foo {
+///     fn try_from_shared(this: ARef<Self>) -> Result<Owned<Self>, ARef<Self>> {
+///         if this.refcount.get() == 1 {
+///             // SAFETY: The `Foo` is still alive as the refcount is 1.
+///             Ok(unsafe { Owned::from_raw(ARef::into_raw(this)) })
+///         } else {
+///             Err(this)
+///         }
+///     }
+/// }
+///
+/// // SAFETY: We are not `AlwaysRefCounted`.
+/// unsafe impl Ownable for Foo {
+///     unsafe fn release(this: NonNull<Self>) {
+///         // SAFETY: Using `dec_ref()` from `RefCounted` to release is okay, as the refcount is
+///         // always 1 for an `Owned<Foo>`.
+///         unsafe{ Foo::dec_ref(this) };
+///     }
+/// }
+///
+/// let foo = Foo::new().unwrap();
+/// let mut foo = ARef::from(foo);
+/// {
+///     let bar = foo.clone();
+///     assert!(Owned::try_from(bar).is_err());
+/// }
+/// assert!(Owned::try_from(foo).is_ok());
+/// ```
+pub unsafe trait OwnableRefCounted: RefCounted + Ownable + Sized {
+    /// Checks if the [`ARef`] is unique and convert it to an [`Owned`] it that is that case.
+    /// Otherwise it returns again an [`ARef`] to the same underlying object.
+    fn try_from_shared(this: ARef<Self>) -> Result<Owned<Self>, ARef<Self>>;
+
+    /// Converts the [`Owned`] into an [`ARef`].
+    fn into_shared(this: Owned<Self>) -> ARef<Self> {
+        // SAFETY: Safe by the requirements on implementing the trait.
+        unsafe { ARef::from_raw(Owned::into_raw(this)) }
+    }
+}
+
+/// This trait allows to implement [`Ownable`] and [`OwnableRefCounted`] together in a simplified
+/// way, only requiring to implement [`RefCounted`] and providing the method
+/// [`is_unique()`](SimpleOwnableRefCounted::is_unique).
+///
+/// For non-standard cases where conversion between [`Ownable`] and [`RefCounted`] does not allow
+/// [`Ownable::release()`] and [`RefCounted::dec_ref()`] to be the same method, [`Ownable`]
+/// and [`OwnableRefCounted`] should be implemented separately.
+///
+/// # Safety
+///
+/// Implementers must ensure that:
+///
+/// - The safety requirements for [`Ownable`] are fulfilled and [`RefCounted::dec_ref()`] can
+///   be used for [`Ownable::release()`].
+/// - [`is_unique`](SimpleOwnableRefCounted::is_unique) must only return `true` in case only one
+///   [`ARef`] exists and it is impossible for one to be obtained other than by cloning an existing
+///   [`ARef`] or converting an [`Owned`] to an [`ARef`].
+/// - It is safe to convert an unique [`ARef`] into an [`Owned`] simply by re-wrapping the
+///   underlying object without modifying the refcount.
+///
+/// # Examples
+///
+/// A minimal example implementation of [`RefCounted`] and [`SimpleOwnableRefCounted`]
+/// and its usage with [`ARef`] and [`Owned`] looks like this:
+///
+/// ```
+/// # #![expect(clippy::disallowed_names)]
+/// use core::cell::Cell;
+/// use core::ptr::NonNull;
+/// use kernel::alloc::{flags, kbox::KBox, AllocError};
+/// use kernel::types::{
+///     ARef, Owned, RefCounted, SimpleOwnableRefCounted,
+/// };
+///
+/// struct Foo {
+///     refcount: Cell<usize>,
+/// }
+///
+/// impl Foo {
+///     fn new() -> Result<Owned<Self>, AllocError> {
+///         // Use a KBox to handle the actual allocation.
+///         let result = KBox::new(
+///             Foo {
+///                 refcount: Cell::new(1),
+///             },
+///             flags::GFP_KERNEL,
+///         )?;
+///         let result = NonNull::new(KBox::into_raw(result))
+///             .expect("Raw pointer to newly allocation KBox is null, this should never happen.");
+///         // SAFETY: We just allocated the `Foo`, thus it is valid.
+///         Ok(unsafe { Owned::from_raw(result) })
+///     }
+/// }
+///
+/// // SAFETY: we ensure that:
+/// // - The `Foo` is only dropped when the refcount is zero.
+/// // - `is_unique()` only returns `true` when the refcount is 1.
+/// unsafe impl RefCounted for Foo {
+///     fn inc_ref(&self) {
+///         self.refcount.replace(self.refcount.get() + 1);
+///     }
+///
+///     unsafe fn dec_ref(this: NonNull<Self>) {
+///         // SAFETY: The underlying object is always valid when the function is called.
+///         let refcount = unsafe { &this.as_ref().refcount };
+///         let new_refcount = refcount.get() - 1;
+///         if new_refcount == 0 {
+///             // The `Foo` will be dropped when KBox goes out of scope.
+///             // SAFETY: The `Box<Foo>` is still alive as the old refcount is 1.
+///             unsafe { KBox::from_raw(this.as_ptr()) };
+///         } else {
+///             refcount.replace(new_refcount);
+///         }
+///     }
+/// }
+///
+/// // SAFETY: we ensure that:
+/// // - `is_unique()` only returns `true` when the refcount is 1.
+/// unsafe impl SimpleOwnableRefCounted for Foo {
+///     fn is_unique(&self) -> bool {
+///         self.refcount.get() == 1
+///     }
+/// }
+///
+/// let foo = Foo::new().unwrap();
+/// let mut foo = ARef::from(foo);
+/// {
+///     let bar = foo.clone();
+///     assert!(Owned::try_from(bar).is_err());
+/// }
+/// assert!(Owned::try_from(foo).is_ok());
+/// ```
+pub unsafe trait SimpleOwnableRefCounted: RefCounted {
+    /// Checks if exactly one [`ARef`] to the object exists. In case the object is [`Sync`], the
+    /// check needs to be race-free.
+    fn is_unique(&self) -> bool;
+}
+
+#[cfg_attr(RUSTC_HAS_DO_NOT_RECOMMEND, diagnostic::do_not_recommend)]
+// SAFETY: Safe by the requirements on implementation of [`SimpleOwnableRefCounted`].
+unsafe impl<T: SimpleOwnableRefCounted> OwnableRefCounted for T {
+    fn try_from_shared(this: ARef<Self>) -> Result<Owned<Self>, ARef<Self>> {
+        if T::is_unique(&*this) {
+            // SAFETY: Safe by the requirements on implementation of [`SimpleOwnable`].
+            Ok(unsafe { Owned::from_raw(ARef::into_raw(this)) })
+        } else {
+            Err(this)
+        }
+    }
+}
+
+#[cfg_attr(RUSTC_HAS_DO_NOT_RECOMMEND, diagnostic::do_not_recommend)]
+// SAFETY: Safe by the requirements on implementation of [`SimpleOwnableRefCounted`].
+unsafe impl<T: SimpleOwnableRefCounted> Ownable for T {
+    unsafe fn release(this: NonNull<Self>) {
+        // SAFETY: Safe by the requirements on implementation of
+        // [`SimpleOwnableRefCounted::dec_ref()`].
+        unsafe { RefCounted::dec_ref(this) };
+    }
+}
+
+impl<T: OwnableRefCounted> TryFrom<ARef<T>> for Owned<T> {
+    type Error = ARef<T>;
+    /// Tries to convert the [`ARef`] to an [`Owned`] by calling
+    /// [`try_from_shared()`](OwnableRefCounted::try_from_shared). In case the [`ARef`] is not
+    /// unique, it returns again an [`ARef`] to the same underlying object.
+    fn try_from(b: ARef<T>) -> Result<Owned<T>, Self::Error> {
+        T::try_from_shared(b)
+    }
+}

-- 
2.49.0

Re: [PATCH v10 5/5] rust: Add OwnableRefCounted and SimpleOwnableRefCounted

[Alice Ryhl]

On Fri, May 02, 2025 at 09:03:04AM +0000, Oliver Mangold wrote:
> Types implementing one of these traits can safely convert between an
> ARef<T> and an Owned<T>.
> 
> This is useful for types which generally are accessed through an ARef
> but have methods which can only safely be called when the reference is
> unique, like e.g. `block::mq::Request::end_ok()`.
> 
> Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
> Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
>  rust/kernel/types.rs         |   8 +-
>  rust/kernel/types/ownable.rs | 244 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 251 insertions(+), 1 deletion(-)
> 
> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index 33d2b4e4a87b991c6d934f4e8d2c6c71a15b1bcb..3a58905599eb9acb0e701c97bd92d3b93b9515cf 100644
> --- a/rust/kernel/types.rs
> +++ b/rust/kernel/types.rs
> @@ -12,7 +12,7 @@
>  use pin_init::{PinInit, Zeroable};
>  
>  pub mod ownable;
> -pub use ownable::{Ownable, OwnableMut, Owned};
> +pub use ownable::{Ownable, OwnableMut, OwnableRefCounted, Owned, SimpleOwnableRefCounted};
>  
>  /// Used to transfer ownership to and from foreign (non-Rust) languages.
>  ///
> @@ -544,6 +544,12 @@ fn from(b: &T) -> Self {
>      }
>  }
>  
> +impl<T: OwnableRefCounted> From<Owned<T>> for ARef<T> {
> +    fn from(b: Owned<T>) -> Self {
> +        T::into_shared(b)
> +    }
> +}
> +
>  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
> diff --git a/rust/kernel/types/ownable.rs b/rust/kernel/types/ownable.rs
> index 52e7a69019f1e2bbbe3cf715651b67a5a5c7c13d..b79459d07870ea4fa4f5df0c7565ac72d65e2c53 100644
> --- a/rust/kernel/types/ownable.rs
> +++ b/rust/kernel/types/ownable.rs
> @@ -2,6 +2,7 @@
>  
>  //! Owned reference types.
>  
> +use crate::types::{ARef, RefCounted};
>  use core::{
>      marker::PhantomData,
>      mem::ManuallyDrop,
> @@ -115,3 +116,246 @@ fn drop(&mut self) {
>          unsafe { T::release(self.ptr) };
>      }
>  }
> +
> +/// A trait for objects that can be wrapped in either one of the reference types [`Owned`] and
> +/// [`ARef`].
> +///
> +/// # Safety
> +///
> +/// Implementers must ensure that:
> +///
> +/// - Both the safety requirements for [`Ownable`] and [`RefCounted`] are fulfilled.
> +/// - [`try_from_shared()`](OwnableRefCounted::into_shared) only returns an [`Owned`] if exactly
> +///   one [`ARef`] exists.
> +/// - [`into_shared()`](OwnableRefCounted::into_shared) set the reference count to the value which
> +///   the returned [`ARef`] expects for an object with a single reference
> +///   in existence. This implies that if [`into_shared()`](OwnableRefCounted::into_shared) is left
> +///   on the default implementation, which just rewraps the underlying object, the reference count
> +///   needs not to be modified when converting a [`Owned`] to an [`ARef`].
> +///
> +/// # Examples
> +///
> +/// A minimal example implementation of [`OwnableRefCounted`], [`Ownable`] and its usage with
> +/// [`ARef`] and [`Owned`] looks like this:
> +///
> +/// ```
> +/// # #![expect(clippy::disallowed_names)]
> +/// use core::cell::Cell;
> +/// use core::ptr::NonNull;
> +/// use kernel::alloc::{flags, kbox::KBox, AllocError};
> +/// use kernel::types::{
> +///     ARef, RefCounted, Owned, Ownable, OwnableRefCounted,
> +/// };
> +///
> +/// struct Foo {
> +///     refcount: Cell<usize>,
> +/// }
> +///
> +/// impl Foo {
> +///     fn new() -> Result<Owned<Self>, AllocError> {
> +///         // Use a `KBox` to handle the actual allocation.
> +///         let result = KBox::new(
> +///             Foo {
> +///                 refcount: Cell::new(1),
> +///             },
> +///             flags::GFP_KERNEL,
> +///         )?;
> +///         let result = NonNull::new(KBox::into_raw(result))
> +///             .expect("Raw pointer to newly allocation KBox is null, this should never happen.");
> +///         // SAFETY: We just allocated the `Foo`, thus it is valid.
> +///         Ok(unsafe { Owned::from_raw(result) })
> +///     }
> +/// }
> +///
> +/// // SAFETY: We increment and decrement each time the respective function is called and only free
> +/// // the `Foo` when the refcount reaches zero.
> +/// unsafe impl RefCounted for Foo {
> +///     fn inc_ref(&self) {
> +///         self.refcount.replace(self.refcount.get() + 1);
> +///     }
> +///
> +///     unsafe fn dec_ref(this: NonNull<Self>) {
> +///         // SAFETY: The underlying object is always valid when the function is called.
> +///         let refcount = unsafe { &this.as_ref().refcount };
> +///         let new_refcount = refcount.get() - 1;
> +///         if new_refcount == 0 {
> +///             // The `Foo` will be dropped when `KBox` goes out of scope.
> +///             // SAFETY: The `Box<Foo>` is still alive as the old refcount is 1.
> +///             unsafe { KBox::from_raw(this.as_ptr()) };
> +///         } else {
> +///             refcount.replace(new_refcount);
> +///         }
> +///     }
> +/// }
> +///
> +/// // SAFETY: We only convert into an `Owned` when the refcount is 1.
> +/// unsafe impl OwnableRefCounted for Foo {
> +///     fn try_from_shared(this: ARef<Self>) -> Result<Owned<Self>, ARef<Self>> {
> +///         if this.refcount.get() == 1 {
> +///             // SAFETY: The `Foo` is still alive as the refcount is 1.
> +///             Ok(unsafe { Owned::from_raw(ARef::into_raw(this)) })
> +///         } else {
> +///             Err(this)
> +///         }
> +///     }
> +/// }
> +///
> +/// // SAFETY: We are not `AlwaysRefCounted`.
> +/// unsafe impl Ownable for Foo {
> +///     unsafe fn release(this: NonNull<Self>) {
> +///         // SAFETY: Using `dec_ref()` from `RefCounted` to release is okay, as the refcount is
> +///         // always 1 for an `Owned<Foo>`.
> +///         unsafe{ Foo::dec_ref(this) };
> +///     }
> +/// }
> +///
> +/// let foo = Foo::new().unwrap();
> +/// let mut foo = ARef::from(foo);
> +/// {
> +///     let bar = foo.clone();
> +///     assert!(Owned::try_from(bar).is_err());
> +/// }
> +/// assert!(Owned::try_from(foo).is_ok());
> +/// ```
> +pub unsafe trait OwnableRefCounted: RefCounted + Ownable + Sized {
> +    /// Checks if the [`ARef`] is unique and convert it to an [`Owned`] it that is that case.
> +    /// Otherwise it returns again an [`ARef`] to the same underlying object.
> +    fn try_from_shared(this: ARef<Self>) -> Result<Owned<Self>, ARef<Self>>;
> +
> +    /// Converts the [`Owned`] into an [`ARef`].
> +    fn into_shared(this: Owned<Self>) -> ARef<Self> {
> +        // SAFETY: Safe by the requirements on implementing the trait.
> +        unsafe { ARef::from_raw(Owned::into_raw(this)) }
> +    }
> +}
> +
> +/// This trait allows to implement [`Ownable`] and [`OwnableRefCounted`] together in a simplified
> +/// way, only requiring to implement [`RefCounted`] and providing the method
> +/// [`is_unique()`](SimpleOwnableRefCounted::is_unique).
> +///
> +/// For non-standard cases where conversion between [`Ownable`] and [`RefCounted`] does not allow
> +/// [`Ownable::release()`] and [`RefCounted::dec_ref()`] to be the same method, [`Ownable`]
> +/// and [`OwnableRefCounted`] should be implemented separately.
> +///
> +/// # Safety
> +///
> +/// Implementers must ensure that:
> +///
> +/// - The safety requirements for [`Ownable`] are fulfilled and [`RefCounted::dec_ref()`] can
> +///   be used for [`Ownable::release()`].
> +/// - [`is_unique`](SimpleOwnableRefCounted::is_unique) must only return `true` in case only one
> +///   [`ARef`] exists and it is impossible for one to be obtained other than by cloning an existing
> +///   [`ARef`] or converting an [`Owned`] to an [`ARef`].
> +/// - It is safe to convert an unique [`ARef`] into an [`Owned`] simply by re-wrapping the
> +///   underlying object without modifying the refcount.
> +///
> +/// # Examples
> +///
> +/// A minimal example implementation of [`RefCounted`] and [`SimpleOwnableRefCounted`]
> +/// and its usage with [`ARef`] and [`Owned`] looks like this:
> +///
> +/// ```
> +/// # #![expect(clippy::disallowed_names)]
> +/// use core::cell::Cell;
> +/// use core::ptr::NonNull;
> +/// use kernel::alloc::{flags, kbox::KBox, AllocError};
> +/// use kernel::types::{
> +///     ARef, Owned, RefCounted, SimpleOwnableRefCounted,
> +/// };
> +///
> +/// struct Foo {
> +///     refcount: Cell<usize>,
> +/// }
> +///
> +/// impl Foo {
> +///     fn new() -> Result<Owned<Self>, AllocError> {
> +///         // Use a KBox to handle the actual allocation.
> +///         let result = KBox::new(
> +///             Foo {
> +///                 refcount: Cell::new(1),
> +///             },
> +///             flags::GFP_KERNEL,
> +///         )?;
> +///         let result = NonNull::new(KBox::into_raw(result))
> +///             .expect("Raw pointer to newly allocation KBox is null, this should never happen.");
> +///         // SAFETY: We just allocated the `Foo`, thus it is valid.
> +///         Ok(unsafe { Owned::from_raw(result) })
> +///     }
> +/// }
> +///
> +/// // SAFETY: we ensure that:
> +/// // - The `Foo` is only dropped when the refcount is zero.
> +/// // - `is_unique()` only returns `true` when the refcount is 1.
> +/// unsafe impl RefCounted for Foo {
> +///     fn inc_ref(&self) {
> +///         self.refcount.replace(self.refcount.get() + 1);
> +///     }
> +///
> +///     unsafe fn dec_ref(this: NonNull<Self>) {
> +///         // SAFETY: The underlying object is always valid when the function is called.
> +///         let refcount = unsafe { &this.as_ref().refcount };
> +///         let new_refcount = refcount.get() - 1;
> +///         if new_refcount == 0 {
> +///             // The `Foo` will be dropped when KBox goes out of scope.
> +///             // SAFETY: The `Box<Foo>` is still alive as the old refcount is 1.
> +///             unsafe { KBox::from_raw(this.as_ptr()) };
> +///         } else {
> +///             refcount.replace(new_refcount);
> +///         }
> +///     }
> +/// }
> +///
> +/// // SAFETY: we ensure that:
> +/// // - `is_unique()` only returns `true` when the refcount is 1.
> +/// unsafe impl SimpleOwnableRefCounted for Foo {
> +///     fn is_unique(&self) -> bool {
> +///         self.refcount.get() == 1
> +///     }
> +/// }
> +///
> +/// let foo = Foo::new().unwrap();
> +/// let mut foo = ARef::from(foo);
> +/// {
> +///     let bar = foo.clone();
> +///     assert!(Owned::try_from(bar).is_err());
> +/// }
> +/// assert!(Owned::try_from(foo).is_ok());
> +/// ```
> +pub unsafe trait SimpleOwnableRefCounted: RefCounted {
> +    /// Checks if exactly one [`ARef`] to the object exists. In case the object is [`Sync`], the
> +    /// check needs to be race-free.
> +    fn is_unique(&self) -> bool;
> +}
> +
> +#[cfg_attr(RUSTC_HAS_DO_NOT_RECOMMEND, diagnostic::do_not_recommend)]
> +// SAFETY: Safe by the requirements on implementation of [`SimpleOwnableRefCounted`].
> +unsafe impl<T: SimpleOwnableRefCounted> OwnableRefCounted for T {
> +    fn try_from_shared(this: ARef<Self>) -> Result<Owned<Self>, ARef<Self>> {
> +        if T::is_unique(&*this) {
> +            // SAFETY: Safe by the requirements on implementation of [`SimpleOwnable`].
> +            Ok(unsafe { Owned::from_raw(ARef::into_raw(this)) })
> +        } else {
> +            Err(this)
> +        }
> +    }
> +}
> +
> +#[cfg_attr(RUSTC_HAS_DO_NOT_RECOMMEND, diagnostic::do_not_recommend)]
> +// SAFETY: Safe by the requirements on implementation of [`SimpleOwnableRefCounted`].
> +unsafe impl<T: SimpleOwnableRefCounted> Ownable for T {
> +    unsafe fn release(this: NonNull<Self>) {
> +        // SAFETY: Safe by the requirements on implementation of
> +        // [`SimpleOwnableRefCounted::dec_ref()`].
> +        unsafe { RefCounted::dec_ref(this) };
> +    }
> +}

I wonder if this is too limiting. It will limit our ability to write
other blanket impls for Ownable and OwnableRefCounted. Using e.g. a
derive macro might be better?

Alice

Re: [PATCH v10 5/5] rust: Add OwnableRefCounted and SimpleOwnableRefCounted

[Oliver Mangold]

On 250502 1143, Alice Ryhl wrote:
> On Fri, May 02, 2025 at 09:03:04AM +0000, Oliver Mangold wrote:
> > +pub unsafe trait SimpleOwnableRefCounted: RefCounted {
> > +    /// Checks if exactly one [`ARef`] to the object exists. In case the object is [`Sync`], the
> > +    /// check needs to be race-free.
> > +    fn is_unique(&self) -> bool;
> > +}
> > +
> > +#[cfg_attr(RUSTC_HAS_DO_NOT_RECOMMEND, diagnostic::do_not_recommend)]
> > +// SAFETY: Safe by the requirements on implementation of [`SimpleOwnableRefCounted`].
> > +unsafe impl<T: SimpleOwnableRefCounted> OwnableRefCounted for T {
> > +    fn try_from_shared(this: ARef<Self>) -> Result<Owned<Self>, ARef<Self>> {
> > +        if T::is_unique(&*this) {
> > +            // SAFETY: Safe by the requirements on implementation of [`SimpleOwnable`].
> > +            Ok(unsafe { Owned::from_raw(ARef::into_raw(this)) })
> > +        } else {
> > +            Err(this)
> > +        }
> > +    }
> > +}
> > +
> > +#[cfg_attr(RUSTC_HAS_DO_NOT_RECOMMEND, diagnostic::do_not_recommend)]
> > +// SAFETY: Safe by the requirements on implementation of [`SimpleOwnableRefCounted`].
> > +unsafe impl<T: SimpleOwnableRefCounted> Ownable for T {
> > +    unsafe fn release(this: NonNull<Self>) {
> > +        // SAFETY: Safe by the requirements on implementation of
> > +        // [`SimpleOwnableRefCounted::dec_ref()`].
> > +        unsafe { RefCounted::dec_ref(this) };
> > +    }
> > +}
> 
> I wonder if this is too limiting. It will limit our ability to write
> other blanket impls for Ownable and OwnableRefCounted. Using e.g. a
> derive macro might be better?
> 
> Alice

You might be right. I don't have a strong opinion on the matter.

I'm not really familiar with procmacros, though. So maybe I should just do
away with SimpleOwnableRefCounted for now?

I mean Andreas doesn't need it for his current use case and this or a derive
macro can always be added later if it makes sense.

Best regards,

Oliver

Re: [PATCH v10 5/5] rust: Add OwnableRefCounted and SimpleOwnableRefCounted

[Alice Ryhl]

On Tue, May 06, 2025 at 11:42:08AM +0000, Oliver Mangold wrote:
> On 250502 1143, Alice Ryhl wrote:
> > On Fri, May 02, 2025 at 09:03:04AM +0000, Oliver Mangold wrote:
> > > +pub unsafe trait SimpleOwnableRefCounted: RefCounted {
> > > +    /// Checks if exactly one [`ARef`] to the object exists. In case the object is [`Sync`], the
> > > +    /// check needs to be race-free.
> > > +    fn is_unique(&self) -> bool;
> > > +}
> > > +
> > > +#[cfg_attr(RUSTC_HAS_DO_NOT_RECOMMEND, diagnostic::do_not_recommend)]
> > > +// SAFETY: Safe by the requirements on implementation of [`SimpleOwnableRefCounted`].
> > > +unsafe impl<T: SimpleOwnableRefCounted> OwnableRefCounted for T {
> > > +    fn try_from_shared(this: ARef<Self>) -> Result<Owned<Self>, ARef<Self>> {
> > > +        if T::is_unique(&*this) {
> > > +            // SAFETY: Safe by the requirements on implementation of [`SimpleOwnable`].
> > > +            Ok(unsafe { Owned::from_raw(ARef::into_raw(this)) })
> > > +        } else {
> > > +            Err(this)
> > > +        }
> > > +    }
> > > +}
> > > +
> > > +#[cfg_attr(RUSTC_HAS_DO_NOT_RECOMMEND, diagnostic::do_not_recommend)]
> > > +// SAFETY: Safe by the requirements on implementation of [`SimpleOwnableRefCounted`].
> > > +unsafe impl<T: SimpleOwnableRefCounted> Ownable for T {
> > > +    unsafe fn release(this: NonNull<Self>) {
> > > +        // SAFETY: Safe by the requirements on implementation of
> > > +        // [`SimpleOwnableRefCounted::dec_ref()`].
> > > +        unsafe { RefCounted::dec_ref(this) };
> > > +    }
> > > +}
> > 
> > I wonder if this is too limiting. It will limit our ability to write
> > other blanket impls for Ownable and OwnableRefCounted. Using e.g. a
> > derive macro might be better?
> > 
> > Alice
> 
> You might be right. I don't have a strong opinion on the matter.
> 
> I'm not really familiar with procmacros, though. So maybe I should just do
> away with SimpleOwnableRefCounted for now?
> 
> I mean Andreas doesn't need it for his current use case and this or a derive
> macro can always be added later if it makes sense.

That's okay for now.

Alice

Re: [PATCH v10 0/5] New trait OwnableRefCounted for ARef<->Owned conversion.

[Andreas Hindborg]

Hi Oliver,

Oliver Mangold <oliver.mangold@pm.me> writes:

> This allows to convert between ARef<T> and Owned<T> by
> implementing the new trait OwnedRefCounted.
>
> This way we will have a shared/unique reference counting scheme
> for types with built-in refcounts in analogy to Arc/UniqueArc.
>
> Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>

Do you intend to send a new version of the series?


Best regards,
Andreas Hindborg

Re: [PATCH v10 0/5] New trait OwnableRefCounted for ARef<->Owned conversion.

[Oliver Mangold]

Hi Andreas,

yes. I didn't have time to work on this during the last weeks, but I 
will post one with the suggested changed next week.

Best,

Oliver

On 13.06.25 15:10, Andreas Hindborg wrote:
> Hi Oliver,
>
> Oliver Mangold <oliver.mangold@pm.me> writes:
>
>> This allows to convert between ARef<T> and Owned<T> by
>> implementing the new trait OwnedRefCounted.
>>
>> This way we will have a shared/unique reference counting scheme
>> for types with built-in refcounts in analogy to Arc/UniqueArc.
>>
>> Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
> Do you intend to send a new version of the series?
>
>
> Best regards,
> Andreas Hindborg
>
>
>