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

[PATCH v9 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 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/lib.rs              |   1 +
 rust/kernel/ownable.rs          | 361 ++++++++++++++++++++++++++++++++++++++++
 rust/kernel/pid_namespace.rs    |   8 +-
 rust/kernel/task.rs             |   6 +-
 rust/kernel/types.rs            |  53 +++---
 10 files changed, 436 insertions(+), 32 deletions(-)
---
base-commit: 4701f33a10702d5fc577c32434eb62adde0a1ae1
change-id: 20250305-unique-ref-29fcd675f9e9

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

[PATCH v9 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/lib.rs     |   1 +
 rust/kernel/ownable.rs | 117 +++++++++++++++++++++++++++++++++++++++++++++++++
 rust/kernel/types.rs   |   2 +
 3 files changed, 120 insertions(+)

diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 7697c60b2d1a670c436246d422de3b22b1520956..52c294bbf8ded260540e0bc07499257bce91383c 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -61,6 +61,7 @@
 #[cfg(CONFIG_NET)]
 pub mod net;
 pub mod of;
+pub mod ownable;
 pub mod page;
 #[cfg(CONFIG_PCI)]
 pub mod pci;
diff --git a/rust/kernel/ownable.rs b/rust/kernel/ownable.rs
new file mode 100644
index 0000000000000000000000000000000000000000..f4bebea23ce1d62f5597e35199ca38ea07b293db
--- /dev/null
+++ b/rust/kernel/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 non-null and 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 unique `&mut T` pointer (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) };
+    }
+}
diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index 2bbaab83b9d65da667a07e85b3c89c7fa881b53c..2cddbd3a2873b601419f7628c386431a63cb9692 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -11,6 +11,8 @@
     ptr::NonNull,
 };
 
+pub use crate::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

-- 
2.49.0

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

[Andreas Hindborg]

Hi Oliver,

"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/lib.rs     |   1 +
>  rust/kernel/ownable.rs | 117 +++++++++++++++++++++++++++++++++++++++++++++++++
>  rust/kernel/types.rs   |   2 +
>  3 files changed, 120 insertions(+)

I would suggest moving ownable.rs to rust/kernel/types/ownable.rs and
then moving `pub mod ownable` to types.rs.

>
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 7697c60b2d1a670c436246d422de3b22b1520956..52c294bbf8ded260540e0bc07499257bce91383c 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -61,6 +61,7 @@
>  #[cfg(CONFIG_NET)]
>  pub mod net;
>  pub mod of;
> +pub mod ownable;
>  pub mod page;
>  #[cfg(CONFIG_PCI)]
>  pub mod pci;
> diff --git a/rust/kernel/ownable.rs b/rust/kernel/ownable.rs
> new file mode 100644
> index 0000000000000000000000000000000000000000..f4bebea23ce1d62f5597e35199ca38ea07b293db
> --- /dev/null
> +++ b/rust/kernel/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 non-null and valid for the lifetime of the [`Owned`] instance.

I am not sure we need the non-null invariant here, since it is an
invariant of `NonNull`. The rest is fine.

> +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 unique `&mut T` pointer (which is safe because `T` is `Send`).

I would drop 'pointer' in 'a unique `&mut T` ~pointer~' here. '`&mut T`'
is sufficient alone.

> +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`).

Like here, I think this is correct (without the pointer wording).

> +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 is acquired.


Best regards,
Andreas Hindborg

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

[Oliver Mangold]

On 250409 1034, Andreas Hindborg wrote:
> Hi Oliver,
> 

Hi Andreas,

> "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/lib.rs     |   1 +
> >  rust/kernel/ownable.rs | 117 +++++++++++++++++++++++++++++++++++++++++++++++++
> >  rust/kernel/types.rs   |   2 +
> >  3 files changed, 120 insertions(+)
> 
> I would suggest moving ownable.rs to rust/kernel/types/ownable.rs and
> then moving `pub mod ownable` to types.rs.

Yes, that makes more sense.

> I am not sure we need the non-null invariant here, since it is an
> invariant of `NonNull`. The rest is fine.

> I would drop 'pointer' in 'a unique `&mut T` ~pointer~' here. '`&mut T`'
> is sufficient alone.

> Like here, I think this is correct (without the pointer wording).

> This part "the underlying object is acquired" is unclear to me. How about:
> 
>   Callers must ensure that *ownership of* the underlying object is acquired.

Agree. I will fix these.

Best,

Oliver

[PATCH v9 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 from a `&`, which cannot be allowed for types which are
also Ownable.

Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
Suggested-by: Alice Ryhl <aliceryhl@google.com>
---
 rust/kernel/block/mq/request.rs | 10 +++++++---
 rust/kernel/cred.rs             |  8 ++++++--
 rust/kernel/device.rs           |  8 ++++++--
 rust/kernel/fs/file.rs          | 10 +++++++---
 rust/kernel/pid_namespace.rs    |  8 ++++++--
 rust/kernel/task.rs             |  6 +++++-
 rust/kernel/types.rs            | 41 ++++++++++++++++++++++++-----------------
 7 files changed, 61 insertions(+), 30 deletions(-)

diff --git a/rust/kernel/block/mq/request.rs b/rust/kernel/block/mq/request.rs
index 7943f43b957532749f5a95eee5546bf4f2b6cc5a..f47a13de1e073d564ae224d2e0d5cfb875f04da6 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 81d67789b16f243e7832ff3b2e5e479a1ab2bf9e..e04d1021130eb1ec46fe48feb088959da7656d66 100644
--- a/rust/kernel/cred.rs
+++ b/rust/kernel/cred.rs
@@ -11,7 +11,7 @@
 use crate::{
     bindings,
     task::Kuid,
-    types::{AlwaysRefCounted, Opaque},
+    types::{AlwaysRefCounted, Opaque, RefCounted},
 };
 
 /// Wraps the kernel's `struct cred`.
@@ -71,7 +71,7 @@ pub fn euid(&self) -> Kuid {
 }
 
 // SAFETY: The type invariants guarantee that `Credential` is always ref-counted.
-unsafe impl AlwaysRefCounted for Credential {
+unsafe impl RefCounted for Credential {
     fn inc_ref(&self) {
         // SAFETY: The existence of a shared reference means that the refcount is nonzero.
         unsafe { bindings::get_cred(self.0.get()) };
@@ -83,3 +83,7 @@ unsafe fn dec_ref(obj: core::ptr::NonNull<Credential>) {
         unsafe { bindings::put_cred(obj.cast().as_ptr()) };
     }
 }
+
+// SAFETY: We do not implement `Ownable`, thus it is okay to can obtain an `ARef<Credential>` from a
+// `&Credential`.
+unsafe impl AlwaysRefCounted for Credential {}
diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index db2d9658ba47d9c492bc813ce3eb2ff29703ca31..189298518dc184405b1d62404b190d4c0b08b7ad 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -7,7 +7,7 @@
 use crate::{
     bindings,
     str::CStr,
-    types::{ARef, Opaque},
+    types::{ARef, AlwaysRefCounted, Opaque, RefCounted},
 };
 use core::{fmt, ptr};
 
@@ -190,7 +190,7 @@ pub fn property_present(&self, name: &CStr) -> bool {
 }
 
 // SAFETY: Instances of `Device` are always reference-counted.
-unsafe impl crate::types::AlwaysRefCounted for Device {
+unsafe impl RefCounted for Device {
     fn inc_ref(&self) {
         // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
         unsafe { bindings::get_device(self.as_raw()) };
@@ -202,6 +202,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
     }
 }
 
+// SAFETY: We do not implement `Ownable`, thus it is okay to can obtain an `Device<Task>` from a
+// `&Device`.
+unsafe impl AlwaysRefCounted for Device {}
+
 // SAFETY: As by the type invariant `Device` can be sent to any thread.
 unsafe impl Send for Device {}
 
diff --git a/rust/kernel/fs/file.rs b/rust/kernel/fs/file.rs
index e03dbe14d62a566349c4100f2f78b17d4c79aab5..a7836cc754e7927b6addc3bd06cfe8c8119f1d9f 100644
--- a/rust/kernel/fs/file.rs
+++ b/rust/kernel/fs/file.rs
@@ -11,7 +11,7 @@
     bindings,
     cred::Credential,
     error::{code::*, Error, Result},
-    types::{ARef, AlwaysRefCounted, NotThreadSafe, Opaque},
+    types::{ARef, AlwaysRefCounted, NotThreadSafe, Opaque, RefCounted},
 };
 use core::ptr;
 
@@ -190,7 +190,7 @@ unsafe impl Sync for File {}
 
 // SAFETY: The type invariants guarantee that `File` is always ref-counted. This implementation
 // makes `ARef<File>` own a normal refcount.
-unsafe impl AlwaysRefCounted for File {
+unsafe impl RefCounted for File {
     #[inline]
     fn inc_ref(&self) {
         // SAFETY: The existence of a shared reference means that the refcount is nonzero.
@@ -205,6 +205,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<File>) {
     }
 }
 
+// SAFETY: We do not implement `Ownable`, thus it is okay to can obtain an `ARef<File>` from a
+/// `&File`.
+unsafe impl AlwaysRefCounted for File {}
+
 /// Wraps the kernel's `struct file`. Not thread safe.
 ///
 /// This type represents a file that is not known to be safe to transfer across thread boundaries.
@@ -225,7 +229,7 @@ pub struct LocalFile {
 
 // SAFETY: The type invariants guarantee that `LocalFile` is always ref-counted. This implementation
 // makes `ARef<File>` own a normal refcount.
-unsafe impl AlwaysRefCounted for LocalFile {
+unsafe impl RefCounted for LocalFile {
     #[inline]
     fn inc_ref(&self) {
         // SAFETY: The existence of a shared reference means that the refcount is nonzero.
diff --git a/rust/kernel/pid_namespace.rs b/rust/kernel/pid_namespace.rs
index 0e93808e4639b37dd77add5d79f64058dac7cb87..3e45e945b7509b9607266b2e0e6ef130e7a1ed39 100644
--- a/rust/kernel/pid_namespace.rs
+++ b/rust/kernel/pid_namespace.rs
@@ -9,7 +9,7 @@
 
 use crate::{
     bindings,
-    types::{AlwaysRefCounted, Opaque},
+    types::{AlwaysRefCounted, RefCounted, Opaque},
 };
 use core::ptr;
 
@@ -44,7 +44,7 @@ pub unsafe fn from_ptr<'a>(ptr: *const bindings::pid_namespace) -> &'a Self {
 }
 
 // SAFETY: Instances of `PidNamespace` are always reference-counted.
-unsafe impl AlwaysRefCounted for PidNamespace {
+unsafe impl RefCounted for PidNamespace {
     #[inline]
     fn inc_ref(&self) {
         // SAFETY: The existence of a shared reference means that the refcount is nonzero.
@@ -58,6 +58,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<PidNamespace>) {
     }
 }
 
+// SAFETY: We do not implement `Ownable`, thus it is okay to can obtain an `ARef<PidNamespace>`
+// from a `&PidNamespace`.
+unsafe impl AlwaysRefCounted for PidNamespace {}
+
 // SAFETY:
 // - `PidNamespace::dec_ref` can be called from any thread.
 // - It is okay to send ownership of `PidNamespace` across thread boundaries.
diff --git a/rust/kernel/task.rs b/rust/kernel/task.rs
index 38da555a2bdbb71d698c671ad1a7a337e50c6600..5bdc0a348e6d549d66b002aa6b892b61ce31214a 100644
--- a/rust/kernel/task.rs
+++ b/rust/kernel/task.rs
@@ -327,7 +327,7 @@ pub fn wake_up(&self) {
 }
 
 // SAFETY: The type invariants guarantee that `Task` is always refcounted.
-unsafe impl crate::types::AlwaysRefCounted for Task {
+unsafe impl crate::types::RefCounted for Task {
     fn inc_ref(&self) {
         // SAFETY: The existence of a shared reference means that the refcount is nonzero.
         unsafe { bindings::get_task_struct(self.as_ptr()) };
@@ -339,6 +339,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
     }
 }
 
+// SAFETY: We do not implement `Ownable`, thus it is okay to can obtain an `ARef<Task>` from a
+// `&Task`.
+unsafe impl crate::types::AlwaysRefCounted for Task {}
+
 impl Kuid {
     /// Get the current euid.
     #[inline]
diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index 2cddbd3a2873b601419f7628c386431a63cb9692..c8b78bcad259132808cc38c56b9f2bd525a0b755 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -388,11 +388,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
@@ -404,9 +402,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);
 
@@ -419,11 +416,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
@@ -434,7 +441,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>,
 }
@@ -443,16 +450,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.
@@ -481,12 +488,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>) {}
     /// }
@@ -504,7 +511,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.
@@ -512,7 +519,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 {
@@ -529,7 +536,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 v9 2/5] rust: Rename AlwaysRefCounted to RefCounted

[kernel test robot]

Hi Oliver,

kernel test robot noticed the following build errors:

[auto build test ERROR on 4701f33a10702d5fc577c32434eb62adde0a1ae1]

url:    https://github.com/intel-lab-lkp/linux/commits/Oliver-Mangold/rust-types-Add-Ownable-Owned-types/20250325-200247
base:   4701f33a10702d5fc577c32434eb62adde0a1ae1
patch link:    https://lore.kernel.org/r/20250325-unique-ref-v9-2-e91618c1de26%40pm.me
patch subject: [PATCH v9 2/5] rust: Rename AlwaysRefCounted to RefCounted
config: x86_64-rhel-9.4-rust (https://download.01.org/0day-ci/archive/20250327/202503270014.4G907YMY-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
rustc: rustc 1.78.0 (9b00956e5 2024-04-29)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250327/202503270014.4G907YMY-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202503270014.4G907YMY-lkp@intel.com/

All errors (new ones prefixed by >>):

   PATH=/opt/cross/clang-18/bin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
   INFO PATH=/opt/cross/rustc-1.78.0-bindgen-0.65.1/cargo/bin:/opt/cross/clang-18/bin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
   /usr/bin/timeout -k 100 12h /usr/bin/make KCFLAGS= -Wno-error=return-type -Wreturn-type -funsigned-char -Wundef W=1 --keep-going LLVM=1 -j32 -C source O=/kbuild/obj/consumer/x86_64-rhel-9.4-rust ARCH=x86_64 SHELL=/bin/bash rustfmtcheck
   make: Entering directory '/kbuild/src/consumer'
   make[1]: Entering directory '/kbuild/obj/consumer/x86_64-rhel-9.4-rust'
>> Diff in rust/kernel/pid_namespace.rs at line 9:
    
    use crate::{
        bindings,
   -    types::{AlwaysRefCounted, RefCounted, Opaque},
   +    types::{AlwaysRefCounted, Opaque, RefCounted},
    };
    use core::ptr;
    
>> Diff in rust/kernel/pid_namespace.rs at line 9:
    
    use crate::{
        bindings,
   -    types::{AlwaysRefCounted, RefCounted, Opaque},
   +    types::{AlwaysRefCounted, Opaque, RefCounted},
    };
    use core::ptr;
    
   make[2]: *** [Makefile:1816: rustfmt] Error 123
   make[2]: Target 'rustfmtcheck' not remade because of errors.
   make[1]: Leaving directory '/kbuild/obj/consumer/x86_64-rhel-9.4-rust'
   make: *** [Makefile:251: __sub-make] Error 2
   make: Target 'rustfmtcheck' not remade because of errors.
   make[1]: *** [Makefile:251: __sub-make] Error 2
   make: Leaving directory '/kbuild/src/consumer'
   make[1]: Target 'rustfmtcheck' not remade because of errors.

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v9 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 from a `&`, which cannot be allowed for types which are
> also Ownable.

I would suggest using `ARef<T>` and `&T` in the commit message.

v6.15-rc1 includes some more uses of `AlwaysRefCounted` that need to be
converted. Otherwise looks good to me.


Best regards,
Andreas Hindborg

[PATCH v9 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 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index c8b78bcad259132808cc38c56b9f2bd525a0b755..db29f7c725e631c11099fa9122901ec2b3f4a039 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -492,7 +492,7 @@ pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
     ///
     /// struct Empty {}
     ///
-    /// # // SAFETY: TODO.
+    /// // SAFETY: We do not free anything.
     /// unsafe impl RefCounted for Empty {
     ///     fn inc_ref(&self) {}
     ///     unsafe fn dec_ref(_obj: NonNull<Self>) {}
@@ -500,7 +500,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 v9 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 | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index c8b78bcad259132808cc38c56b9f2bd525a0b755..db29f7c725e631c11099fa9122901ec2b3f4a039 100644
> --- a/rust/kernel/types.rs
> +++ b/rust/kernel/types.rs
> @@ -492,7 +492,7 @@ pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
>      ///
>      /// struct Empty {}
>      ///
> -    /// # // SAFETY: TODO.
> +    /// // SAFETY: We do not free anything.

How about:

  This implementation will never free the underlying object, so the
  object is kept alive longer than the safety requirement specifies.


>      /// unsafe impl RefCounted for Empty {
>      ///     fn inc_ref(&self) {}
>      ///     unsafe fn dec_ref(_obj: NonNull<Self>) {}
> @@ -500,7 +500,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`.

This is not sufficient. The safety requirement is:

  Callers must ensure that the reference count was incremented at least once, and that they
  are properly relinquishing one increment. That is, if there is only one increment, callers
  must not use the underlying object anymore -- it is only safe to do so via the newly
  created [`ARef`].

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

I think this example actually exposes a soundness hole. When the
underlying object is allocated on the stack, the safety requirements are
not sufficient to ensure the lifetime of the object. We could safely
return `data_ref` and have the underlying object go away. We should add
to the safety requirement of `ARef::from_raw`:

  `ptr` must be valid while the refcount is positive.

That would allow the code in this example, but prevent the issue
outlined above.


Best regards,
Andreas Hindborg

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

[Oliver Mangold]

On 250409 1126, Andreas Hindborg wrote:
> "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 | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> > index c8b78bcad259132808cc38c56b9f2bd525a0b755..db29f7c725e631c11099fa9122901ec2b3f4a039 100644
> > --- a/rust/kernel/types.rs
> > +++ b/rust/kernel/types.rs
> > @@ -492,7 +492,7 @@ pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
> >      ///
> >      /// struct Empty {}
> >      ///
> > -    /// # // SAFETY: TODO.
> > +    /// // SAFETY: We do not free anything.
> 
> How about:
> 
>   This implementation will never free the underlying object, so the
>   object is kept alive longer than the safety requirement specifies.

Yes, was rather sloppy wording. Thanks, I will use your version.

> >      /// unsafe impl RefCounted for Empty {
> >      ///     fn inc_ref(&self) {}
> >      ///     unsafe fn dec_ref(_obj: NonNull<Self>) {}
> > @@ -500,7 +500,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`.
> 
> This is not sufficient. The safety requirement is:
> 
>   Callers must ensure that the reference count was incremented at least once, and that they
>   are properly relinquishing one increment. That is, if there is only one increment, callers
>   must not use the underlying object anymore -- it is only safe to do so via the newly
>   created [`ARef`].
> 
> 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`.
> 
> I think this example actually exposes a soundness hole. When the
> underlying object is allocated on the stack, the safety requirements are
> not sufficient to ensure the lifetime of the object. We could safely
> return `data_ref` and have the underlying object go away. We should add
> to the safety requirement of `ARef::from_raw`:
> 
>   `ptr` must be valid while the refcount is positive.

Wouldn't this already be covered by the below in the doc for
AlwaysRefCounted?

    Implementers must ensure that increments to the reference count keep
    the object alive in memory at least until matching decrements are
    performed."

OTOH, it also says this (which would be violated):

    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.)"

Should I change the example to one with an actual reference count?
Not sure, would make it more complex and less readable, of course.

Best regards,

Oliver

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

[Andreas Hindborg]

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

> On 250409 1126, Andreas Hindborg wrote:
>> "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 | 4 ++--
>> >  1 file changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
>> > index c8b78bcad259132808cc38c56b9f2bd525a0b755..db29f7c725e631c11099fa9122901ec2b3f4a039 100644
>> > --- a/rust/kernel/types.rs
>> > +++ b/rust/kernel/types.rs
>> > @@ -492,7 +492,7 @@ pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
>> >      ///
>> >      /// struct Empty {}
>> >      ///
>> > -    /// # // SAFETY: TODO.
>> > +    /// // SAFETY: We do not free anything.
>> 
>> How about:
>> 
>>   This implementation will never free the underlying object, so the
>>   object is kept alive longer than the safety requirement specifies.
>
> Yes, was rather sloppy wording. Thanks, I will use your version.
>
>> >      /// unsafe impl RefCounted for Empty {
>> >      ///     fn inc_ref(&self) {}
>> >      ///     unsafe fn dec_ref(_obj: NonNull<Self>) {}
>> > @@ -500,7 +500,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`.
>> 
>> This is not sufficient. The safety requirement is:
>> 
>>   Callers must ensure that the reference count was incremented at least once, and that they
>>   are properly relinquishing one increment. That is, if there is only one increment, callers
>>   must not use the underlying object anymore -- it is only safe to do so via the newly
>>   created [`ARef`].
>> 
>> 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`.
>> 
>> I think this example actually exposes a soundness hole. When the
>> underlying object is allocated on the stack, the safety requirements are
>> not sufficient to ensure the lifetime of the object. We could safely
>> return `data_ref` and have the underlying object go away. We should add
>> to the safety requirement of `ARef::from_raw`:
>> 
>>   `ptr` must be valid while the refcount is positive.
>
> Wouldn't this already be covered by the below in the doc for
> AlwaysRefCounted?
>
>     Implementers must ensure that increments to the reference count keep
>     the object alive in memory at least until matching decrements are
>     performed."

No, I don't think that is enough. We can implement the trait properly
with refcounts and still we can allocate an object on the stack and then
do a `from_raw` on that object without violating any safety
requirements. I think the `ARef::from_raw` should have the safety
requirement above. But we can do that as a separate patch.

>
> OTOH, it also says this (which would be violated):
>
>     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.)"
>
> Should I change the example to one with an actual reference count?
> Not sure, would make it more complex and less readable, of course.

No I think that is fine. `Empty` is reference counted in the sense that
the refcount can considered to always be positive.



Best regards,
Andreas Hindborg

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

[Andreas Hindborg]

Andreas Hindborg <a.hindborg@kernel.org> writes:

> Oliver Mangold <oliver.mangold@pm.me> writes:
>
>> On 250409 1126, Andreas Hindborg wrote:
>>> "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 | 4 ++--
>>> >  1 file changed, 2 insertions(+), 2 deletions(-)
>>> >
>>> > diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
>>> > index c8b78bcad259132808cc38c56b9f2bd525a0b755..db29f7c725e631c11099fa9122901ec2b3f4a039 100644
>>> > --- a/rust/kernel/types.rs
>>> > +++ b/rust/kernel/types.rs
>>> > @@ -492,7 +492,7 @@ pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
>>> >      ///
>>> >      /// struct Empty {}
>>> >      ///
>>> > -    /// # // SAFETY: TODO.
>>> > +    /// // SAFETY: We do not free anything.
>>> 
>>> How about:
>>> 
>>>   This implementation will never free the underlying object, so the
>>>   object is kept alive longer than the safety requirement specifies.
>>
>> Yes, was rather sloppy wording. Thanks, I will use your version.
>>
>>> >      /// unsafe impl RefCounted for Empty {
>>> >      ///     fn inc_ref(&self) {}
>>> >      ///     unsafe fn dec_ref(_obj: NonNull<Self>) {}
>>> > @@ -500,7 +500,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`.
>>> 
>>> This is not sufficient. The safety requirement is:
>>> 
>>>   Callers must ensure that the reference count was incremented at least once, and that they
>>>   are properly relinquishing one increment. That is, if there is only one increment, callers
>>>   must not use the underlying object anymore -- it is only safe to do so via the newly
>>>   created [`ARef`].
>>> 
>>> 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`.
>>> 
>>> I think this example actually exposes a soundness hole. When the
>>> underlying object is allocated on the stack, the safety requirements are
>>> not sufficient to ensure the lifetime of the object. We could safely
>>> return `data_ref` and have the underlying object go away. We should add
>>> to the safety requirement of `ARef::from_raw`:
>>> 
>>>   `ptr` must be valid while the refcount is positive.
>>
>> Wouldn't this already be covered by the below in the doc for
>> AlwaysRefCounted?
>>
>>     Implementers must ensure that increments to the reference count keep
>>     the object alive in memory at least until matching decrements are
>>     performed."
>
> No, I don't think that is enough. We can implement the trait properly
> with refcounts and still we can allocate an object on the stack and then
> do a `from_raw` on that object without violating any safety
> requirements. I think the `ARef::from_raw` should have the safety
> requirement above. But we can do that as a separate patch.

On second thought, I think you are right. I was trying to implement a
counter example, and I think it is not possible to implement
`RefCounted` while following the safety requirements in a way that would
trigger this issue. Implementers will have to make sure that the the
type that implement `RefCounted` cannot be directly constructed. In
other words the implementation for `Empty` is illegal in this regard.

We can do this instead for the example


    mod empty {
        use crate::alloc::KBox;
        use core::ptr::NonNull;

        pub struct Empty {
            // Prevent direct construction
            _p: (),
        }

        // 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 super::RefCounted for Empty {
            fn inc_ref(&self) {}

            unsafe fn dec_ref(_obj: NonNull<Self>) {}
        }

        impl Empty {
            pub fn new() -> NonNull<Self> {
                NonNull::new(KBox::into_raw(
                    KBox::new(Self { _p: () }, kernel::alloc::flags::GFP_KERNEL).unwrap(),
                ))
                .unwrap()
            }
        }
    }

    let ptr = empty::Empty::new();

    // 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`.
    let data_ref: ARef<empty::Empty> = unsafe { ARef::from_raw(ptr) };
    let _raw_ptr: NonNull<empty::Empty> = ARef::into_raw(data_ref);



Best regards,
Andreas Hindborg

[PATCH v9 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>
---
 init/Kconfig | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/init/Kconfig b/init/Kconfig
index 324c2886b2ea31e84d6ff221338a1effa06298f2..6dd58788636ef26e017f704b2c0088401f28943c 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -132,6 +132,9 @@ config CC_HAS_COUNTED_BY
 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

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

[Andreas Hindborg]

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

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


Best regards,
Andreas Hindborg

[PATCH v9 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 access through an ARef
but have methods which can only safely called when the reference is unique,
like e.g. `block::mq::Request::end_ok()`.

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

diff --git a/rust/kernel/ownable.rs b/rust/kernel/ownable.rs
index f4bebea23ce1d62f5597e35199ca38ea07b293db..59f84a838bbc4e80fc86dd8dc91f1672fc649403 100644
--- a/rust/kernel/ownable.rs
+++ b/rust/kernel/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)
+    }
+}
diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index db29f7c725e631c11099fa9122901ec2b3f4a039..a72ac86befc4923ee0bbbc1eea733a245189706a 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -11,7 +11,7 @@
     ptr::NonNull,
 };
 
-pub use crate::ownable::{Ownable, OwnableMut, Owned};
+pub use crate::ownable::{Ownable, OwnableMut, OwnableRefCounted, Owned, SimpleOwnableRefCounted};
 
 /// Used to transfer ownership to and from foreign (non-Rust) languages.
 ///
@@ -536,6 +536,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

-- 
2.49.0

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

[Andreas Hindborg]

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

> 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 access through an ARef
> but have methods which can only safely called when the reference is unique,
> like e.g. `block::mq::Request::end_ok()`.

You have a few typos here

  This is useful for types which generally are *accessed* through an ARef
  but have methods which can only *be* safely 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>


Best regards,
Andreas Hindborg