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

[Onur Özkan]

On Wed, 17 Jun 2026 15:10:30 +0200
Danilo Krummrich <dakr@xxxxxxxxxx> wrote:

> 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@xxxxxxxxxx [1]
> > Signed-off-by: Onur Özkan <work@xxxxxxxxxxxxx>
> 
> Suggested-by: Danilo Krummrich <dakr@xxxxxxxxxx>
> 
> > +/// 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().

Perhaps new() alone would be better? ScopedQueue can only work with ordered queue
anyway.

Onur

> 
> > +        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,
> > +                )
> > +            })
> > +        }
> > +    }
> > +}