Re: [PATCH v3 2/2] rust: workqueue: add creation of workqueues

[Alice Ryhl <aliceryhl@google.com>]

On Fri, Feb 27, 2026 at 04:04:57PM +0000, Gary Guo wrote:
> On Fri Feb 27, 2026 at 2:53 PM 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.rs | 190 ++++++++++++++++++++++++++++++++++++++++++++++-
> >  2 files changed, 194 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.rs b/rust/kernel/workqueue.rs
> > index 1acd113c04ee..4ef02a537cd9 100644
> > --- a/rust/kernel/workqueue.rs
> > +++ b/rust/kernel/workqueue.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,11 @@
> >      time::Jiffies,
> >      types::Opaque,
> >  };
> > -use core::marker::PhantomData;
> > +use core::{
> > +    marker::PhantomData,
> > +    ops::Deref,
> > +    ptr::NonNull, //
> > +};
> >  
> >  /// Creates a [`Work`] initialiser with the given name and a newly-created lock class.
> >  #[macro_export]
> > @@ -340,7 +347,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 +360,183 @@ pub fn try_spawn<T: 'static + Send + FnOnce()>(
> >      }
> >  }
> >  
> > +/// 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 {
> > +    flags: bindings::wq_flags,
> > +    max_active: i32,
> > +}
> > +
> > +impl Builder {
> > +    /// Not bound to any cpu.
> > +    #[inline]
> > +    pub fn unbound() -> Builder {
> 
> These can all be "-> Self".
> 
> > +        Builder {
> > +            flags: bindings::wq_flags_WQ_UNBOUND,
> > +            max_active: 0,
> > +        }
> > +    }
> > +
> > +    /// Bound to a specific cpu.
> > +    #[inline]
> > +    pub fn percpu() -> Builder {
> > +        Builder {
> > +            flags: bindings::wq_flags_WQ_PERCPU,
> > +            max_active: 0,
> > +        }
> > +    }
> > +
> > +    /// Set the maximum number of active cpus.
> > +    ///
> > +    /// If not set, a reasonable default value is used. The maximum value is `WQ_MAX_ACTIVE`.
> > +    #[inline]
> > +    pub fn max_active(mut self, max_active: u32) -> Builder {
> > +        self.max_active = i32::try_from(max_active).unwrap_or(i32::MAX);
> > +        self
> > +    }
> > +
> > +    /// Allow this workqueue to be frozen during suspend.
> > +    #[inline]
> > +    pub fn freezable(mut self) -> Self {
> > +        self.flags |= bindings::wq_flags_WQ_FREEZABLE;
> > +        self
> > +    }
> > +
> > +    /// This workqueue may be used during memory reclaim.
> > +    #[inline]
> > +    pub fn mem_reclaim(mut self) -> Self {
> > +        self.flags |= bindings::wq_flags_WQ_MEM_RECLAIM;
> > +        self
> > +    }
> > +
> > +    /// Mark this workqueue as cpu intensive.
> > +    #[inline]
> > +    pub fn cpu_intensive(mut self) -> Self {
> > +        self.flags |= bindings::wq_flags_WQ_CPU_INTENSIVE;
> > +        self
> > +    }
> > +
> > +    /// Make this workqueue visible in sysfs.
> > +    #[inline]
> > +    pub fn sysfs(mut self) -> Self {
> > +        self.flags |= bindings::wq_flags_WQ_SYSFS;
> > +        self
> > +    }
> > +
> > +    /// Mark this workqueue high priority.
> > +    #[inline]
> > +    pub fn highpri(mut self) -> Self {
> > +        self.flags |= bindings::wq_flags_WQ_HIGHPRI;
> > +        self
> > +    }
> > +
> > +    /// Allocates a new workqueue.
> > +    ///
> > +    /// The provided name is used verbatim as the workqueue name.
> > +    ///
> > +    /// # Examples
> > +    ///
> > +    /// ```
> > +    /// use kernel::workqueue;
> > +    ///
> > +    /// // create an unbound workqueue registered with sysfs
> > +    /// let wq = workqueue::Builder::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]
> > +    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 comments?
> 
> > +
> > +        Ok(OwnedQueue {
> > +            queue: NonNull::new(ptr).ok_or(AllocError)?.cast(),
> > +        })
> > +    }
> > +
> > +    /// Allocates a new 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::{
> > +    ///     alloc::AllocError,
> > +    ///     workqueue::{self, OwnedQueue},
> > +    /// };
> > +    ///
> > +    /// fn my_wq(num: u32) -> Result<OwnedQueue, AllocError> {
> > +    ///     // create a percpu workqueue called my-wq-{num}
> > +    ///     workqueue::Builder::percpu().build_fmt(fmt!("my-wq-{num}"))
> > +    /// }
> > +    /// ```
> > +    #[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,
> > +                core::ptr::from_ref(&name).cast::<c_void>(),
> > +            )
> > +        };
> > +
> > +        Ok(OwnedQueue {
> > +            queue: NonNull::new(ptr).ok_or(AllocError)?.cast(),
> > +        })
> > +    }
> > +}
> > +
> > +/// 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: The `OwnedQueue` is being destroyed, so we can destroy the workqueue it owns.
> 
> Hmm, is this correct? This should say *why* the call is safe, not what it is
> doing.
> 
> I think this should mention: (1) the pointer is valid and (2) no delayed work is
> being scheduled on this queue.

Although it is missing the part about delayed work, I do think this
talks about why. The pointer being valid is far insufficient. We need
actual ownership of the workqueue - nobody can use it after this. We can
destroy the inner workqueue *because* the OwnedQueue owning it is being
dropped.

Alice