From: Asahi Lina <lina+kernel@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 and commit message
]
Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
---
rust/kernel/types.rs | 7 +++
rust/kernel/types/ownable.rs | 134 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 141 insertions(+)
diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index 22985b6f69820d6df8ff3aae0bf815fad36a9d92..c12ff4d2a3f2d79b760c34c0b84a51b507d0cfb1 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
@@ -425,6 +428,10 @@ pub const fn raw_get(this: *const Self) -> *mut T {
/// Rust code, the recommendation is to use [`Arc`](crate::sync::Arc) to create reference-counted
/// instances of a type.
///
+/// Note: Implementing this trait allows types to be wrapped in an [`ARef<Self>`]. It requires an
+/// internal reference count and provides only shared references. If unique references are required
+/// [`Ownable`] should be implemented which allows types to be wrapped in an [`Owned<Self>`].
+///
/// # Safety
///
/// Implementers must ensure that increments to the reference count keep the object alive in memory
diff --git a/rust/kernel/types/ownable.rs b/rust/kernel/types/ownable.rs
new file mode 100644
index 0000000000000000000000000000000000000000..f4065a0d627a62d3ecb15edabf306e9b812556e1
--- /dev/null
+++ b/rust/kernel/types/ownable.rs
@@ -0,0 +1,134 @@
+// 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.
+///
+/// Note: Implementing this trait allows types to be wrapped in an [`Owned<Self>`]. This does not
+/// provide reference counting but represents a unique, owned reference. If reference counting is
+/// required [`AlwaysRefCounted`](crate::types::AlwaysRefCounted) should be implemented which allows
+/// types to be wrapped in an [`ARef<Self>`](crate::types::ARef).
+///
+/// # Safety
+///
+/// Implementers must ensure that:
+/// - The [`release()`](Ownable::release) method leaves the underlying object in a state which the
+/// kernel expects after ownership has been relinquished (i.e. no dangling references in the
+/// kernel is case it frees the object, etc.).
+pub unsafe trait Ownable {
+ /// Releases the object (frees it or returns it to foreign ownership).
+ ///
+ /// # Safety
+ ///
+ /// Callers must ensure that:
+ /// - `this` points to a valid `Self`.
+ /// - The object is no longer referenced after this call.
+ unsafe fn release(this: NonNull<Self>);
+}
+
+/// Type where [`Owned<Self>`] derefs to `&mut Self`.
+///
+/// # Safety
+///
+/// Implementers must ensure that access to a `&mut T` is safe, implying that:
+/// - It is safe to call [`core::mem::swap`] on the [`Ownable`]. This excludes pinned types
+/// (i.e. most kernel types).
+/// - The kernel will never access the underlying object (excluding internal mutability that follows
+/// the usual rules) while Rust owns it.
+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` can be considered owned by 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:
+ /// - Ownership of the underlying object can be transferred to the `Owned<T>` (i.e. operations
+ /// which require ownership will be safe).
+ /// - No other Rust references to the underlying object exist. This implies that the underlying
+ /// object is not accessed through `ptr` anymore after the function call (at least until the
+ /// the `Owned<T>` is dropped).
+ /// - The C code follows the usual shared reference requirements. That is, the kernel will never
+ /// mutate or free the underlying object (excluding interior mutability that follows the usual
+ /// rules) while Rust owns it.
+ /// - In case `T` implements [`OwnableMut`] the previous requirement is extended from shared to
+ /// mutable reference requirements. That is, the kernel will not mutate or free the underlying
+ /// object and is okay with it being modified by Rust code.
+ 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"Oliver Mangold" <oliver.mangold@pm.me> writes:
> From: Asahi Lina <lina+kernel@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 and commit message
> ]
> Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
> Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
> ---
> rust/kernel/types.rs | 7 +++
> rust/kernel/types/ownable.rs | 134 +++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 141 insertions(+)
>
> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index 22985b6f69820d6df8ff3aae0bf815fad36a9d92..c12ff4d2a3f2d79b760c34c0b84a51b507d0cfb1 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
> @@ -425,6 +428,10 @@ pub const fn raw_get(this: *const Self) -> *mut T {
> /// Rust code, the recommendation is to use [`Arc`](crate::sync::Arc) to create reference-counted
> /// instances of a type.
> ///
> +/// Note: Implementing this trait allows types to be wrapped in an [`ARef<Self>`]. It requires an
> +/// internal reference count and provides only shared references. If unique references are required
> +/// [`Ownable`] should be implemented which allows types to be wrapped in an [`Owned<Self>`].
> +///
> /// # Safety
> ///
> /// Implementers must ensure that increments to the reference count keep the object alive in memory
> diff --git a/rust/kernel/types/ownable.rs b/rust/kernel/types/ownable.rs
> new file mode 100644
> index 0000000000000000000000000000000000000000..f4065a0d627a62d3ecb15edabf306e9b812556e1
> --- /dev/null
> +++ b/rust/kernel/types/ownable.rs
> @@ -0,0 +1,134 @@
> +// 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.
> +///
> +/// Note: Implementing this trait allows types to be wrapped in an [`Owned<Self>`]. This does not
> +/// provide reference counting but represents a unique, owned reference. If reference counting is
> +/// required [`AlwaysRefCounted`](crate::types::AlwaysRefCounted) should be implemented which allows
> +/// types to be wrapped in an [`ARef<Self>`](crate::types::ARef).
> +///
> +/// # Safety
> +///
> +/// Implementers must ensure that:
> +/// - The [`release()`](Ownable::release) method leaves the underlying object in a state which the
> +/// kernel expects after ownership has been relinquished (i.e. no dangling references in the
> +/// kernel is case it frees the object, etc.).
> +pub unsafe trait Ownable {
> + /// Releases the object (frees it or returns it to foreign ownership).
> + ///
> + /// # Safety
> + ///
> + /// Callers must ensure that:
> + /// - `this` points to a valid `Self`.
> + /// - The object is no longer referenced after this call.
> + unsafe fn release(this: NonNull<Self>);
> +}
> +
> +/// Type where [`Owned<Self>`] derefs to `&mut Self`.
> +///
> +/// # Safety
> +///
> +/// Implementers must ensure that access to a `&mut T` is safe, implying that:
> +/// - It is safe to call [`core::mem::swap`] on the [`Ownable`]. This excludes pinned types
> +/// (i.e. most kernel types).
> +/// - The kernel will never access the underlying object (excluding internal mutability that follows
> +/// the usual rules) while Rust owns it.
> +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` can be considered owned by 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:
> + /// - Ownership of the underlying object can be transferred to the `Owned<T>` (i.e. operations
> + /// which require ownership will be safe).
> + /// - No other Rust references to the underlying object exist. This implies that the underlying
> + /// object is not accessed through `ptr` anymore after the function call (at least until the
> + /// the `Owned<T>` is dropped).
> + /// - The C code follows the usual shared reference requirements. That is, the kernel will never
> + /// mutate or free the underlying object (excluding interior mutability that follows the usual
> + /// rules) while Rust owns it.
> + /// - In case `T` implements [`OwnableMut`] the previous requirement is extended from shared to
> + /// mutable reference requirements. That is, the kernel will not mutate or free the underlying
> + /// object and is okay with it being modified by Rust code.
> + 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() }
> + }
> +}
I think someone mentioned this before, but handing out mutable
references can be a problem if `T: !Unpin`. For instance, we don't want
to hand out `&mut Page` in case of `Owned<Page>`.
Could we do something like this:
diff --git a/rust/kernel/types/ownable.rs b/rust/kernel/types/ownable.rs
index 29729dc10cb4..52d21e41c26b 100644
--- a/rust/kernel/types/ownable.rs
+++ b/rust/kernel/types/ownable.rs
@@ -3,6 +3,7 @@
//! Owned reference types.
use crate::{
+ prelude::*,
sync::aref::{ARef, RefCounted},
types::ForeignOwnable,
};
@@ -112,6 +113,17 @@ pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
pub fn into_raw(me: Self) -> NonNull<T> {
ManuallyDrop::new(me).ptr
}
+
+ /// Get a pinned mutable reference to the data owned by this `Owned<T>`.
+ pub fn get_pin_mut(&mut self) -> Pin<&mut T> {
+ // SAFETY: The type invariants guarantee that the object is valid, and that we can safely
+ // return a mutable reference to it.
+ let unpinned = unsafe { self.ptr.as_mut() };
+
+ // SAFETY: We never hand out unpinned mutable references to the data in
+ // `Self`, unless the contained type is `Unpin`.
+ unsafe { Pin::new_unchecked(unpinned) }
+ }
}
impl<T: Ownable> Deref for Owned<T> {
@@ -123,7 +135,7 @@ fn deref(&self) -> &Self::Target {
}
}
-impl<T: OwnableMut> DerefMut for Owned<T> {
+impl<T: OwnableMut + Unpin> 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.
Best regards,
Andreas Hindborg
On 250818 1446, Andreas Hindborg wrote:
> "Oliver Mangold" <oliver.mangold@pm.me> writes:
>
> > From: Asahi Lina <lina+kernel@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 and commit message
> > ]
> > Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
> > Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
> > ---
> > rust/kernel/types.rs | 7 +++
> > rust/kernel/types/ownable.rs | 134 +++++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 141 insertions(+)
> >
> > diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> > index 22985b6f69820d6df8ff3aae0bf815fad36a9d92..c12ff4d2a3f2d79b760c34c0b84a51b507d0cfb1 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
> > @@ -425,6 +428,10 @@ pub const fn raw_get(this: *const Self) -> *mut T {
> > /// Rust code, the recommendation is to use [`Arc`](crate::sync::Arc) to create reference-counted
> > /// instances of a type.
> > ///
> > +/// Note: Implementing this trait allows types to be wrapped in an [`ARef<Self>`]. It requires an
> > +/// internal reference count and provides only shared references. If unique references are required
> > +/// [`Ownable`] should be implemented which allows types to be wrapped in an [`Owned<Self>`].
> > +///
> > /// # Safety
> > ///
> > /// Implementers must ensure that increments to the reference count keep the object alive in memory
> > diff --git a/rust/kernel/types/ownable.rs b/rust/kernel/types/ownable.rs
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..f4065a0d627a62d3ecb15edabf306e9b812556e1
> > --- /dev/null
> > +++ b/rust/kernel/types/ownable.rs
> > @@ -0,0 +1,134 @@
> > +// 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.
> > +///
> > +/// Note: Implementing this trait allows types to be wrapped in an [`Owned<Self>`]. This does not
> > +/// provide reference counting but represents a unique, owned reference. If reference counting is
> > +/// required [`AlwaysRefCounted`](crate::types::AlwaysRefCounted) should be implemented which allows
> > +/// types to be wrapped in an [`ARef<Self>`](crate::types::ARef).
> > +///
> > +/// # Safety
> > +///
> > +/// Implementers must ensure that:
> > +/// - The [`release()`](Ownable::release) method leaves the underlying object in a state which the
> > +/// kernel expects after ownership has been relinquished (i.e. no dangling references in the
> > +/// kernel is case it frees the object, etc.).
> > +pub unsafe trait Ownable {
> > + /// Releases the object (frees it or returns it to foreign ownership).
> > + ///
> > + /// # Safety
> > + ///
> > + /// Callers must ensure that:
> > + /// - `this` points to a valid `Self`.
> > + /// - The object is no longer referenced after this call.
> > + unsafe fn release(this: NonNull<Self>);
> > +}
> > +
> > +/// Type where [`Owned<Self>`] derefs to `&mut Self`.
> > +///
> > +/// # Safety
> > +///
> > +/// Implementers must ensure that access to a `&mut T` is safe, implying that:
> > +/// - It is safe to call [`core::mem::swap`] on the [`Ownable`]. This excludes pinned types
> > +/// (i.e. most kernel types).
> > +/// - The kernel will never access the underlying object (excluding internal mutability that follows
> > +/// the usual rules) while Rust owns it.
> > +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` can be considered owned by 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:
> > + /// - Ownership of the underlying object can be transferred to the `Owned<T>` (i.e. operations
> > + /// which require ownership will be safe).
> > + /// - No other Rust references to the underlying object exist. This implies that the underlying
> > + /// object is not accessed through `ptr` anymore after the function call (at least until the
> > + /// the `Owned<T>` is dropped).
> > + /// - The C code follows the usual shared reference requirements. That is, the kernel will never
> > + /// mutate or free the underlying object (excluding interior mutability that follows the usual
> > + /// rules) while Rust owns it.
> > + /// - In case `T` implements [`OwnableMut`] the previous requirement is extended from shared to
> > + /// mutable reference requirements. That is, the kernel will not mutate or free the underlying
> > + /// object and is okay with it being modified by Rust code.
> > + 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() }
> > + }
> > +}
>
> I think someone mentioned this before, but handing out mutable
> references can be a problem if `T: !Unpin`. For instance, we don't want
> to hand out `&mut Page` in case of `Owned<Page>`.
>
That was the reason, why `OwnableMut` was introduced in the first place.
It's clear, I guess, that as-is it cannot be implemented on many classes.
> Could we do something like this:
>
> diff --git a/rust/kernel/types/ownable.rs b/rust/kernel/types/ownable.rs
> index 29729dc10cb4..52d21e41c26b 100644
> --- a/rust/kernel/types/ownable.rs
> +++ b/rust/kernel/types/ownable.rs
> @@ -3,6 +3,7 @@
> //! Owned reference types.
>
> use crate::{
> + prelude::*,
> sync::aref::{ARef, RefCounted},
> types::ForeignOwnable,
> };
> @@ -112,6 +113,17 @@ pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
> pub fn into_raw(me: Self) -> NonNull<T> {
> ManuallyDrop::new(me).ptr
> }
> +
> + /// Get a pinned mutable reference to the data owned by this `Owned<T>`.
> + pub fn get_pin_mut(&mut self) -> Pin<&mut T> {
> + // SAFETY: The type invariants guarantee that the object is valid, and that we can safely
> + // return a mutable reference to it.
> + let unpinned = unsafe { self.ptr.as_mut() };
> +
> + // SAFETY: We never hand out unpinned mutable references to the data in
> + // `Self`, unless the contained type is `Unpin`.
> + unsafe { Pin::new_unchecked(unpinned) }
> + }
> }
>
> impl<T: Ownable> Deref for Owned<T> {
> @@ -123,7 +135,7 @@ fn deref(&self) -> &Self::Target {
> }
> }
>
> -impl<T: OwnableMut> DerefMut for Owned<T> {
> +impl<T: OwnableMut + Unpin> 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.
>
Good question, I have been thinking about it, too. But it might
be, that it isn't needed at all. As I understand, usually Rust wrappers
are around non-movable C structs. Do we actually have a useful application
for OwnableMut?
Best,
Oliver
On Mon Aug 18, 2025 at 3:04 PM CEST, Oliver Mangold wrote:
> On 250818 1446, Andreas Hindborg wrote:
>> "Oliver Mangold" <oliver.mangold@pm.me> writes:
>> > +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() }
>> > + }
>> > +}
>>
>> I think someone mentioned this before, but handing out mutable
>> references can be a problem if `T: !Unpin`. For instance, we don't want
>> to hand out `&mut Page` in case of `Owned<Page>`.
>>
>
> That was the reason, why `OwnableMut` was introduced in the first place.
> It's clear, I guess, that as-is it cannot be implemented on many classes.
Yeah the safety requirements ensure that you can't implement it on
`!Unpin` types.
But I'm not sure it's useful then? As you said there aren't many types
that will implement the type then, so how about we change the meaning
and make it give out a pinned mutable reference instead?
> Good question, I have been thinking about it, too. But it might
> be, that it isn't needed at all. As I understand, usually Rust wrappers
> are around non-movable C structs. Do we actually have a useful application
> for OwnableMut?
Also, do we even need two different traits? Which types would only
implement `Ownable` but not `OwnableMut`?
---
Cheers,
Benno
On 250819 0027, Benno Lossin wrote:
> On Mon Aug 18, 2025 at 3:04 PM CEST, Oliver Mangold wrote:
> > On 250818 1446, Andreas Hindborg wrote:
> >> "Oliver Mangold" <oliver.mangold@pm.me> writes:
> >> > +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() }
> >> > + }
> >> > +}
> >>
> >> I think someone mentioned this before, but handing out mutable
> >> references can be a problem if `T: !Unpin`. For instance, we don't want
> >> to hand out `&mut Page` in case of `Owned<Page>`.
> >>
> >
> > That was the reason, why `OwnableMut` was introduced in the first place.
> > It's clear, I guess, that as-is it cannot be implemented on many classes.
>
> Yeah the safety requirements ensure that you can't implement it on
> `!Unpin` types.
>
> But I'm not sure it's useful then? As you said there aren't many types
> that will implement the type then, so how about we change the meaning
> and make it give out a pinned mutable reference instead?
Making `deref_mut()` give out a pinned type won't work. The return types of
deref() are required to match.
> > Good question, I have been thinking about it, too. But it might
> > be, that it isn't needed at all. As I understand, usually Rust wrappers
> > are around non-movable C structs. Do we actually have a useful application
> > for OwnableMut?
>
> Also, do we even need two different traits? Which types would only
> implement `Ownable` but not `OwnableMut`?
I'm not 100% sure, but on a quick glance it looks indeed be safe to
substitute `OwnableMut` by `Unpin`.
If we add `get_pin_mut(&mut self) -> Pin<&mut T>` as Andreas suggested,
it would be possible to obtain an `&mut T` anyway, then, if T is `Unpin`.
On Tue Aug 19, 2025 at 8:04 AM CEST, Oliver Mangold wrote:
> On 250819 0027, Benno Lossin wrote:
>> On Mon Aug 18, 2025 at 3:04 PM CEST, Oliver Mangold wrote:
>> > On 250818 1446, Andreas Hindborg wrote:
>> >> "Oliver Mangold" <oliver.mangold@pm.me> writes:
>> >> > +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() }
>> >> > + }
>> >> > +}
>> >>
>> >> I think someone mentioned this before, but handing out mutable
>> >> references can be a problem if `T: !Unpin`. For instance, we don't want
>> >> to hand out `&mut Page` in case of `Owned<Page>`.
>> >>
>> >
>> > That was the reason, why `OwnableMut` was introduced in the first place.
>> > It's clear, I guess, that as-is it cannot be implemented on many classes.
>>
>> Yeah the safety requirements ensure that you can't implement it on
>> `!Unpin` types.
>>
>> But I'm not sure it's useful then? As you said there aren't many types
>> that will implement the type then, so how about we change the meaning
>> and make it give out a pinned mutable reference instead?
>
> Making `deref_mut()` give out a pinned type won't work. The return types of
> deref() are required to match.
I meant the changes that Andreas suggested.
>> > Good question, I have been thinking about it, too. But it might
>> > be, that it isn't needed at all. As I understand, usually Rust wrappers
>> > are around non-movable C structs. Do we actually have a useful application
>> > for OwnableMut?
>>
>> Also, do we even need two different traits? Which types would only
>> implement `Ownable` but not `OwnableMut`?
>
> I'm not 100% sure, but on a quick glance it looks indeed be safe to
> substitute `OwnableMut` by `Unpin`.
We just have to change the safety requirements of `OwnableMut`.
> If we add `get_pin_mut(&mut self) -> Pin<&mut T>` as Andreas suggested,
> it would be possible to obtain an `&mut T` anyway, then, if T is `Unpin`.
Well the `DerefMut` impl still is convenient in the `Unpin` case.
---
Cheers,
Benno
"Benno Lossin" <lossin@kernel.org> writes:
> On Tue Aug 19, 2025 at 8:04 AM CEST, Oliver Mangold wrote:
>> On 250819 0027, Benno Lossin wrote:
>>> On Mon Aug 18, 2025 at 3:04 PM CEST, Oliver Mangold wrote:
>>> > On 250818 1446, Andreas Hindborg wrote:
>>> >> "Oliver Mangold" <oliver.mangold@pm.me> writes:
>>> >> > +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() }
>>> >> > + }
>>> >> > +}
>>> >>
>>> >> I think someone mentioned this before, but handing out mutable
>>> >> references can be a problem if `T: !Unpin`. For instance, we don't want
>>> >> to hand out `&mut Page` in case of `Owned<Page>`.
>>> >>
>>> >
>>> > That was the reason, why `OwnableMut` was introduced in the first place.
>>> > It's clear, I guess, that as-is it cannot be implemented on many classes.
>>>
>>> Yeah the safety requirements ensure that you can't implement it on
>>> `!Unpin` types.
>>>
>>> But I'm not sure it's useful then? As you said there aren't many types
>>> that will implement the type then, so how about we change the meaning
>>> and make it give out a pinned mutable reference instead?
>>
>> Making `deref_mut()` give out a pinned type won't work. The return types of
>> deref() are required to match.
>
> I meant the changes that Andreas suggested.
Not sure what you are asking, but I need to assert exclusive access to
an `Page`. I could either get this by taking a `&mut Owned<Page>` or a
`Pin<&mut Page>`. I think the latter is more agnostic.
>
>>> > Good question, I have been thinking about it, too. But it might
>>> > be, that it isn't needed at all. As I understand, usually Rust wrappers
>>> > are around non-movable C structs. Do we actually have a useful application
>>> > for OwnableMut?
>>>
>>> Also, do we even need two different traits? Which types would only
>>> implement `Ownable` but not `OwnableMut`?
>>
>> I'm not 100% sure, but on a quick glance it looks indeed be safe to
>> substitute `OwnableMut` by `Unpin`.
>
> We just have to change the safety requirements of `OwnableMut`.
`OwnableMut` already requires `Unpin`, it just does not say so directly:
/// - It is safe to call [`core::mem::swap`] on the [`Ownable`]. This excludes pinned types
/// (i.e. most kernel types).
We could remove this and then just add a trait bound on `Unpin`.
>
>> If we add `get_pin_mut(&mut self) -> Pin<&mut T>` as Andreas suggested,
>> it would be possible to obtain an `&mut T` anyway, then, if T is `Unpin`.
>
> Well the `DerefMut` impl still is convenient in the `Unpin` case.
`OwnableMut` is probably not that useful, since all the types we want to
implement `Ownable` for is `!Unpin`. We could remove it, but I felt it
was neat to add the `DerefMut` impl for `Unpin` types.
Best regards,
Andreas Hindborg
On Tue Aug 19, 2025 at 10:53 AM CEST, Andreas Hindborg wrote:
> "Benno Lossin" <lossin@kernel.org> writes:
>> On Tue Aug 19, 2025 at 8:04 AM CEST, Oliver Mangold wrote:
>>> On 250819 0027, Benno Lossin wrote:
>>>> On Mon Aug 18, 2025 at 3:04 PM CEST, Oliver Mangold wrote:
>>>> > On 250818 1446, Andreas Hindborg wrote:
>>>> >> "Oliver Mangold" <oliver.mangold@pm.me> writes:
>>>> >> > +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() }
>>>> >> > + }
>>>> >> > +}
>>>> >>
>>>> >> I think someone mentioned this before, but handing out mutable
>>>> >> references can be a problem if `T: !Unpin`. For instance, we don't want
>>>> >> to hand out `&mut Page` in case of `Owned<Page>`.
>>>> >>
>>>> >
>>>> > That was the reason, why `OwnableMut` was introduced in the first place.
>>>> > It's clear, I guess, that as-is it cannot be implemented on many classes.
>>>>
>>>> Yeah the safety requirements ensure that you can't implement it on
>>>> `!Unpin` types.
>>>>
>>>> But I'm not sure it's useful then? As you said there aren't many types
>>>> that will implement the type then, so how about we change the meaning
>>>> and make it give out a pinned mutable reference instead?
>>>
>>> Making `deref_mut()` give out a pinned type won't work. The return types of
>>> deref() are required to match.
>>
>> I meant the changes that Andreas suggested.
>
> Not sure what you are asking, but I need to assert exclusive access to
> an `Page`. I could either get this by taking a `&mut Owned<Page>` or a
> `Pin<&mut Page>`. I think the latter is more agnostic.
The former isn't really correct? It's like having a `&mut Box<Page>`
which is weird. I was saying we can have a `DerefMut` impl gated on `T:
Unpin` and a `fn get_pin_mut(&mut self) -> Pin<&mut T>`.
>>>> > Good question, I have been thinking about it, too. But it might
>>>> > be, that it isn't needed at all. As I understand, usually Rust wrappers
>>>> > are around non-movable C structs. Do we actually have a useful application
>>>> > for OwnableMut?
>>>>
>>>> Also, do we even need two different traits? Which types would only
>>>> implement `Ownable` but not `OwnableMut`?
>>>
>>> I'm not 100% sure, but on a quick glance it looks indeed be safe to
>>> substitute `OwnableMut` by `Unpin`.
>>
>> We just have to change the safety requirements of `OwnableMut`.
>
> `OwnableMut` already requires `Unpin`, it just does not say so directly:
>
>
> /// - It is safe to call [`core::mem::swap`] on the [`Ownable`]. This excludes pinned types
> /// (i.e. most kernel types).
>
> We could remove this and then just add a trait bound on `Unpin`.
Oh I happened to not have read it that thoroughly then... I don't think
it makes sense to have `OwnableMut` then. So I agree with your suggested
change :)
>>> If we add `get_pin_mut(&mut self) -> Pin<&mut T>` as Andreas suggested,
>>> it would be possible to obtain an `&mut T` anyway, then, if T is `Unpin`.
>>
>> Well the `DerefMut` impl still is convenient in the `Unpin` case.
>
> `OwnableMut` is probably not that useful, since all the types we want to
> implement `Ownable` for is `!Unpin`. We could remove it, but I felt it
> was neat to add the `DerefMut` impl for `Unpin` types.
But we don't need `OwnableMut` in that case?
Let's just do the following if it makes sense:
* remove `OwnableMut`
* allow obtaining a `Pin<&mut T>` from `Owned<T>` via a `&mut self`
method (we need a new safety requirement for this on `Ownable`)
* have the `DerefMut` impl require `T: Unpin`
---
Cheers,
Benno
On 250819 1913, Benno Lossin wrote:
> On Tue Aug 19, 2025 at 10:53 AM CEST, Andreas Hindborg wrote:
> > "Benno Lossin" <lossin@kernel.org> writes:
> >> On Tue Aug 19, 2025 at 8:04 AM CEST, Oliver Mangold wrote:
> >>> On 250819 0027, Benno Lossin wrote:
> >>>> On Mon Aug 18, 2025 at 3:04 PM CEST, Oliver Mangold wrote:
> >>>> > On 250818 1446, Andreas Hindborg wrote:
> >>>> >> "Oliver Mangold" <oliver.mangold@pm.me> writes:
> >>>> >> > +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() }
> >>>> >> > + }
> >>>> >> > +}
> >>>> >>
> >>>> >> I think someone mentioned this before, but handing out mutable
> >>>> >> references can be a problem if `T: !Unpin`. For instance, we don't want
> >>>> >> to hand out `&mut Page` in case of `Owned<Page>`.
> >>>> >>
> >>>> >
> >>>> > That was the reason, why `OwnableMut` was introduced in the first place.
> >>>> > It's clear, I guess, that as-is it cannot be implemented on many classes.
> >>>>
> >>>> Yeah the safety requirements ensure that you can't implement it on
> >>>> `!Unpin` types.
> >>>>
> >>>> But I'm not sure it's useful then? As you said there aren't many types
> >>>> that will implement the type then, so how about we change the meaning
> >>>> and make it give out a pinned mutable reference instead?
> >>>
> >>> Making `deref_mut()` give out a pinned type won't work. The return types of
> >>> deref() are required to match.
> >>
> >> I meant the changes that Andreas suggested.
> >
> > Not sure what you are asking, but I need to assert exclusive access to
> > an `Page`. I could either get this by taking a `&mut Owned<Page>` or a
> > `Pin<&mut Page>`. I think the latter is more agnostic.
>
> The former isn't really correct? It's like having a `&mut Box<Page>`
> which is weird. I was saying we can have a `DerefMut` impl gated on `T:
> Unpin` and a `fn get_pin_mut(&mut self) -> Pin<&mut T>`.
Yes. I think `Page` is the wrong example, as it already has owned semantics
and does its own cleanup. Wrapping it in an Owned would be redundant.
Best,
Oliver
On Wed Aug 20, 2025 at 8:02 AM CEST, Oliver Mangold wrote:
> On 250819 1913, Benno Lossin wrote:
>> On Tue Aug 19, 2025 at 10:53 AM CEST, Andreas Hindborg wrote:
>> > "Benno Lossin" <lossin@kernel.org> writes:
>> >> On Tue Aug 19, 2025 at 8:04 AM CEST, Oliver Mangold wrote:
>> >>> On 250819 0027, Benno Lossin wrote:
>> >>>> On Mon Aug 18, 2025 at 3:04 PM CEST, Oliver Mangold wrote:
>> >>>> > On 250818 1446, Andreas Hindborg wrote:
>> >>>> >> "Oliver Mangold" <oliver.mangold@pm.me> writes:
>> >>>> >> > +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() }
>> >>>> >> > + }
>> >>>> >> > +}
>> >>>> >>
>> >>>> >> I think someone mentioned this before, but handing out mutable
>> >>>> >> references can be a problem if `T: !Unpin`. For instance, we don't want
>> >>>> >> to hand out `&mut Page` in case of `Owned<Page>`.
>> >>>> >>
>> >>>> >
>> >>>> > That was the reason, why `OwnableMut` was introduced in the first place.
>> >>>> > It's clear, I guess, that as-is it cannot be implemented on many classes.
>> >>>>
>> >>>> Yeah the safety requirements ensure that you can't implement it on
>> >>>> `!Unpin` types.
>> >>>>
>> >>>> But I'm not sure it's useful then? As you said there aren't many types
>> >>>> that will implement the type then, so how about we change the meaning
>> >>>> and make it give out a pinned mutable reference instead?
>> >>>
>> >>> Making `deref_mut()` give out a pinned type won't work. The return types of
>> >>> deref() are required to match.
>> >>
>> >> I meant the changes that Andreas suggested.
>> >
>> > Not sure what you are asking, but I need to assert exclusive access to
>> > an `Page`. I could either get this by taking a `&mut Owned<Page>` or a
>> > `Pin<&mut Page>`. I think the latter is more agnostic.
>>
>> The former isn't really correct? It's like having a `&mut Box<Page>`
>> which is weird. I was saying we can have a `DerefMut` impl gated on `T:
>> Unpin` and a `fn get_pin_mut(&mut self) -> Pin<&mut T>`.
>
> Yes. I think `Page` is the wrong example, as it already has owned semantics
> and does its own cleanup. Wrapping it in an Owned would be redundant.
After we have these owned patches, we are going to change `Page` to
`Opaque<bindings::page>`.
---
Cheers,
Benno
On 250820 0941, Benno Lossin wrote:
> On Wed Aug 20, 2025 at 8:02 AM CEST, Oliver Mangold wrote:
> > On 250819 1913, Benno Lossin wrote:
> >> On Tue Aug 19, 2025 at 10:53 AM CEST, Andreas Hindborg wrote:
> >> > "Benno Lossin" <lossin@kernel.org> writes:
> >> >> On Tue Aug 19, 2025 at 8:04 AM CEST, Oliver Mangold wrote:
> >> >>> On 250819 0027, Benno Lossin wrote:
> >> >>>> On Mon Aug 18, 2025 at 3:04 PM CEST, Oliver Mangold wrote:
> >> >>>> > On 250818 1446, Andreas Hindborg wrote:
> >> >>>> >> "Oliver Mangold" <oliver.mangold@pm.me> writes:
> >> >>>> >> > +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() }
> >> >>>> >> > + }
> >> >>>> >> > +}
> >> >>>> >>
> >> >>>> >> I think someone mentioned this before, but handing out mutable
> >> >>>> >> references can be a problem if `T: !Unpin`. For instance, we don't want
> >> >>>> >> to hand out `&mut Page` in case of `Owned<Page>`.
> >> >>>> >>
> >> >>>> >
> >> >>>> > That was the reason, why `OwnableMut` was introduced in the first place.
> >> >>>> > It's clear, I guess, that as-is it cannot be implemented on many classes.
> >> >>>>
> >> >>>> Yeah the safety requirements ensure that you can't implement it on
> >> >>>> `!Unpin` types.
> >> >>>>
> >> >>>> But I'm not sure it's useful then? As you said there aren't many types
> >> >>>> that will implement the type then, so how about we change the meaning
> >> >>>> and make it give out a pinned mutable reference instead?
> >> >>>
> >> >>> Making `deref_mut()` give out a pinned type won't work. The return types of
> >> >>> deref() are required to match.
> >> >>
> >> >> I meant the changes that Andreas suggested.
> >> >
> >> > Not sure what you are asking, but I need to assert exclusive access to
> >> > an `Page`. I could either get this by taking a `&mut Owned<Page>` or a
> >> > `Pin<&mut Page>`. I think the latter is more agnostic.
> >>
> >> The former isn't really correct? It's like having a `&mut Box<Page>`
> >> which is weird. I was saying we can have a `DerefMut` impl gated on `T:
> >> Unpin` and a `fn get_pin_mut(&mut self) -> Pin<&mut T>`.
> >
> > Yes. I think `Page` is the wrong example, as it already has owned semantics
> > and does its own cleanup. Wrapping it in an Owned would be redundant.
>
> After we have these owned patches, we are going to change `Page` to
> `Opaque<bindings::page>`.
Ah, okay. Makes sense, I guess.
Best,
Oliver
"Oliver Mangold" <oliver.mangold@pm.me> writes:
> On 250820 0941, Benno Lossin wrote:
>> On Wed Aug 20, 2025 at 8:02 AM CEST, Oliver Mangold wrote:
>> > On 250819 1913, Benno Lossin wrote:
>> >> On Tue Aug 19, 2025 at 10:53 AM CEST, Andreas Hindborg wrote:
>> >> > "Benno Lossin" <lossin@kernel.org> writes:
>> >> >> On Tue Aug 19, 2025 at 8:04 AM CEST, Oliver Mangold wrote:
>> >> >>> On 250819 0027, Benno Lossin wrote:
>> >> >>>> On Mon Aug 18, 2025 at 3:04 PM CEST, Oliver Mangold wrote:
>> >> >>>> > On 250818 1446, Andreas Hindborg wrote:
>> >> >>>> >> "Oliver Mangold" <oliver.mangold@pm.me> writes:
>> >> >>>> >> > +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() }
>> >> >>>> >> > + }
>> >> >>>> >> > +}
>> >> >>>> >>
>> >> >>>> >> I think someone mentioned this before, but handing out mutable
>> >> >>>> >> references can be a problem if `T: !Unpin`. For instance, we don't want
>> >> >>>> >> to hand out `&mut Page` in case of `Owned<Page>`.
>> >> >>>> >>
>> >> >>>> >
>> >> >>>> > That was the reason, why `OwnableMut` was introduced in the first place.
>> >> >>>> > It's clear, I guess, that as-is it cannot be implemented on many classes.
>> >> >>>>
>> >> >>>> Yeah the safety requirements ensure that you can't implement it on
>> >> >>>> `!Unpin` types.
>> >> >>>>
>> >> >>>> But I'm not sure it's useful then? As you said there aren't many types
>> >> >>>> that will implement the type then, so how about we change the meaning
>> >> >>>> and make it give out a pinned mutable reference instead?
>> >> >>>
>> >> >>> Making `deref_mut()` give out a pinned type won't work. The return types of
>> >> >>> deref() are required to match.
>> >> >>
>> >> >> I meant the changes that Andreas suggested.
>> >> >
>> >> > Not sure what you are asking, but I need to assert exclusive access to
>> >> > an `Page`. I could either get this by taking a `&mut Owned<Page>` or a
>> >> > `Pin<&mut Page>`. I think the latter is more agnostic.
>> >>
>> >> The former isn't really correct? It's like having a `&mut Box<Page>`
>> >> which is weird. I was saying we can have a `DerefMut` impl gated on `T:
>> >> Unpin` and a `fn get_pin_mut(&mut self) -> Pin<&mut T>`.
>> >
>> > Yes. I think `Page` is the wrong example, as it already has owned semantics
>> > and does its own cleanup. Wrapping it in an Owned would be redundant.
>>
>> After we have these owned patches, we are going to change `Page` to
>> `Opaque<bindings::page>`.
>
> Ah, okay. Makes sense, I guess.
I applied Linas patch with this change to my tree [1].
Best regards,
Andreas Hindborg
[1] https://lore.kernel.org/all/20250202-rust-page-v1-2-e3170d7fe55e@asahilina.net/
"Benno Lossin" <lossin@kernel.org> writes:
> On Tue Aug 19, 2025 at 10:53 AM CEST, Andreas Hindborg wrote:
>> "Benno Lossin" <lossin@kernel.org> writes:
>>> On Tue Aug 19, 2025 at 8:04 AM CEST, Oliver Mangold wrote:
>>>> On 250819 0027, Benno Lossin wrote:
>>>>> On Mon Aug 18, 2025 at 3:04 PM CEST, Oliver Mangold wrote:
>>>>> > On 250818 1446, Andreas Hindborg wrote:
>>>>> >> "Oliver Mangold" <oliver.mangold@pm.me> writes:
>>>>> >> > +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() }
>>>>> >> > + }
>>>>> >> > +}
>>>>> >>
>>>>> >> I think someone mentioned this before, but handing out mutable
>>>>> >> references can be a problem if `T: !Unpin`. For instance, we don't want
>>>>> >> to hand out `&mut Page` in case of `Owned<Page>`.
>>>>> >>
>>>>> >
>>>>> > That was the reason, why `OwnableMut` was introduced in the first place.
>>>>> > It's clear, I guess, that as-is it cannot be implemented on many classes.
>>>>>
>>>>> Yeah the safety requirements ensure that you can't implement it on
>>>>> `!Unpin` types.
>>>>>
>>>>> But I'm not sure it's useful then? As you said there aren't many types
>>>>> that will implement the type then, so how about we change the meaning
>>>>> and make it give out a pinned mutable reference instead?
>>>>
>>>> Making `deref_mut()` give out a pinned type won't work. The return types of
>>>> deref() are required to match.
>>>
>>> I meant the changes that Andreas suggested.
>>
>> Not sure what you are asking, but I need to assert exclusive access to
>> an `Page`. I could either get this by taking a `&mut Owned<Page>` or a
>> `Pin<&mut Page>`. I think the latter is more agnostic.
>
> The former isn't really correct? It's like having a `&mut Box<Page>`
> which is weird. I was saying we can have a `DerefMut` impl gated on `T:
> Unpin` and a `fn get_pin_mut(&mut self) -> Pin<&mut T>`.
>
>>>>> > Good question, I have been thinking about it, too. But it might
>>>>> > be, that it isn't needed at all. As I understand, usually Rust wrappers
>>>>> > are around non-movable C structs. Do we actually have a useful application
>>>>> > for OwnableMut?
>>>>>
>>>>> Also, do we even need two different traits? Which types would only
>>>>> implement `Ownable` but not `OwnableMut`?
>>>>
>>>> I'm not 100% sure, but on a quick glance it looks indeed be safe to
>>>> substitute `OwnableMut` by `Unpin`.
>>>
>>> We just have to change the safety requirements of `OwnableMut`.
>>
>> `OwnableMut` already requires `Unpin`, it just does not say so directly:
>>
>>
>> /// - It is safe to call [`core::mem::swap`] on the [`Ownable`]. This excludes pinned types
>> /// (i.e. most kernel types).
>>
>> We could remove this and then just add a trait bound on `Unpin`.
>
> Oh I happened to not have read it that thoroughly then... I don't think
> it makes sense to have `OwnableMut` then. So I agree with your suggested
> change :)
>
>>>> If we add `get_pin_mut(&mut self) -> Pin<&mut T>` as Andreas suggested,
>>>> it would be possible to obtain an `&mut T` anyway, then, if T is `Unpin`.
>>>
>>> Well the `DerefMut` impl still is convenient in the `Unpin` case.
>>
>> `OwnableMut` is probably not that useful, since all the types we want to
>> implement `Ownable` for is `!Unpin`. We could remove it, but I felt it
>> was neat to add the `DerefMut` impl for `Unpin` types.
>
> But we don't need `OwnableMut` in that case?
Right, we can directly limit the `DerefMut` impl.
>
> Let's just do the following if it makes sense:
> * remove `OwnableMut`
> * allow obtaining a `Pin<&mut T>` from `Owned<T>` via a `&mut self`
> method (we need a new safety requirement for this on `Ownable`)
> * have the `DerefMut` impl require `T: Unpin`
Sounds good to me 👍
Best regards,
Andreas Hindborg
On 250819 1026, Benno Lossin wrote:
> On Tue Aug 19, 2025 at 8:04 AM CEST, Oliver Mangold wrote:
> > On 250819 0027, Benno Lossin wrote:
> >> On Mon Aug 18, 2025 at 3:04 PM CEST, Oliver Mangold wrote:
> >> > On 250818 1446, Andreas Hindborg wrote:
> >> >> "Oliver Mangold" <oliver.mangold@pm.me> writes:
> >> >> > +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() }
> >> >> > + }
> >> >> > +}
> >> >>
> >> >> I think someone mentioned this before, but handing out mutable
> >> >> references can be a problem if `T: !Unpin`. For instance, we don't want
> >> >> to hand out `&mut Page` in case of `Owned<Page>`.
> >> >>
> >> >
> >> > That was the reason, why `OwnableMut` was introduced in the first place.
> >> > It's clear, I guess, that as-is it cannot be implemented on many classes.
> >>
> >> Yeah the safety requirements ensure that you can't implement it on
> >> `!Unpin` types.
> >>
> >> But I'm not sure it's useful then? As you said there aren't many types
> >> that will implement the type then, so how about we change the meaning
> >> and make it give out a pinned mutable reference instead?
> >
> > Making `deref_mut()` give out a pinned type won't work. The return types of
> > deref() are required to match.
>
> I meant the changes that Andreas suggested.
>
> >> > Good question, I have been thinking about it, too. But it might
> >> > be, that it isn't needed at all. As I understand, usually Rust wrappers
> >> > are around non-movable C structs. Do we actually have a useful application
> >> > for OwnableMut?
> >>
> >> Also, do we even need two different traits? Which types would only
> >> implement `Ownable` but not `OwnableMut`?
> >
> > I'm not 100% sure, but on a quick glance it looks indeed be safe to
> > substitute `OwnableMut` by `Unpin`.
>
> We just have to change the safety requirements of `OwnableMut`.
You mean of `Ownable`, when `OwnableMut` is removed? Yes. A good question
in that context is, what it actually means to have an `&mut Opaque<T>`
where `T` is `Unpin`. If that implies being allowed to obtain an `&mut T`,
it would we easy, I guess.
> > If we add `get_pin_mut(&mut self) -> Pin<&mut T>` as Andreas suggested,
> > it would be possible to obtain an `&mut T` anyway, then, if T is `Unpin`.
>
> Well the `DerefMut` impl still is convenient in the `Unpin` case.
I agree. What I meant is, it could not introduce an extra safety
requirement having it, if that indirect method can be used anyway.
But what I am wondering is, if we actually want to start using `Pin`
at all. Isn't `Opaque` currently used about everywhere pinning is needed?
Best,
Oliver
"Oliver Mangold" <oliver.mangold@pm.me> writes:
> On 250819 1026, Benno Lossin wrote:
>> On Tue Aug 19, 2025 at 8:04 AM CEST, Oliver Mangold wrote:
>> > On 250819 0027, Benno Lossin wrote:
>> >> On Mon Aug 18, 2025 at 3:04 PM CEST, Oliver Mangold wrote:
>> >> > On 250818 1446, Andreas Hindborg wrote:
>> >> >> "Oliver Mangold" <oliver.mangold@pm.me> writes:
>> >> >> > +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() }
>> >> >> > + }
>> >> >> > +}
>> >> >>
>> >> >> I think someone mentioned this before, but handing out mutable
>> >> >> references can be a problem if `T: !Unpin`. For instance, we don't want
>> >> >> to hand out `&mut Page` in case of `Owned<Page>`.
>> >> >>
>> >> >
>> >> > That was the reason, why `OwnableMut` was introduced in the first place.
>> >> > It's clear, I guess, that as-is it cannot be implemented on many classes.
>> >>
>> >> Yeah the safety requirements ensure that you can't implement it on
>> >> `!Unpin` types.
>> >>
>> >> But I'm not sure it's useful then? As you said there aren't many types
>> >> that will implement the type then, so how about we change the meaning
>> >> and make it give out a pinned mutable reference instead?
>> >
>> > Making `deref_mut()` give out a pinned type won't work. The return types of
>> > deref() are required to match.
>>
>> I meant the changes that Andreas suggested.
>>
>> >> > Good question, I have been thinking about it, too. But it might
>> >> > be, that it isn't needed at all. As I understand, usually Rust wrappers
>> >> > are around non-movable C structs. Do we actually have a useful application
>> >> > for OwnableMut?
>> >>
>> >> Also, do we even need two different traits? Which types would only
>> >> implement `Ownable` but not `OwnableMut`?
>> >
>> > I'm not 100% sure, but on a quick glance it looks indeed be safe to
>> > substitute `OwnableMut` by `Unpin`.
>>
>> We just have to change the safety requirements of `OwnableMut`.
>
> You mean of `Ownable`, when `OwnableMut` is removed? Yes. A good question
> in that context is, what it actually means to have an `&mut Opaque<T>`
> where `T` is `Unpin`. If that implies being allowed to obtain an `&mut T`,
> it would we easy, I guess.
You should not be able to get a `&mut T` from a `&mut Opaque<T>`.
`Opaque` opts out of invariants that normally hold for rust references.
>
>> > If we add `get_pin_mut(&mut self) -> Pin<&mut T>` as Andreas suggested,
>> > it would be possible to obtain an `&mut T` anyway, then, if T is `Unpin`.
>>
>> Well the `DerefMut` impl still is convenient in the `Unpin` case.
>
> I agree. What I meant is, it could not introduce an extra safety
> requirement having it, if that indirect method can be used anyway.
As I mention in my other email, I think we can still have `OwnableMut`
if we add a trait bound on `Unpin`.
>
> But what I am wondering is, if we actually want to start using `Pin`
> at all. Isn't `Opaque` currently used about everywhere pinning is needed?
`Opaque` is `!Unpin`, but pinning guarantees does not come into effect
until we produce a `Pin<Opaque<T>>`.
Best regards,
Andreas Hindborg
On Tue Aug 19, 2025 at 11:00 AM CEST, Andreas Hindborg wrote: > "Oliver Mangold" <oliver.mangold@pm.me> writes: >> You mean of `Ownable`, when `OwnableMut` is removed? Yes. A good question >> in that context is, what it actually means to have an `&mut Opaque<T>` >> where `T` is `Unpin`. If that implies being allowed to obtain an `&mut T`, >> it would we easy, I guess. > > You should not be able to get a `&mut T` from a `&mut Opaque<T>`. > `Opaque` opts out of invariants that normally hold for rust references. Yes, that function mustn't exist. >> But what I am wondering is, if we actually want to start using `Pin` >> at all. Isn't `Opaque` currently used about everywhere pinning is needed? > > `Opaque` is `!Unpin`, but pinning guarantees does not come into effect > until we produce a `Pin<Opaque<T>>`. `Pin<Opaque<T>>` isn't really a thing. You still need a pointer indirection, so `Pin<P>` where `P: DerefMut<Target = Opaque<T>>` so for example `Pin<Box<Opaque<T>>>`. --- Cheers, Benno
"Benno Lossin" <lossin@kernel.org> writes: > On Tue Aug 19, 2025 at 11:00 AM CEST, Andreas Hindborg wrote: >> "Oliver Mangold" <oliver.mangold@pm.me> writes: >>> You mean of `Ownable`, when `OwnableMut` is removed? Yes. A good question >>> in that context is, what it actually means to have an `&mut Opaque<T>` >>> where `T` is `Unpin`. If that implies being allowed to obtain an `&mut T`, >>> it would we easy, I guess. >> >> You should not be able to get a `&mut T` from a `&mut Opaque<T>`. >> `Opaque` opts out of invariants that normally hold for rust references. > > Yes, that function mustn't exist. > >>> But what I am wondering is, if we actually want to start using `Pin` >>> at all. Isn't `Opaque` currently used about everywhere pinning is needed? >> >> `Opaque` is `!Unpin`, but pinning guarantees does not come into effect >> until we produce a `Pin<Opaque<T>>`. > > `Pin<Opaque<T>>` isn't really a thing. You still need a pointer > indirection, so `Pin<P>` where `P: DerefMut<Target = Opaque<T>>` so for > example `Pin<Box<Opaque<T>>>`. Right, that is what I meant, but not what I typed. Thanks for pointing that out :) Best regards, Andreas Hindborg
On Wed Jun 18, 2025 at 2:27 PM CEST, Oliver Mangold wrote:
> From: Asahi Lina <lina+kernel@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 and commit message
> ]
> Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
> Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
> ---
> rust/kernel/types.rs | 7 +++
> rust/kernel/types/ownable.rs | 134 +++++++++++++++++++++++++++++++++++++++++++
I think we should name this file `owned.rs` instead. It's also what
we'll have for `ARef` when that is moved to `sync/`.
Also, I do wonder does this really belong into the `types` module? I
feel like it's becoming our `utils` module and while it does fit, I
think we should just make this a top level module. So
`rust/kernel/owned.rs`. Thoughts?
> 2 files changed, 141 insertions(+)
>
> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index 22985b6f69820d6df8ff3aae0bf815fad36a9d92..c12ff4d2a3f2d79b760c34c0b84a51b507d0cfb1 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
> @@ -425,6 +428,10 @@ pub const fn raw_get(this: *const Self) -> *mut T {
> /// Rust code, the recommendation is to use [`Arc`](crate::sync::Arc) to create reference-counted
> /// instances of a type.
> ///
> +/// Note: Implementing this trait allows types to be wrapped in an [`ARef<Self>`]. It requires an
> +/// internal reference count and provides only shared references. If unique references are required
> +/// [`Ownable`] should be implemented which allows types to be wrapped in an [`Owned<Self>`].
> +///
> /// # Safety
> ///
> /// Implementers must ensure that increments to the reference count keep the object alive in memory
> diff --git a/rust/kernel/types/ownable.rs b/rust/kernel/types/ownable.rs
> new file mode 100644
> index 0000000000000000000000000000000000000000..f4065a0d627a62d3ecb15edabf306e9b812556e1
> --- /dev/null
> +++ b/rust/kernel/types/ownable.rs
> @@ -0,0 +1,134 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Owned reference types.
I think it's a good idea to expand the docs here a bit.
> +
> +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.
This seems wrong, `var: Owned<T>` should life until `min(var, T)`, so
whatever is earlier: until the user drops the `var` or `T`'s lifetime
ends.
How about we just say:
Type allocated and destroyed on the C side, but owned by Rust.
> +///
> +/// It allows such types to define their own custom destructor function to be called when a
> +/// Rust-owned reference is dropped.
We shouldn't call this a reference. Also we should start the first
paragraph with how this trait enables the usage of `Owned<Self>`.
> +///
> +/// This is usually implemented by wrappers to existing structures on the C side of the code.
> +///
> +/// Note: Implementing this trait allows types to be wrapped in an [`Owned<Self>`]. This does not
> +/// provide reference counting but represents a unique, owned reference. If reference counting is
> +/// required [`AlwaysRefCounted`](crate::types::AlwaysRefCounted) should be implemented which allows
> +/// types to be wrapped in an [`ARef<Self>`](crate::types::ARef).
I think this is more confusing than helpful. We should mention
`AlwaysRefCounted`, but the phrasing needs to be changed. Something
along the lines: if you need reference counting, implement
`AlwaysRefCounted` instead.
> +///
> +/// # Safety
> +///
> +/// Implementers must ensure that:
> +/// - The [`release()`](Ownable::release) method leaves the underlying object in a state which the
> +/// kernel expects after ownership has been relinquished (i.e. no dangling references in the
> +/// kernel is case it frees the object, etc.).
This invariant sounds weird to me. It's vague "a state which the kernel
expects" and difficult to use (what needs this invariant?).
> +pub unsafe trait Ownable {
> + /// Releases the object (frees it or returns it to foreign ownership).
Let's remove the part in parenthesis.
> + ///
> + /// # Safety
> + ///
> + /// Callers must ensure that:
> + /// - `this` points to a valid `Self`.
> + /// - The object is no longer referenced after this call.
s/The object/`*this`/
s/referenced/used/
> + unsafe fn release(this: NonNull<Self>);
> +}
> +
> +/// Type where [`Owned<Self>`] derefs to `&mut Self`.
How about:
Type allowing mutable access via [`Owned<Self>`].
> +///
> +/// # Safety
> +///
> +/// Implementers must ensure that access to a `&mut T` is safe, implying that:
s/T/Self/
> +/// - It is safe to call [`core::mem::swap`] on the [`Ownable`]. This excludes pinned types
> +/// (i.e. most kernel types).
Can't we implicitly pin `Owned`?
> +/// - The kernel will never access the underlying object (excluding internal mutability that follows
> +/// the usual rules) while Rust owns it.
> +pub unsafe trait OwnableMut: Ownable {}
> +
> +/// An owned reference to an ownable kernel object.
How about
An owned `T`.
> +///
> +/// The object is automatically freed or released when an instance of [`Owned`] is
> +/// dropped.
I don't think we need to say this, I always assume this for all Rust
types except they document otherwise (eg `ManuallyDrop`, `MaybeUninit`
and thus also `Opaque`.)
How about we provide some examples here?
> +///
> +/// # Invariants
> +///
> +/// The pointer stored in `ptr` can be considered owned by the [`Owned`] instance.
What exactly is "owned" supposed to mean? It depends on the concrete `T`
and that isn't well-defined (since it's a generic)...
Maybe we should give `Ownable` the task to document the exact ownership
semantics of `T`?
> +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`).
How does this amount to sending a `&mut T`?
I guess this also needs to be guaranteed by `Owned::from_raw`... ah the
list grows...
I'll try to come up with something to simplify this design a bit wrt the
safety docs.
> +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`).
Same here.
> +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:
> + /// - Ownership of the underlying object can be transferred to the `Owned<T>` (i.e. operations
> + /// which require ownership will be safe).
> + /// - No other Rust references to the underlying object exist. This implies that the underlying
> + /// object is not accessed through `ptr` anymore after the function call (at least until the
> + /// the `Owned<T>` is dropped).
> + /// - The C code follows the usual shared reference requirements. That is, the kernel will never
> + /// mutate or free the underlying object (excluding interior mutability that follows the usual
> + /// rules) while Rust owns it.
> + /// - In case `T` implements [`OwnableMut`] the previous requirement is extended from shared to
> + /// mutable reference requirements. That is, the kernel will not mutate or free the underlying
> + /// object and is okay with it being modified by Rust code.
> + pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
> + // INVARIANT: The safety requirements guarantee that the new instance now owns the
> + // reference.
This doesn't follow for me. Well the first issue is that the safety
invariant of `Self` isn't well-defined, so let's revisit this when that
is fixed.
---
Cheers,
Benno
> + 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) };
> + }
> +}
On 250702 1303, Benno Lossin wrote:
> On Wed Jun 18, 2025 at 2:27 PM CEST, Oliver Mangold wrote:
> > From: Asahi Lina <lina+kernel@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 and commit message
> > ]
> > Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
> > Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
> > ---
> > rust/kernel/types.rs | 7 +++
> > rust/kernel/types/ownable.rs | 134 +++++++++++++++++++++++++++++++++++++++++++
>
> I think we should name this file `owned.rs` instead. It's also what
> we'll have for `ARef` when that is moved to `sync/`.
>
> Also, I do wonder does this really belong into the `types` module? I
> feel like it's becoming our `utils` module and while it does fit, I
> think we should just make this a top level module. So
> `rust/kernel/owned.rs`. Thoughts?
I don't have much of an opinion on on that. But maybe refactoring types.rs
should be an independent task?
>
> > +
> > +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.
>
> This seems wrong, `var: Owned<T>` should life until `min(var, T)`, so
> whatever is earlier: until the user drops the `var` or `T`'s lifetime
> ends.
Yes, I guess that sounds sloppy.
> How about we just say:
>
> Type allocated and destroyed on the C side, but owned by Rust.
Would be okay with me.
> > +///
> > +/// It allows such types to define their own custom destructor function to be called when a
> > +/// Rust-owned reference is dropped.
>
> We shouldn't call this a reference. Also we should start the first
> paragraph with how this trait enables the usage of `Owned<Self>`.
>
> > +///
> > +/// This is usually implemented by wrappers to existing structures on the C side of the code.
> > +///
> > +/// Note: Implementing this trait allows types to be wrapped in an [`Owned<Self>`]. This does not
> > +/// provide reference counting but represents a unique, owned reference. If reference counting is
> > +/// required [`AlwaysRefCounted`](crate::types::AlwaysRefCounted) should be implemented which allows
> > +/// types to be wrapped in an [`ARef<Self>`](crate::types::ARef).
>
> I think this is more confusing than helpful. We should mention
> `AlwaysRefCounted`, but the phrasing needs to be changed. Something
> along the lines: if you need reference counting, implement
> `AlwaysRefCounted` instead.
I guess you have a point. The shortened version is probably good enough.
> > +///
> > +/// # Safety
> > +///
> > +/// Implementers must ensure that:
> > +/// - The [`release()`](Ownable::release) method leaves the underlying object in a state which the
> > +/// kernel expects after ownership has been relinquished (i.e. no dangling references in the
> > +/// kernel is case it frees the object, etc.).
>
> This invariant sounds weird to me. It's vague "a state which the kernel
> expects" and difficult to use (what needs this invariant?).
>
> > +pub unsafe trait Ownable {
> > + /// Releases the object (frees it or returns it to foreign ownership).
>
> Let's remove the part in parenthesis.
>
> > + ///
> > + /// # Safety
> > + ///
> > + /// Callers must ensure that:
> > + /// - `this` points to a valid `Self`.
> > + /// - The object is no longer referenced after this call.
>
> s/The object/`*this`/
>
> s/referenced/used/
>
> > + unsafe fn release(this: NonNull<Self>);
> > +}
> > +
> > +/// Type where [`Owned<Self>`] derefs to `&mut Self`.
>
> How about:
>
> Type allowing mutable access via [`Owned<Self>`].
>
> > +///
> > +/// # Safety
> > +///
> > +/// Implementers must ensure that access to a `&mut T` is safe, implying that:
>
> s/T/Self/
Okay with all of the above.
> > +/// - It is safe to call [`core::mem::swap`] on the [`Ownable`]. This excludes pinned types
> > +/// (i.e. most kernel types).
>
> Can't we implicitly pin `Owned`?
I have been thinking about that. But this would mean that the blanket
implementation for `Deref` would conflict with the one for `OwnableMut`.
> > +/// - The kernel will never access the underlying object (excluding internal mutability that follows
> > +/// the usual rules) while Rust owns it.
> > +pub unsafe trait OwnableMut: Ownable {}
> > +
> > +/// An owned reference to an ownable kernel object.
>
> How about
>
> An owned `T`.
A wrapper around `T`.
maybe?
> > +///
> > +/// The object is automatically freed or released when an instance of [`Owned`] is
> > +/// dropped.
>
> I don't think we need to say this, I always assume this for all Rust
> types except they document otherwise (eg `ManuallyDrop`, `MaybeUninit`
> and thus also `Opaque`.)
Hmm, it is an important feature of the wrapper that it turns the `*Ownable`
into an object that is automatically dropped. So shouldn't that be
mentioned here?
> How about we provide some examples here?
>
> > +///
> > +/// # Invariants
> > +///
> > +/// The pointer stored in `ptr` can be considered owned by the [`Owned`] instance.
>
> What exactly is "owned" supposed to mean? It depends on the concrete `T`
> and that isn't well-defined (since it's a generic)...
"owned" means that access to the `T` is exclusive through the `Owned<T>`,
so normal Rust semantics can be applied.
> Maybe we should give `Ownable` the task to document the exact ownership
> semantics of `T`?
>
> > +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`).
>
> How does this amount to sending a `&mut T`?
Good point. That documentation hasn't been updated since `&mut T` access
has been split out into `OwnableMut`. Not sure how to phrase it now.
> I guess this also needs to be guaranteed by `Owned::from_raw`... ah the
> list grows...
>
> I'll try to come up with something to simplify this design a bit wrt the
> safety docs.
>
> > +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`).
>
> Same here.
>
> > +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:
> > + /// - Ownership of the underlying object can be transferred to the `Owned<T>` (i.e. operations
> > + /// which require ownership will be safe).
> > + /// - No other Rust references to the underlying object exist. This implies that the underlying
> > + /// object is not accessed through `ptr` anymore after the function call (at least until the
> > + /// the `Owned<T>` is dropped).
> > + /// - The C code follows the usual shared reference requirements. That is, the kernel will never
> > + /// mutate or free the underlying object (excluding interior mutability that follows the usual
> > + /// rules) while Rust owns it.
> > + /// - In case `T` implements [`OwnableMut`] the previous requirement is extended from shared to
> > + /// mutable reference requirements. That is, the kernel will not mutate or free the underlying
> > + /// object and is okay with it being modified by Rust code.
> > + pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
> > + // INVARIANT: The safety requirements guarantee that the new instance now owns the
> > + // reference.
>
> This doesn't follow for me. Well the first issue is that the safety
> invariant of `Self` isn't well-defined, so let's revisit this when that
> is fixed.
I guess it follows from:
> > + /// It takes over ownership of the underlying object.
Okay, sure, it not in the safety requirement.
Best,
Oliver
On Mon, Jul 7, 2025 at 8:58 AM Oliver Mangold <oliver.mangold@pm.me> wrote: > > I don't have much of an opinion on on that. But maybe refactoring types.rs > should be an independent task? Yeah; however, putting things in the final place now (even if we don't move the rest, which as you say, is independent) avoids a move later on. Moves are themselves easy, but they can end up being painful to coordinate between trees / across kernel cycles, and they also make things harder for backports. So, if possible, it is best to try to add them in the right place, and move the rest later. Thanks! Cheers, Miguel
On Mon Jul 7, 2025 at 8:58 AM CEST, Oliver Mangold wrote:
> On 250702 1303, Benno Lossin wrote:
>> On Wed Jun 18, 2025 at 2:27 PM CEST, Oliver Mangold wrote:
>> > From: Asahi Lina <lina+kernel@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 and commit message
>> > ]
>> > Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
>> > Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
>> > ---
>> > rust/kernel/types.rs | 7 +++
>> > rust/kernel/types/ownable.rs | 134 +++++++++++++++++++++++++++++++++++++++++++
>>
>> I think we should name this file `owned.rs` instead. It's also what
>> we'll have for `ARef` when that is moved to `sync/`.
>>
>> Also, I do wonder does this really belong into the `types` module? I
>> feel like it's becoming our `utils` module and while it does fit, I
>> think we should just make this a top level module. So
>> `rust/kernel/owned.rs`. Thoughts?
>
> I don't have much of an opinion on on that. But maybe refactoring types.rs
> should be an independent task?
But you're adding the new file there? Just add it under
`rust/kernel/owned.rs` instead.
>> > +/// - It is safe to call [`core::mem::swap`] on the [`Ownable`]. This excludes pinned types
>> > +/// (i.e. most kernel types).
>>
>> Can't we implicitly pin `Owned`?
>
> I have been thinking about that. But this would mean that the blanket
> implementation for `Deref` would conflict with the one for `OwnableMut`.
Yeah we could not implement `DerefMut` in that case (or only for `T:
Unpin`).
>> > +/// - The kernel will never access the underlying object (excluding internal mutability that follows
>> > +/// the usual rules) while Rust owns it.
>> > +pub unsafe trait OwnableMut: Ownable {}
>> > +
>> > +/// An owned reference to an ownable kernel object.
>>
>> How about
>>
>> An owned `T`.
>
> A wrapper around `T`.
>
> maybe?
"wrapper" seems wrong, since a wrapper in my mind is a newtype. So it
would be `struct Owned(T)` which is wrong. The `T` is stored elsewhere.
>> > +///
>> > +/// The object is automatically freed or released when an instance of [`Owned`] is
>> > +/// dropped.
>>
>> I don't think we need to say this, I always assume this for all Rust
>> types except they document otherwise (eg `ManuallyDrop`, `MaybeUninit`
>> and thus also `Opaque`.)
>
> Hmm, it is an important feature of the wrapper that it turns the `*Ownable`
> into an object that is automatically dropped. So shouldn't that be
> mentioned here?
I would expect that that happens, but sure we can leave it here.
>> How about we provide some examples here?
>>
>> > +///
>> > +/// # Invariants
>> > +///
>> > +/// The pointer stored in `ptr` can be considered owned by the [`Owned`] instance.
>>
>> What exactly is "owned" supposed to mean? It depends on the concrete `T`
>> and that isn't well-defined (since it's a generic)...
>
> "owned" means that access to the `T` is exclusive through the `Owned<T>`,
> so normal Rust semantics can be applied.
Okay, in that case just say that `ptr` has exclusive access.
>> Maybe we should give `Ownable` the task to document the exact ownership
>> semantics of `T`?
>>
>> > +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`).
>>
>> How does this amount to sending a `&mut T`?
>
> Good point. That documentation hasn't been updated since `&mut T` access
> has been split out into `OwnableMut`. Not sure how to phrase it now.
I'm also unsure, also for the correct trait bounds on this impl.
---
Cheers,
Benno
>> I guess this also needs to be guaranteed by `Owned::from_raw`... ah the
>> list grows...
>>
>> I'll try to come up with something to simplify this design a bit wrt the
>> safety docs.
>>
>> > +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`).
>>
>> Same here.
>>
>> > +unsafe impl<T: Ownable + Sync> Sync for Owned<T> {}
On 250707 1123, Benno Lossin wrote:
> On Mon Jul 7, 2025 at 8:58 AM CEST, Oliver Mangold wrote:
> > On 250702 1303, Benno Lossin wrote:
> >> On Wed Jun 18, 2025 at 2:27 PM CEST, Oliver Mangold wrote:
> >> > From: Asahi Lina <lina+kernel@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 and commit message
> >> > ]
> >> > Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
> >> > Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
> >> > ---
> >> > rust/kernel/types.rs | 7 +++
> >> > rust/kernel/types/ownable.rs | 134 +++++++++++++++++++++++++++++++++++++++++++
> >>
> >> I think we should name this file `owned.rs` instead. It's also what
> >> we'll have for `ARef` when that is moved to `sync/`.
> >>
> >> Also, I do wonder does this really belong into the `types` module? I
> >> feel like it's becoming our `utils` module and while it does fit, I
> >> think we should just make this a top level module. So
> >> `rust/kernel/owned.rs`. Thoughts?
> >
> > I don't have much of an opinion on on that. But maybe refactoring types.rs
> > should be an independent task?
>
> But you're adding the new file there? Just add it under
> `rust/kernel/owned.rs` instead.
To be honest, I don't really mind.
Note, though, that I already moved it from types.rs to types/ownable.rs on
request. It seems to me different people here have different ideas where it
should be placed. I feel now, that it would make sense to come to an
agreement between the interested parties about where it should finally be
placed, before I move it again. Could I ask that we settle that question
once and for all before my next revision?
> >> > +/// - It is safe to call [`core::mem::swap`] on the [`Ownable`]. This excludes pinned types
> >> > +/// (i.e. most kernel types).
> >>
> >> Can't we implicitly pin `Owned`?
> >
> > I have been thinking about that. But this would mean that the blanket
> > implementation for `Deref` would conflict with the one for `OwnableMut`.
>
> Yeah we could not implement `DerefMut` in that case (or only for `T:
> Unpin`).
>
> >> > +/// - The kernel will never access the underlying object (excluding internal mutability that follows
> >> > +/// the usual rules) while Rust owns it.
> >> > +pub unsafe trait OwnableMut: Ownable {}
> >> > +
> >> > +/// An owned reference to an ownable kernel object.
> >>
> >> How about
> >>
> >> An owned `T`.
> >
> > A wrapper around `T`.
> >
> > maybe?
>
> "wrapper" seems wrong, since a wrapper in my mind is a newtype. So it
> would be `struct Owned(T)` which is wrong. The `T` is stored elsewhere.
>
> >> > +///
> >> > +/// The object is automatically freed or released when an instance of [`Owned`] is
> >> > +/// dropped.
> >>
> >> I don't think we need to say this, I always assume this for all Rust
> >> types except they document otherwise (eg `ManuallyDrop`, `MaybeUninit`
> >> and thus also `Opaque`.)
> >
> > Hmm, it is an important feature of the wrapper that it turns the `*Ownable`
> > into an object that is automatically dropped. So shouldn't that be
> > mentioned here?
>
> I would expect that that happens, but sure we can leave it here.
>
> >> How about we provide some examples here?
> >>
> >> > +///
> >> > +/// # Invariants
> >> > +///
> >> > +/// The pointer stored in `ptr` can be considered owned by the [`Owned`] instance.
> >>
> >> What exactly is "owned" supposed to mean? It depends on the concrete `T`
> >> and that isn't well-defined (since it's a generic)...
> >
> > "owned" means that access to the `T` is exclusive through the `Owned<T>`,
> > so normal Rust semantics can be applied.
>
> Okay, in that case just say that `ptr` has exclusive access.
Or, ehm, sorry, I forgot, ownership also implies that the allocation of the
underlying resource/object is now under the responsibility of the owner,
i.e. the owner should free it at the appropriate time.
In short, just the standard meaning of ownership in Rust.
https://doc.rust-lang.org/book/ch04-01-what-is-ownership.html
> >> Maybe we should give `Ownable` the task to document the exact ownership
> >> semantics of `T`?
> >>
> >> > +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`).
> >>
> >> How does this amount to sending a `&mut T`?
> >
> > Good point. That documentation hasn't been updated since `&mut T` access
> > has been split out into `OwnableMut`. Not sure how to phrase it now.
>
> I'm also unsure, also for the correct trait bounds on this impl.
>
> ---
> Cheers,
> Benno
>
> >> I guess this also needs to be guaranteed by `Owned::from_raw`... ah the
> >> list grows...
> >>
> >> I'll try to come up with something to simplify this design a bit wrt the
> >> safety docs.
> >>
> >> > +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`).
> >>
> >> Same here.
> >>
> >> > +unsafe impl<T: Ownable + Sync> Sync for Owned<T> {}
On Tue Jul 8, 2025 at 11:56 AM CEST, Oliver Mangold wrote:
> On 250707 1123, Benno Lossin wrote:
>> On Mon Jul 7, 2025 at 8:58 AM CEST, Oliver Mangold wrote:
>> > On 250702 1303, Benno Lossin wrote:
>> >> On Wed Jun 18, 2025 at 2:27 PM CEST, Oliver Mangold wrote:
>> >> > +///
>> >> > +/// # Invariants
>> >> > +///
>> >> > +/// The pointer stored in `ptr` can be considered owned by the [`Owned`] instance.
>> >>
>> >> What exactly is "owned" supposed to mean? It depends on the concrete `T`
>> >> and that isn't well-defined (since it's a generic)...
>> >
>> > "owned" means that access to the `T` is exclusive through the `Owned<T>`,
>> > so normal Rust semantics can be applied.
>>
>> Okay, in that case just say that `ptr` has exclusive access.
>
> Or, ehm, sorry, I forgot, ownership also implies that the allocation of the
> underlying resource/object is now under the responsibility of the owner,
> i.e. the owner should free it at the appropriate time.
>
> In short, just the standard meaning of ownership in Rust.
>
> https://doc.rust-lang.org/book/ch04-01-what-is-ownership.html
Okay that's good to hear. I think what tripped me up the most was the
"can be considered" wording. Let's just say:
/// # Invariants
///
/// - `ptr` is valid,
/// - `*ptr` is owned by `self`,
/// - `ptr` is an "owning pointer" according to the [`Ownable`] implementation for `T`.
And then on `Ownable` we add:
/// # Invariants
///
/// An implementer of this trait needs to define which pointers can be supplied to
/// [`Self::release`]. These pointers are called "owning pointers".
This should be as general as possible and still give us exactly the
guarantees that we need to implement `Owned`.
`Owned::from_raw` can then require that the pointer is an owning
pointer (& it's valid) and that the caller yields ownership to
`from_raw`.
---
Cheers,
Benno
On Tue, Jul 8, 2025 at 11:57 AM Oliver Mangold <oliver.mangold@pm.me> wrote: > > Note, though, that I already moved it from types.rs to types/ownable.rs on > request. It seems to me different people here have different ideas where it > should be placed. I feel now, that it would make sense to come to an > agreement between the interested parties about where it should finally be > placed, before I move it again. Could I ask that we settle that question > once and for all before my next revision? Yeah, if there is a disagreement with something said previously, then it should be resolved before starting to ping-pong between approaches with more and more patch versions. Reviewers can forget or they may not have read an earlier comment, but you did the right thing mentioning there is such a conflict in opinions. Cheers, Miguel
"Miguel Ojeda" <miguel.ojeda.sandonis@gmail.com> writes: > On Tue, Jul 8, 2025 at 11:57 AM Oliver Mangold <oliver.mangold@pmme> wrote: >> >> Note, though, that I already moved it from types.rs to types/ownable.rs on >> request. It seems to me different people here have different ideas where it >> should be placed. I feel now, that it would make sense to come to an >> agreement between the interested parties about where it should finally be >> placed, before I move it again. Could I ask that we settle that question >> once and for all before my next revision? > > Yeah, if there is a disagreement with something said previously, then > it should be resolved before starting to ping-pong between approaches > with more and more patch versions. Reviewers can forget or they may > not have read an earlier comment, but you did the right thing > mentioning there is such a conflict in opinions. Didn't we start with this placed in the top level kernel crate? Anyway, I think it should go next to `ARef` and `AlwaysRefCounted`, which is in `types`. I think I asked to have it moved to a submodule at some point to keep types.rs manageable. If we place this one to top level, we should move `ARef` and `AlwaysRefCounted` as well. Best regards, Andreas Hindborg
On Tue Jul 8, 2025 at 3:22 PM CEST, Andreas Hindborg wrote: > "Miguel Ojeda" <miguel.ojeda.sandonis@gmail.com> writes: >> On Tue, Jul 8, 2025 at 11:57 AM Oliver Mangold <oliver.mangold@pmme> wrote: >>> Note, though, that I already moved it from types.rs to types/ownable.rs on >>> request. It seems to me different people here have different ideas where it >>> should be placed. I feel now, that it would make sense to come to an >>> agreement between the interested parties about where it should finally be >>> placed, before I move it again. Could I ask that we settle that question >>> once and for all before my next revision? >> >> Yeah, if there is a disagreement with something said previously, then >> it should be resolved before starting to ping-pong between approaches >> with more and more patch versions. Reviewers can forget or they may >> not have read an earlier comment, but you did the right thing >> mentioning there is such a conflict in opinions. > > Didn't we start with this placed in the top level kernel crate? > > Anyway, I think it should go next to `ARef` and `AlwaysRefCounted`, > which is in `types`. I think I asked to have it moved to a submodule at > some point to keep types.rs manageable. I don't think we should have the `types` module at all, see my other response. > If we place this one to top level, we should move `ARef` and > `AlwaysRefCounted` as well. I already created an issue [1] & a patch was sent [2]. [1]: https://github.com/Rust-for-Linux/linux/issues/1173 [2]: https://lore.kernel.org/rust-for-linux/20250623192530.266103-1-shankari.ak0208@gmail.com/ --- Cheers, Benno
On Tue Jul 8, 2025 at 12:16 PM CEST, Miguel Ojeda wrote: > On Tue, Jul 8, 2025 at 11:57 AM Oliver Mangold <oliver.mangold@pm.me> wrote: >> >> Note, though, that I already moved it from types.rs to types/ownable.rs on >> request. It seems to me different people here have different ideas where it >> should be placed. I feel now, that it would make sense to come to an >> agreement between the interested parties about where it should finally be >> placed, before I move it again. Could I ask that we settle that question >> once and for all before my next revision? > > Yeah, if there is a disagreement with something said previously, then > it should be resolved before starting to ping-pong between approaches > with more and more patch versions. Reviewers can forget or they may > not have read an earlier comment, but you did the right thing > mentioning there is such a conflict in opinions. Yeah, I checked and that was Andreas on v9. @Andreas what do you think? I think we should just get rid of `types.rs` and split it into: * `opaque.rs` * `foreign.rs` * `scope_guard.rs` (this might need a better name) `Either` can just be removed entirely, `AlwaysRefcounted` & `ARef` should be in the `sync` module (I already created an issue for this) as well as `NotThreadSafe` (or we could create a `marker` module for that). Thoughts? --- Cheers, Benno
"Benno Lossin" <lossin@kernel.org> writes: > On Tue Jul 8, 2025 at 12:16 PM CEST, Miguel Ojeda wrote: >> On Tue, Jul 8, 2025 at 11:57 AM Oliver Mangold <oliver.mangold@pm.me> wrote: >>> >>> Note, though, that I already moved it from types.rs to types/ownable.rs on >>> request. It seems to me different people here have different ideas where it >>> should be placed. I feel now, that it would make sense to come to an >>> agreement between the interested parties about where it should finally be >>> placed, before I move it again. Could I ask that we settle that question >>> once and for all before my next revision? >> >> Yeah, if there is a disagreement with something said previously, then >> it should be resolved before starting to ping-pong between approaches >> with more and more patch versions. Reviewers can forget or they may >> not have read an earlier comment, but you did the right thing >> mentioning there is such a conflict in opinions. > > Yeah, I checked and that was Andreas on v9. @Andreas what do you think? > > I think we should just get rid of `types.rs` and split it into: > > * `opaque.rs` > * `foreign.rs` > * `scope_guard.rs` (this might need a better name) > > `Either` can just be removed entirely, `AlwaysRefcounted` & `ARef` > should be in the `sync` module (I already created an issue for this) as > well as `NotThreadSafe` (or we could create a `marker` module for that). > Thoughts? Sounds good. I just wanted to prevent us from cramming everything into types.rs. But we should probably move `Owned` into `sync` with `ARef` et. al., right? Best regards, Andreas Hindborg
On Tue Jul 8, 2025 at 8:30 PM CEST, Andreas Hindborg wrote: > "Benno Lossin" <lossin@kernel.org> writes: > >> On Tue Jul 8, 2025 at 12:16 PM CEST, Miguel Ojeda wrote: >>> On Tue, Jul 8, 2025 at 11:57 AM Oliver Mangold <oliver.mangold@pm.me> wrote: >>>> >>>> Note, though, that I already moved it from types.rs to types/ownable.rs on >>>> request. It seems to me different people here have different ideas where it >>>> should be placed. I feel now, that it would make sense to come to an >>>> agreement between the interested parties about where it should finally be >>>> placed, before I move it again. Could I ask that we settle that question >>>> once and for all before my next revision? >>> >>> Yeah, if there is a disagreement with something said previously, then >>> it should be resolved before starting to ping-pong between approaches >>> with more and more patch versions. Reviewers can forget or they may >>> not have read an earlier comment, but you did the right thing >>> mentioning there is such a conflict in opinions. >> >> Yeah, I checked and that was Andreas on v9. @Andreas what do you think? >> >> I think we should just get rid of `types.rs` and split it into: >> >> * `opaque.rs` >> * `foreign.rs` >> * `scope_guard.rs` (this might need a better name) >> >> `Either` can just be removed entirely, `AlwaysRefcounted` & `ARef` >> should be in the `sync` module (I already created an issue for this) as >> well as `NotThreadSafe` (or we could create a `marker` module for that). >> Thoughts? > > Sounds good. I just wanted to prevent us from cramming everything into > types.rs. Yeah me too :) > But we should probably move `Owned` into `sync` with `ARef` et. al., > right? I don't think it fits into `sync`... It's more like a `Box` and could very well store something that is `!Send` & `!Sync`. --- Cheers, Benno
"Benno Lossin" <lossin@kernel.org> writes: > On Tue Jul 8, 2025 at 8:30 PM CEST, Andreas Hindborg wrote: >> "Benno Lossin" <lossin@kernel.org> writes: >> >>> On Tue Jul 8, 2025 at 12:16 PM CEST, Miguel Ojeda wrote: >>>> On Tue, Jul 8, 2025 at 11:57 AM Oliver Mangold <oliver.mangold@pm.me> wrote: >>>>> >>>>> Note, though, that I already moved it from types.rs to types/ownable.rs on >>>>> request. It seems to me different people here have different ideas where it >>>>> should be placed. I feel now, that it would make sense to come to an >>>>> agreement between the interested parties about where it should finally be >>>>> placed, before I move it again. Could I ask that we settle that question >>>>> once and for all before my next revision? >>>> >>>> Yeah, if there is a disagreement with something said previously, then >>>> it should be resolved before starting to ping-pong between approaches >>>> with more and more patch versions. Reviewers can forget or they may >>>> not have read an earlier comment, but you did the right thing >>>> mentioning there is such a conflict in opinions. >>> >>> Yeah, I checked and that was Andreas on v9. @Andreas what do you think? >>> >>> I think we should just get rid of `types.rs` and split it into: >>> >>> * `opaque.rs` >>> * `foreign.rs` >>> * `scope_guard.rs` (this might need a better name) >>> >>> `Either` can just be removed entirely, `AlwaysRefcounted` & `ARef` >>> should be in the `sync` module (I already created an issue for this) as >>> well as `NotThreadSafe` (or we could create a `marker` module for that). >>> Thoughts? >> >> Sounds good. I just wanted to prevent us from cramming everything into >> types.rs. > > Yeah me too :) > >> But we should probably move `Owned` into `sync` with `ARef` et. al., >> right? > > I don't think it fits into `sync`... It's more like a `Box` and could > very well store something that is `!Send` & `!Sync`. Good point. My primary use case is `OwnableRefCounted`. Where do you want tput that? It is strongly tied to `Ownable` and `Owned`, but revolving around refcounts. Best regards, Andreas Hindborg
On Wed Jul 9, 2025 at 10:53 AM CEST, Andreas Hindborg wrote: > "Benno Lossin" <lossin@kernel.org> writes: > >> On Tue Jul 8, 2025 at 8:30 PM CEST, Andreas Hindborg wrote: >>> "Benno Lossin" <lossin@kernel.org> writes: >>> >>>> On Tue Jul 8, 2025 at 12:16 PM CEST, Miguel Ojeda wrote: >>>>> On Tue, Jul 8, 2025 at 11:57 AM Oliver Mangold <oliver.mangold@pm.me> wrote: >>>>>> >>>>>> Note, though, that I already moved it from types.rs to types/ownable.rs on >>>>>> request. It seems to me different people here have different ideas where it >>>>>> should be placed. I feel now, that it would make sense to come to an >>>>>> agreement between the interested parties about where it should finally be >>>>>> placed, before I move it again. Could I ask that we settle that question >>>>>> once and for all before my next revision? >>>>> >>>>> Yeah, if there is a disagreement with something said previously, then >>>>> it should be resolved before starting to ping-pong between approaches >>>>> with more and more patch versions. Reviewers can forget or they may >>>>> not have read an earlier comment, but you did the right thing >>>>> mentioning there is such a conflict in opinions. >>>> >>>> Yeah, I checked and that was Andreas on v9. @Andreas what do you think? >>>> >>>> I think we should just get rid of `types.rs` and split it into: >>>> >>>> * `opaque.rs` >>>> * `foreign.rs` >>>> * `scope_guard.rs` (this might need a better name) >>>> >>>> `Either` can just be removed entirely, `AlwaysRefcounted` & `ARef` >>>> should be in the `sync` module (I already created an issue for this) as >>>> well as `NotThreadSafe` (or we could create a `marker` module for that). >>>> Thoughts? >>> >>> Sounds good. I just wanted to prevent us from cramming everything into >>> types.rs. >> >> Yeah me too :) >> >>> But we should probably move `Owned` into `sync` with `ARef` et. al., >>> right? >> >> I don't think it fits into `sync`... It's more like a `Box` and could >> very well store something that is `!Send` & `!Sync`. > > Good point. My primary use case is `OwnableRefCounted`. Where do you > want tput that? It is strongly tied to `Ownable` and `Owned`, but > revolving around refcounts. Yeah... Let's just put it in owned.rs too. --- Cheers, Benno
© 2016 - 2025 Red Hat, Inc.