public inbox for rust-for-linux@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] rust: workqueue: fix SAFETY comment Arc refs in Pin<KBox<T>>
@ 2026-04-25 13:57 Sagar Taunk
  2026-04-25 15:15 ` Alexandre Courbot
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Sagar Taunk @ 2026-04-25 13:57 UTC (permalink / raw)
  To: ojeda
  Cc: aliceryhl, bjorn3_gh, boqun, gary, lossin, a.hindborg, tmgross,
	dakr, contact, rust-for-linux, linux-kernel, Sagar Taunk

The `WorkItemPointer` implementation for `Pin<KBox<T>>` contained SAFETY
comments that incorrectly referenced `Arc::into_raw` instead of
`KBox::into_raw`. This implementation uses `KBox`, not `Arc`, so update
the comments to accurately reflect the actual ownership transfer.

Suggested-by: Onur Özkan <contact@onurozkan.dev>
Link: https://github.com/Rust-for-Linux/linux/issues/1233
Signed-off-by: Sagar Taunk <sagartaunk2@gmail.com>
---
 rust/kernel/workqueue.rs | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
index 7e253b6f299c..74c59f2b1c09 100644
--- a/rust/kernel/workqueue.rs
+++ b/rust/kernel/workqueue.rs
@@ -890,9 +890,10 @@ unsafe impl<T, const ID: u64> WorkItemPointer<ID> for Pin<KBox<T>>
     unsafe extern "C" fn run(ptr: *mut bindings::work_struct) {
         // The `__enqueue` method always uses a `work_struct` stored in a `Work<T, ID>`.
         let ptr = ptr.cast::<Work<T, ID>>();
-        // SAFETY: This computes the pointer that `__enqueue` got from `Arc::into_raw`.
+        // SAFETY: This computes the pointer that `__enqueue` got from `KBox::into_raw`.
         let ptr = unsafe { T::work_container_of(ptr) };
-        // SAFETY: This pointer comes from `Arc::into_raw` and we've been given back ownership.
+        // SAFETY: This pointer comes from `KBox::into_raw` and we have been given back ownership,
+        // as the workqueue guarantees `run` is called exactly once.
         let boxed = unsafe { KBox::from_raw(ptr) };
         // SAFETY: The box was already pinned when it was enqueued.
         let pinned = unsafe { Pin::new_unchecked(boxed) };
-- 
2.54.0


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

* Re: [PATCH] rust: workqueue: fix SAFETY comment Arc refs in Pin<KBox<T>>
  2026-04-25 13:57 [PATCH] rust: workqueue: fix SAFETY comment Arc refs in Pin<KBox<T>> Sagar Taunk
@ 2026-04-25 15:15 ` Alexandre Courbot
  2026-04-26 13:38 ` Miguel Ojeda
  2026-04-27 14:56 ` Alice Ryhl
  2 siblings, 0 replies; 4+ messages in thread
From: Alexandre Courbot @ 2026-04-25 15:15 UTC (permalink / raw)
  To: Sagar Taunk
  Cc: ojeda, aliceryhl, bjorn3_gh, boqun, gary, lossin, a.hindborg,
	tmgross, dakr, contact, rust-for-linux, linux-kernel

On Sat Apr 25, 2026 at 10:57 PM JST, Sagar Taunk wrote:
> The `WorkItemPointer` implementation for `Pin<KBox<T>>` contained SAFETY
> comments that incorrectly referenced `Arc::into_raw` instead of
> `KBox::into_raw`. This implementation uses `KBox`, not `Arc`, so update
> the comments to accurately reflect the actual ownership transfer.
>
> Suggested-by: Onur Özkan <contact@onurozkan.dev>
> Link: https://github.com/Rust-for-Linux/linux/issues/1233
> Signed-off-by: Sagar Taunk <sagartaunk2@gmail.com>
> ---
>  rust/kernel/workqueue.rs | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
> index 7e253b6f299c..74c59f2b1c09 100644
> --- a/rust/kernel/workqueue.rs
> +++ b/rust/kernel/workqueue.rs
> @@ -890,9 +890,10 @@ unsafe impl<T, const ID: u64> WorkItemPointer<ID> for Pin<KBox<T>>
>      unsafe extern "C" fn run(ptr: *mut bindings::work_struct) {
>          // The `__enqueue` method always uses a `work_struct` stored in a `Work<T, ID>`.
>          let ptr = ptr.cast::<Work<T, ID>>();
> -        // SAFETY: This computes the pointer that `__enqueue` got from `Arc::into_raw`.
> +        // SAFETY: This computes the pointer that `__enqueue` got from `KBox::into_raw`.
>          let ptr = unsafe { T::work_container_of(ptr) };
> -        // SAFETY: This pointer comes from `Arc::into_raw` and we've been given back ownership.
> +        // SAFETY: This pointer comes from `KBox::into_raw` and we have been given back ownership,
> +        // as the workqueue guarantees `run` is called exactly once.

Not sure if extending the comment is useful, but the comment looks correct.

Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>

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

* Re: [PATCH] rust: workqueue: fix SAFETY comment Arc refs in Pin<KBox<T>>
  2026-04-25 13:57 [PATCH] rust: workqueue: fix SAFETY comment Arc refs in Pin<KBox<T>> Sagar Taunk
  2026-04-25 15:15 ` Alexandre Courbot
@ 2026-04-26 13:38 ` Miguel Ojeda
  2026-04-27 14:56 ` Alice Ryhl
  2 siblings, 0 replies; 4+ messages in thread
From: Miguel Ojeda @ 2026-04-26 13:38 UTC (permalink / raw)
  To: Sagar Taunk
  Cc: ojeda, aliceryhl, bjorn3_gh, boqun, gary, lossin, a.hindborg,
	tmgross, dakr, contact, rust-for-linux, linux-kernel

On Sat, Apr 25, 2026 at 3:57 PM Sagar Taunk <sagartaunk2@gmail.com> wrote:
>
> The `WorkItemPointer` implementation for `Pin<KBox<T>>` contained SAFETY
> comments that incorrectly referenced `Arc::into_raw` instead of
> `KBox::into_raw`. This implementation uses `KBox`, not `Arc`, so update
> the comments to accurately reflect the actual ownership transfer.
>
> Suggested-by: Onur Özkan <contact@onurozkan.dev>
> Link: https://github.com/Rust-for-Linux/linux/issues/1233
> Signed-off-by: Sagar Taunk <sagartaunk2@gmail.com>

I am copying a procedural comment since I just wrote it for another patch:

  For this case it doesn't matter too much, but in general, when
  something is a fix, we add a Fixes: tag (and in most cases Cc: stable@
  to accompany it). That helps backporting (when needed), adds context
  e.g. for reviewing (at times), etc.

Thanks!

Cheers,
Miguel

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

* Re: [PATCH] rust: workqueue: fix SAFETY comment Arc refs in Pin<KBox<T>>
  2026-04-25 13:57 [PATCH] rust: workqueue: fix SAFETY comment Arc refs in Pin<KBox<T>> Sagar Taunk
  2026-04-25 15:15 ` Alexandre Courbot
  2026-04-26 13:38 ` Miguel Ojeda
@ 2026-04-27 14:56 ` Alice Ryhl
  2 siblings, 0 replies; 4+ messages in thread
From: Alice Ryhl @ 2026-04-27 14:56 UTC (permalink / raw)
  To: Sagar Taunk
  Cc: ojeda, bjorn3_gh, boqun, gary, lossin, a.hindborg, tmgross, dakr,
	contact, rust-for-linux, linux-kernel

On Sat, Apr 25, 2026 at 07:27:01PM +0530, Sagar Taunk wrote:
> The `WorkItemPointer` implementation for `Pin<KBox<T>>` contained SAFETY
> comments that incorrectly referenced `Arc::into_raw` instead of
> `KBox::into_raw`. This implementation uses `KBox`, not `Arc`, so update
> the comments to accurately reflect the actual ownership transfer.
> 
> Suggested-by: Onur Özkan <contact@onurozkan.dev>
> Link: https://github.com/Rust-for-Linux/linux/issues/1233
> Signed-off-by: Sagar Taunk <sagartaunk2@gmail.com>

Thanks!

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

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

end of thread, other threads:[~2026-04-27 14:56 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-25 13:57 [PATCH] rust: workqueue: fix SAFETY comment Arc refs in Pin<KBox<T>> Sagar Taunk
2026-04-25 15:15 ` Alexandre Courbot
2026-04-26 13:38 ` Miguel Ojeda
2026-04-27 14:56 ` Alice Ryhl

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox