Re: [PATCH v2 6/6] rust: workqueue: add ScopedWork for non-'static work items

[Alice Ryhl <aliceryhl@google.com>]

On Fri, Aug 07, 2026 at 06:52:49PM +0200, Danilo Krummrich wrote:
> Add ScopedWork<T>, a work item wrapper whose destructor calls
> cancel_work_sync(), allowing T to carry non-'static lifetimes. Ownership
> of the data is not transferred to the workqueue; instead, the
> synchronous cancellation on drop guarantees the work function is not
> running when the data is freed.
> 
> ScopedWork uses the existing Work/HasWork/WorkItem infrastructure with
> NonNull<ScopedWorkRef<T>> as WorkItem::Pointer for the callback path,
> and implements RawWorkItem for &ScopedWork<T> and &ScopedWorkRef<T> for
> the enqueue path (requiring T: Sync for cross-thread shared access
> safety).
> 
> Two enqueue paths are provided:
>   - Queue::enqueue_scoped() (unsafe): the caller must ensure the work
>     item is not forgotten.
>   - ScopedQueue::enqueue() (safe): when the work item's lifetime
>     satisfies the queue's 'scope bound.
> 
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
>      pub fn enqueue<W, const ID: u64>(&self, w: W) -> W::EnqueueOutput
>      where
>          W: RawWorkItem<ID> + Send + 'static,
> [...]
> +    pub unsafe fn enqueue_scoped<W, const ID: u64>(&self, w: W) -> W::EnqueueOutput
> +    where
> +        W: RawWorkItem<ID> + Send,

I'm not convinced that enqueue_scoped() should be the unsafe operation.
Rather, I think that should be the constructor of ScopedWork. If we
promise to not forget it in ::new(), we can make this safe.

> +impl Deref for ScopedQueue<'_> {
> +    type Target = Queue;
> +
> +    #[inline]
> +    fn deref(&self) -> &Queue {
> +        &self.inner
> +    }
> +}

Should be part of previous patch.

> +/// The work function's view of a [`ScopedWork`] item.
> +///
> +/// The work function callback receives `&ScopedWorkRef<T>`, which [`Deref`]s to `&T` and can be
> +/// passed to queue enqueue methods for re-enqueueing from within the work function.
> +#[pin_data]
> +pub struct ScopedWorkRef<T: ScopedWorkItem> {

Structs with 'Ref' in their name sound like they are a
reference/pointer, but this struct has no indirection.

> +impl<T: ScopedWorkItem> WorkItem for ScopedWorkRef<T> {
> +    type Pointer = NonNull<Self>;
> +
> +    #[inline]
> +    fn run(this: NonNull<Self>) {
> +        // SAFETY: `this` points to a valid, pinned `ScopedWorkRef`. `cancel_work_sync()` in
> +        // `ScopedWork`'s `PinnedDrop` prevents use-after-drop.
> +        let work = unsafe { &*this.as_ptr() };
> +
> +        T::run(work);
> +    }
> +}

This is unsound because its a safe function that dereferences a raw
pointer.

I think you can avoid all of this logic by just not implementing
WorkItem. You can skip all of that and just implement RawWorkItem for
&ScopedWorkRef<T> and remove all the other implementations. You do not
need the WorkItem impl.

Alice