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

Alice Ryhl posted 2 patches 1 month, 1 week ago
There is a newer version of this series
[PATCH v3 2/2] rust: workqueue: add creation of workqueues
Posted by Alice Ryhl 1 month, 1 week ago
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 {
+        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>(),
+            )
+        };
+
+        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.
+        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 v3 2/2] rust: workqueue: add creation of workqueues
Posted by kernel test robot 1 month, 1 week ago
Hi Alice,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f]

url:    https://github.com/intel-lab-lkp/linux/commits/Alice-Ryhl/rust-workqueue-restrict-delayed-work-to-global-wqs/20260227-230029
base:   6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f
patch link:    https://lore.kernel.org/r/20260227-create-workqueue-v3-2-87de133f7849%40google.com
patch subject: [PATCH v3 2/2] rust: workqueue: add creation of workqueues
config: riscv-randconfig-r063-20260228 (https://download.01.org/0day-ci/archive/20260228/202602281033.afIyhHsn-lkp@intel.com/config)
compiler: clang version 23.0.0git (https://github.com/llvm/llvm-project 9a109fbb6e184ec9bcce10615949f598f4c974a9)
rustc: rustc 1.88.0 (6b00bc388 2025-06-23)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260228/202602281033.afIyhHsn-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202602281033.afIyhHsn-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> clang diag: include/linux/workqueue.h:513:76: warning: diagnostic behavior may be improved by adding the 'format(printf, 1, 4)' attribute to the declaration of 'rust_helper_alloc_workqueue' [-Wmissing-format-attribute]
--
   In file included from rust/helpers/helpers.c:66:
>> rust/helpers/workqueue.c:22:9: warning: diagnostic behavior may be improved by adding the 'format(printf, 1, 4)' attribute to the declaration of 'rust_helper_alloc_workqueue' [-Wmissing-format-attribute]
      19 | struct workqueue_struct *rust_helper_alloc_workqueue(const char *fmt, unsigned int flags,
         | __attribute__((format(printf, 1, 4))) 
      20 |                                                      int max_active, const void *data)
      21 | {
      22 |         return alloc_workqueue(fmt, flags, max_active, data);
         |                ^
   include/linux/workqueue.h:513:76: note: expanded from macro 'alloc_workqueue'
     513 | #define alloc_workqueue(...)    alloc_hooks(alloc_workqueue_noprof(__VA_ARGS__))
         |                                                                               ^
   include/linux/alloc_tag.h:265:31: note: expanded from macro 'alloc_hooks'
     265 |         alloc_hooks_tag(&_alloc_tag, _do_alloc);                        \
         |                                      ^
   include/linux/alloc_tag.h:251:9: note: expanded from macro 'alloc_hooks_tag'
     251 |         typeof(_do_alloc) _res;                                         \
         |                ^
   rust/helpers/workqueue.c:19:26: note: 'rust_helper_alloc_workqueue' declared here
      19 | struct workqueue_struct *rust_helper_alloc_workqueue(const char *fmt, unsigned int flags,
         |                          ^
   1 warning generated.
--
   llvm-nm: error: rust/helpers/helpers.o: No such file or directory
>> clang diag: include/linux/workqueue.h:513:76: warning: diagnostic behavior may be improved by adding the 'format(printf, 1, 4)' attribute to the declaration of 'rust_helper_alloc_workqueue' [-Wmissing-format-attribute]

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH v3 2/2] rust: workqueue: add creation of workqueues
Posted by Gary Guo 1 month, 1 week ago
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.

Best,
Gary

> +        unsafe { bindings::destroy_workqueue(self.queue.as_ptr().cast()) }
> +    }
> +}
> +
>  /// A helper type used in [`try_spawn`].
>  ///
>  /// [`try_spawn`]: Queue::try_spawn
Re: [PATCH v3 2/2] rust: workqueue: add creation of workqueues
Posted by Alice Ryhl 1 month, 1 week ago
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
Re: [PATCH v3 2/2] rust: workqueue: add creation of workqueues
Posted by Danilo Krummrich 1 month, 1 week ago
On Fri Feb 27, 2026 at 3:53 PM CET, Alice Ryhl wrote:
> +    /// Set the maximum number of active cpus.
> +    ///
> +    /// If not set, a reasonable default value is used. The maximum value is `WQ_MAX_ACTIVE`.

Should we just mention the default value?

> +    #[inline]
> +    pub fn max_active(mut self, max_active: u32) -> Builder {
> +        self.max_active = i32::try_from(max_active).unwrap_or(i32::MAX);

The workqueue code prints a warning for max_active >  WQ_MAX_ACTIVE. Maybe use
debug_assert()?

It's also a bit unfortunate that alloc_ordered_workqueue() becomes
.max_active(1).

At the same time having a separate ordered() method competes with max_active().

Mybe a type state, i.e. Builder<Ordered> that doesn't have max_active()?

> +        self
> +    }
Re: [PATCH v3 2/2] rust: workqueue: add creation of workqueues
Posted by Alice Ryhl 1 month, 1 week ago
On Fri, Feb 27, 2026 at 04:30:59PM +0100, Danilo Krummrich wrote:
> On Fri Feb 27, 2026 at 3:53 PM CET, Alice Ryhl wrote:
> > +    /// Set the maximum number of active cpus.
> > +    ///
> > +    /// If not set, a reasonable default value is used. The maximum value is `WQ_MAX_ACTIVE`.
> 
> Should we just mention the default value?

I can mention the constant name, but I'd like to avoid mentioning a
value that might change.

> > +    #[inline]
> > +    pub fn max_active(mut self, max_active: u32) -> Builder {
> > +        self.max_active = i32::try_from(max_active).unwrap_or(i32::MAX);
> 
> The workqueue code prints a warning for max_active >  WQ_MAX_ACTIVE. Maybe use
> debug_assert()?

What's wrong with just making use of the C-side warning?

> It's also a bit unfortunate that alloc_ordered_workqueue() becomes
> .max_active(1).
> 
> At the same time having a separate ordered() method competes with max_active().
> 
> Mybe a type state, i.e. Builder<Ordered> that doesn't have max_active()?

Sorry I'm a bit confused by this. Why does an ordered() compete with
max_active()?

Alice
Re: [PATCH v3 2/2] rust: workqueue: add creation of workqueues
Posted by Danilo Krummrich 1 month, 1 week ago
On Fri Feb 27, 2026 at 8:05 PM CET, Alice Ryhl wrote:
> On Fri, Feb 27, 2026 at 04:30:59PM +0100, Danilo Krummrich wrote:
>> On Fri Feb 27, 2026 at 3:53 PM CET, Alice Ryhl wrote:
>> > +    /// Set the maximum number of active cpus.
>> > +    ///
>> > +    /// If not set, a reasonable default value is used. The maximum value is `WQ_MAX_ACTIVE`.
>> 
>> Should we just mention the default value?
>
> I can mention the constant name, but I'd like to avoid mentioning a
> value that might change.

Yes, that's what I meant.

>> > +    #[inline]
>> > +    pub fn max_active(mut self, max_active: u32) -> Builder {
>> > +        self.max_active = i32::try_from(max_active).unwrap_or(i32::MAX);
>> 
>> The workqueue code prints a warning for max_active >  WQ_MAX_ACTIVE. Maybe use
>> debug_assert()?
>
> What's wrong with just making use of the C-side warning?

IIRC, we have the same pattern in other Rust code that we use debug_assert()
when a value got clamped, e.g. in udelay().

>> It's also a bit unfortunate that alloc_ordered_workqueue() becomes
>> .max_active(1).
>> 
>> At the same time having a separate ordered() method competes with max_active().
>> 
>> Mybe a type state, i.e. Builder<Ordered> that doesn't have max_active()?
>
> Sorry I'm a bit confused by this. Why does an ordered() compete with
> max_active()?

Because you could get an inconsistent state with __WQ_ORDERED and
max_active > 1.

It also conflicts with sysfs() I think [1].

[1] https://elixir.bootlin.com/linux/v6.19.3/source/kernel/workqueue.c#L7417
Re: [PATCH v3 2/2] rust: workqueue: add creation of workqueues
Posted by Alice Ryhl 1 month, 1 week ago
On Fri, Feb 27, 2026 at 08:23:44PM +0100, Danilo Krummrich wrote:
> On Fri Feb 27, 2026 at 8:05 PM CET, Alice Ryhl wrote:
> > On Fri, Feb 27, 2026 at 04:30:59PM +0100, Danilo Krummrich wrote:
> >> On Fri Feb 27, 2026 at 3:53 PM CET, Alice Ryhl wrote:
> >> > +    #[inline]
> >> > +    pub fn max_active(mut self, max_active: u32) -> Builder {
> >> > +        self.max_active = i32::try_from(max_active).unwrap_or(i32::MAX);
> >> 
> >> The workqueue code prints a warning for max_active >  WQ_MAX_ACTIVE. Maybe use
> >> debug_assert()?
> >
> > What's wrong with just making use of the C-side warning?
> 
> IIRC, we have the same pattern in other Rust code that we use debug_assert()
> when a value got clamped, e.g. in udelay().

In udelay(), the clamping happens on the Rust side, so it makes sense
that Rust is the one to warn about it.

Here, the clamping happens in C code. To warn about it, I'd have to
duplicate the existing C-side check to clamp in Rust.

> >> It's also a bit unfortunate that alloc_ordered_workqueue() becomes
> >> .max_active(1).
> >> 
> >> At the same time having a separate ordered() method competes with max_active().
> >> 
> >> Mybe a type state, i.e. Builder<Ordered> that doesn't have max_active()?
> >
> > Sorry I'm a bit confused by this. Why does an ordered() compete with
> > max_active()?
> 
> Because you could get an inconsistent state with __WQ_ORDERED and
> max_active > 1.
> 
> It also conflicts with sysfs() I think [1].
> 
> [1] https://elixir.bootlin.com/linux/v6.19.3/source/kernel/workqueue.c#L7417

And I guess the further argument is that we have a use-case for ordered
workqueues?

Alice
Re: [PATCH v3 2/2] rust: workqueue: add creation of workqueues
Posted by Danilo Krummrich 1 month, 1 week ago
On Sat Feb 28, 2026 at 1:59 PM CET, Alice Ryhl wrote:
> On Fri, Feb 27, 2026 at 08:23:44PM +0100, Danilo Krummrich wrote:
>> On Fri Feb 27, 2026 at 8:05 PM CET, Alice Ryhl wrote:
>> > On Fri, Feb 27, 2026 at 04:30:59PM +0100, Danilo Krummrich wrote:
>> >> On Fri Feb 27, 2026 at 3:53 PM CET, Alice Ryhl wrote:
>> >> > +    #[inline]
>> >> > +    pub fn max_active(mut self, max_active: u32) -> Builder {
>> >> > +        self.max_active = i32::try_from(max_active).unwrap_or(i32::MAX);
>> >> 
>> >> The workqueue code prints a warning for max_active >  WQ_MAX_ACTIVE. Maybe use
>> >> debug_assert()?
>> >
>> > What's wrong with just making use of the C-side warning?
>> 
>> IIRC, we have the same pattern in other Rust code that we use debug_assert()
>> when a value got clamped, e.g. in udelay().
>
> In udelay(), the clamping happens on the Rust side, so it makes sense
> that Rust is the one to warn about it.
>
> Here, the clamping happens in C code. To warn about it, I'd have to
> duplicate the existing C-side check to clamp in Rust.

That's fair, although I also think that it is not unreasonable. Given that this
uses the builder pattern, I think it would be nice to ensure that nothing
"invalid" can be built in the first place.

Maybe we can use a bounded integer?

>> >> It's also a bit unfortunate that alloc_ordered_workqueue() becomes
>> >> .max_active(1).
>> >> 
>> >> At the same time having a separate ordered() method competes with max_active().
>> >> 
>> >> Mybe a type state, i.e. Builder<Ordered> that doesn't have max_active()?
>> >
>> > Sorry I'm a bit confused by this. Why does an ordered() compete with
>> > max_active()?
>> 
>> Because you could get an inconsistent state with __WQ_ORDERED and
>> max_active > 1.
>> 
>> It also conflicts with sysfs() I think [1].
>> 
>> [1] https://elixir.bootlin.com/linux/v6.19.3/source/kernel/workqueue.c#L7417
>
> And I guess the further argument is that we have a use-case for ordered
> workqueues?

In the context of

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

I think we do.

Depending on the final implementation details and the driver it may be needed by
the job queue.

They are also pretty common outside the scheduler use-case in GPU drivers. I
think panthor has one as well, so you might also need one in Tyr. In nova-core I
expect this to be used in MM code.

But even without that, I think it would be reasonble to consider ordered queues
for this abstraction, since alloc_ordered_workqueue() and
create_singlethread_workqueue() seem to have more users than the non-ordered
constructors (without checking whether alloc_workqueue() is also used directly
to create ordered queues).
Re: [PATCH v3 2/2] rust: workqueue: add creation of workqueues
Posted by Alice Ryhl 1 month, 1 week ago
On Sat, Feb 28, 2026 at 03:43:02PM +0100, Danilo Krummrich wrote:
> On Sat Feb 28, 2026 at 1:59 PM CET, Alice Ryhl wrote:
> > On Fri, Feb 27, 2026 at 08:23:44PM +0100, Danilo Krummrich wrote:
> >> On Fri Feb 27, 2026 at 8:05 PM CET, Alice Ryhl wrote:
> >> > On Fri, Feb 27, 2026 at 04:30:59PM +0100, Danilo Krummrich wrote:
> >> >> On Fri Feb 27, 2026 at 3:53 PM CET, Alice Ryhl wrote:
> >> >> > +    #[inline]
> >> >> > +    pub fn max_active(mut self, max_active: u32) -> Builder {
> >> >> > +        self.max_active = i32::try_from(max_active).unwrap_or(i32::MAX);
> >> >> 
> >> >> The workqueue code prints a warning for max_active >  WQ_MAX_ACTIVE. Maybe use
> >> >> debug_assert()?
> >> >
> >> > What's wrong with just making use of the C-side warning?
> >> 
> >> IIRC, we have the same pattern in other Rust code that we use debug_assert()
> >> when a value got clamped, e.g. in udelay().
> >
> > In udelay(), the clamping happens on the Rust side, so it makes sense
> > that Rust is the one to warn about it.
> >
> > Here, the clamping happens in C code. To warn about it, I'd have to
> > duplicate the existing C-side check to clamp in Rust.
> 
> That's fair, although I also think that it is not unreasonable. Given that this
> uses the builder pattern, I think it would be nice to ensure that nothing
> "invalid" can be built in the first place.
> 
> Maybe we can use a bounded integer?

Bounded integers allow zero, which is also illegal.

I think it's a bit much honestly.

> >> >> It's also a bit unfortunate that alloc_ordered_workqueue() becomes
> >> >> .max_active(1).
> >> >> 
> >> >> At the same time having a separate ordered() method competes with max_active().
> >> >> 
> >> >> Mybe a type state, i.e. Builder<Ordered> that doesn't have max_active()?
> >> >
> >> > Sorry I'm a bit confused by this. Why does an ordered() compete with
> >> > max_active()?
> >> 
> >> Because you could get an inconsistent state with __WQ_ORDERED and
> >> max_active > 1.
> >> 
> >> It also conflicts with sysfs() I think [1].
> >> 
> >> [1] https://elixir.bootlin.com/linux/v6.19.3/source/kernel/workqueue.c#L7417
> >
> > And I guess the further argument is that we have a use-case for ordered
> > workqueues?
> 
> In the context of
> 
> 	GPU drivers often need to create their own workqueues for various
> 	reasons. Add the ability to do so.
> 
> I think we do.
> 
> Depending on the final implementation details and the driver it may be needed by
> the job queue.
> 
> They are also pretty common outside the scheduler use-case in GPU drivers. I
> think panthor has one as well, so you might also need one in Tyr. In nova-core I
> expect this to be used in MM code.
> 
> But even without that, I think it would be reasonble to consider ordered queues
> for this abstraction, since alloc_ordered_workqueue() and
> create_singlethread_workqueue() seem to have more users than the non-ordered
> constructors (without checking whether alloc_workqueue() is also used directly
> to create ordered queues).

Ok, I'll consider this for next version.

Alice
Re: [PATCH v3 2/2] rust: workqueue: add creation of workqueues
Posted by Philipp Stanner 1 month ago
On Sun, 2026-03-01 at 11:55 +0000, Alice Ryhl wrote:
> On Sat, Feb 28, 2026 at 03:43:02PM +0100, Danilo Krummrich wrote:
> > On Sat Feb 28, 2026 at 1:59 PM CET, Alice Ryhl wrote:
> > > On Fri, Feb 27, 2026 at 08:23:44PM +0100, Danilo Krummrich wrote:
> > > > On Fri Feb 27, 2026 at 8:05 PM CET, Alice Ryhl wrote:
> > > > > On Fri, Feb 27, 2026 at 04:30:59PM +0100, Danilo Krummrich wrote:
> > > > > > On Fri Feb 27, 2026 at 3:53 PM CET, Alice Ryhl wrote:
> > > > > > > +    #[inline]
> > > > > > > +    pub fn max_active(mut self, max_active: u32) -> Builder {
> > > > > > > +        self.max_active = i32::try_from(max_active).unwrap_or(i32::MAX);
> > > > > > 
> > > > > > The workqueue code prints a warning for max_active >  WQ_MAX_ACTIVE. Maybe use
> > > > > > debug_assert()?
> > > > > 
> > > > > What's wrong with just making use of the C-side warning?
> > > > 
> > > > IIRC, we have the same pattern in other Rust code that we use debug_assert()
> > > > when a value got clamped, e.g. in udelay().
> > > 
> > > In udelay(), the clamping happens on the Rust side, so it makes sense
> > > that Rust is the one to warn about it.
> > > 
> > > Here, the clamping happens in C code. To warn about it, I'd have to
> > > duplicate the existing C-side check to clamp in Rust.
> > 
> > That's fair, although I also think that it is not unreasonable. Given that this
> > uses the builder pattern, I think it would be nice to ensure that nothing
> > "invalid" can be built in the first place.
> > 
> > Maybe we can use a bounded integer?
> 
> Bounded integers allow zero, which is also illegal.
> 
> I think it's a bit much honestly.


My two cents here would be too that it's more elegant to just leverage
the C side's warning.

P.
Re: [PATCH v3 2/2] rust: workqueue: add creation of workqueues
Posted by Gary Guo 1 month, 1 week ago
On Fri Feb 27, 2026 at 3:30 PM GMT, Danilo Krummrich wrote:
> On Fri Feb 27, 2026 at 3:53 PM CET, Alice Ryhl wrote:
>> +    /// Set the maximum number of active cpus.
>> +    ///
>> +    /// If not set, a reasonable default value is used. The maximum value is `WQ_MAX_ACTIVE`.
>
> Should we just mention the default value?
>
>> +    #[inline]
>> +    pub fn max_active(mut self, max_active: u32) -> Builder {
>> +        self.max_active = i32::try_from(max_active).unwrap_or(i32::MAX);
>
> The workqueue code prints a warning for max_active >  WQ_MAX_ACTIVE. Maybe use
> debug_assert()?
>
> It's also a bit unfortunate that alloc_ordered_workqueue() becomes
> .max_active(1).

I think ordered workqueue still to have __WQ_ORDERED flag, too?

>
> At the same time having a separate ordered() method competes with max_active().

I don't think its too bad to have both, and before constructing checks that it's
used correctly:

    warn_on!(self.flags & __WQ_ORDERED != 0 && self.max_active != 1);

but type state works, too.

Best,
Gary

>
> Mybe a type state, i.e. Builder<Ordered> that doesn't have max_active()?
>
>> +        self
>> +    }
Re: [PATCH v3 2/2] rust: workqueue: add creation of workqueues
Posted by Danilo Krummrich 1 month, 1 week ago
On Fri Feb 27, 2026 at 5:00 PM CET, Gary Guo wrote:
> On Fri Feb 27, 2026 at 3:30 PM GMT, Danilo Krummrich wrote:
>> On Fri Feb 27, 2026 at 3:53 PM CET, Alice Ryhl wrote:
>>> +    /// Set the maximum number of active cpus.
>>> +    ///
>>> +    /// If not set, a reasonable default value is used. The maximum value is `WQ_MAX_ACTIVE`.
>>
>> Should we just mention the default value?
>>
>>> +    #[inline]
>>> +    pub fn max_active(mut self, max_active: u32) -> Builder {
>>> +        self.max_active = i32::try_from(max_active).unwrap_or(i32::MAX);
>>
>> The workqueue code prints a warning for max_active >  WQ_MAX_ACTIVE. Maybe use
>> debug_assert()?
>>
>> It's also a bit unfortunate that alloc_ordered_workqueue() becomes
>> .max_active(1).
>
> I think ordered workqueue still to have __WQ_ORDERED flag, too?
>
>>
>> At the same time having a separate ordered() method competes with max_active().
>
> I don't think its too bad to have both, and before constructing checks that it's
> used correctly:
>
>     warn_on!(self.flags & __WQ_ORDERED != 0 && self.max_active != 1);
>
> but type state works, too.

I think we can ensure this at compile time, so we should do it.