rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] rust: adding UniqueRefCounted and UniqueRef types
  2025-02-28 14:43 [PATCH] " Oliver Mangold
@ 2025-02-28 18:01 ` Oliver Mangold
  2025-02-28 18:09   ` Boqun Feng
                     ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Oliver Mangold @ 2025-02-28 18:01 UTC (permalink / raw)
  To: Andreas Hindborg
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
	linux-kernel, rust-for-linux

For usage with block-mq, a variant of ARef
which is guaranteed to be unique would be useful.
As chances are it is useful in general, This implements it
as kernel::types::UniqueRef.
The difference between ARef and UniqueRef
is basically the same as between Arc and UniqueArc.

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

diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index 55ddd50e8aaa..72a973d9e1c7 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -543,6 +543,12 @@ fn from(b: &T) -> Self {
     }
 }
 
+impl<T: UniqueRefCounted> From<UniqueRef<T>> for ARef<T> {
+    fn from(b: UniqueRef<T>) -> Self {
+        UniqueRefCounted::unique_to_shared(b)
+    }
+}
+
 impl<T: AlwaysRefCounted> Drop for ARef<T> {
     fn drop(&mut self) {
         // SAFETY: The type invariants guarantee that the `ARef` owns the reference we're about to
@@ -551,6 +557,153 @@ fn drop(&mut self) {
     }
 }
 
+/// Types that are [`AlwaysRefCounted`] and can be safely converted to an [`UniqueRef`]
+///
+/// # Safety
+///
+/// Implementers must ensure that the methods of the trait
+/// change the reference count of the underlying object such that:
+/// - the uniqueness invariant is upheld, i.e. it is not possible
+///   to obtain another reference by any means (other than through the [`UniqueRef`])
+///   until the [`UniqueRef`] is dropped or converted to an [`ARef`].
+/// - [`UniqueRefCounted::dec_ref`] correctly frees the underlying object.
+/// - [`UniqueRefCounted::unique_to_shared`] set the reference count to the value
+/// - that the returned [`ARef`] expects for an object with a single reference
+///   in existence.
+pub unsafe trait UniqueRefCounted: AlwaysRefCounted + Sized {
+    /// Checks if the [`ARef`] is unique and convert it
+    /// to an [`UniqueRef`] it that is that case.
+    /// Otherwise it returns again an [`ARef`] to the same
+    /// underlying object.
+    fn try_shared_to_unique(this: ARef<Self>) -> Result<UniqueRef<Self>, ARef<Self>>;
+    /// Converts the [`UniqueRef`] into an [`ARef`].
+    fn unique_to_shared(this: UniqueRef<Self>) -> ARef<Self>;
+    /// Decrements the reference count on the object when the [`UniqueRef`] is dropped.
+    ///
+    /// Frees the object when the count reaches zero.
+    ///
+    /// It defaults to [`AlwaysRefCounted::dec_ref`],
+    /// but overriding it may be useful, e.g. in case of non-standard refcounting
+    /// schemes.
+    ///
+    /// # Safety
+    ///
+    /// The same safety constraints as for [`AlwaysRefCounted::dec_ref`] apply,
+    /// but as the reference is unique, it can be assumed that the function
+    /// will not be called twice. In case the default implementation is not
+    /// overridden, it has to be ensured that the call to [`AlwaysRefCounted::dec_ref`]
+    /// can be used for an [`UniqueRef`], too.
+    unsafe fn dec_ref(obj: NonNull<Self>) {
+        // SAFETY: correct by function safety requirements
+        unsafe { AlwaysRefCounted::dec_ref(obj) };
+    }
+}
+
+/// An unique, owned reference to an [`AlwaysRefCounted`] object.
+///
+/// It works the same ways as [`ARef`] but ensures that the reference is unique
+/// and thus can be dereferenced mutably.
+///
+/// # Invariants
+///
+/// - The pointer stored in `ptr` is non-null and valid for the lifetime of the [`UniqueRef`]
+///   instance. In particular, the [`UniqueRef`] instance owns an increment
+///   on the underlying object's reference count.
+/// - No other references to the underlying object exist while the [`UniqueRef`] is live.
+pub struct UniqueRef<T: UniqueRefCounted> {
+    ptr: NonNull<T>,
+    _p: PhantomData<T>,
+}
+
+// SAFETY: It is safe to send `UniqueRef<T>` to another thread
+// when the underlying `T` is `Sync` because
+// it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
+// `T` to be `Send` because any thread that has an `UniqueRef<T>` may ultimately access `T` using a
+// mutable reference, for example, when the reference count reaches zero and `T` is dropped.
+unsafe impl<T: UniqueRefCounted + Sync + Send> Send for UniqueRef<T> {}
+
+// SAFETY: It is safe to send `&UniqueRef<T>` to another thread when the underlying `T` is `Sync`
+// because it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally,
+// it needs `T` to be `Send` because any thread that has a `&UniqueRef<T>` may clone it and get an
+// `UniqueRef<T>` on that thread, so the thread may ultimately access `T`
+// using a mutable reference, for example, when the reference count reaches zero and `T` is dropped.
+unsafe impl<T: UniqueRefCounted + Sync + Send> Sync for UniqueRef<T> {}
+
+impl<T: UniqueRefCounted> UniqueRef<T> {
+    /// Creates a new instance of [`UniqueRef`].
+    ///
+    /// It takes over an increment of the reference count on the underlying object.
+    ///
+    /// # Safety
+    ///
+    /// Callers must ensure that the reference count is set to such a value
+    /// that a call to [`UniqueRefCounted::dec_ref`] will release the underlying object
+    /// in the way which is expected when the last reference is dropped.
+    /// Callers must not use the underlying object anymore --
+    /// it is only safe to do so via the newly created [`UniqueRef`].
+    pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
+        // INVARIANT: The safety requirements guarantee that the new instance now owns the
+        // increment on the refcount.
+        Self {
+            ptr,
+            _p: PhantomData,
+        }
+    }
+
+    /// Consumes the [`UniqueRef`], returning a raw pointer.
+    ///
+    /// This function does not change the refcount. After calling this function, the caller is
+    /// responsible for the refcount previously managed by the [`UniqueRef`].
+    pub fn into_raw(me: Self) -> NonNull<T> {
+        ManuallyDrop::new(me).ptr
+    }
+}
+
+impl<T: UniqueRefCounted> Deref for UniqueRef<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: UniqueRefCounted> DerefMut for UniqueRef<T> {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        // SAFETY: The type invariants guarantee that the object is valid.
+        unsafe { self.ptr.as_mut() }
+    }
+}
+
+impl<T: UniqueRefCounted> From<&T> for UniqueRef<T> {
+    /// Converts the [`UniqueRef`] into an [`ARef`]
+    /// by calling [`UniqueRefCounted::unique_to_shared`] on it.
+    fn from(b: &T) -> Self {
+        b.inc_ref();
+        // SAFETY: We just incremented the refcount above.
+        unsafe { Self::from_raw(NonNull::from(b)) }
+    }
+}
+
+impl<T: UniqueRefCounted> TryFrom<ARef<T>> for UniqueRef<T> {
+    type Error = ARef<T>;
+    /// Tries to convert the [`ARef`] to an [`UniqueRef`]
+    /// by calling [`UniqueRefCounted::try_shared_to_unique`].
+    /// In case the [`ARef`] is not unique it returns again an [`ARef`] to the same
+    /// underlying object.
+    fn try_from(b: ARef<T>) -> Result<UniqueRef<T>, Self::Error> {
+        UniqueRefCounted::try_shared_to_unique(b)
+    }
+}
+
+impl<T: UniqueRefCounted> Drop for UniqueRef<T> {
+    fn drop(&mut self) {
+        // SAFETY: The type invariants guarantee that the [`UniqueRef`] owns the reference
+        // we're about to decrement.
+        unsafe { UniqueRefCounted::dec_ref(self.ptr) };
+    }
+}
+
 /// A sum type that always holds either a value of type `L` or `R`.
 ///
 /// # Examples
-- 
2.48.1

Best regards,

Oliver


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] rust: adding UniqueRefCounted and UniqueRef types
  2025-02-28 18:01 ` [PATCH v2] " Oliver Mangold
@ 2025-02-28 18:09   ` Boqun Feng
  2025-02-28 18:16     ` Boqun Feng
  2025-02-28 18:29   ` Andreas Hindborg
  2025-03-03 14:09   ` Andreas Hindborg
  2 siblings, 1 reply; 14+ messages in thread
From: Boqun Feng @ 2025-02-28 18:09 UTC (permalink / raw)
  To: Oliver Mangold
  Cc: Andreas Hindborg, Miguel Ojeda, Alex Gaynor, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
	linux-kernel, rust-for-linux

On Fri, Feb 28, 2025 at 06:01:55PM +0000, Oliver Mangold wrote:
> For usage with block-mq, a variant of ARef
> which is guaranteed to be unique would be useful.
> As chances are it is useful in general, This implements it
> as kernel::types::UniqueRef.
> The difference between ARef and UniqueRef
> is basically the same as between Arc and UniqueArc.
> 

Please add the "# Examples" section as I requested:

	https://lore.kernel.org/rust-for-linux/Z8HcHVtPiG-X6ujP@Mac.home/

I also would like to know why do you think it's OK to ignore my previous
comment, thanks!

Regards,
Boqun

> Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
> ---
>  rust/kernel/types.rs | 153 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 153 insertions(+)
> 
> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index 55ddd50e8aaa..72a973d9e1c7 100644
> --- a/rust/kernel/types.rs
> +++ b/rust/kernel/types.rs
> @@ -543,6 +543,12 @@ fn from(b: &T) -> Self {
>      }
>  }
>  
> +impl<T: UniqueRefCounted> From<UniqueRef<T>> for ARef<T> {
> +    fn from(b: UniqueRef<T>) -> Self {
> +        UniqueRefCounted::unique_to_shared(b)
> +    }
> +}
> +
>  impl<T: AlwaysRefCounted> Drop for ARef<T> {
>      fn drop(&mut self) {
>          // SAFETY: The type invariants guarantee that the `ARef` owns the reference we're about to
> @@ -551,6 +557,153 @@ fn drop(&mut self) {
>      }
>  }
>  
> +/// Types that are [`AlwaysRefCounted`] and can be safely converted to an [`UniqueRef`]
> +///
> +/// # Safety
> +///
> +/// Implementers must ensure that the methods of the trait
> +/// change the reference count of the underlying object such that:
> +/// - the uniqueness invariant is upheld, i.e. it is not possible
> +///   to obtain another reference by any means (other than through the [`UniqueRef`])
> +///   until the [`UniqueRef`] is dropped or converted to an [`ARef`].
> +/// - [`UniqueRefCounted::dec_ref`] correctly frees the underlying object.
> +/// - [`UniqueRefCounted::unique_to_shared`] set the reference count to the value
> +/// - that the returned [`ARef`] expects for an object with a single reference
> +///   in existence.
> +pub unsafe trait UniqueRefCounted: AlwaysRefCounted + Sized {
> +    /// Checks if the [`ARef`] is unique and convert it
> +    /// to an [`UniqueRef`] it that is that case.
> +    /// Otherwise it returns again an [`ARef`] to the same
> +    /// underlying object.
> +    fn try_shared_to_unique(this: ARef<Self>) -> Result<UniqueRef<Self>, ARef<Self>>;
> +    /// Converts the [`UniqueRef`] into an [`ARef`].
> +    fn unique_to_shared(this: UniqueRef<Self>) -> ARef<Self>;
> +    /// Decrements the reference count on the object when the [`UniqueRef`] is dropped.
> +    ///
> +    /// Frees the object when the count reaches zero.
> +    ///
> +    /// It defaults to [`AlwaysRefCounted::dec_ref`],
> +    /// but overriding it may be useful, e.g. in case of non-standard refcounting
> +    /// schemes.
> +    ///
> +    /// # Safety
> +    ///
> +    /// The same safety constraints as for [`AlwaysRefCounted::dec_ref`] apply,
> +    /// but as the reference is unique, it can be assumed that the function
> +    /// will not be called twice. In case the default implementation is not
> +    /// overridden, it has to be ensured that the call to [`AlwaysRefCounted::dec_ref`]
> +    /// can be used for an [`UniqueRef`], too.
> +    unsafe fn dec_ref(obj: NonNull<Self>) {
> +        // SAFETY: correct by function safety requirements
> +        unsafe { AlwaysRefCounted::dec_ref(obj) };
> +    }
> +}
> +
> +/// An unique, owned reference to an [`AlwaysRefCounted`] object.
> +///
> +/// It works the same ways as [`ARef`] but ensures that the reference is unique
> +/// and thus can be dereferenced mutably.
> +///
> +/// # Invariants
> +///
> +/// - The pointer stored in `ptr` is non-null and valid for the lifetime of the [`UniqueRef`]
> +///   instance. In particular, the [`UniqueRef`] instance owns an increment
> +///   on the underlying object's reference count.
> +/// - No other references to the underlying object exist while the [`UniqueRef`] is live.
> +pub struct UniqueRef<T: UniqueRefCounted> {
> +    ptr: NonNull<T>,
> +    _p: PhantomData<T>,
> +}
> +
> +// SAFETY: It is safe to send `UniqueRef<T>` to another thread
> +// when the underlying `T` is `Sync` because
> +// it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
> +// `T` to be `Send` because any thread that has an `UniqueRef<T>` may ultimately access `T` using a
> +// mutable reference, for example, when the reference count reaches zero and `T` is dropped.
> +unsafe impl<T: UniqueRefCounted + Sync + Send> Send for UniqueRef<T> {}
> +
> +// SAFETY: It is safe to send `&UniqueRef<T>` to another thread when the underlying `T` is `Sync`
> +// because it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally,
> +// it needs `T` to be `Send` because any thread that has a `&UniqueRef<T>` may clone it and get an
> +// `UniqueRef<T>` on that thread, so the thread may ultimately access `T`
> +// using a mutable reference, for example, when the reference count reaches zero and `T` is dropped.
> +unsafe impl<T: UniqueRefCounted + Sync + Send> Sync for UniqueRef<T> {}
> +
> +impl<T: UniqueRefCounted> UniqueRef<T> {
> +    /// Creates a new instance of [`UniqueRef`].
> +    ///
> +    /// It takes over an increment of the reference count on the underlying object.
> +    ///
> +    /// # Safety
> +    ///
> +    /// Callers must ensure that the reference count is set to such a value
> +    /// that a call to [`UniqueRefCounted::dec_ref`] will release the underlying object
> +    /// in the way which is expected when the last reference is dropped.
> +    /// Callers must not use the underlying object anymore --
> +    /// it is only safe to do so via the newly created [`UniqueRef`].
> +    pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
> +        // INVARIANT: The safety requirements guarantee that the new instance now owns the
> +        // increment on the refcount.
> +        Self {
> +            ptr,
> +            _p: PhantomData,
> +        }
> +    }
> +
> +    /// Consumes the [`UniqueRef`], returning a raw pointer.
> +    ///
> +    /// This function does not change the refcount. After calling this function, the caller is
> +    /// responsible for the refcount previously managed by the [`UniqueRef`].
> +    pub fn into_raw(me: Self) -> NonNull<T> {
> +        ManuallyDrop::new(me).ptr
> +    }
> +}
> +
> +impl<T: UniqueRefCounted> Deref for UniqueRef<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: UniqueRefCounted> DerefMut for UniqueRef<T> {
> +    fn deref_mut(&mut self) -> &mut Self::Target {
> +        // SAFETY: The type invariants guarantee that the object is valid.
> +        unsafe { self.ptr.as_mut() }
> +    }
> +}
> +
> +impl<T: UniqueRefCounted> From<&T> for UniqueRef<T> {
> +    /// Converts the [`UniqueRef`] into an [`ARef`]
> +    /// by calling [`UniqueRefCounted::unique_to_shared`] on it.
> +    fn from(b: &T) -> Self {
> +        b.inc_ref();
> +        // SAFETY: We just incremented the refcount above.
> +        unsafe { Self::from_raw(NonNull::from(b)) }
> +    }
> +}
> +
> +impl<T: UniqueRefCounted> TryFrom<ARef<T>> for UniqueRef<T> {
> +    type Error = ARef<T>;
> +    /// Tries to convert the [`ARef`] to an [`UniqueRef`]
> +    /// by calling [`UniqueRefCounted::try_shared_to_unique`].
> +    /// In case the [`ARef`] is not unique it returns again an [`ARef`] to the same
> +    /// underlying object.
> +    fn try_from(b: ARef<T>) -> Result<UniqueRef<T>, Self::Error> {
> +        UniqueRefCounted::try_shared_to_unique(b)
> +    }
> +}
> +
> +impl<T: UniqueRefCounted> Drop for UniqueRef<T> {
> +    fn drop(&mut self) {
> +        // SAFETY: The type invariants guarantee that the [`UniqueRef`] owns the reference
> +        // we're about to decrement.
> +        unsafe { UniqueRefCounted::dec_ref(self.ptr) };
> +    }
> +}
> +
>  /// A sum type that always holds either a value of type `L` or `R`.
>  ///
>  /// # Examples
> -- 
> 2.48.1
> 
> Best regards,
> 
> Oliver
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] rust: adding UniqueRefCounted and UniqueRef types
  2025-02-28 18:09   ` Boqun Feng
@ 2025-02-28 18:16     ` Boqun Feng
  0 siblings, 0 replies; 14+ messages in thread
From: Boqun Feng @ 2025-02-28 18:16 UTC (permalink / raw)
  To: Oliver Mangold
  Cc: Andreas Hindborg, Miguel Ojeda, Alex Gaynor, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
	linux-kernel, rust-for-linux

On Fri, Feb 28, 2025 at 10:09:11AM -0800, Boqun Feng wrote:
> On Fri, Feb 28, 2025 at 06:01:55PM +0000, Oliver Mangold wrote:
> > For usage with block-mq, a variant of ARef
> > which is guaranteed to be unique would be useful.
> > As chances are it is useful in general, This implements it
> > as kernel::types::UniqueRef.
> > The difference between ARef and UniqueRef
> > is basically the same as between Arc and UniqueArc.
> > 
> 
> Please add the "# Examples" section as I requested:
> 
> 	https://lore.kernel.org/rust-for-linux/Z8HcHVtPiG-X6ujP@Mac.home/
> 

More particularly, I gave an example here:

	https://lore.kernel.org/rust-for-linux/Z8HikJloVjoZXBbc@Mac.home/	

maybe I wasn't clear, sorry about that. I meant to say you need to
include the examples with the introduction of the API, and that provides
us the sample code and the tests at the first day of the new API.

Regards,
Boqun

> I also would like to know why do you think it's OK to ignore my previous
> comment, thanks!
> 
> Regards,
> Boqun
> 
> > Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
> > ---
> >  rust/kernel/types.rs | 153 +++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 153 insertions(+)
> > 
> > diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> > index 55ddd50e8aaa..72a973d9e1c7 100644
> > --- a/rust/kernel/types.rs
> > +++ b/rust/kernel/types.rs
> > @@ -543,6 +543,12 @@ fn from(b: &T) -> Self {
> >      }
> >  }
> >  
> > +impl<T: UniqueRefCounted> From<UniqueRef<T>> for ARef<T> {
> > +    fn from(b: UniqueRef<T>) -> Self {
> > +        UniqueRefCounted::unique_to_shared(b)
> > +    }
> > +}
> > +
> >  impl<T: AlwaysRefCounted> Drop for ARef<T> {
> >      fn drop(&mut self) {
> >          // SAFETY: The type invariants guarantee that the `ARef` owns the reference we're about to
> > @@ -551,6 +557,153 @@ fn drop(&mut self) {
> >      }
> >  }
> >  
> > +/// Types that are [`AlwaysRefCounted`] and can be safely converted to an [`UniqueRef`]
> > +///
> > +/// # Safety
> > +///
> > +/// Implementers must ensure that the methods of the trait
> > +/// change the reference count of the underlying object such that:
> > +/// - the uniqueness invariant is upheld, i.e. it is not possible
> > +///   to obtain another reference by any means (other than through the [`UniqueRef`])
> > +///   until the [`UniqueRef`] is dropped or converted to an [`ARef`].
> > +/// - [`UniqueRefCounted::dec_ref`] correctly frees the underlying object.
> > +/// - [`UniqueRefCounted::unique_to_shared`] set the reference count to the value
> > +/// - that the returned [`ARef`] expects for an object with a single reference
> > +///   in existence.
> > +pub unsafe trait UniqueRefCounted: AlwaysRefCounted + Sized {
> > +    /// Checks if the [`ARef`] is unique and convert it
> > +    /// to an [`UniqueRef`] it that is that case.
> > +    /// Otherwise it returns again an [`ARef`] to the same
> > +    /// underlying object.
> > +    fn try_shared_to_unique(this: ARef<Self>) -> Result<UniqueRef<Self>, ARef<Self>>;
> > +    /// Converts the [`UniqueRef`] into an [`ARef`].
> > +    fn unique_to_shared(this: UniqueRef<Self>) -> ARef<Self>;
> > +    /// Decrements the reference count on the object when the [`UniqueRef`] is dropped.
> > +    ///
> > +    /// Frees the object when the count reaches zero.
> > +    ///
> > +    /// It defaults to [`AlwaysRefCounted::dec_ref`],
> > +    /// but overriding it may be useful, e.g. in case of non-standard refcounting
> > +    /// schemes.
> > +    ///
> > +    /// # Safety
> > +    ///
> > +    /// The same safety constraints as for [`AlwaysRefCounted::dec_ref`] apply,
> > +    /// but as the reference is unique, it can be assumed that the function
> > +    /// will not be called twice. In case the default implementation is not
> > +    /// overridden, it has to be ensured that the call to [`AlwaysRefCounted::dec_ref`]
> > +    /// can be used for an [`UniqueRef`], too.
> > +    unsafe fn dec_ref(obj: NonNull<Self>) {
> > +        // SAFETY: correct by function safety requirements
> > +        unsafe { AlwaysRefCounted::dec_ref(obj) };
> > +    }
> > +}
> > +
> > +/// An unique, owned reference to an [`AlwaysRefCounted`] object.
> > +///
> > +/// It works the same ways as [`ARef`] but ensures that the reference is unique
> > +/// and thus can be dereferenced mutably.
> > +///
> > +/// # Invariants
> > +///
> > +/// - The pointer stored in `ptr` is non-null and valid for the lifetime of the [`UniqueRef`]
> > +///   instance. In particular, the [`UniqueRef`] instance owns an increment
> > +///   on the underlying object's reference count.
> > +/// - No other references to the underlying object exist while the [`UniqueRef`] is live.
> > +pub struct UniqueRef<T: UniqueRefCounted> {
> > +    ptr: NonNull<T>,
> > +    _p: PhantomData<T>,
> > +}
> > +
> > +// SAFETY: It is safe to send `UniqueRef<T>` to another thread
> > +// when the underlying `T` is `Sync` because
> > +// it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
> > +// `T` to be `Send` because any thread that has an `UniqueRef<T>` may ultimately access `T` using a
> > +// mutable reference, for example, when the reference count reaches zero and `T` is dropped.
> > +unsafe impl<T: UniqueRefCounted + Sync + Send> Send for UniqueRef<T> {}
> > +
> > +// SAFETY: It is safe to send `&UniqueRef<T>` to another thread when the underlying `T` is `Sync`
> > +// because it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally,
> > +// it needs `T` to be `Send` because any thread that has a `&UniqueRef<T>` may clone it and get an
> > +// `UniqueRef<T>` on that thread, so the thread may ultimately access `T`
> > +// using a mutable reference, for example, when the reference count reaches zero and `T` is dropped.
> > +unsafe impl<T: UniqueRefCounted + Sync + Send> Sync for UniqueRef<T> {}
> > +
> > +impl<T: UniqueRefCounted> UniqueRef<T> {
> > +    /// Creates a new instance of [`UniqueRef`].
> > +    ///
> > +    /// It takes over an increment of the reference count on the underlying object.
> > +    ///
> > +    /// # Safety
> > +    ///
> > +    /// Callers must ensure that the reference count is set to such a value
> > +    /// that a call to [`UniqueRefCounted::dec_ref`] will release the underlying object
> > +    /// in the way which is expected when the last reference is dropped.
> > +    /// Callers must not use the underlying object anymore --
> > +    /// it is only safe to do so via the newly created [`UniqueRef`].
> > +    pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
> > +        // INVARIANT: The safety requirements guarantee that the new instance now owns the
> > +        // increment on the refcount.
> > +        Self {
> > +            ptr,
> > +            _p: PhantomData,
> > +        }
> > +    }
> > +
> > +    /// Consumes the [`UniqueRef`], returning a raw pointer.
> > +    ///
> > +    /// This function does not change the refcount. After calling this function, the caller is
> > +    /// responsible for the refcount previously managed by the [`UniqueRef`].
> > +    pub fn into_raw(me: Self) -> NonNull<T> {
> > +        ManuallyDrop::new(me).ptr
> > +    }
> > +}
> > +
> > +impl<T: UniqueRefCounted> Deref for UniqueRef<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: UniqueRefCounted> DerefMut for UniqueRef<T> {
> > +    fn deref_mut(&mut self) -> &mut Self::Target {
> > +        // SAFETY: The type invariants guarantee that the object is valid.
> > +        unsafe { self.ptr.as_mut() }
> > +    }
> > +}
> > +
> > +impl<T: UniqueRefCounted> From<&T> for UniqueRef<T> {
> > +    /// Converts the [`UniqueRef`] into an [`ARef`]
> > +    /// by calling [`UniqueRefCounted::unique_to_shared`] on it.
> > +    fn from(b: &T) -> Self {
> > +        b.inc_ref();
> > +        // SAFETY: We just incremented the refcount above.
> > +        unsafe { Self::from_raw(NonNull::from(b)) }
> > +    }
> > +}
> > +
> > +impl<T: UniqueRefCounted> TryFrom<ARef<T>> for UniqueRef<T> {
> > +    type Error = ARef<T>;
> > +    /// Tries to convert the [`ARef`] to an [`UniqueRef`]
> > +    /// by calling [`UniqueRefCounted::try_shared_to_unique`].
> > +    /// In case the [`ARef`] is not unique it returns again an [`ARef`] to the same
> > +    /// underlying object.
> > +    fn try_from(b: ARef<T>) -> Result<UniqueRef<T>, Self::Error> {
> > +        UniqueRefCounted::try_shared_to_unique(b)
> > +    }
> > +}
> > +
> > +impl<T: UniqueRefCounted> Drop for UniqueRef<T> {
> > +    fn drop(&mut self) {
> > +        // SAFETY: The type invariants guarantee that the [`UniqueRef`] owns the reference
> > +        // we're about to decrement.
> > +        unsafe { UniqueRefCounted::dec_ref(self.ptr) };
> > +    }
> > +}
> > +
> >  /// A sum type that always holds either a value of type `L` or `R`.
> >  ///
> >  /// # Examples
> > -- 
> > 2.48.1
> > 
> > Best regards,
> > 
> > Oliver
> > 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] rust: adding UniqueRefCounted and UniqueRef types
@ 2025-02-28 18:22 Oliver Mangold
  2025-02-28 18:33 ` Boqun Feng
  0 siblings, 1 reply; 14+ messages in thread
From: Oliver Mangold @ 2025-02-28 18:22 UTC (permalink / raw)
  To: Boqun Feng
  Cc: Andreas Hindborg, Miguel Ojeda, Alex Gaynor, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
	linux-kernel, rust-for-linux

On 250228 1009, Boqun Feng wrote:
> 
> Please add the "# Examples" section as I requested:
> 
> 	https://lore.kernel.org/rust-for-linux/Z8HcHVtPiG-X6ujP@Mac.home/
> 
> I also would like to know why do you think it's OK to ignore my previous
> comment, thanks!
> 

I intend to. Don't worry. I totally have it in mind that, this is missing.
As said, I will do it in a couple of days. Sorry if the update
caused confusion.

I am aware this won't be the final version, but I thought
I still post this one correctly formatted, as Miguel requested.

I assure you, I'm not ignoring your comment and request at all.

Oliver


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] rust: adding UniqueRefCounted and UniqueRef types
  2025-02-28 18:01 ` [PATCH v2] " Oliver Mangold
  2025-02-28 18:09   ` Boqun Feng
@ 2025-02-28 18:29   ` Andreas Hindborg
  2025-03-03 14:09   ` Andreas Hindborg
  2 siblings, 0 replies; 14+ messages in thread
From: Andreas Hindborg @ 2025-02-28 18:29 UTC (permalink / raw)
  To: Oliver Mangold
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
	linux-kernel, rust-for-linux

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

> For usage with block-mq, a variant of ARef
> which is guaranteed to be unique would be useful.
> As chances are it is useful in general, This implements it
> as kernel::types::UniqueRef.
> The difference between ARef and UniqueRef
> is basically the same as between Arc and UniqueArc.

I would suggest splitting out the use case from the commit message:

  rust: add `UniqueRef` for `ARef` references

  Add `UniqeRef` as a variabt of `ARef` that is guaranteed to be unique.
  This is useful when mutable access to the underlying type is required
  and we can guarantee uniqueness, and when APIs that would normally take
  an `ARef` require uniqueness.

  ---

  Upcomming changes to the rust block device driver API depend on this
  patch. Please see [1] for a use casel.

  [1] https://github.com/metaspace/linux/blob/41034cb4ea7cd20b981ce320136b6526a9fa9db6/rust/kernel/block/mq/request.rs#L406


The comments after the --- will not got in the commit log, they are a
message for the review process only.


Best regards,
Andreas Hindborg



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] rust: adding UniqueRefCounted and UniqueRef types
  2025-02-28 18:22 [PATCH v2] rust: adding UniqueRefCounted and UniqueRef types Oliver Mangold
@ 2025-02-28 18:33 ` Boqun Feng
  2025-02-28 19:01   ` Andreas Hindborg
  0 siblings, 1 reply; 14+ messages in thread
From: Boqun Feng @ 2025-02-28 18:33 UTC (permalink / raw)
  To: Oliver Mangold
  Cc: Andreas Hindborg, Miguel Ojeda, Alex Gaynor, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
	linux-kernel, rust-for-linux

On Fri, Feb 28, 2025 at 06:22:58PM +0000, Oliver Mangold wrote:
> On 250228 1009, Boqun Feng wrote:
> > 
> > Please add the "# Examples" section as I requested:
> > 
> > 	https://lore.kernel.org/rust-for-linux/Z8HcHVtPiG-X6ujP@Mac.home/
> > 
> > I also would like to know why do you think it's OK to ignore my previous
> > comment, thanks!
> > 
> 
> I intend to. Don't worry. I totally have it in mind that, this is missing.
> As said, I will do it in a couple of days. Sorry if the update
> caused confusion.
> 

I should appologize as well, after re-reading the email exchange between
us, I realized I wasn't clear about whether my request was including the
tests with the patch or not. So it's reasonable you thought a separate
patch would work.

Now as we cleared things, I want to make sure that it's clear that I
would like to see the patch with examples in it. 

> I am aware this won't be the final version, but I thought
> I still post this one correctly formatted, as Miguel requested.
> 

Yes, I figured that's what you planned to do, although it's better if
you could provide some information between versions, one example would
be:

	https://lore.kernel.org/rust-for-linux/20250227193522.198344-1-lyude@redhat.com/

i.e. you can add some description after the "---" line, and that won't
affect git to apply the patches. Usually people put changes between
versions, and in this case since you just want to update a version that
is not the final version, you can add some description about that there
as well.

> I assure you, I'm not ignoring your comment and request at all.
> 

Thanks! Again, sorry for not being clear in previous emails.

Regards,
Boqun

> Oliver
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] rust: adding UniqueRefCounted and UniqueRef types
@ 2025-02-28 18:41 Oliver Mangold
  0 siblings, 0 replies; 14+ messages in thread
From: Oliver Mangold @ 2025-02-28 18:41 UTC (permalink / raw)
  To: Andreas Hindborg
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
	linux-kernel, rust-for-linux

On 250228 1929, Andreas Hindborg wrote:
> 
> I would suggest splitting out the use case from the commit message:
> 
Yes, that makes sense. Will do.
> 
> The comments after the --- will not got in the commit log, they are a
> message for the review process only.
> 
Ahhh. Honestly I'm not very familiar with the format-patch/am
method, yet. That was a gap in my understanding.
Thanks for pointing that out. Now everything makes sense.

On 250228 1033, Boqun Feng wrote:
> 
> Now as we cleared things, I want to make sure that it's clear that I
> would like to see the patch with examples in it.
>
That's fine. That's a reasonable request. Will do.
> 
> i.e. you can add some description after the "---" line, and that won't
> affect git to apply the patches. Usually people put changes between
> versions, and in this case since you just want to update a version that
> is not the final version, you can add some description about that there
> as well.
> 
Yes. I just learned that it's working like this. See above :)
> 
> Thanks! Again, sorry for not being clear in previous emails.
>
Don't worry.


Best,

Oliver


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] rust: adding UniqueRefCounted and UniqueRef types
  2025-02-28 18:33 ` Boqun Feng
@ 2025-02-28 19:01   ` Andreas Hindborg
  2025-02-28 19:06     ` Boqun Feng
  0 siblings, 1 reply; 14+ messages in thread
From: Andreas Hindborg @ 2025-02-28 19:01 UTC (permalink / raw)
  To: Boqun Feng
  Cc: Oliver Mangold, Miguel Ojeda, Alex Gaynor, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
	linux-kernel, rust-for-linux

"Boqun Feng" <boqun.feng@gmail.com> writes:

> On Fri, Feb 28, 2025 at 06:22:58PM +0000, Oliver Mangold wrote:
>> On 250228 1009, Boqun Feng wrote:
>> >

Something strange is going on with this thread. Some replies are not
attached to the patch in my email client. Lore also has some trouble.

What is going on?


Best regards,
Andreas Hindborg



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] rust: adding UniqueRefCounted and UniqueRef types
  2025-02-28 19:01   ` Andreas Hindborg
@ 2025-02-28 19:06     ` Boqun Feng
  2025-02-28 19:16       ` Oliver Mangold
  0 siblings, 1 reply; 14+ messages in thread
From: Boqun Feng @ 2025-02-28 19:06 UTC (permalink / raw)
  To: Andreas Hindborg
  Cc: Oliver Mangold, Miguel Ojeda, Alex Gaynor, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
	linux-kernel, rust-for-linux

On Fri, Feb 28, 2025 at 08:01:55PM +0100, Andreas Hindborg wrote:
> "Boqun Feng" <boqun.feng@gmail.com> writes:
> 
> > On Fri, Feb 28, 2025 at 06:22:58PM +0000, Oliver Mangold wrote:
> >> On 250228 1009, Boqun Feng wrote:
> >> >
> 
> Something strange is going on with this thread. Some replies are not
> attached to the patch in my email client. Lore also has some trouble.
> 
> What is going on?
> 

Probably because some of Oliver's replys don't have the "in-reply-to"
field in the email header. Maybe it's a known issue of protonmail?

Regards,
Boqun

> 
> Best regards,
> Andreas Hindborg
> 
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] rust: adding UniqueRefCounted and UniqueRef types
  2025-02-28 19:06     ` Boqun Feng
@ 2025-02-28 19:16       ` Oliver Mangold
  2025-02-28 19:25         ` Andreas Hindborg
  0 siblings, 1 reply; 14+ messages in thread
From: Oliver Mangold @ 2025-02-28 19:16 UTC (permalink / raw)
  To: Boqun Feng
  Cc: Andreas Hindborg, Miguel Ojeda, Alex Gaynor, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
	linux-kernel, rust-for-linux

On 250228 1106, Boqun Feng wrote:
> 
> Probably because some of Oliver's replys don't have the "in-reply-to"
> field in the email header. Maybe it's a known issue of protonmail?
> 
I think you are right.

Google couldn't tell what to do about it during the last 5 min, though.
I'm using mutt through protonmail-bridge for mailing.

I will look into it, but if someone has an advice, it would also be
welcome.

Oliver


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] rust: adding UniqueRefCounted and UniqueRef types
  2025-02-28 19:16       ` Oliver Mangold
@ 2025-02-28 19:25         ` Andreas Hindborg
  2025-02-28 19:47           ` Oliver Mangold
  2025-02-28 20:07           ` Oliver Mangold
  0 siblings, 2 replies; 14+ messages in thread
From: Andreas Hindborg @ 2025-02-28 19:25 UTC (permalink / raw)
  To: Oliver Mangold
  Cc: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
	linux-kernel, rust-for-linux

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

> On 250228 1106, Boqun Feng wrote:
>>
>> Probably because some of Oliver's replys don't have the "in-reply-to"
>> field in the email header. Maybe it's a known issue of protonmail?
>>
> I think you are right.
>
> Google couldn't tell what to do about it during the last 5 min, though.
> I'm using mutt through protonmail-bridge for mailing.
>
> I will look into it, but if someone has an advice, it would also be
> welcome.

I am using protonmail-bridge for sending as well. I don't think the
issue is in the bridge, it must be in the way you compose/reply in mutt.
I am not a mutt user, so I don't know how that works.


Best regards,
Andreas Hindborg




^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] rust: adding UniqueRefCounted and UniqueRef types
  2025-02-28 19:25         ` Andreas Hindborg
@ 2025-02-28 19:47           ` Oliver Mangold
  2025-02-28 20:07           ` Oliver Mangold
  1 sibling, 0 replies; 14+ messages in thread
From: Oliver Mangold @ 2025-02-28 19:47 UTC (permalink / raw)
  To: Andreas Hindborg
  Cc: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
	linux-kernel, rust-for-linux

On 250228 2025, Andreas Hindborg wrote:
> 
> I am using protonmail-bridge for sending as well. I don't think the
> issue is in the bridge, it must be in the way you compose/reply in mutt.
> I am not a mutt user, so I don't know how that works.
> 
I'm usually just doing group-reply. What I read is that protonmail-bridge
drops 'in-reply-to' if there is no 'references', which looks about right,
when I check last mails. Need to find out, why the 'references' vanishes
sometimes, though.

Oliver


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] rust: adding UniqueRefCounted and UniqueRef types
  2025-02-28 19:25         ` Andreas Hindborg
  2025-02-28 19:47           ` Oliver Mangold
@ 2025-02-28 20:07           ` Oliver Mangold
  1 sibling, 0 replies; 14+ messages in thread
From: Oliver Mangold @ 2025-02-28 20:07 UTC (permalink / raw)
  To: Andreas Hindborg
  Cc: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
	linux-kernel, rust-for-linux

On 250228 2025, Andreas Hindborg wrote:
> 
> I am using protonmail-bridge for sending as well. I don't think the
> issue is in the bridge, it must be in the way you compose/reply in mutt.
> I am not a mutt user, so I don't know how that works.
> 
I think I got it now. A rather subtle combination of issues:

- I have mutt configured to use the Drafts folder of Protonmail
  for postponed messages
- Protonmail drops the 'references' header when a mail gets stored
  in Drafts like this
- protonmail-bridge drops the 'in-reply-to' header when there is no
  'references' header

Thus when I postpone a mail and send it later, we see the current problem.

I just configured mutt to store drafts locally. So it should be okay
from now on.

Best,

Oliver


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] rust: adding UniqueRefCounted and UniqueRef types
  2025-02-28 18:01 ` [PATCH v2] " Oliver Mangold
  2025-02-28 18:09   ` Boqun Feng
  2025-02-28 18:29   ` Andreas Hindborg
@ 2025-03-03 14:09   ` Andreas Hindborg
  2 siblings, 0 replies; 14+ messages in thread
From: Andreas Hindborg @ 2025-03-03 14:09 UTC (permalink / raw)
  To: Oliver Mangold
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
	linux-kernel, rust-for-linux

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

[...]

> +/// Types that are [`AlwaysRefCounted`] and can be safely converted to an [`UniqueRef`]
> +///
> +/// # Safety
> +///
> +/// Implementers must ensure that the methods of the trait
> +/// change the reference count of the underlying object such that:
> +/// - the uniqueness invariant is upheld, i.e. it is not possible
> +///   to obtain another reference by any means (other than through the [`UniqueRef`])
> +///   until the [`UniqueRef`] is dropped or converted to an [`ARef`].
> +/// - [`UniqueRefCounted::dec_ref`] correctly frees the underlying object.
> +/// - [`UniqueRefCounted::unique_to_shared`] set the reference count to the value
> +/// - that the returned [`ARef`] expects for an object with a single reference
> +///   in existence.
> +pub unsafe trait UniqueRefCounted: AlwaysRefCounted + Sized {
> +    /// Checks if the [`ARef`] is unique and convert it
> +    /// to an [`UniqueRef`] it that is that case.
> +    /// Otherwise it returns again an [`ARef`] to the same
> +    /// underlying object.
> +    fn try_shared_to_unique(this: ARef<Self>) -> Result<UniqueRef<Self>, ARef<Self>>;

This could just be `try_into_unique`, since the type of `this` gives the
rest of the context.

> +    /// Converts the [`UniqueRef`] into an [`ARef`].
> +    fn unique_to_shared(this: UniqueRef<Self>) -> ARef<Self>;

Similarly, this could be `into_shared`.


Best regards,
Andreas Hindborg



^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2025-03-03 14:10 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-28 18:22 [PATCH v2] rust: adding UniqueRefCounted and UniqueRef types Oliver Mangold
2025-02-28 18:33 ` Boqun Feng
2025-02-28 19:01   ` Andreas Hindborg
2025-02-28 19:06     ` Boqun Feng
2025-02-28 19:16       ` Oliver Mangold
2025-02-28 19:25         ` Andreas Hindborg
2025-02-28 19:47           ` Oliver Mangold
2025-02-28 20:07           ` Oliver Mangold
  -- strict thread matches above, loose matches on Subject: below --
2025-02-28 18:41 Oliver Mangold
2025-02-28 14:43 [PATCH] " Oliver Mangold
2025-02-28 18:01 ` [PATCH v2] " Oliver Mangold
2025-02-28 18:09   ` Boqun Feng
2025-02-28 18:16     ` Boqun Feng
2025-02-28 18:29   ` Andreas Hindborg
2025-03-03 14:09   ` Andreas Hindborg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).