The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH v2 0/1] rust: workqueue: add cancel_sync support
@ 2026-06-12 19:45 Onur Özkan
  2026-06-12 19:45 ` [PATCH v2 1/1] " Onur Özkan
  0 siblings, 1 reply; 3+ messages in thread
From: Onur Özkan @ 2026-06-12 19:45 UTC (permalink / raw)
  To: rust-for-linux, linux-kernel
  Cc: ojeda, boqun, gary, bjorn3_gh, lossin, a.hindborg, aliceryhl,
	tmgross, dakr, tamird, daniel.almeida, Onur Özkan

Changes since v1:
	- Created SupportsCancelling trait for from_raw_work function.
	- Removed from_raw_work from WorkItemPointer.

v1: https://lore.kernel.org/all/20260510082211.207450-1-work@onurozkan.dev

Onur Özkan (1):
  rust: workqueue: add cancel_sync support

 rust/kernel/workqueue.rs | 116 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 116 insertions(+)

-- 
2.51.2


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

* [PATCH v2 1/1] rust: workqueue: add cancel_sync support
  2026-06-12 19:45 [PATCH v2 0/1] rust: workqueue: add cancel_sync support Onur Özkan
@ 2026-06-12 19:45 ` Onur Özkan
  2026-06-15  7:08   ` Alice Ryhl
  0 siblings, 1 reply; 3+ messages in thread
From: Onur Özkan @ 2026-06-12 19:45 UTC (permalink / raw)
  To: rust-for-linux, linux-kernel
  Cc: ojeda, boqun, gary, bjorn3_gh, lossin, a.hindborg, aliceryhl,
	tmgross, dakr, tamird, daniel.almeida, Onur Özkan

Drivers can use this during teardown to cancel pending work and wait for
running work to finish before dropping related resources.

This is not implemented for Pin<KBox<T>> because queuing a boxed work
item transfers ownership of the box to the workqueue. There is therefore
no separate safe owner that can cancel the boxed work while it is pending.

Signed-off-by: Onur Özkan <work@onurozkan.dev>
---
 rust/kernel/workqueue.rs | 116 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 116 insertions(+)

diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
index 7e253b6f299c..4d61d7a10fae 100644
--- a/rust/kernel/workqueue.rs
+++ b/rust/kernel/workqueue.rs
@@ -471,6 +471,23 @@ pub trait WorkItem<const ID: u64 = 0> {
     fn run(this: Self::Pointer);
 }
 
+/// Work item pointers that support cancellation.
+///
+/// # Safety
+///
+/// Implementers must ensure that `from_raw_work` rebuilds the exact ownership transferred
+/// by a successful [`RawWorkItem::__enqueue`] call.
+pub unsafe trait SupportsCancelling<const ID: u64>: WorkItemPointer<ID> + Sized {
+    /// Rebuild this work item's pointer from its embedded `work_struct`.
+    ///
+    /// # Safety
+    ///
+    /// The provided `work_struct` pointer must originate from a previous call to
+    /// [`RawWorkItem::__enqueue`] where the `queue_work_on` closure returned true
+    /// and the pointer must still be valid.
+    unsafe fn from_raw_work(ptr: *mut bindings::work_struct) -> Self;
+}
+
 /// Links for a work item.
 ///
 /// This struct contains a function pointer to the [`run`] function from the [`WorkItemPointer`]
@@ -537,6 +554,32 @@ pub unsafe fn raw_get(ptr: *const Self) -> *mut bindings::work_struct {
         // the compiler does not complain that the `work` field is unused.
         unsafe { Opaque::cast_into(core::ptr::addr_of!((*ptr).work)) }
     }
+
+    /// Cancels this work item if it is pending and waits for any running execution to finish.
+    ///
+    /// On return, the work item is guaranteed to not be pending or executing as long as there are
+    /// no racing re-enqueues.
+    ///
+    /// # Note
+    ///
+    /// Should be called from a sleepable context if the work was last queued on a non-BH
+    /// workqueue.
+    #[inline]
+    pub fn cancel_sync(&self) -> Option<T::Pointer>
+    where
+        T: WorkItem<ID>,
+        T::Pointer: SupportsCancelling<ID>,
+    {
+        let ptr = self.work.get();
+        // SAFETY: `ptr` is a valid embedded `work_struct`.
+        if unsafe { bindings::cancel_work_sync(ptr) } {
+            // SAFETY: A `true` return means the work was pending and got canceled, so the queued
+            // ownership transfer performed by `__enqueue` is reclaimed here.
+            Some(unsafe { T::Pointer::from_raw_work(ptr) })
+        } else {
+            None
+        }
+    }
 }
 
 /// Declares that a type contains a [`Work<T, ID>`].
@@ -725,6 +768,34 @@ pub unsafe fn raw_as_work(ptr: *const Self) -> *mut Work<T, ID> {
         // CAST: Work and work_struct have compatible layouts.
         wrk.cast()
     }
+
+    /// Cancels this delayed work item if it is pending and waits for any running execution to
+    /// finish.
+    ///
+    /// On return, the work item is guaranteed to not be pending or executing as long as there are
+    /// no racing re-enqueues.
+    ///
+    /// # Note
+    ///
+    /// Should be called from a sleepable context if the work was last queued on a non-BH
+    /// workqueue.
+    #[inline]
+    pub fn cancel_sync(&self) -> Option<T::Pointer>
+    where
+        T: WorkItem<ID>,
+        T::Pointer: SupportsCancelling<ID>,
+    {
+        let ptr = self.dwork.get();
+
+        // SAFETY: `ptr` is a valid embedded `delayed_work`.
+        if unsafe { bindings::cancel_delayed_work_sync(ptr) } {
+            // SAFETY: A `true` return means the work was pending and got canceled, so the queued
+            // ownership transfer performed by `__enqueue` is reclaimed here.
+            Some(unsafe { T::Pointer::from_raw_work(core::ptr::addr_of_mut!((*ptr).work)) })
+        } else {
+            None
+        }
+    }
 }
 
 /// Declares that a type contains a [`DelayedWork<T, ID>`].
@@ -871,6 +942,24 @@ unsafe fn __enqueue<F>(self, queue_work_on: F) -> Self::EnqueueOutput
     }
 }
 
+// SAFETY: `from_raw_work()` reconstructs exactly the `Arc<T>` ownership previously transferred by
+// `Arc::into_raw()` in `RawWorkItem::__enqueue`, using the same `Work<T, ID>` field selected by
+// `HasWork<T, ID>`.
+unsafe impl<T, const ID: u64> SupportsCancelling<ID> for Arc<T>
+where
+    T: WorkItem<ID, Pointer = Self>,
+    T: HasWork<T, ID>,
+{
+    unsafe fn from_raw_work(ptr: *mut bindings::work_struct) -> Self {
+        // 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`.
+        let ptr = unsafe { T::work_container_of(ptr) };
+        // SAFETY: This pointer comes from `Arc::into_raw` and we've been given back ownership.
+        unsafe { Arc::from_raw(ptr) }
+    }
+}
+
 // SAFETY: By the safety requirements of `HasDelayedWork`, the `work_struct` returned by methods in
 // `HasWork` provides a `work_struct` that is the `work` field of a `delayed_work`, and the rest of
 // the `delayed_work` has the same access rules as its `work` field.
@@ -1013,6 +1102,33 @@ unsafe fn __enqueue<F>(self, queue_work_on: F) -> Self::EnqueueOutput
     }
 }
 
+// SAFETY: `from_raw_work()` reconstructs exactly the `ARef<T>` ownership previously transferred by
+// `ARef::into_raw()` in `RawWorkItem::__enqueue`, using the same `Work<T, ID>` field selected by
+// `HasWork<T, ID>`.
+unsafe impl<T, const ID: u64> SupportsCancelling<ID> for ARef<T>
+where
+    T: AlwaysRefCounted,
+    T: WorkItem<ID, Pointer = Self>,
+    T: HasWork<T, ID>,
+{
+    unsafe fn from_raw_work(ptr: *mut bindings::work_struct) -> Self {
+        // 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
+        // `ARef::into_raw`.
+        let ptr = unsafe { T::work_container_of(ptr) };
+
+        // SAFETY: The safety contract of `work_container_of` ensures that it
+        // returns a valid non-null pointer.
+        let ptr = unsafe { NonNull::new_unchecked(ptr) };
+
+        // SAFETY: This pointer comes from `ARef::into_raw` and we've been given
+        // back ownership.
+        unsafe { ARef::from_raw(ptr) }
+    }
+}
+
 // SAFETY: By the safety requirements of `HasDelayedWork`, the `work_struct` returned by methods in
 // `HasWork` provides a `work_struct` that is the `work` field of a `delayed_work`, and the rest of
 // the `delayed_work` has the same access rules as its `work` field.
-- 
2.51.2


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

* Re: [PATCH v2 1/1] rust: workqueue: add cancel_sync support
  2026-06-12 19:45 ` [PATCH v2 1/1] " Onur Özkan
@ 2026-06-15  7:08   ` Alice Ryhl
  0 siblings, 0 replies; 3+ messages in thread
From: Alice Ryhl @ 2026-06-15  7:08 UTC (permalink / raw)
  To: Onur Özkan
  Cc: rust-for-linux, linux-kernel, ojeda, boqun, gary, bjorn3_gh,
	lossin, a.hindborg, tmgross, dakr, tamird, daniel.almeida

On Fri, Jun 12, 2026 at 10:45:42PM +0300, Onur Özkan wrote:
> Drivers can use this during teardown to cancel pending work and wait for
> running work to finish before dropping related resources.
> 
> This is not implemented for Pin<KBox<T>> because queuing a boxed work
> item transfers ownership of the box to the workqueue. There is therefore
> no separate safe owner that can cancel the boxed work while it is pending.
> 
> Signed-off-by: Onur Özkan <work@onurozkan.dev>

Overall looks reasonable to me, but some comments below.

>  rust/kernel/workqueue.rs | 116 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 116 insertions(+)
> 
> diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
> index 7e253b6f299c..4d61d7a10fae 100644
> --- a/rust/kernel/workqueue.rs
> +++ b/rust/kernel/workqueue.rs
> @@ -471,6 +471,23 @@ pub trait WorkItem<const ID: u64 = 0> {
>      fn run(this: Self::Pointer);
>  }
>  
> +/// Work item pointers that support cancellation.
> +///
> +/// # Safety
> +///
> +/// Implementers must ensure that `from_raw_work` rebuilds the exact ownership transferred
> +/// by a successful [`RawWorkItem::__enqueue`] call.
> +pub unsafe trait SupportsCancelling<const ID: u64>: WorkItemPointer<ID> + Sized {

Nit; I think it reads nicer as SupportsCancel.

> +    /// Rebuild this work item's pointer from its embedded `work_struct`.
> +    ///
> +    /// # Safety
> +    ///
> +    /// The provided `work_struct` pointer must originate from a previous call to
> +    /// [`RawWorkItem::__enqueue`] where the `queue_work_on` closure returned true
> +    /// and the pointer must still be valid.
> +    unsafe fn from_raw_work(ptr: *mut bindings::work_struct) -> Self;

I think you're missing some condition here about ownership having been
transferred *out* from the workqueue by cancel_sync() or similar
methods. Otherwise this currently says I can call the method even though
the workqueue still owns the work item.

> +    /// # Note
> +    ///
> +    /// Should be called from a sleepable context if the work was last queued on a non-BH
> +    /// workqueue.

Nit: I'd either reword "Note" to something more specific to what the
note is about, or remove the heading.

> +    #[inline]
> +    pub fn cancel_sync(&self) -> Option<T::Pointer>
> +    where
> +        T: WorkItem<ID>,
> +        T::Pointer: SupportsCancelling<ID>,
> +    {
> +        let ptr = self.dwork.get();
> +
> +        // SAFETY: `ptr` is a valid embedded `delayed_work`.
> +        if unsafe { bindings::cancel_delayed_work_sync(ptr) } {
> +            // SAFETY: A `true` return means the work was pending and got canceled, so the queued
> +            // ownership transfer performed by `__enqueue` is reclaimed here.
> +            Some(unsafe { T::Pointer::from_raw_work(core::ptr::addr_of_mut!((*ptr).work)) })

Nit: The addr_of_mut! macro is no longer required. You can do: &raw mut (*ptr).work

Alice

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

end of thread, other threads:[~2026-06-15  7:08 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-12 19:45 [PATCH v2 0/1] rust: workqueue: add cancel_sync support Onur Özkan
2026-06-12 19:45 ` [PATCH v2 1/1] " Onur Özkan
2026-06-15  7:08   ` Alice Ryhl

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