rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Rust: Implement a unique reference type URef supplementing ARef
@ 2025-02-21  8:04 ` Oliver Mangold
  2025-02-21  8:12   ` Greg KH
                     ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Oliver Mangold @ 2025-02-21  8:04 UTC (permalink / raw)
  To: Andreas Hindborg; +Cc: linux-kernel, rust-for-linux

For usage with block-mq, we found that having a variant of ARef which is guaranteed to be unique is useful. As chances are it is useful in general, I implemented it as kernel::types::URef. The difference between ARef and URef is basically the same as between Arc and UniqueArc.

Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
---
 rust/kernel/block/mq/request.rs | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------
 1 file changed, 61 insertions(+), 36 deletions(-)

diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index 55ddd50e8aaa..80dc9edef1b9 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -543,6 +543,12 @@ fn from(b: &T) -> Self {
     }
 }
 
+impl<T: UniqueRefCounted> From<URef<T>> for ARef<T> {
+    fn from(b: URef<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,119 @@ fn drop(&mut self) {
     }
 }
 
+/// Types that are `AlwaysRefCounted` and can be safely converted to an `URef`
+///
+pub unsafe trait UniqueRefCounted: AlwaysRefCounted + Sized {
+    /// Checks if the the [ARef] is unique and convert it
+    /// to an [URef] 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<URef<Self>,ARef<Self>>;
+    /// Converts the [URef] into an [ARef].
+    fn unique_to_shared(this: URef<Self>) -> ARef<Self>;
+}
+
+/// An unique owned reference to an always-reference-counted 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 [`ARef`] instance. In
+/// particular, the [`ARef`] instance owns an increment on the underlying object's reference count.
+pub struct URef<T: UniqueRefCounted> {
+    ptr: NonNull<T>,
+    _p: PhantomData<T>,
+}
+
+// SAFETY: It is safe to send `URef<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 `URef<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 URef<T> {}
+
+// SAFETY: It is safe to send `&URef<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 `&URef<T>` may clone it and get an
+// `URef<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 URef<T> {}
+
+impl<T: UniqueRefCounted> URef<T> {
+    /// Creates a new instance of [`URef`].
+    ///
+    /// 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 AlwaysRefCounted::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 [`URef`].
+    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 `URef`, 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 `URef`.
+    pub fn into_raw(me: Self) -> NonNull<T> {
+        ManuallyDrop::new(me).ptr
+    }
+}
+
+impl<T: UniqueRefCounted> Deref for URef<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 URef<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 URef<T> {
+    /// Converts the [URef] 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 URef<T> {
+    type Error = ARef<T>;
+    /// Tries to convert the [ARef] to an [URef] 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<URef<T>,Self::Error> {
+        UniqueRefCounted::try_shared_to_unique(b)
+    }
+}
+
+impl<T: UniqueRefCounted> Drop for URef<T> {
+    fn drop(&mut self) {
+        // SAFETY: The type invariants guarantee that the `URef` owns the reference we're about to
+        // decrement.
+        unsafe { T::dec_ref(self.ptr) };
+    }
+}
+
 /// A sum type that always holds either a value of type `L` or `R`.
 ///
 /// # Examples
---


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

* Re: [PATCH] Rust: Implement a unique reference type URef supplementing ARef
  2025-02-21  8:04 ` [PATCH] Rust: Implement a unique reference type URef supplementing ARef Oliver Mangold
@ 2025-02-21  8:12   ` Greg KH
  2025-02-21  8:35     ` Oliver Mangold
  2025-02-28 11:10   ` Andreas Hindborg
  2025-03-04  6:19   ` kernel test robot
  2 siblings, 1 reply; 10+ messages in thread
From: Greg KH @ 2025-02-21  8:12 UTC (permalink / raw)
  To: Oliver Mangold; +Cc: Andreas Hindborg, linux-kernel, rust-for-linux

On Fri, Feb 21, 2025 at 08:04:34AM +0000, Oliver Mangold wrote:
> For usage with block-mq, we found that having a variant of ARef which is guaranteed to be unique is useful. As chances are it is useful in general, I implemented it as kernel::types::URef. The difference between ARef and URef is basically the same as between Arc and UniqueArc.

Nit, please wrap your changelog text at 72 columns or so, checkpatch
should have warned you about this, right?

thanks,

greg k-h

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

* Re: [PATCH] Rust: Implement a unique reference type URef supplementing ARef
  2025-02-21  8:12   ` Greg KH
@ 2025-02-21  8:35     ` Oliver Mangold
  2025-02-21  9:33       ` Andreas Hindborg
  0 siblings, 1 reply; 10+ messages in thread
From: Oliver Mangold @ 2025-02-21  8:35 UTC (permalink / raw)
  To: Greg KH; +Cc: Andreas Hindborg, linux-kernel, rust-for-linux

On 250221 0912, Greg KH wrote:
> Nit, please wrap your changelog text at 72 columns or so, checkpatch
> should have warned you about this, right?

Sorry, I did checkpatch before adding the description. Learned something new, I guess :)


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

* Re: [PATCH] Rust: Implement a unique reference type URef supplementing ARef
  2025-02-21  8:35     ` Oliver Mangold
@ 2025-02-21  9:33       ` Andreas Hindborg
  0 siblings, 0 replies; 10+ messages in thread
From: Andreas Hindborg @ 2025-02-21  9:33 UTC (permalink / raw)
  To: Oliver Mangold; +Cc: Greg KH, linux-kernel, rust-for-linux

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

> On 250221 0912, Greg KH wrote:
>> Nit, please wrap your changelog text at 72 columns or so, checkpatch
>> should have warned you about this, right?
>
> Sorry, I did checkpatch before adding the description. Learned something new, I guess :)

Thanks for sending this!

I can absolutely recommend b4 for sending your work [1]. It will warn
you if you try to send but did not run checkpatch after your latest change.


Best regards,
Andreas Hindborg


[1] https://b4.docs.kernel.org/en/latest/


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

* Re: [PATCH] Rust: Implement a unique reference type URef supplementing ARef
  2025-02-21  8:04 ` [PATCH] Rust: Implement a unique reference type URef supplementing ARef Oliver Mangold
  2025-02-21  8:12   ` Greg KH
@ 2025-02-28 11:10   ` Andreas Hindborg
  2025-02-28 11:25     ` Andreas Hindborg
                       ` (2 more replies)
  2025-03-04  6:19   ` kernel test robot
  2 siblings, 3 replies; 10+ messages in thread
From: Andreas Hindborg @ 2025-02-28 11:10 UTC (permalink / raw)
  To: Oliver Mangold; +Cc: linux-kernel, rust-for-linux

Hi Oliver,

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

> For usage with block-mq, we found that having a variant of ARef which is guaranteed to be unique is useful. As chances are it is useful in general, I implemented it as kernel::types::URef. The difference between ARef and URef is basically the same as between Arc and UniqueArc.

Wrap at 75 characters please :)

>
> Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
> ---
>  rust/kernel/block/mq/request.rs | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------
>  1 file changed, 61 insertions(+), 36 deletions(-)
>
> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index 55ddd50e8aaa..80dc9edef1b9 100644
> --- a/rust/kernel/types.rs
> +++ b/rust/kernel/types.rs
> @@ -543,6 +543,12 @@ fn from(b: &T) -> Self {
>      }
>  }
>
> +impl<T: UniqueRefCounted> From<URef<T>> for ARef<T> {
> +    fn from(b: URef<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,119 @@ fn drop(&mut self) {
>      }
>  }
>
> +/// Types that are `AlwaysRefCounted` and can be safely converted to an `URef`
> +///

When the trait is unsafe, we need to specify the conditions under which
it is safe to implement:

# Safety

- Requirement 1
- Requirement 2
- etc

See `AlwaysRefCounted` for reference on formatting.

> +pub unsafe trait UniqueRefCounted: AlwaysRefCounted + Sized {
> +    /// Checks if the the [ARef] is unique and convert it
> +    /// to an [URef] it that is that case.

Please use back ticks for types: [`ARef`], [`URref`]

> +    /// Otherwise it returns again an [ARef] to the same
> +    /// underlying object.
> +    fn try_shared_to_unique(this: ARef<Self>) -> Result<URef<Self>,ARef<Self>>;
> +    /// Converts the [URef] into an [ARef].
> +    fn unique_to_shared(this: URef<Self>) -> ARef<Self>;
> +}
> +
> +/// An unique owned reference to an always-reference-counted 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 [`ARef`] instance. In
> +/// particular, the [`ARef`] instance owns an increment on the underlying object's reference count.
> +pub struct URef<T: UniqueRefCounted> {
> +    ptr: NonNull<T>,
> +    _p: PhantomData<T>,
> +}
> +
> +// SAFETY: It is safe to send `URef<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 `URef<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 URef<T> {}
> +
> +// SAFETY: It is safe to send `&URef<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 `&URef<T>` may clone it and get an
> +// `URef<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 URef<T> {}
> +
> +impl<T: UniqueRefCounted> URef<T> {

I would prefer `UniqueRef`. I know `ARef` has a different naming scheme,
but I think `UniqueRef` is sufficiently short and significantly more
descriptive than `URef`.

> +    /// Creates a new instance of [`URef`].
> +    ///
> +    /// 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 AlwaysRefCounted::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 [`URef`].
> +    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 `URef`, 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 `URef`.
> +    pub fn into_raw(me: Self) -> NonNull<T> {
> +        ManuallyDrop::new(me).ptr
> +    }
> +}
> +
> +impl<T: UniqueRefCounted> Deref for URef<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 URef<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 URef<T> {
> +    /// Converts the [URef] 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 URef<T> {
> +    type Error = ARef<T>;
> +    /// Tries to convert the [ARef] to an [URef] 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<URef<T>,Self::Error> {
> +        UniqueRefCounted::try_shared_to_unique(b)
> +    }
> +}
> +
> +impl<T: UniqueRefCounted> Drop for URef<T> {
> +    fn drop(&mut self) {
> +        // SAFETY: The type invariants guarantee that the `URef` owns the reference we're about to
> +        // decrement.
> +        unsafe { T::dec_ref(self.ptr) };
> +    }
> +}
> +
>  /// A sum type that always holds either a value of type `L` or `R`.
>  ///
>  /// # Examples
> ---

For your next version, can you run `make rustfmt`?:


diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index 58556d417cae..49903fd0446e 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -542,13 +542,12 @@ fn drop(&mut self) {
 }
 
 /// Types that are `AlwaysRefCounted` and can be safely converted to an `URef`
-///
 pub unsafe trait UniqueRefCounted: AlwaysRefCounted + Sized {
     /// Checks if the the [ARef] is unique and convert it
     /// to an [URef] 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<URef<Self>,ARef<Self>>;
+    fn try_shared_to_unique(this: ARef<Self>) -> Result<URef<Self>, ARef<Self>>;
     /// Converts the [URef] into an [ARef].
     fn unique_to_shared(this: URef<Self>) -> ARef<Self>;
 }
@@ -620,7 +619,6 @@ fn deref(&self) -> &Self::Target {
 }
 
 impl<T: UniqueRefCounted> DerefMut for URef<T> {
-
     fn deref_mut(&mut self) -> &mut Self::Target {
         // SAFETY: The type invariants guarantee that the object is valid.
         unsafe { self.ptr.as_mut() }
@@ -638,10 +636,10 @@ fn from(b: &T) -> Self {
 
 impl<T: UniqueRefCounted> TryFrom<ARef<T>> for URef<T> {
     type Error = ARef<T>;
-    /// Tries to convert the [ARef] to an [URef] 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<URef<T>,Self::Error> {
+    /// Tries to convert the [ARef] to an [URef] 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<URef<T>, Self::Error> {
         UniqueRefCounted::try_shared_to_unique(b)
     }
 }


Also it would be great if you include your "rust: for fix dec_ref for
URef<Request>" folded in.

Best regards,
Andreas Hindborg



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

* Re: [PATCH] Rust: Implement a unique reference type URef supplementing ARef
  2025-02-28 11:10   ` Andreas Hindborg
@ 2025-02-28 11:25     ` Andreas Hindborg
  2025-02-28 11:31     ` Oliver Mangold
  2025-02-28 13:17     ` Miguel Ojeda
  2 siblings, 0 replies; 10+ messages in thread
From: Andreas Hindborg @ 2025-02-28 11:25 UTC (permalink / raw)
  To: Oliver Mangold
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, linux-kernel, rust-for-linux

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

> Hi Oliver,
>
> "Oliver Mangold" <oliver.mangold@pm.me> writes:
>
>> For usage with block-mq, we found that having a variant of ARef which is guaranteed to be unique is useful. As chances are it is useful in general, I implemented it as kernel::types::URef. The difference between ARef and URef is basically the same as between Arc and UniqueArc.
>
> Wrap at 75 characters please :)
>

Also, be sure to Cc relevant maintainers and reviewers (I did so in this
reply). Use scripts/checkpatch.pl and refer to [1]. You can also use
`b4` [2] to get a lot of automation.


Best regards,
Andreas Hindborg


[1] https://docs.kernel.org/process/submitting-patches.html
[2] https://b4.docs.kernel.org/en/latest/contributor/prep.html


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

* Re: [PATCH] Rust: Implement a unique reference type URef supplementing ARef
  2025-02-28 11:10   ` Andreas Hindborg
  2025-02-28 11:25     ` Andreas Hindborg
@ 2025-02-28 11:31     ` Oliver Mangold
  2025-02-28 12:16       ` Andreas Hindborg
  2025-02-28 13:17     ` Miguel Ojeda
  2 siblings, 1 reply; 10+ messages in thread
From: Oliver Mangold @ 2025-02-28 11:31 UTC (permalink / raw)
  To: Andreas Hindborg; +Cc: linux-kernel, rust-for-linux

On 250228 1210, Andreas Hindborg wrote:

Hi Andreas,

> Wrap at 75 characters please :)

> See `AlwaysRefCounted` for reference on formatting.

> Please use back ticks for types: [`ARef`], [`URref`]

> For your next version, can you run `make rustfmt`?:

Sorry for all that. Apparently I need to get used a bit more to how things
are supposed to be done here. Please be patient with me for a bit :)

> When the trait is unsafe, we need to specify the conditions under which
> it is safe to implement:

Of course. Sorry, missed that.

> I would prefer `UniqueRef`. I know `ARef` has a different naming scheme,
> but I think `UniqueRef` is sufficiently short and significantly more
> descriptive than `URef`.

Ok, will do. Honestly I also prefer UniqueRef.

> Also it would be great if you include your "rust: for fix dec_ref for
> URef<Request>" folded in.

Are your sure? Wouldn't the patches have to be ordered like this?

    this patch (mine)
    rust: block: change `queue_rq` request type to unique (yours)
    rust: block: simplify reference counting scheme (yours)
    rust: for fix dec_ref for URef<Request> (mine)

IOW, the latter requests depends on the 2 patches of yours, while these depend on this patch. You know what I mean?

Best regards,

Oliver


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

* Re: [PATCH] Rust: Implement a unique reference type URef supplementing ARef
  2025-02-28 11:31     ` Oliver Mangold
@ 2025-02-28 12:16       ` Andreas Hindborg
  0 siblings, 0 replies; 10+ messages in thread
From: Andreas Hindborg @ 2025-02-28 12:16 UTC (permalink / raw)
  To: Oliver Mangold; +Cc: linux-kernel, rust-for-linux

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

> On 250228 1210, Andreas Hindborg wrote:
>
> Hi Andreas,
>
>> Wrap at 75 characters please :)
>
>> See `AlwaysRefCounted` for reference on formatting.
>
>> Please use back ticks for types: [`ARef`], [`URref`]
>
>> For your next version, can you run `make rustfmt`?:
>
> Sorry for all that. Apparently I need to get used a bit more to how things
> are supposed to be done here. Please be patient with me for a bit :)
>
>> When the trait is unsafe, we need to specify the conditions under which
>> it is safe to implement:
>
> Of course. Sorry, missed that.
>
>> I would prefer `UniqueRef`. I know `ARef` has a different naming scheme,
>> but I think `UniqueRef` is sufficiently short and significantly more
>> descriptive than `URef`.
>
> Ok, will do. Honestly I also prefer UniqueRef.
>
>> Also it would be great if you include your "rust: for fix dec_ref for
>> URef<Request>" folded in.
>
> Are your sure? Wouldn't the patches have to be ordered like this?
>
>     this patch (mine)
>     rust: block: change `queue_rq` request type to unique (yours)
>     rust: block: simplify reference counting scheme (yours)
>     rust: for fix dec_ref for URef<Request> (mine)
>
> IOW, the latter requests depends on the 2 patches of yours, while these depend on this patch. You know what I mean?

Sorry, yes I meant "rust: allow to override dec_ref() for URef's".
Either include it in the series or fold it in, whatever makes most
sense.

The request update can go later.


Best regards,
Andreas Hindborg




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

* Re: [PATCH] Rust: Implement a unique reference type URef supplementing ARef
  2025-02-28 11:10   ` Andreas Hindborg
  2025-02-28 11:25     ` Andreas Hindborg
  2025-02-28 11:31     ` Oliver Mangold
@ 2025-02-28 13:17     ` Miguel Ojeda
  2 siblings, 0 replies; 10+ messages in thread
From: Miguel Ojeda @ 2025-02-28 13:17 UTC (permalink / raw)
  To: Andreas Hindborg; +Cc: Oliver Mangold, linux-kernel, rust-for-linux

On Fri, Feb 28, 2025 at 12:11 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> When the trait is unsafe, we need to specify the conditions under which
> it is safe to implement:
>
> # Safety
>
> - Requirement 1
> - Requirement 2
> - etc
>
> See `AlwaysRefCounted` for reference on formatting.

I think should be caught by Clippy -- Oliver, please try building with
`CLIPPY=1` (we would like Clippy-clean builds as much as reasonably
possible), you should be getting something like:

    error: docs for unsafe trait missing `# Safety` section
      --> rust/........rs:23:1
       |
    23 | pub unsafe trait A {}
       | ^^^^^^^^^^^^^^^^^^
       |
       = help: for further information visit
https://rust-lang.github.io/rust-clippy/master/index.html#missing_safety_doc
       = note: `-D clippy::missing-safety-doc` implied by `-D warnings`
       = help: to override `-D warnings` add
`#[allow(clippy::missing_safety_doc)]`

Thanks!

Cheers,
Miguel

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

* Re: [PATCH] Rust: Implement a unique reference type URef supplementing ARef
  2025-02-21  8:04 ` [PATCH] Rust: Implement a unique reference type URef supplementing ARef Oliver Mangold
  2025-02-21  8:12   ` Greg KH
  2025-02-28 11:10   ` Andreas Hindborg
@ 2025-03-04  6:19   ` kernel test robot
  2 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2025-03-04  6:19 UTC (permalink / raw)
  To: Oliver Mangold, Andreas Hindborg
  Cc: oe-kbuild-all, linux-kernel, rust-for-linux

Hi Oliver,

kernel test robot noticed the following build warnings:

[auto build test WARNING on rust/rust-next]
[also build test WARNING on linus/master v6.14-rc5 next-20250303]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Oliver-Mangold/Rust-Implement-a-unique-reference-type-URef-supplementing-ARef/20250221-160625
base:   https://github.com/Rust-for-Linux/linux rust-next
patch link:    https://lore.kernel.org/r/SpXhwnZO__ST8sHQ3HQ3ygThOcnmn0x2JlJ_nwJglL87vw5XfQA8sDH6HdkrGgFVycLhPBlCc6-UtEImTvY26A%3D%3D%40protonmail.internalid
patch subject: [PATCH] Rust: Implement a unique reference type URef supplementing ARef
config: x86_64-rhel-9.4-rust (https://download.01.org/0day-ci/archive/20250304/202503041325.UnOn2m7a-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250304/202503041325.UnOn2m7a-lkp@intel.com/reproduce)

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

All warnings (new ones prefixed by >>):

>> warning: docs for unsafe trait missing `# Safety` section
   --> rust/kernel/types.rs:546:1
   |
   546 | pub unsafe trait UniqueRefCounted: AlwaysRefCounted + Sized {
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#missing_safety_doc
   = note: `-W clippy::missing-safety-doc` implied by `-W clippy::all`
   = help: to override `-W clippy::all` add `#[allow(clippy::missing_safety_doc)]`

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

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

end of thread, other threads:[~2025-03-04  6:20 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <jeyp4dToznpiAQYWHAABrEBIHVfxaIf7ntexop3d2AXQgnlytw3J8YFkX8E8EFKc1-USf_fVZqKhEmuNGE3O0w==@protonmail.internalid>
2025-02-21  8:04 ` [PATCH] Rust: Implement a unique reference type URef supplementing ARef Oliver Mangold
2025-02-21  8:12   ` Greg KH
2025-02-21  8:35     ` Oliver Mangold
2025-02-21  9:33       ` Andreas Hindborg
2025-02-28 11:10   ` Andreas Hindborg
2025-02-28 11:25     ` Andreas Hindborg
2025-02-28 11:31     ` Oliver Mangold
2025-02-28 12:16       ` Andreas Hindborg
2025-02-28 13:17     ` Miguel Ojeda
2025-03-04  6:19   ` kernel test robot

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