Re: [PATCH v1] rust: workqueue: add cancel_sync support

[Onur Özkan]

On Sun, 10 May 2026 13:43:59 +0000
Alice Ryhl <aliceryhl@xxxxxxxxxx> wrote:

> On Sun, May 10, 2026 at 11:21:57AM +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.
> > 
> > The immediate motivation is the Tyr reset infrastructure [1], which needs
> > to cancel pending reset work and wait for any running reset work during
> > teardown before dropping the resources used by that work.
> > 
> > [1]: https://lore.kernel.org/all/20260416171728.205141-1-work@xxxxxxxxxxxxx
> > 
> > Signed-off-by: Onur Özkan <work@xxxxxxxxxxxxx>
> > ---
> >  rust/kernel/workqueue.rs | 134 ++++++++++++++++++++++++++++++++-------
> >  1 file changed, 112 insertions(+), 22 deletions(-)
> > 
> > diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
> > index 7e253b6f299c..a10daa2763ac 100644
> > --- a/rust/kernel/workqueue.rs
> > +++ b/rust/kernel/workqueue.rs
> > @@ -442,23 +442,44 @@ pub unsafe trait RawDelayedWorkItem<const ID: u64>: RawWorkItem<ID> {}
> >  ///
> >  /// # Safety
> >  ///
> > -/// Implementers must ensure that [`__enqueue`] uses a `work_struct` initialized with the [`run`]
> > -/// method of this trait as the function pointer.
> > +/// Implementers must ensure that [`__enqueue`] uses a `work_struct` initialized with [`run`] as
> > +/// its function pointer, and that [`from_raw_work`] rebuilds the exact ownership transferred by
> > +/// a successful [`__enqueue`] call.
> >  ///
> >  /// [`__enqueue`]: RawWorkItem::__enqueue
> > +/// [`from_raw_work`]: WorkItemPointer::from_raw_work
> >  /// [`run`]: WorkItemPointer::run
> > -pub unsafe trait WorkItemPointer<const ID: u64>: RawWorkItem<ID> {
> > -    /// Run this work item.
> > +pub unsafe trait WorkItemPointer<const ID: u64>: RawWorkItem<ID> + Sized {
> > +    /// The work item type containing the embedded `work_struct`.
> > +    type Item: WorkItem<ID, Pointer = Self> + ?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 [`__enqueue`]
> > -    /// where the `queue_work_on` closure returned true, and the pointer must still be valid.
> > +    /// 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;
> > +
> > +    /// Run this work item.
> >      ///
> > -    /// [`__enqueue`]: RawWorkItem::__enqueue
> > -    unsafe extern "C" fn run(ptr: *mut bindings::work_struct);
> > +    /// # Safety
> > +    ///
> > +    /// The provided `work_struct` pointer must satisfy the same requirements as
> > +    /// [`WorkItemPointer::from_raw_work`].
> > +    #[inline]
> > +    unsafe extern "C" fn run(ptr: *mut bindings::work_struct) {
> > +        <Self::Item as WorkItem<ID>>::run(
> > +            // SAFETY: The requirements for `run` are exactly those of `from_raw_work`.
> > +            unsafe { Self::from_raw_work(ptr) },
> > +        );
> > +    }
> >  }
> >  
> > +/// Marker for work item types that support cancellation.
> > +pub trait SupportsCancelling<const ID: u64>: WorkItemPointer<ID> {}
> 
> Shouldn't 'from_raw_work()' be a method on SupportsCancelling instead?

Not necessarily. I wanted to keep it like that because it removes a bit of
duplication and makes run() simpler. It's also more extensible e.g. can be
used for disable_sync too if needed in the future.

-Onur

> 
> Alice