Rust for Linux List
 help / color / mirror / Atom feed
From: "Onur Özkan" <work@onurozkan.dev>
To: Alice Ryhl <aliceryhl@google.com>
Cc: linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org,
	tj@kernel.org, jiangshanlai@gmail.com, ojeda@kernel.org,
	boqun@kernel.org, gary@garyguo.net, bjorn3_gh@protonmail.com,
	lossin@kernel.org, a.hindborg@kernel.org, tmgross@umich.edu,
	dakr@kernel.org
Subject: Re: [PATCH v1] rust: workqueue: add ScopedQueue for lifetime bound items
Date: Wed, 17 Jun 2026 15:49:03 +0300	[thread overview]
Message-ID: <20260617124911.90218-1-work@onurozkan.dev> (raw)
In-Reply-To: <CAH5fLggQDAQ02YLibAO=NpQF8xxj_0GCEz5+cVZh8LmYo+xXvw@mail.gmail.com>

On Mon, 15 Jun 2026 17:18:02 +0200
Alice Ryhl <aliceryhl@google.com> wrote:

> On Mon, Jun 15, 2026 at 1:56 PM Onur Özkan <work@onurozkan.dev> 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>
> > ---
> >  rust/kernel/workqueue/mod.rs          |   3 +
> >  rust/kernel/workqueue/scoped_queue.rs | 179 ++++++++++++++++++++++++++
> >  2 files changed, 182 insertions(+)
> >  create mode 100644 rust/kernel/workqueue/scoped_queue.rs
> >
> > diff --git a/rust/kernel/workqueue/mod.rs b/rust/kernel/workqueue/mod.rs
> > index e30c21214a81..5e2baa84c143 100644
> > --- a/rust/kernel/workqueue/mod.rs
> > +++ b/rust/kernel/workqueue/mod.rs
> > @@ -212,6 +212,9 @@
> >  mod builder;
> >  pub use self::builder::Builder;
> >
> > +mod scoped_queue;
> > +pub use self::scoped_queue::ScopedQueue;
> > +
> >  /// Creates a [`Work`] initialiser with the given name and a newly-created lock class.
> >  #[macro_export]
> >  macro_rules! new_work {
> > diff --git a/rust/kernel/workqueue/scoped_queue.rs b/rust/kernel/workqueue/scoped_queue.rs
> > new file mode 100644
> > index 000000000000..e0457b521a19
> > --- /dev/null
> > +++ b/rust/kernel/workqueue/scoped_queue.rs
> > @@ -0,0 +1,179 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +//! Lifetime-scoped workqueues.
> > +//!
> > +//! Provides [`ScopedQueue`] for work items that may borrow data with some
> > +//! non-`'static` lifetime.
> > +//!
> > +//! Unlike [`Queue`] which only accepts `'static` work items, [`ScopedQueue`]
> > +//! owns its underlying queue and relies on that queue being dropped to drain
> > +//! pending and running work before borrowed data can go out of scope.
> > +//!
> > +//! TODO: Remove `ignore` once KUnit supports `compile_fail` on doc-tests.
> > +//! ```compile_fail,ignore
> > +//! use kernel::prelude::*;
> > +//! use kernel::workqueue::ScopedQueue;
> > +//!
> > +//! /// # Safety
> > +//! ///
> > +//! /// Returned queue must not be leaked.
> > +//! unsafe fn new_queue_with_lt<'bound>(_: &'bound ()) -> Result<ScopedQueue<'bound>> {
> > +//!     // SAFETY: Caller guarantees that the returned queue is not leaked.
> > +//!     unsafe { ScopedQueue::new_ordered_with_lt(c"scoped_queue") }
> > +//! }
> > +//!
> > +//! fn queue_outlives_borrowed_data() -> Result {
> > +//!     let queue;
> > +//!
> > +//!     {
> > +//!         let data = ();
> > +//!         // SAFETY: Queue is not leaked.
> > +//!         queue = unsafe { new_queue_with_lt(&data)? };
> > +//!     }
> > +//!     // Here the `compile_fail` is fulfilled as `queue` would be dropped
> > +//!     // after `data`.
> > +//!     Ok(())
> > +//! }
> > +//! ```
> > +//!
> > +//! TODO: Remove `ignore` once KUnit supports `compile_fail` on doc-tests.
> > +//! ```compile_fail,ignore
> > +//! use kernel::prelude::*;
> > +//! use kernel::sync::Arc;
> > +//! use kernel::workqueue::{
> > +//!     impl_has_work,
> > +//!     new_work,
> > +//!     ScopedQueue,
> > +//!     Work,
> > +//!     WorkItem,
> > +//! };
> > +//!
> > +//! #[pin_data]
> > +//! struct BorrowedWork<'bound> {
> > +//!     data: &'bound (),
> > +//!     #[pin]
> > +//!     work: Work<BorrowedWork<'bound>>,
> > +//! }
> > +//!
> > +//! impl_has_work! {
> > +//!     impl{'bound} HasWork<BorrowedWork<'bound>> for BorrowedWork<'bound> { self.work }
> > +//! }
> > +//!
> > +//! impl<'bound> WorkItem for BorrowedWork<'bound> {
> > +//!     type Pointer = Arc<Self>;
> > +//!
> > +//!     fn run(_this: Arc<Self>) {}
> > +//! }
> > +//!
> > +//! impl<'bound> BorrowedWork<'bound> {
> > +//!     fn new(data: &'bound ()) -> Result<Arc<Self>> {
> > +//!         Arc::pin_init(
> > +//!             pin_init!(Self {
> > +//!                 data,
> > +//!                 work <- new_work!("BorrowedWork::work"),
> > +//!             }),
> > +//!             GFP_KERNEL,
> > +//!         )
> > +//!     }
> > +//! }
> > +//!
> > +//! struct Handle<'bound> {
> > +//!     work: Arc<BorrowedWork<'bound>>,
> > +//!     wq: ScopedQueue<'bound>,
> > +//! }
> > +//!
> > +//! impl<'bound> Handle<'bound> {
> > +//!     /// # Safety
> > +//!     ///
> > +//!     /// Returned handle must not be leaked.
> > +//!     unsafe fn new_with_lt(data: &'bound ()) -> Result<Self> {
> > +//!         Ok(Self {
> > +//!             work: BorrowedWork::new(data)?,
> > +//!             // SAFETY: Caller guarantees that the returned handle is not leaked.
> > +//!             wq: unsafe { ScopedQueue::new_ordered_with_lt(c"handle_wq")? },
> > +//!         })
> > +//!     }
> > +//! }
> > +//!
> > +//! fn handle_outlives_borrowed_data() -> Result {
> > +//!     let handle;
> > +//!
> > +//!     {
> > +//!         let data = ();
> > +//!         // SAFETY: Handle is not leaked.
> > +//!         handle = unsafe { Handle::new_with_lt(&data)? };
> > +//!
> > +//!         let _ = handle.wq.enqueue(handle.work.clone());
> > +//!     }
> > +//!     // Here the `compile_fail` is fulfilled as `handle` would be dropped
> > +//!     // after `data`.
> > +//!     Ok(())
> > +//! }
> > +//! ```
> > +
> > +use super::{
> > +    OwnedQueue,
> > +    Queue,
> > +    RawWorkItem, //
> > +};
> > +
> > +use crate::{
> > +    bindings,
> > +    ffi,
> > +    prelude::*, //
> > +};
> > +
> > +use core::marker::PhantomData;
> > +
> > +/// 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 ()>,
> 
> I think this can be contra-variant:
> 
> _scope: PhantomData<fn(&'scope ())>,

With that, lifetime of the queue is not restricted enough i.e. can live longer
than expected.

For instance, module doc-test should fail to compile when removing
`compile_fail,ignore` but it doesn't with your suggestion.

Thanks,
Onur

> 
> Alice

  reply	other threads:[~2026-06-17 12:49 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-15 11:56 [PATCH v1] rust: workqueue: add ScopedQueue for lifetime bound items Onur Özkan
2026-06-15 15:18 ` Alice Ryhl
2026-06-17 12:49   ` Onur Özkan [this message]
2026-06-17 14:08   ` Gary Guo
2026-06-17 13:10 ` Danilo Krummrich
2026-06-17 13:21   ` Onur Özkan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260617124911.90218-1-work@onurozkan.dev \
    --to=work@onurozkan.dev \
    --cc=a.hindborg@kernel.org \
    --cc=aliceryhl@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun@kernel.org \
    --cc=dakr@kernel.org \
    --cc=gary@garyguo.net \
    --cc=jiangshanlai@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tj@kernel.org \
    --cc=tmgross@umich.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox