Re: [PATCH v1 1/1] rust: add Work::disable_sync

["Gary Guo" <gary@garyguo.net>]

On Tue Apr 28, 2026 at 11:44 AM BST, Onur Özkan wrote:
> Adds Work::disable_sync() as a safe wrapper for disable_work_sync().
>
> Drivers can use this during teardown to stop new queueing and wait for
> queued or running work to finish before dropping related resources.
>
> Signed-off-by: Onur Özkan <work@onurozkan.dev>
> ---
>  rust/kernel/workqueue.rs | 102 +++++++++++++++++++++++++++++----------
>  1 file changed, 76 insertions(+), 26 deletions(-)
>
> diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
> index 7e253b6f299c..443fc84dbeeb 100644
> --- a/rust/kernel/workqueue.rs
> +++ b/rust/kernel/workqueue.rs
> @@ -442,21 +442,48 @@ 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.
> -///
> -/// [`__enqueue`]: RawWorkItem::__enqueue
> -/// [`run`]: WorkItemPointer::run
> -pub unsafe trait WorkItemPointer<const ID: u64>: RawWorkItem<ID> {
> +/// Implementers must ensure that [`WorkItemPointer::from_raw_work`] rebuilds the exact ownership
> +/// transferred by a successful [`RawWorkItem::__enqueue`] call.
> +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
> +    /// [`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.
>      ///
>      /// # 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 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) },
> +        );
> +    }
> +
> +    /// Reclaim a previously enqueued work item that will no longer run.
> +    ///
> +    /// # Safety
>      ///
> -    /// [`__enqueue`]: RawWorkItem::__enqueue
> -    unsafe extern "C" fn run(ptr: *mut bindings::work_struct);
> +    /// The provided `work_struct` pointer must satisfy the same requirements as
> +    /// [`WorkItemPointer::from_raw_work`].
> +    #[inline]
> +    unsafe fn cancel(ptr: *mut bindings::work_struct) {
> +        drop(
> +            // SAFETY: The requirements for `cancel` are exactly those of `from_raw_work`.
> +            unsafe { Self::from_raw_work(ptr) },
> +        );
> +    }
>  }
>  
>  /// Defines the method that should be called when this work item is executed.
> @@ -537,6 +564,29 @@ 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)) }
>      }
> +
> +    /// Disables this work item and waits for queued/running executions to finish.
> +    ///
> +    /// # Note
> +    ///
> +    /// Should be called from a sleepable context if the work was last queued on a non-BH
> +    /// workqueue.
> +    #[inline]
> +    pub fn disable_sync(&self)
> +    where
> +        T: WorkItem<ID>,
> +    {
> +        let ptr: *const Self = self;
> +        // SAFETY: `self` points to a valid initialized work.
> +        let raw_work = unsafe { Self::raw_get(ptr) };
> +        // SAFETY: `raw_work` is a valid embedded `work_struct`.
> +        if unsafe { bindings::disable_work_sync(raw_work) } {
> +            // SAFETY: A `true` return means the work was pending and got canceled, so the queued
> +            // ownership transfer performed by `__enqueue` must be reclaimed here and the work
> +            // item will not subsequently run.
> +            unsafe { T::Pointer::cancel(raw_work) };

I think have an explicit drop is clearer and remove the need of additional
function in the trait.

    // SAFETY: A `true` return means the work was pending and got canceled, so the queued
    // ownership transfer performed by `__enqueue` is reclaimed here.
    drop(unsafe { T::Pointer::from_raw_work(ptr) })

The `cancel` name is confusing because this is called *after* the work has
already been cancelled.

Best,
Gary

> +        }
> +    }
>  }
>  
>  /// Declares that a type contains a [`Work<T, ID>`].
> @@ -817,22 +867,22 @@ unsafe fn work_container_of(
>  //   - `Work::new` makes sure that `T::Pointer::run` is passed to `init_work_with_key`.
>  //   - Finally `Work` and `RawWorkItem` guarantee that the correct `Work` field
>  //     will be used because of the ID const generic bound. This makes sure that `T::raw_get_work`
> -//     uses the correct offset for the `Work` field, and `Work::new` picks the correct
> -//     implementation of `WorkItemPointer` for `Arc<T>`.
> +//     uses the correct offset for the `Work` field, and `T::Pointer::from_raw_work` rebuilds the
> +//     correct pointer type for `Arc<T>`.
>  unsafe impl<T, const ID: u64> WorkItemPointer<ID> for Arc<T>
>  where
>      T: WorkItem<ID, Pointer = Self>,
>      T: HasWork<T, ID>,
>  {
> -    unsafe extern "C" fn run(ptr: *mut bindings::work_struct) {
> +    type Item = T;
> +
> +    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.
> -        let arc = unsafe { Arc::from_raw(ptr) };
> -
> -        T::run(arc)
> +        unsafe { Arc::from_raw(ptr) }
>      }
>  }
>  
> @@ -887,7 +937,9 @@ unsafe impl<T, const ID: u64> WorkItemPointer<ID> for Pin<KBox<T>>
>      T: WorkItem<ID, Pointer = Self>,
>      T: HasWork<T, ID>,
>  {
> -    unsafe extern "C" fn run(ptr: *mut bindings::work_struct) {
> +    type Item = T;
> +
> +    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`.
> @@ -895,9 +947,7 @@ unsafe impl<T, const ID: u64> WorkItemPointer<ID> for Pin<KBox<T>>
>          // SAFETY: This pointer comes from `Arc::into_raw` and we've been given back ownership.
>          let boxed = unsafe { KBox::from_raw(ptr) };
>          // SAFETY: The box was already pinned when it was enqueued.
> -        let pinned = unsafe { Pin::new_unchecked(boxed) };
> -
> -        T::run(pinned)
> +        unsafe { Pin::new_unchecked(boxed) }
>      }
>  }
>  
> @@ -950,15 +1000,17 @@ unsafe impl<T, const ID: u64> RawDelayedWorkItem<ID> for Pin<KBox<T>>
>  //   - `Work::new` makes sure that `T::Pointer::run` is passed to `init_work_with_key`.
>  //   - Finally `Work` and `RawWorkItem` guarantee that the correct `Work` field
>  //     will be used because of the ID const generic bound. This makes sure that `T::raw_get_work`
> -//     uses the correct offset for the `Work` field, and `Work::new` picks the correct
> -//     implementation of `WorkItemPointer` for `ARef<T>`.
> +//     uses the correct offset for the `Work` field, and `T::Pointer::from_raw_work` rebuilds the
> +//     correct pointer type for `ARef<T>`.
>  unsafe impl<T, const ID: u64> WorkItemPointer<ID> for ARef<T>
>  where
>      T: AlwaysRefCounted,
>      T: WorkItem<ID, Pointer = Self>,
>      T: HasWork<T, ID>,
>  {
> -    unsafe extern "C" fn run(ptr: *mut bindings::work_struct) {
> +    type Item = T;
> +
> +    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>>();
>  
> @@ -972,9 +1024,7 @@ unsafe impl<T, const ID: u64> WorkItemPointer<ID> for ARef<T>
>  
>          // SAFETY: This pointer comes from `ARef::into_raw` and we've been given
>          // back ownership.
> -        let aref = unsafe { ARef::from_raw(ptr) };
> -
> -        T::run(aref)
> +        unsafe { ARef::from_raw(ptr) }
>      }
>  }
>