Re: [PATCH v1] rust: workqueue: add ScopedQueue for lifetime bound items

["Danilo Krummrich" <dakr@kernel.org>]

On Mon Jun 15, 2026 at 1:56 PM CEST, Onur Özkan wrote:
> Add a workqueue wrapper for work items that are not 'static.
>
> Tyr reset work is queued from a handle that owns a Controller<'bound>
> where the work item holds references tied to the lifetime of the bound
> device and its mapped IO state. The existing API only accepts 'static
> work items which cannot express that relationship.
>
> Introduce ScopedQueue for this case. It owns the underlying workqueue
> and ties enqueued work to the queue lifetime so borrowed state cannot
> outlive the queue that may still run it.
>
> Construction is unsafe because the queue must not be leaked.
>
> `compile_fail` doc-tests are ignored for now as KUnit doesn't support
> that. Enabling those tests as regular code block would raise this error:
>
>     ERROR:root:error[E0597]: `data` does not live long enough
> 	 --> rust/doctests_kernel_generated.rs:22029:44
> 	  |
>     22027 |         let data = ();
> 	  |             ---- binding `data` declared here
>     22028 |         // SAFETY: Queue is not leaked.
>     22029 |         queue = unsafe { new_queue_with_lt(&data)? };
> 	  |                                            ^^^^^ borrowed value does not live long enough
>     22030 |     }
> 	  |     - `data` dropped here while still borrowed
>     ...
>     22034 | }
> 	  | - borrow might be used here, when `queue` is dropped and runs the `Drop` code for type `ScopedQueue`
> 	  |
> 	  = note: values in a scope are dropped in the opposite order they are defined
>
> which is exactly the constraint ScopedQueue is meant to enforce.
>
> This series is based on Alice's "Creation of workqueues in Rust" [1]
> series.
>
> Link: https://lore.kernel.org/all/20260312-create-workqueue-v4-0-ea39c351c38f@google.com [1]
> Signed-off-by: Onur Özkan <work@onurozkan.dev>

Suggested-by: Danilo Krummrich <dakr@kernel.org>

> +/// An owned workqueue that can enqueue work items borrowing from `'scope`.
> +///
> +/// A `ScopedQueue` must not outlive data borrowed by its work items.
> +pub struct ScopedQueue<'scope> {
> +    inner: OwnedQueue,
> +    _scope: PhantomData<&'scope mut &'scope ()>,
> +}
> +
> +impl<'scope> ScopedQueue<'scope> {
> +    /// Creates an ordered scoped workqueue.
> +    ///
> +    /// # Safety
> +    ///
> +    /// The caller must not leak the returned queue or otherwise prevent its
> +    /// [`Drop`] implementation from running since dropping the queue drains
> +    /// pending and running work that may borrow from `'scope`.
> +    pub unsafe fn new_ordered_with_lt(name: &'static CStr) -> Result<Self> {

I think the with_lt naming is a bit redundant; the ScopedQueue name and it's
lifetime already implies that, so I think ScopedQueue::new_ordered() is
sufficient.

Also note that we only use the with_lt suffix when there's also a 'static
version called new().

> +        Ok(Self {
> +            inner: Queue::new_ordered().build(name)?,
> +            _scope: PhantomData,
> +        })
> +    }
> +
> +    /// Enqueues a work item on this scoped queue.
> +    pub fn enqueue<W, const ID: u64>(&self, work: W) -> W::EnqueueOutput
> +    where
> +        W: RawWorkItem<ID> + Send + 'scope,
> +    {
> +        let queue_ptr = self.inner.0.get();
> +
> +        // SAFETY: `W: 'scope` and dropck keep borrowed data alive until this
> +        // queue is dropped. The constructor requires that the queue is not
> +        // leaked and dropping `inner` drains pending and running work.

I think it is covered implicitly, but maybe we can spell out more explicitly how
this requirement is justified:

	/// If the work item type is annotated with any lifetimes, then you must not call the function
	/// pointer after any such lifetime expires. (Never calling the function pointer is okay.)

Also, I think __enqueue() has three distinct safety requirements, so I think it
would be good to address them with separate bullet points.

> +        unsafe {
> +            work.__enqueue(move |work_ptr| {
> +                bindings::queue_work_on(
> +                    bindings::wq_misc_consts_WORK_CPU_UNBOUND as ffi::c_int,
> +                    queue_ptr,
> +                    work_ptr,
> +                )
> +            })
> +        }
> +    }
> +}