[PATCH v4 0/3] Creation of workqueues in Rust

[PATCH v4 0/3] Creation of workqueues in Rust

[Alice Ryhl]

GPU drivers often need to create their own workqueues for various
reasons. Add the ability to do so.

Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
Changes in v4:
- Add link to delayed work fix.
- Redo workqueue creation to prevent invalid configurations.
- Introduce a directory as workqueue.rs was getting really large.
- Link to v3: https://lore.kernel.org/r/20260227-create-workqueue-v3-0-87de133f7849@google.com

Changes in v3:
- Switch to builder pattern.
- Drop BH workqueues for now.
- Mark delayed wq change as fix.
- Link to v2: https://lore.kernel.org/r/20251113-create-workqueue-v2-0-8b45277119bc@google.com

Changes in v2:
- Redo how flagging works.
- Restrict delayed work to not be usable on custom workqueues.
- Link to v1: https://lore.kernel.org/r/20250411-create-workqueue-v1-1-f7dbe7f1e05f@google.com

---
Alice Ryhl (3):
      rust: workqueue: restrict delayed work to global wqs
      rust: workqueue: create workqueue subdirectory
      rust: workqueue: add creation of workqueues

 MAINTAINERS                                    |   1 +
 rust/helpers/workqueue.c                       |   7 +
 rust/kernel/workqueue/builder.rs               | 380 +++++++++++++++++++++++++
 rust/kernel/{workqueue.rs => workqueue/mod.rs} |  53 +++-
 4 files changed, 437 insertions(+), 4 deletions(-)
---
base-commit: df9c51269a5e2a6fbca2884a756a4011a5e78748
change-id: 20250411-create-workqueue-d053158c7a4b

Best regards,
-- 
Alice Ryhl <aliceryhl@google.com>

[PATCH v4 1/3] rust: workqueue: restrict delayed work to global wqs

[Alice Ryhl]

When a workqueue is shut down, delayed work that is pending but not
scheduled does not get properly cleaned up, so it's not safe to use
`enqueue_delayed` on a workqueue that might be destroyed. To fix this,
restricted `enqueue_delayed` to static queues.

This may be fixed in the future by an approach along the lines of [1].

Cc: stable@vger.kernel.org
Fixes: 7c098cd5eaae ("workqueue: rust: add delayed work items")
Reviewed-by: John Hubbard <jhubbard@nvidia.com>
Reviewed-by: Danilo Krummrich <dakr@kernel.org>
Reviewed-by: Gary Guo <gary@garyguo.net>
Link: https://lore.kernel.org/r/20250423-destroy-workqueue-flush-v1-1-3d74820780a5@google.com [1]
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 rust/kernel/workqueue.rs | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
index 706e833e9702..1acd113c04ee 100644
--- a/rust/kernel/workqueue.rs
+++ b/rust/kernel/workqueue.rs
@@ -296,8 +296,15 @@ pub fn enqueue<W, const ID: u64>(&self, w: W) -> W::EnqueueOutput
     ///
     /// This may fail if the work item is already enqueued in a workqueue.
     ///
+    /// This is only valid for global workqueues (with static lifetimes) because those are the only
+    /// ones that outlive all possible delayed work items.
+    ///
     /// The work item will be submitted using `WORK_CPU_UNBOUND`.
-    pub fn enqueue_delayed<W, const ID: u64>(&self, w: W, delay: Jiffies) -> W::EnqueueOutput
+    pub fn enqueue_delayed<W, const ID: u64>(
+        &'static self,
+        w: W,
+        delay: Jiffies,
+    ) -> W::EnqueueOutput
     where
         W: RawDelayedWorkItem<ID> + Send + 'static,
     {

-- 
2.53.0.473.g4a7958ca14-goog

[PATCH v4 2/3] rust: workqueue: create workqueue subdirectory

[Alice Ryhl]

The following patch will implement a workqueue builder in a separate
file. To prepare for that, create a rust/kernel/workqueue subdirectory
and move the existing file.

Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 MAINTAINERS                                    | 1 +
 rust/kernel/{workqueue.rs => workqueue/mod.rs} | 0
 2 files changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 8fb7b70dfb47..8a12db6bd127 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -28511,6 +28511,7 @@ F:	Documentation/core-api/workqueue.rst
 F:	include/linux/workqueue.h
 F:	kernel/workqueue.c
 F:	kernel/workqueue_internal.h
+F:	rust/kernel/workqueue/
 
 WWAN DRIVERS
 M:	Loic Poulain <loic.poulain@oss.qualcomm.com>
diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue/mod.rs
similarity index 100%
rename from rust/kernel/workqueue.rs
rename to rust/kernel/workqueue/mod.rs

-- 
2.53.0.473.g4a7958ca14-goog

[PATCH v4 3/3] rust: workqueue: add creation of workqueues

[Alice Ryhl]

Creating workqueues is needed by various GPU drivers. Not only does it
give you better control over execution, it also allows devices to ensure
that all tasks have exited before the device is unbound (or similar) by
running the workqueue destructor.

Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 rust/helpers/workqueue.c         |   7 +
 rust/kernel/workqueue/builder.rs | 380 +++++++++++++++++++++++++++++++++++++++
 rust/kernel/workqueue/mod.rs     |  44 ++++-
 3 files changed, 428 insertions(+), 3 deletions(-)

diff --git a/rust/helpers/workqueue.c b/rust/helpers/workqueue.c
index ce1c3a5b2150..e4b9d1b3d6bf 100644
--- a/rust/helpers/workqueue.c
+++ b/rust/helpers/workqueue.c
@@ -14,3 +14,10 @@ __rust_helper void rust_helper_init_work_with_key(struct work_struct *work,
 	INIT_LIST_HEAD(&work->entry);
 	work->func = func;
 }
+
+__rust_helper
+struct workqueue_struct *rust_helper_alloc_workqueue(const char *fmt, unsigned int flags,
+						     int max_active, const void *data)
+{
+	return alloc_workqueue(fmt, flags, max_active, data);
+}
diff --git a/rust/kernel/workqueue/builder.rs b/rust/kernel/workqueue/builder.rs
new file mode 100644
index 000000000000..d4d77b96f9c4
--- /dev/null
+++ b/rust/kernel/workqueue/builder.rs
@@ -0,0 +1,380 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Workqueue builders.
+
+use kernel::{
+    alloc::AllocError,
+    prelude::*,
+    workqueue::{
+        OwnedQueue, //
+        Queue,
+    }, //
+};
+
+use core::{
+    marker::PhantomData, //
+    ptr::{self, NonNull},
+};
+
+/// Workqueue builder.
+///
+/// A valid combination of workqueue flags contains one of the base flags (`WQ_UNBOUND`, `WQ_BH`,
+/// or `WQ_PERCPU`) and a combination of modifier flags that are compatible with the selected base
+/// flag.
+///
+/// For details, please refer to `Documentation/core-api/workqueue.rst`.
+pub struct Builder<T> {
+    flags: bindings::wq_flags,
+    max_active: i32,
+    _type: PhantomData<T>,
+}
+
+pub enum TypeUnbound {}
+pub enum TypePercpu {}
+pub enum TypePowerEfficient {}
+pub enum TypeBH {}
+pub enum TypeOrdered {}
+
+/// Entry-points to the builder API.
+impl Queue {
+    /// Build a workqueue whose work may execute on any cpu.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use kernel::workqueue::Queue;
+    ///
+    /// let wq = Queue::new_unbound().build(c"my-wq")?;
+    /// wq.try_spawn(GFP_KERNEL, || pr_info!("Hello from unbound wq"))?;
+    /// # Ok::<(), Error>(())
+    /// ```
+    #[inline]
+    #[doc(alias = "WQ_UNBOUND")]
+    pub fn new_unbound() -> Builder<TypeUnbound> {
+        Builder {
+            flags: bindings::wq_flags_WQ_UNBOUND,
+            max_active: 0,
+            _type: PhantomData,
+        }
+    }
+
+    /// Build a workqueue whose work is bound to a specific cpu.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use kernel::workqueue::Queue;
+    ///
+    /// let wq = Queue::new_percpu().build(c"my-wq")?;
+    /// wq.try_spawn(GFP_KERNEL, || pr_info!("Hello from percpu wq"))?;
+    /// # Ok::<(), Error>(())
+    /// ```
+    #[inline]
+    #[doc(alias = "WQ_PERCPU")]
+    pub fn new_percpu() -> Builder<TypePercpu> {
+        Builder {
+            flags: bindings::wq_flags_WQ_PERCPU,
+            max_active: 0,
+            _type: PhantomData,
+        }
+    }
+
+    /// Build a power-efficient workqueue.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use kernel::workqueue::Queue;
+    ///
+    /// let wq = Queue::new_power_efficient().build(c"my-wq")?;
+    /// wq.try_spawn(GFP_KERNEL, || pr_info!("Hello from power-efficient wq"))?;
+    /// # Ok::<(), Error>(())
+    /// ```
+    #[inline]
+    #[doc(alias = "WQ_POWER_EFFICIENT")]
+    pub fn new_power_efficient() -> Builder<TypePowerEfficient> {
+        Builder {
+            flags: bindings::wq_flags_WQ_POWER_EFFICIENT,
+            max_active: 0,
+            _type: PhantomData,
+        }
+    }
+
+    /// Build a single-threaded workqueue that executes jobs in order.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use kernel::workqueue::Queue;
+    ///
+    /// let wq = Queue::new_ordered().build(c"my-wq")?;
+    /// wq.try_spawn(GFP_KERNEL, || pr_info!("Hello from ordered wq"))?;
+    /// # Ok::<(), Error>(())
+    /// ```
+    #[inline]
+    #[doc(alias = "alloc_ordered_workqueue")]
+    #[doc(alias = "__WQ_ORDERED")]
+    pub fn new_ordered() -> Builder<TypeOrdered> {
+        Builder {
+            flags: bindings::wq_flags_WQ_UNBOUND | bindings::wq_flags___WQ_ORDERED,
+            max_active: 0,
+            _type: PhantomData,
+        }
+    }
+
+    /// Build a workqueue that executes in bottom-half (softirq) context.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use kernel::workqueue::Queue;
+    ///
+    /// let wq = Queue::new_bh().build(c"my-wq")?;
+    /// wq.try_spawn(GFP_KERNEL, || pr_info!("Hello from BH wq"))?;
+    /// # Ok::<(), Error>(())
+    /// ```
+    #[inline]
+    #[doc(alias = "WQ_BH")]
+    pub fn new_bh() -> Builder<TypeBH> {
+        Builder {
+            flags: bindings::wq_flags_WQ_BH,
+            max_active: 0,
+            _type: PhantomData,
+        }
+    }
+}
+
+/// Options that may be used with all workqueue types.
+impl<T> Builder<T> {
+    /// Mark this workqueue high priority.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use kernel::workqueue::Queue;
+    ///
+    /// let wq = Queue::new_unbound().highpri().build(c"my-wq")?;
+    /// wq.try_spawn(GFP_KERNEL, || pr_info!("Hello from highpri wq"))?;
+    /// # Ok::<(), Error>(())
+    /// ```
+    #[inline]
+    #[doc(alias = "WQ_HIGHPRI")]
+    pub fn highpri(mut self) -> Self {
+        self.flags |= bindings::wq_flags_WQ_HIGHPRI;
+        self
+    }
+
+    /// Creates the workqueue.
+    ///
+    /// The provided name is used verbatim as the workqueue name.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use kernel::workqueue::Queue;
+    ///
+    /// // create an unbound workqueue registered with sysfs
+    /// let wq = Queue::new_unbound().sysfs().build(c"my-wq")?;
+    ///
+    /// // spawn a work item on it
+    /// wq.try_spawn(
+    ///     GFP_KERNEL,
+    ///     || pr_warn!("Printing from my-wq"),
+    /// )?;
+    /// # Ok::<(), Error>(())
+    /// ```
+    #[inline]
+    #[doc(alias = "alloc_workqueue")]
+    pub fn build(self, name: &CStr) -> Result<OwnedQueue, AllocError> {
+        // SAFETY:
+        // * c"%s" is compatible with passing the name as a c-string.
+        // * the builder only permits valid flag combinations
+        let ptr = unsafe {
+            bindings::alloc_workqueue(
+                c"%s".as_char_ptr(),
+                self.flags,
+                self.max_active,
+                name.as_char_ptr().cast::<c_void>(),
+            )
+        };
+
+        // INVARIANT: We successfully created the workqueue, so we can return ownership to the
+        // caller.
+        Ok(OwnedQueue {
+            queue: NonNull::new(ptr).ok_or(AllocError)?.cast(),
+        })
+    }
+
+    /// Creates the workqueue.
+    ///
+    /// # Examples
+    ///
+    /// This example shows how to pass a Rust string formatter to the workqueue name, creating
+    /// workqueues with names such as `my-wq-1` and `my-wq-2`.
+    ///
+    /// ```
+    /// use kernel::workqueue::{Queue, OwnedQueue};
+    ///
+    /// fn my_wq(num: u32) -> Result<OwnedQueue> {
+    ///     // create a percpu workqueue called my-wq-{num}
+    ///     let wq = Queue::new_percpu().build_fmt(fmt!("my-wq-{num}"))?;
+    ///     Ok(wq)
+    /// }
+    /// ```
+    #[inline]
+    pub fn build_fmt(self, name: kernel::fmt::Arguments<'_>) -> Result<OwnedQueue, AllocError> {
+        // SAFETY:
+        // * c"%pA" is compatible with passing an `Arguments` pointer.
+        // * the builder only permits valid flag combinations
+        let ptr = unsafe {
+            bindings::alloc_workqueue(
+                c"%pA".as_char_ptr(),
+                self.flags,
+                self.max_active,
+                ptr::from_ref(&name).cast::<c_void>(),
+            )
+        };
+
+        // INVARIANT: We successfully created the workqueue, so we can return ownership to the
+        // caller.
+        Ok(OwnedQueue {
+            queue: NonNull::new(ptr).ok_or(AllocError)?.cast(),
+        })
+    }
+}
+
+/// Indicates that this workqueue is threaded.
+pub trait TypeThreaded {}
+impl TypeThreaded for TypeUnbound {}
+impl TypeThreaded for TypePercpu {}
+impl TypeThreaded for TypePowerEfficient {}
+
+/// Options that are not available on BH or ordered workqueues.
+impl<T: TypeThreaded> Builder<T> {
+    /// Set the maximum number of active cpus.
+    ///
+    /// If not set, a default value of `WQ_DFL_ACTIVE` is used. The maximum value is
+    /// `WQ_MAX_ACTIVE`.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use kernel::workqueue::Queue;
+    ///
+    /// let wq = Queue::new_unbound().max_active(16).build(c"my-wq")?;
+    /// wq.try_spawn(GFP_KERNEL, || pr_info!("Hello from wq with max_active=16"))?;
+    /// # Ok::<(), Error>(())
+    /// ```
+    #[inline]
+    pub fn max_active(mut self, max_active: u32) -> Self {
+        // If provided `max_active` is greater than `i32::MAX`, then we need to trigger the C-side
+        // comparison with `WQ_MAX_ACTIVE`, which we can do by clamping to `i32::MAX`.
+        self.max_active = i32::try_from(max_active).unwrap_or(i32::MAX);
+        self
+    }
+
+    /// Mark this workqueue as cpu intensive.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use kernel::workqueue::Queue;
+    ///
+    /// let wq = Queue::new_unbound().cpu_intensive().build(c"my-wq")?;
+    /// wq.try_spawn(GFP_KERNEL, || pr_info!("Hello from cpu-intensive wq"))?;
+    /// # Ok::<(), Error>(())
+    /// ```
+    #[inline]
+    #[doc(alias = "WQ_CPU_INTENSIVE")]
+    pub fn cpu_intensive(mut self) -> Self {
+        self.flags |= bindings::wq_flags_WQ_CPU_INTENSIVE;
+        self
+    }
+
+    /// Make this workqueue visible in sysfs.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use kernel::workqueue::Queue;
+    ///
+    /// let wq = Queue::new_unbound().sysfs().build(c"my-wq")?;
+    /// wq.try_spawn(GFP_KERNEL, || pr_info!("Hello from sysfs wq"))?;
+    /// # Ok::<(), Error>(())
+    /// ```
+    #[inline]
+    #[doc(alias = "WQ_SYSFS")]
+    pub fn sysfs(mut self) -> Self {
+        self.flags |= bindings::wq_flags_WQ_SYSFS;
+        self
+    }
+}
+
+/// Indicates that this workqueue runs in a normal context (as opposed to softirq context).
+pub trait TypeNormal {}
+impl TypeNormal for TypeUnbound {}
+impl TypeNormal for TypePercpu {}
+impl TypeNormal for TypePowerEfficient {}
+impl TypeNormal for TypeOrdered {}
+
+/// Options that are not available on BH workqueues.
+impl<T: TypeNormal> Builder<T> {
+    /// Allow this workqueue to be frozen during suspend.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use kernel::workqueue::Queue;
+    ///
+    /// let wq = Queue::new_unbound().freezable().build(c"my-wq")?;
+    /// wq.try_spawn(GFP_KERNEL, || pr_info!("Hello from freezable wq"))?;
+    /// # Ok::<(), Error>(())
+    /// ```
+    #[inline]
+    #[doc(alias = "WQ_FREEZABLE")]
+    pub fn freezable(mut self) -> Self {
+        self.flags |= bindings::wq_flags_WQ_FREEZABLE;
+        self
+    }
+
+    /// This workqueue may be used during memory reclaim.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use kernel::workqueue::Queue;
+    ///
+    /// let wq = Queue::new_unbound().mem_reclaim().build(c"my-wq")?;
+    /// wq.try_spawn(GFP_KERNEL, || pr_info!("Hello from mem_reclaim wq"))?;
+    /// # Ok::<(), Error>(())
+    /// ```
+    #[inline]
+    #[doc(alias = "WQ_MEM_RECLAIM")]
+    pub fn mem_reclaim(mut self) -> Self {
+        self.flags |= bindings::wq_flags_WQ_MEM_RECLAIM;
+        self
+    }
+}
+
+/// Options only available on a BH workqueue.
+impl Builder<TypeBH> {
+    /// Configure this BH workqueue to be percpu.
+    ///
+    /// To configure a non-BH percpu workqueue, use [`Queue::new_percpu`] instead.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use kernel::workqueue::Queue;
+    ///
+    /// let wq = Queue::new_bh().percpu().build(c"my-wq")?;
+    /// wq.try_spawn(GFP_KERNEL, || pr_info!("Hello from percpu BH wq"))?;
+    /// # Ok::<(), Error>(())
+    /// ```
+    #[inline]
+    #[doc(alias = "WQ_PERCPU")]
+    pub fn percpu(mut self) -> Self {
+        self.flags |= bindings::wq_flags_WQ_PERCPU;
+        self
+    }
+}
diff --git a/rust/kernel/workqueue/mod.rs b/rust/kernel/workqueue/mod.rs
index 1acd113c04ee..6049c0e8e4b6 100644
--- a/rust/kernel/workqueue/mod.rs
+++ b/rust/kernel/workqueue/mod.rs
@@ -186,7 +186,10 @@
 //! C header: [`include/linux/workqueue.h`](srctree/include/linux/workqueue.h)
 
 use crate::{
-    alloc::{AllocError, Flags},
+    alloc::{
+        self,
+        AllocError, //
+    },
     container_of,
     prelude::*,
     sync::Arc,
@@ -194,7 +197,14 @@
     time::Jiffies,
     types::Opaque,
 };
-use core::marker::PhantomData;
+use core::{
+    marker::PhantomData,
+    ops::Deref,
+    ptr::NonNull, //
+};
+
+mod builder;
+pub use self::builder::Builder;
 
 /// Creates a [`Work`] initialiser with the given name and a newly-created lock class.
 #[macro_export]
@@ -340,7 +350,7 @@ pub fn enqueue_delayed<W, const ID: u64>(
     /// This method can fail because it allocates memory to store the work item.
     pub fn try_spawn<T: 'static + Send + FnOnce()>(
         &self,
-        flags: Flags,
+        flags: alloc::Flags,
         func: T,
     ) -> Result<(), AllocError> {
         let init = pin_init!(ClosureWork {
@@ -353,6 +363,34 @@ pub fn try_spawn<T: 'static + Send + FnOnce()>(
     }
 }
 
+/// An owned kernel work queue.
+///
+/// Dropping a workqueue blocks on all pending work.
+///
+/// # Invariants
+///
+/// `queue` points at a valid workqueue that is owned by this `OwnedQueue`.
+pub struct OwnedQueue {
+    queue: NonNull<Queue>,
+}
+
+impl Deref for OwnedQueue {
+    type Target = Queue;
+    fn deref(&self) -> &Queue {
+        // SAFETY: By the type invariants, this pointer references a valid queue.
+        unsafe { &*self.queue.as_ptr() }
+    }
+}
+
+impl Drop for OwnedQueue {
+    fn drop(&mut self) {
+        // SAFETY: This `OwnedQueue` owns a valid workqueue, so we can destroy it. There is no
+        // delayed work scheduled on this queue that may attempt to use it after this call, as
+        // scheduling delayed work requires a 'static reference.
+        unsafe { bindings::destroy_workqueue(self.queue.as_ptr().cast()) }
+    }
+}
+
 /// A helper type used in [`try_spawn`].
 ///
 /// [`try_spawn`]: Queue::try_spawn

-- 
2.53.0.473.g4a7958ca14-goog

Re: [PATCH v4 3/3] rust: workqueue: add creation of workqueues

[Gary Guo]

On Thu Mar 12, 2026 at 9:23 AM GMT, Alice Ryhl wrote:
> Creating workqueues is needed by various GPU drivers. Not only does it
> give you better control over execution, it also allows devices to ensure
> that all tasks have exited before the device is unbound (or similar) by
> running the workqueue destructor.
>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
>  rust/helpers/workqueue.c         |   7 +
>  rust/kernel/workqueue/builder.rs | 380 +++++++++++++++++++++++++++++++++++++++
>  rust/kernel/workqueue/mod.rs     |  44 ++++-
>  3 files changed, 428 insertions(+), 3 deletions(-)
>
> diff --git a/rust/helpers/workqueue.c b/rust/helpers/workqueue.c
> index ce1c3a5b2150..e4b9d1b3d6bf 100644
> --- a/rust/helpers/workqueue.c
> +++ b/rust/helpers/workqueue.c
> @@ -14,3 +14,10 @@ __rust_helper void rust_helper_init_work_with_key(struct work_struct *work,
>  	INIT_LIST_HEAD(&work->entry);
>  	work->func = func;
>  }
> +
> +__rust_helper
> +struct workqueue_struct *rust_helper_alloc_workqueue(const char *fmt, unsigned int flags,
> +						     int max_active, const void *data)
> +{
> +	return alloc_workqueue(fmt, flags, max_active, data);
> +}
> diff --git a/rust/kernel/workqueue/builder.rs b/rust/kernel/workqueue/builder.rs
> new file mode 100644
> index 000000000000..d4d77b96f9c4
> --- /dev/null
> +++ b/rust/kernel/workqueue/builder.rs
> @@ -0,0 +1,380 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Workqueue builders.
> +
> +use kernel::{
> +    alloc::AllocError,
> +    prelude::*,
> +    workqueue::{
> +        OwnedQueue, //
> +        Queue,
> +    }, //
> +};
> +
> +use core::{
> +    marker::PhantomData, //
> +    ptr::{self, NonNull},

This is formatted incorrectly.

> +};
> +
> +/// Workqueue builder.
> +///
> +/// A valid combination of workqueue flags contains one of the base flags (`WQ_UNBOUND`, `WQ_BH`,
> +/// or `WQ_PERCPU`) and a combination of modifier flags that are compatible with the selected base
> +/// flag.
> +///
> +/// For details, please refer to `Documentation/core-api/workqueue.rst`.
> +pub struct Builder<T> {

I would name the generic parameter `Kind` rather than `T`. `T` is too generic to
indicate it's for specific kind/type of workqueue.

> +    flags: bindings::wq_flags,
> +    max_active: i32,
> +    _type: PhantomData<T>,

Hmm, it is somewhat awkward to me that we are having a `PhantomData<T>` here,
as `PhantomData` is documented to "behave as it it owns a `T`", but all the
possible `T`s that we use here are uninhabited.

> +}
> +
> +pub enum TypeUnbound {}
> +pub enum TypePercpu {}
> +pub enum TypePowerEfficient {}
> +pub enum TypeBH {}
> +pub enum TypeOrdered {}
> +
> +/// Entry-points to the builder API.
> +impl Queue {
> +    /// Build a workqueue whose work may execute on any cpu.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// use kernel::workqueue::Queue;
> +    ///
> +    /// let wq = Queue::new_unbound().build(c"my-wq")?;
> +    /// wq.try_spawn(GFP_KERNEL, || pr_info!("Hello from unbound wq"))?;
> +    /// # Ok::<(), Error>(())
> +    /// ```
> +    #[inline]
> +    #[doc(alias = "WQ_UNBOUND")]
> +    pub fn new_unbound() -> Builder<TypeUnbound> {
> +        Builder {
> +            flags: bindings::wq_flags_WQ_UNBOUND,
> +            max_active: 0,
> +            _type: PhantomData,
> +        }
> +    }
> +
> +    /// Build a workqueue whose work is bound to a specific cpu.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// use kernel::workqueue::Queue;
> +    ///
> +    /// let wq = Queue::new_percpu().build(c"my-wq")?;
> +    /// wq.try_spawn(GFP_KERNEL, || pr_info!("Hello from percpu wq"))?;
> +    /// # Ok::<(), Error>(())
> +    /// ```
> +    #[inline]
> +    #[doc(alias = "WQ_PERCPU")]
> +    pub fn new_percpu() -> Builder<TypePercpu> {
> +        Builder {
> +            flags: bindings::wq_flags_WQ_PERCPU,
> +            max_active: 0,
> +            _type: PhantomData,
> +        }
> +    }
> +
> +    /// Build a power-efficient workqueue.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// use kernel::workqueue::Queue;
> +    ///
> +    /// let wq = Queue::new_power_efficient().build(c"my-wq")?;
> +    /// wq.try_spawn(GFP_KERNEL, || pr_info!("Hello from power-efficient wq"))?;
> +    /// # Ok::<(), Error>(())
> +    /// ```
> +    #[inline]
> +    #[doc(alias = "WQ_POWER_EFFICIENT")]
> +    pub fn new_power_efficient() -> Builder<TypePowerEfficient> {
> +        Builder {
> +            flags: bindings::wq_flags_WQ_POWER_EFFICIENT,
> +            max_active: 0,
> +            _type: PhantomData,
> +        }
> +    }
> +
> +    /// Build a single-threaded workqueue that executes jobs in order.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// use kernel::workqueue::Queue;
> +    ///
> +    /// let wq = Queue::new_ordered().build(c"my-wq")?;
> +    /// wq.try_spawn(GFP_KERNEL, || pr_info!("Hello from ordered wq"))?;
> +    /// # Ok::<(), Error>(())
> +    /// ```
> +    #[inline]
> +    #[doc(alias = "alloc_ordered_workqueue")]
> +    #[doc(alias = "__WQ_ORDERED")]
> +    pub fn new_ordered() -> Builder<TypeOrdered> {
> +        Builder {
> +            flags: bindings::wq_flags_WQ_UNBOUND | bindings::wq_flags___WQ_ORDERED,
> +            max_active: 0,

This should be 1 instead of 0.

> +            _type: PhantomData,
> +        }
> +    }
> +
> +    /// Build a workqueue that executes in bottom-half (softirq) context.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// use kernel::workqueue::Queue;
> +    ///
> +    /// let wq = Queue::new_bh().build(c"my-wq")?;
> +    /// wq.try_spawn(GFP_KERNEL, || pr_info!("Hello from BH wq"))?;
> +    /// # Ok::<(), Error>(())
> +    /// ```
> +    #[inline]
> +    #[doc(alias = "WQ_BH")]
> +    pub fn new_bh() -> Builder<TypeBH> {
> +        Builder {
> +            flags: bindings::wq_flags_WQ_BH,
> +            max_active: 0,
> +            _type: PhantomData,
> +        }
> +    }
> +}
> +
>
> [snip]
>
> diff --git a/rust/kernel/workqueue/mod.rs b/rust/kernel/workqueue/mod.rs
> index 1acd113c04ee..6049c0e8e4b6 100644
> --- a/rust/kernel/workqueue/mod.rs
> +++ b/rust/kernel/workqueue/mod.rs
> @@ -186,7 +186,10 @@
>  //! C header: [`include/linux/workqueue.h`](srctree/include/linux/workqueue.h)
>  
>  use crate::{
> -    alloc::{AllocError, Flags},
> +    alloc::{
> +        self,
> +        AllocError, //
> +    },
>      container_of,
>      prelude::*,
>      sync::Arc,
> @@ -194,7 +197,14 @@
>      time::Jiffies,
>      types::Opaque,
>  };
> -use core::marker::PhantomData;
> +use core::{
> +    marker::PhantomData,
> +    ops::Deref,
> +    ptr::NonNull, //
> +};
> +
> +mod builder;
> +pub use self::builder::Builder;
>  
>  /// Creates a [`Work`] initialiser with the given name and a newly-created lock class.
>  #[macro_export]
> @@ -340,7 +350,7 @@ pub fn enqueue_delayed<W, const ID: u64>(
>      /// This method can fail because it allocates memory to store the work item.
>      pub fn try_spawn<T: 'static + Send + FnOnce()>(
>          &self,
> -        flags: Flags,
> +        flags: alloc::Flags,
>          func: T,
>      ) -> Result<(), AllocError> {
>          let init = pin_init!(ClosureWork {
> @@ -353,6 +363,34 @@ pub fn try_spawn<T: 'static + Send + FnOnce()>(
>      }
>  }
>  
> +/// An owned kernel work queue.
> +///
> +/// Dropping a workqueue blocks on all pending work.
> +///
> +/// # Invariants
> +///
> +/// `queue` points at a valid workqueue that is owned by this `OwnedQueue`.
> +pub struct OwnedQueue {
> +    queue: NonNull<Queue>,
> +}

This looks like something that can become `Owned<Queue>` when Andreas' series
land?

> +
> +impl Deref for OwnedQueue {
> +    type Target = Queue;

#[inline]

Best,
Gary

> +    fn deref(&self) -> &Queue {
> +        // SAFETY: By the type invariants, this pointer references a valid queue.
> +        unsafe { &*self.queue.as_ptr() }
> +    }
> +}
> +
> +impl Drop for OwnedQueue {
> +    fn drop(&mut self) {
> +        // SAFETY: This `OwnedQueue` owns a valid workqueue, so we can destroy it. There is no
> +        // delayed work scheduled on this queue that may attempt to use it after this call, as
> +        // scheduling delayed work requires a 'static reference.
> +        unsafe { bindings::destroy_workqueue(self.queue.as_ptr().cast()) }
> +    }
> +}
> +
>  /// A helper type used in [`try_spawn`].
>  ///
>  /// [`try_spawn`]: Queue::try_spawn

Re: [PATCH v4 3/3] rust: workqueue: add creation of workqueues

[Danilo Krummrich]

On Thu Mar 12, 2026 at 10:23 AM CET, Alice Ryhl wrote:
> +impl Queue {
> +    /// Build a workqueue whose work may execute on any cpu.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// use kernel::workqueue::Queue;
> +    ///
> +    /// let wq = Queue::new_unbound().build(c"my-wq")?;
> +    /// wq.try_spawn(GFP_KERNEL, || pr_info!("Hello from unbound wq"))?;
> +    /// # Ok::<(), Error>(())
> +    /// ```
> +    #[inline]
> +    #[doc(alias = "WQ_UNBOUND")]
> +    pub fn new_unbound() -> Builder<TypeUnbound> {

Wouldn't it be more straight forward if those where constructors of Builder?

	let wq = wq::Builder::new_unbound().build(c"my-wq")?;

Queue::new_unbound() suggests that it returns a new Queue object, but instead it
returns a new Builder object.

Re: [PATCH v4 3/3] rust: workqueue: add creation of workqueues

[Alice Ryhl]

On Thu, Mar 12, 2026 at 04:39:00PM +0000, Gary Guo wrote:
> On Thu Mar 12, 2026 at 9:23 AM GMT, Alice Ryhl wrote:
> > Creating workqueues is needed by various GPU drivers. Not only does it
> > give you better control over execution, it also allows devices to ensure
> > that all tasks have exited before the device is unbound (or similar) by
> > running the workqueue destructor.
> >
> > Signed-off-by: Alice Ryhl <aliceryhl@google.com>

> > +use core::{
> > +    marker::PhantomData, //
> > +    ptr::{self, NonNull},
> 
> This is formatted incorrectly.

Sigh. Ok.

> > +};
> > +
> > +/// Workqueue builder.
> > +///
> > +/// A valid combination of workqueue flags contains one of the base flags (`WQ_UNBOUND`, `WQ_BH`,
> > +/// or `WQ_PERCPU`) and a combination of modifier flags that are compatible with the selected base
> > +/// flag.
> > +///
> > +/// For details, please refer to `Documentation/core-api/workqueue.rst`.
> > +pub struct Builder<T> {
> 
> I would name the generic parameter `Kind` rather than `T`. `T` is too generic to
> indicate it's for specific kind/type of workqueue.

I guess 'Kind' works here, yeah.

> > +    flags: bindings::wq_flags,
> > +    max_active: i32,
> > +    _type: PhantomData<T>,
> 
> Hmm, it is somewhat awkward to me that we are having a `PhantomData<T>` here,
> as `PhantomData` is documented to "behave as it it owns a `T`", but all the
> possible `T`s that we use here are uninhabited.

I don't think this is an issue.

> > +    /// Build a single-threaded workqueue that executes jobs in order.
> > +    ///
> > +    /// # Examples
> > +    ///
> > +    /// ```
> > +    /// use kernel::workqueue::Queue;
> > +    ///
> > +    /// let wq = Queue::new_ordered().build(c"my-wq")?;
> > +    /// wq.try_spawn(GFP_KERNEL, || pr_info!("Hello from ordered wq"))?;
> > +    /// # Ok::<(), Error>(())
> > +    /// ```
> > +    #[inline]
> > +    #[doc(alias = "alloc_ordered_workqueue")]
> > +    #[doc(alias = "__WQ_ORDERED")]
> > +    pub fn new_ordered() -> Builder<TypeOrdered> {
> > +        Builder {
> > +            flags: bindings::wq_flags_WQ_UNBOUND | bindings::wq_flags___WQ_ORDERED,
> > +            max_active: 0,
> 
> This should be 1 instead of 0.

Hmm, it's weird that the doc-test didn't catch this.

> > +/// An owned kernel work queue.
> > +///
> > +/// Dropping a workqueue blocks on all pending work.
> > +///
> > +/// # Invariants
> > +///
> > +/// `queue` points at a valid workqueue that is owned by this `OwnedQueue`.
> > +pub struct OwnedQueue {
> > +    queue: NonNull<Queue>,
> > +}
> 
> This looks like something that can become `Owned<Queue>` when Andreas' series
> land?

That's right.

Alice

Re: [PATCH v4 3/3] rust: workqueue: add creation of workqueues

[Gary Guo]

On Thu Mar 12, 2026 at 10:56 PM GMT, Alice Ryhl wrote:
> On Thu, Mar 12, 2026 at 04:39:00PM +0000, Gary Guo wrote:
>> > +    flags: bindings::wq_flags,
>> > +    max_active: i32,
>> > +    _type: PhantomData<T>,
>> 
>> Hmm, it is somewhat awkward to me that we are having a `PhantomData<T>` here,
>> as `PhantomData` is documented to "behave as it it owns a `T`", but all the
>> possible `T`s that we use here are uninhabited.
>
> I don't think this is an issue.

Fair. I also don't think it's an issue, just awkward.

I would rather make the uninhabited enums be just unit structs, though (like what we do
for atomic ordering and device contexts).

>
>> > +    /// Build a single-threaded workqueue that executes jobs in order.
>> > +    ///
>> > +    /// # Examples
>> > +    ///
>> > +    /// ```
>> > +    /// use kernel::workqueue::Queue;
>> > +    ///
>> > +    /// let wq = Queue::new_ordered().build(c"my-wq")?;
>> > +    /// wq.try_spawn(GFP_KERNEL, || pr_info!("Hello from ordered wq"))?;
>> > +    /// # Ok::<(), Error>(())
>> > +    /// ```
>> > +    #[inline]
>> > +    #[doc(alias = "alloc_ordered_workqueue")]
>> > +    #[doc(alias = "__WQ_ORDERED")]
>> > +    pub fn new_ordered() -> Builder<TypeOrdered> {
>> > +        Builder {
>> > +            flags: bindings::wq_flags_WQ_UNBOUND | bindings::wq_flags___WQ_ORDERED,
>> > +            max_active: 0,
>> 
>> This should be 1 instead of 0.
>
> Hmm, it's weird that the doc-test didn't catch this.

It seems that there's no check for this in the __alloc_workqueue (which I guess
makes sense, because there's a C macro for allocating ordered workqueue which
doesn't take max_active and pass 1 along).

The only assertion is in workqueue_set_max_active to prevent changing it after
creation.

Best,
Gary

>
>> > +/// An owned kernel work queue.
>> > +///
>> > +/// Dropping a workqueue blocks on all pending work.
>> > +///
>> > +/// # Invariants
>> > +///
>> > +/// `queue` points at a valid workqueue that is owned by this `OwnedQueue`.
>> > +pub struct OwnedQueue {
>> > +    queue: NonNull<Queue>,
>> > +}
>> 
>> This looks like something that can become `Owned<Queue>` when Andreas' series
>> land?
>
> That's right.
>
> Alice

Re: [PATCH v4 3/3] rust: workqueue: add creation of workqueues

[Gary Guo]

On Fri Mar 13, 2026 at 1:25 PM GMT, Gary Guo wrote:
> On Thu Mar 12, 2026 at 10:56 PM GMT, Alice Ryhl wrote:
>> On Thu, Mar 12, 2026 at 04:39:00PM +0000, Gary Guo wrote:
>>> > +    flags: bindings::wq_flags,
>>> > +    max_active: i32,
>>> > +    _type: PhantomData<T>,
>>> 
>>> Hmm, it is somewhat awkward to me that we are having a `PhantomData<T>` here,
>>> as `PhantomData` is documented to "behave as it it owns a `T`", but all the
>>> possible `T`s that we use here are uninhabited.
>>
>> I don't think this is an issue.
>
> Fair. I also don't think it's an issue, just awkward.
>
> I would rather make the uninhabited enums be just unit structs, though (like what we do
> for atomic ordering and device contexts).

One interesting idea with unit structs, is to allow the type to be passed in as
argument rather than having multiple new_ variants:

    Queue::builder(wq::TypeUnbound).max_active(..).build(...)

This is the trick that we use to make atomic API looks nice, although in this
case it seems that it's not worth the hassle as it needs an extra import...

Best,
Gary

>
>>
>>> > +    /// Build a single-threaded workqueue that executes jobs in order.
>>> > +    ///
>>> > +    /// # Examples
>>> > +    ///
>>> > +    /// ```
>>> > +    /// use kernel::workqueue::Queue;
>>> > +    ///
>>> > +    /// let wq = Queue::new_ordered().build(c"my-wq")?;
>>> > +    /// wq.try_spawn(GFP_KERNEL, || pr_info!("Hello from ordered wq"))?;
>>> > +    /// # Ok::<(), Error>(())
>>> > +    /// ```
>>> > +    #[inline]
>>> > +    #[doc(alias = "alloc_ordered_workqueue")]
>>> > +    #[doc(alias = "__WQ_ORDERED")]
>>> > +    pub fn new_ordered() -> Builder<TypeOrdered> {
>>> > +        Builder {
>>> > +            flags: bindings::wq_flags_WQ_UNBOUND | bindings::wq_flags___WQ_ORDERED,
>>> > +            max_active: 0,
>>> 
>>> This should be 1 instead of 0.
>>
>> Hmm, it's weird that the doc-test didn't catch this.
>
> It seems that there's no check for this in the __alloc_workqueue (which I guess
> makes sense, because there's a C macro for allocating ordered workqueue which
> doesn't take max_active and pass 1 along).
>
> The only assertion is in workqueue_set_max_active to prevent changing it after
> creation.
>
> Best,
> Gary
>
>>
>>> > +/// An owned kernel work queue.
>>> > +///
>>> > +/// Dropping a workqueue blocks on all pending work.
>>> > +///
>>> > +/// # Invariants
>>> > +///
>>> > +/// `queue` points at a valid workqueue that is owned by this `OwnedQueue`.
>>> > +pub struct OwnedQueue {
>>> > +    queue: NonNull<Queue>,
>>> > +}
>>> 
>>> This looks like something that can become `Owned<Queue>` when Andreas' series
>>> land?
>>
>> That's right.
>>
>> Alice

Re: [PATCH v4 3/3] rust: workqueue: add creation of workqueues

[Alice Ryhl]

On Thu, Mar 12, 2026 at 06:59:31PM +0100, Danilo Krummrich wrote:
> On Thu Mar 12, 2026 at 10:23 AM CET, Alice Ryhl wrote:
> > +impl Queue {
> > +    /// Build a workqueue whose work may execute on any cpu.
> > +    ///
> > +    /// # Examples
> > +    ///
> > +    /// ```
> > +    /// use kernel::workqueue::Queue;
> > +    ///
> > +    /// let wq = Queue::new_unbound().build(c"my-wq")?;
> > +    /// wq.try_spawn(GFP_KERNEL, || pr_info!("Hello from unbound wq"))?;
> > +    /// # Ok::<(), Error>(())
> > +    /// ```
> > +    #[inline]
> > +    #[doc(alias = "WQ_UNBOUND")]
> > +    pub fn new_unbound() -> Builder<TypeUnbound> {
> 
> Wouldn't it be more straight forward if those where constructors of Builder?
> 
> 	let wq = wq::Builder::new_unbound().build(c"my-wq")?;

Crates out there do both, but I actually like this pattern quite a lot.
After all, given 'wq::Builder::new_unbound()' and 'Queue::new_unbound()'
I think the latter is the one that makes it more clear that you are
building a 'Queue'.

Also, it avoids an extra import.

> Queue::new_unbound() suggests that it returns a new Queue object, but instead it
> returns a new Builder object.

I'm only really concerned about confusion if it falls into one of these
categories:

* You write the wrong code, it compiles, and silently does the wrong
  thing.
* You read the code, and misunderstand what it does.

I don't think this falls into either of those cases. If you forget the
.build() call, all you get is a compiler error, and it's not hard to
figure out what to do when you see the error.

Alice

Re: [PATCH v4 3/3] rust: workqueue: add creation of workqueues

[Alice Ryhl]

On Fri, Mar 13, 2026 at 01:29:41PM +0000, Gary Guo wrote:
> On Fri Mar 13, 2026 at 1:25 PM GMT, Gary Guo wrote:
> > On Thu Mar 12, 2026 at 10:56 PM GMT, Alice Ryhl wrote:
> >> On Thu, Mar 12, 2026 at 04:39:00PM +0000, Gary Guo wrote:
> >>> > +    flags: bindings::wq_flags,
> >>> > +    max_active: i32,
> >>> > +    _type: PhantomData<T>,
> >>> 
> >>> Hmm, it is somewhat awkward to me that we are having a `PhantomData<T>` here,
> >>> as `PhantomData` is documented to "behave as it it owns a `T`", but all the
> >>> possible `T`s that we use here are uninhabited.
> >>
> >> I don't think this is an issue.
> >
> > Fair. I also don't think it's an issue, just awkward.
> >
> > I would rather make the uninhabited enums be just unit structs, though (like what we do
> > for atomic ordering and device contexts).
> 
> One interesting idea with unit structs, is to allow the type to be passed in as
> argument rather than having multiple new_ variants:
> 
>     Queue::builder(wq::TypeUnbound).max_active(..).build(...)
> 
> This is the trick that we use to make atomic API looks nice, although in this
> case it seems that it's not worth the hassle as it needs an extra import...

Yeah, I don't think this is worth it at all.

Alice

Re: [PATCH v4 1/3] rust: workqueue: restrict delayed work to global wqs

[Andreas Hindborg]

"Alice Ryhl" <aliceryhl@google.com> writes:

> When a workqueue is shut down, delayed work that is pending but not
> scheduled does not get properly cleaned up, so it's not safe to use
> `enqueue_delayed` on a workqueue that might be destroyed. To fix this,
> restricted `enqueue_delayed` to static queues.
>
> This may be fixed in the future by an approach along the lines of [1].
>
> Cc: stable@vger.kernel.org
> Fixes: 7c098cd5eaae ("workqueue: rust: add delayed work items")
> Reviewed-by: John Hubbard <jhubbard@nvidia.com>
> Reviewed-by: Danilo Krummrich <dakr@kernel.org>
> Reviewed-by: Gary Guo <gary@garyguo.net>
> Link: https://lore.kernel.org/r/20250423-destroy-workqueue-flush-v1-1-3d74820780a5@google.com [1]
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>

Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>



Best regards,
Andreas Hindborg

Re: [PATCH v4 3/3] rust: workqueue: add creation of workqueues

[Andreas Hindborg]

"Alice Ryhl" <aliceryhl@google.com> writes:

> Creating workqueues is needed by various GPU drivers. Not only does it
> give you better control over execution, it also allows devices to ensure
> that all tasks have exited before the device is unbound (or similar) by
> running the workqueue destructor.
>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>

Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>


Best regards,
Andreas Hindborg

Re: [PATCH v4 3/3] rust: workqueue: add creation of workqueues

[Danilo Krummrich]

On Sat Mar 14, 2026 at 11:30 AM CET, Alice Ryhl wrote:
> On Thu, Mar 12, 2026 at 06:59:31PM +0100, Danilo Krummrich wrote:
>> On Thu Mar 12, 2026 at 10:23 AM CET, Alice Ryhl wrote:
>> > +impl Queue {
>> > +    /// Build a workqueue whose work may execute on any cpu.
>> > +    ///
>> > +    /// # Examples
>> > +    ///
>> > +    /// ```
>> > +    /// use kernel::workqueue::Queue;
>> > +    ///
>> > +    /// let wq = Queue::new_unbound().build(c"my-wq")?;
>> > +    /// wq.try_spawn(GFP_KERNEL, || pr_info!("Hello from unbound wq"))?;
>> > +    /// # Ok::<(), Error>(())
>> > +    /// ```
>> > +    #[inline]
>> > +    #[doc(alias = "WQ_UNBOUND")]
>> > +    pub fn new_unbound() -> Builder<TypeUnbound> {
>> 
>> Wouldn't it be more straight forward if those where constructors of Builder?
>> 
>> 	let wq = wq::Builder::new_unbound().build(c"my-wq")?;
>
> Crates out there do both, but I actually like this pattern quite a lot.
> After all, given 'wq::Builder::new_unbound()' and 'Queue::new_unbound()'
> I think the latter is the one that makes it more clear that you are
> building a 'Queue'.
>
> Also, it avoids an extra import.

If it is about avoiding an extra import, what about

	Queue::builder_unbound()

this makes the name less misleading and avoids the import.

>> Queue::new_unbound() suggests that it returns a new Queue object, but instead it
>> returns a new Builder object.
>
> I'm only really concerned about confusion if it falls into one of these
> categories:
>
> * You write the wrong code, it compiles, and silently does the wrong
>   thing.
> * You read the code, and misunderstand what it does.
>
> I don't think this falls into either of those cases. If you forget the
> .build() call, all you get is a compiler error, and it's not hard to
> figure out what to do when you see the error.
>
> Alice

Re: [PATCH v4 3/3] rust: workqueue: add creation of workqueues

[Andreas Hindborg]

"Alice Ryhl" <aliceryhl@google.com> writes:

> Creating workqueues is needed by various GPU drivers. Not only does it
> give you better control over execution, it also allows devices to ensure
> that all tasks have exited before the device is unbound (or similar) by
> running the workqueue destructor.
>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
>  rust/helpers/workqueue.c         |   7 +
>  rust/kernel/workqueue/builder.rs | 380 +++++++++++++++++++++++++++++++++++++++
>  rust/kernel/workqueue/mod.rs     |  44 ++++-
>  3 files changed, 428 insertions(+), 3 deletions(-)
>
> diff --git a/rust/helpers/workqueue.c b/rust/helpers/workqueue.c
> index ce1c3a5b2150..e4b9d1b3d6bf 100644
> --- a/rust/helpers/workqueue.c
> +++ b/rust/helpers/workqueue.c
> @@ -14,3 +14,10 @@ __rust_helper void rust_helper_init_work_with_key(struct work_struct *work,
>  	INIT_LIST_HEAD(&work->entry);
>  	work->func = func;
>  }
> +
> +__rust_helper
> +struct workqueue_struct *rust_helper_alloc_workqueue(const char *fmt, unsigned int flags,
> +						     int max_active, const void *data)
> +{
> +	return alloc_workqueue(fmt, flags, max_active, data);
> +}
> diff --git a/rust/kernel/workqueue/builder.rs b/rust/kernel/workqueue/builder.rs
> new file mode 100644
> index 000000000000..d4d77b96f9c4
> --- /dev/null
> +++ b/rust/kernel/workqueue/builder.rs
> @@ -0,0 +1,380 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Workqueue builders.
> +
> +use kernel::{
> +    alloc::AllocError,
> +    prelude::*,
> +    workqueue::{
> +        OwnedQueue, //
> +        Queue,
> +    }, //
> +};
> +
> +use core::{
> +    marker::PhantomData, //
> +    ptr::{self, NonNull},
> +};
> +
> +/// Workqueue builder.
> +///
> +/// A valid combination of workqueue flags contains one of the base flags (`WQ_UNBOUND`, `WQ_BH`,
> +/// or `WQ_PERCPU`) and a combination of modifier flags that are compatible with the selected base
> +/// flag.
> +///
> +/// For details, please refer to `Documentation/core-api/workqueue.rst`.
> +pub struct Builder<T> {
> +    flags: bindings::wq_flags,
> +    max_active: i32,
> +    _type: PhantomData<T>,
> +}
> +
> +pub enum TypeUnbound {}
> +pub enum TypePercpu {}
> +pub enum TypePowerEfficient {}
> +pub enum TypeBH {}

Could you expand the name here?

> +pub enum TypeOrdered {}
> +
> +/// Entry-points to the builder API.
> +impl Queue {
> +    /// Build a workqueue whose work may execute on any cpu.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// use kernel::workqueue::Queue;
> +    ///
> +    /// let wq = Queue::new_unbound().build(c"my-wq")?;
> +    /// wq.try_spawn(GFP_KERNEL, || pr_info!("Hello from unbound wq"))?;
> +    /// # Ok::<(), Error>(())
> +    /// ```
> +    #[inline]
> +    #[doc(alias = "WQ_UNBOUND")]
> +    pub fn new_unbound() -> Builder<TypeUnbound> {
> +        Builder {
> +            flags: bindings::wq_flags_WQ_UNBOUND,
> +            max_active: 0,
> +            _type: PhantomData,
> +        }
> +    }
> +
> +    /// Build a workqueue whose work is bound to a specific cpu.

Could you elaborate what this means? When are work items moved between
CPUs normally? If it is a specific CPU, how is the CPU specified?

> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// use kernel::workqueue::Queue;
> +    ///
> +    /// let wq = Queue::new_percpu().build(c"my-wq")?;
> +    /// wq.try_spawn(GFP_KERNEL, || pr_info!("Hello from percpu wq"))?;
> +    /// # Ok::<(), Error>(())
> +    /// ```
> +    #[inline]
> +    #[doc(alias = "WQ_PERCPU")]
> +    pub fn new_percpu() -> Builder<TypePercpu> {
> +        Builder {
> +            flags: bindings::wq_flags_WQ_PERCPU,
> +            max_active: 0,
> +            _type: PhantomData,
> +        }
> +    }
> +
> +    /// Build a power-efficient workqueue.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// use kernel::workqueue::Queue;
> +    ///
> +    /// let wq = Queue::new_power_efficient().build(c"my-wq")?;
> +    /// wq.try_spawn(GFP_KERNEL, || pr_info!("Hello from power-efficient wq"))?;
> +    /// # Ok::<(), Error>(())
> +    /// ```
> +    #[inline]
> +    #[doc(alias = "WQ_POWER_EFFICIENT")]
> +    pub fn new_power_efficient() -> Builder<TypePowerEfficient> {
> +        Builder {
> +            flags: bindings::wq_flags_WQ_POWER_EFFICIENT,
> +            max_active: 0,
> +            _type: PhantomData,
> +        }
> +    }
> +
> +    /// Build a single-threaded workqueue that executes jobs in order.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// use kernel::workqueue::Queue;
> +    ///
> +    /// let wq = Queue::new_ordered().build(c"my-wq")?;
> +    /// wq.try_spawn(GFP_KERNEL, || pr_info!("Hello from ordered wq"))?;
> +    /// # Ok::<(), Error>(())
> +    /// ```
> +    #[inline]
> +    #[doc(alias = "alloc_ordered_workqueue")]
> +    #[doc(alias = "__WQ_ORDERED")]
> +    pub fn new_ordered() -> Builder<TypeOrdered> {
> +        Builder {
> +            flags: bindings::wq_flags_WQ_UNBOUND | bindings::wq_flags___WQ_ORDERED,
> +            max_active: 0,
> +            _type: PhantomData,
> +        }
> +    }
> +
> +    /// Build a workqueue that executes in bottom-half (softirq) context.

Is bottom-half and softirq context the same?

> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// use kernel::workqueue::Queue;
> +    ///
> +    /// let wq = Queue::new_bh().build(c"my-wq")?;
> +    /// wq.try_spawn(GFP_KERNEL, || pr_info!("Hello from BH wq"))?;
> +    /// # Ok::<(), Error>(())
> +    /// ```
> +    #[inline]
> +    #[doc(alias = "WQ_BH")]
> +    pub fn new_bh() -> Builder<TypeBH> {
> +        Builder {
> +            flags: bindings::wq_flags_WQ_BH,
> +            max_active: 0,
> +            _type: PhantomData,
> +        }
> +    }
> +}
> +
> +/// Options that may be used with all workqueue types.
> +impl<T> Builder<T> {
> +    /// Mark this workqueue high priority.

Could you elaborate the meaning of "high priority"?

> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// use kernel::workqueue::Queue;
> +    ///
> +    /// let wq = Queue::new_unbound().highpri().build(c"my-wq")?;
> +    /// wq.try_spawn(GFP_KERNEL, || pr_info!("Hello from highpri wq"))?;
> +    /// # Ok::<(), Error>(())
> +    /// ```
> +    #[inline]
> +    #[doc(alias = "WQ_HIGHPRI")]
> +    pub fn highpri(mut self) -> Self {
> +        self.flags |= bindings::wq_flags_WQ_HIGHPRI;
> +        self
> +    }
> +
> +    /// Creates the workqueue.
> +    ///
> +    /// The provided name is used verbatim as the workqueue name.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// use kernel::workqueue::Queue;
> +    ///
> +    /// // create an unbound workqueue registered with sysfs
> +    /// let wq = Queue::new_unbound().sysfs().build(c"my-wq")?;
> +    ///
> +    /// // spawn a work item on it
> +    /// wq.try_spawn(
> +    ///     GFP_KERNEL,
> +    ///     || pr_warn!("Printing from my-wq"),
> +    /// )?;
> +    /// # Ok::<(), Error>(())
> +    /// ```
> +    #[inline]
> +    #[doc(alias = "alloc_workqueue")]
> +    pub fn build(self, name: &CStr) -> Result<OwnedQueue, AllocError> {
> +        // SAFETY:
> +        // * c"%s" is compatible with passing the name as a c-string.
> +        // * the builder only permits valid flag combinations
> +        let ptr = unsafe {
> +            bindings::alloc_workqueue(
> +                c"%s".as_char_ptr(),
> +                self.flags,
> +                self.max_active,
> +                name.as_char_ptr().cast::<c_void>(),
> +            )
> +        };
> +
> +        // INVARIANT: We successfully created the workqueue, so we can return ownership to the
> +        // caller.
> +        Ok(OwnedQueue {
> +            queue: NonNull::new(ptr).ok_or(AllocError)?.cast(),
> +        })
> +    }
> +
> +    /// Creates the workqueue.
> +    ///
> +    /// # Examples
> +    ///
> +    /// This example shows how to pass a Rust string formatter to the workqueue name, creating
> +    /// workqueues with names such as `my-wq-1` and `my-wq-2`.
> +    ///
> +    /// ```
> +    /// use kernel::workqueue::{Queue, OwnedQueue};
> +    ///
> +    /// fn my_wq(num: u32) -> Result<OwnedQueue> {
> +    ///     // create a percpu workqueue called my-wq-{num}
> +    ///     let wq = Queue::new_percpu().build_fmt(fmt!("my-wq-{num}"))?;
> +    ///     Ok(wq)
> +    /// }
> +    /// ```
> +    #[inline]
> +    pub fn build_fmt(self, name: kernel::fmt::Arguments<'_>) -> Result<OwnedQueue, AllocError> {
> +        // SAFETY:
> +        // * c"%pA" is compatible with passing an `Arguments` pointer.
> +        // * the builder only permits valid flag combinations
> +        let ptr = unsafe {
> +            bindings::alloc_workqueue(
> +                c"%pA".as_char_ptr(),
> +                self.flags,
> +                self.max_active,
> +                ptr::from_ref(&name).cast::<c_void>(),
> +            )
> +        };
> +
> +        // INVARIANT: We successfully created the workqueue, so we can return ownership to the
> +        // caller.
> +        Ok(OwnedQueue {
> +            queue: NonNull::new(ptr).ok_or(AllocError)?.cast(),
> +        })
> +    }
> +}
> +
> +/// Indicates that this workqueue is threaded.
> +pub trait TypeThreaded {}
> +impl TypeThreaded for TypeUnbound {}
> +impl TypeThreaded for TypePercpu {}
> +impl TypeThreaded for TypePowerEfficient {}

Could implementing one of these traits for the wrong type cause
unsoundness? If so, should they be unsafe?

> +
> +/// Options that are not available on BH or ordered workqueues.
> +impl<T: TypeThreaded> Builder<T> {
> +    /// Set the maximum number of active cpus.

Please clarify more clearly what "maximum number of active cpus" mean
for a workqueue.

> +    ///
> +    /// If not set, a default value of `WQ_DFL_ACTIVE` is used. The maximum value is
> +    /// `WQ_MAX_ACTIVE`.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// use kernel::workqueue::Queue;
> +    ///
> +    /// let wq = Queue::new_unbound().max_active(16).build(c"my-wq")?;
> +    /// wq.try_spawn(GFP_KERNEL, || pr_info!("Hello from wq with max_active=16"))?;
> +    /// # Ok::<(), Error>(())
> +    /// ```
> +    #[inline]
> +    pub fn max_active(mut self, max_active: u32) -> Self {
> +        // If provided `max_active` is greater than `i32::MAX`, then we need to trigger the C-side
> +        // comparison with `WQ_MAX_ACTIVE`, which we can do by clamping to `i32::MAX`.
> +        self.max_active = i32::try_from(max_active).unwrap_or(i32::MAX);
> +        self
> +    }
> +
> +    /// Mark this workqueue as cpu intensive.

What does the kernel do to a queue when it is marked cpu intensive?

> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// use kernel::workqueue::Queue;
> +    ///
> +    /// let wq = Queue::new_unbound().cpu_intensive().build(c"my-wq")?;
> +    /// wq.try_spawn(GFP_KERNEL, || pr_info!("Hello from cpu-intensive wq"))?;
> +    /// # Ok::<(), Error>(())
> +    /// ```
> +    #[inline]
> +    #[doc(alias = "WQ_CPU_INTENSIVE")]
> +    pub fn cpu_intensive(mut self) -> Self {
> +        self.flags |= bindings::wq_flags_WQ_CPU_INTENSIVE;
> +        self
> +    }
> +
> +    /// Make this workqueue visible in sysfs.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// use kernel::workqueue::Queue;
> +    ///
> +    /// let wq = Queue::new_unbound().sysfs().build(c"my-wq")?;
> +    /// wq.try_spawn(GFP_KERNEL, || pr_info!("Hello from sysfs wq"))?;
> +    /// # Ok::<(), Error>(())
> +    /// ```
> +    #[inline]
> +    #[doc(alias = "WQ_SYSFS")]
> +    pub fn sysfs(mut self) -> Self {
> +        self.flags |= bindings::wq_flags_WQ_SYSFS;
> +        self
> +    }
> +}
> +
> +/// Indicates that this workqueue runs in a normal context (as opposed to softirq context).
> +pub trait TypeNormal {}
> +impl TypeNormal for TypeUnbound {}
> +impl TypeNormal for TypePercpu {}
> +impl TypeNormal for TypePowerEfficient {}
> +impl TypeNormal for TypeOrdered {}
> +
> +/// Options that are not available on BH workqueues.
> +impl<T: TypeNormal> Builder<T> {
> +    /// Allow this workqueue to be frozen during suspend.

What happens to queues that are not frozen during suspend? They just
continue executing tasks until the system is powered off?


Best regards,
Andreas Hindborg