* [PATCH v2 0/2] Creation of workqueues in Rust
@ 2025-11-13 10:01 Alice Ryhl
2025-11-13 10:01 ` [PATCH v2 1/2] rust: workqueue: restrict delayed work to global wqs Alice Ryhl
2025-11-13 10:01 ` [PATCH v2 2/2] rust: workqueue: add creation of workqueues Alice Ryhl
0 siblings, 2 replies; 12+ messages in thread
From: Alice Ryhl @ 2025-11-13 10:01 UTC (permalink / raw)
To: Tejun Heo, Miguel Ojeda
Cc: Lai Jiangshan, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Trevor Gross, Danilo Krummrich, Daniel Almeida,
John Hubbard, Philipp Stanner, Tamir Duberstein, rust-for-linux,
linux-kernel, Alice Ryhl, Benno Lossin
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 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 (2):
rust: workqueue: restrict delayed work to global wqs
rust: workqueue: add creation of workqueues
rust/helpers/workqueue.c | 6 ++
rust/kernel/workqueue.rs | 187 ++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 189 insertions(+), 4 deletions(-)
---
base-commit: 211ddde0823f1442e4ad052a2f30f050145ccada
change-id: 20250411-create-workqueue-d053158c7a4b
Best regards,
--
Alice Ryhl <aliceryhl@google.com>
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH v2 1/2] rust: workqueue: restrict delayed work to global wqs 2025-11-13 10:01 [PATCH v2 0/2] Creation of workqueues in Rust Alice Ryhl @ 2025-11-13 10:01 ` Alice Ryhl 2025-11-13 10:06 ` Miguel Ojeda 2025-11-13 20:40 ` John Hubbard 2025-11-13 10:01 ` [PATCH v2 2/2] rust: workqueue: add creation of workqueues Alice Ryhl 1 sibling, 2 replies; 12+ messages in thread From: Alice Ryhl @ 2025-11-13 10:01 UTC (permalink / raw) To: Tejun Heo, Miguel Ojeda Cc: Lai Jiangshan, Boqun Feng, Gary Guo, Björn Roy Baron, Andreas Hindborg, Trevor Gross, Danilo Krummrich, Daniel Almeida, John Hubbard, Philipp Stanner, Tamir Duberstein, rust-for-linux, linux-kernel, Alice Ryhl, Benno Lossin 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. Signed-off-by: Alice Ryhl <aliceryhl@google.com> --- rust/kernel/workqueue.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs index 706e833e9702ba4ba9aa7756b0e1fa80079a63fc..901102a8bca54c9fb58655d80fc9624b4dfe1dc1 100644 --- a/rust/kernel/workqueue.rs +++ b/rust/kernel/workqueue.rs @@ -297,7 +297,7 @@ 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. /// /// 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.51.2.1041.gc1ab5b90ca-goog ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] rust: workqueue: restrict delayed work to global wqs 2025-11-13 10:01 ` [PATCH v2 1/2] rust: workqueue: restrict delayed work to global wqs Alice Ryhl @ 2025-11-13 10:06 ` Miguel Ojeda 2025-11-13 10:08 ` Alice Ryhl 2025-11-13 20:40 ` John Hubbard 1 sibling, 1 reply; 12+ messages in thread From: Miguel Ojeda @ 2025-11-13 10:06 UTC (permalink / raw) To: Alice Ryhl Cc: Tejun Heo, Miguel Ojeda, Lai Jiangshan, Boqun Feng, Gary Guo, Björn Roy Baron, Andreas Hindborg, Trevor Gross, Danilo Krummrich, Daniel Almeida, John Hubbard, Philipp Stanner, Tamir Duberstein, rust-for-linux, linux-kernel, Benno Lossin On Thu, Nov 13, 2025 at 11:01 AM Alice Ryhl <aliceryhl@google.com> wrote: > > 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. Should this have a Fixes and Cc: stable? Cheers, Miguel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] rust: workqueue: restrict delayed work to global wqs 2025-11-13 10:06 ` Miguel Ojeda @ 2025-11-13 10:08 ` Alice Ryhl 2025-11-13 11:58 ` Miguel Ojeda 0 siblings, 1 reply; 12+ messages in thread From: Alice Ryhl @ 2025-11-13 10:08 UTC (permalink / raw) To: Miguel Ojeda Cc: Tejun Heo, Miguel Ojeda, Lai Jiangshan, Boqun Feng, Gary Guo, Björn Roy Baron, Andreas Hindborg, Trevor Gross, Danilo Krummrich, Daniel Almeida, John Hubbard, Philipp Stanner, Tamir Duberstein, rust-for-linux, linux-kernel, Benno Lossin On Thu, Nov 13, 2025 at 11:06 AM Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote: > > On Thu, Nov 13, 2025 at 11:01 AM Alice Ryhl <aliceryhl@google.com> wrote: > > > > 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. > > Should this have a Fixes and Cc: stable? I could go either way. Without the next patch, there's no way to use a non-global workqueue. But probably is useful to backport anyway and there's certainly no harm. Alice ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] rust: workqueue: restrict delayed work to global wqs 2025-11-13 10:08 ` Alice Ryhl @ 2025-11-13 11:58 ` Miguel Ojeda 0 siblings, 0 replies; 12+ messages in thread From: Miguel Ojeda @ 2025-11-13 11:58 UTC (permalink / raw) To: Alice Ryhl Cc: Tejun Heo, Miguel Ojeda, Lai Jiangshan, Boqun Feng, Gary Guo, Björn Roy Baron, Andreas Hindborg, Trevor Gross, Danilo Krummrich, Daniel Almeida, John Hubbard, Philipp Stanner, Tamir Duberstein, rust-for-linux, linux-kernel, Benno Lossin On Thu, Nov 13, 2025 at 11:08 AM Alice Ryhl <aliceryhl@google.com> wrote: > > I could go either way. Without the next patch, there's no way to use a > non-global workqueue. But probably is useful to backport anyway and > there's certainly no harm. Yeah, I would say then let's propose it for stable to be safe than sorry in case this gets missed later on (or someone else out there uses only the other one etc.). Cc: stable@vger.kernel.org Fixes: 7c098cd5eaae ("workqueue: rust: add delayed work items") Thanks! Cheers, Miguel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] rust: workqueue: restrict delayed work to global wqs 2025-11-13 10:01 ` [PATCH v2 1/2] rust: workqueue: restrict delayed work to global wqs Alice Ryhl 2025-11-13 10:06 ` Miguel Ojeda @ 2025-11-13 20:40 ` John Hubbard 2025-11-13 21:06 ` Miguel Ojeda 1 sibling, 1 reply; 12+ messages in thread From: John Hubbard @ 2025-11-13 20:40 UTC (permalink / raw) To: Alice Ryhl, Tejun Heo, Miguel Ojeda Cc: Lai Jiangshan, Boqun Feng, Gary Guo, Björn Roy Baron, Andreas Hindborg, Trevor Gross, Danilo Krummrich, Daniel Almeida, Philipp Stanner, Tamir Duberstein, rust-for-linux, linux-kernel, Benno Lossin On 11/13/25 2:01 AM, Alice Ryhl wrote: > 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. > > Signed-off-by: Alice Ryhl <aliceryhl@google.com> > --- > rust/kernel/workqueue.rs | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs > index 706e833e9702ba4ba9aa7756b0e1fa80079a63fc..901102a8bca54c9fb58655d80fc9624b4dfe1dc1 100644 > --- a/rust/kernel/workqueue.rs > +++ b/rust/kernel/workqueue.rs > @@ -297,7 +297,7 @@ 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. > /// > /// 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 Hi Alice, Looks good, just a documentation suggestion below. Reviewed-by: John Hubbard <jhubbard@nvidia.com> I think it deserves a documentation comment (even though patch 2/2 has not yet arrived, we can still document this accurately), for example: diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs index 313d897fe93c..9e55d68f3b0d 100644 --- a/rust/kernel/workqueue.rs +++ b/rust/kernel/workqueue.rs @@ -299,6 +299,8 @@ pub fn enqueue<W, const ID: u64>(&self, w: W) -> W::EnqueueOutput /// Enqueues a delayed work item. /// /// 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>(&'static self, w: W, delay: Jiffies) -> W::EnqueueOutput Also, rustfmt (1.8.0-stable) split the changed line for me, upon saving the file, I'm not sure if it will do that for everyone. Just a heads up. thanks, -- John Hubbard ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] rust: workqueue: restrict delayed work to global wqs 2025-11-13 20:40 ` John Hubbard @ 2025-11-13 21:06 ` Miguel Ojeda 0 siblings, 0 replies; 12+ messages in thread From: Miguel Ojeda @ 2025-11-13 21:06 UTC (permalink / raw) To: John Hubbard Cc: Alice Ryhl, Tejun Heo, Miguel Ojeda, Lai Jiangshan, Boqun Feng, Gary Guo, Björn Roy Baron, Andreas Hindborg, Trevor Gross, Danilo Krummrich, Daniel Almeida, Philipp Stanner, Tamir Duberstein, rust-for-linux, linux-kernel, Benno Lossin On Thu, Nov 13, 2025 at 9:40 PM John Hubbard <jhubbard@nvidia.com> wrote: > > Also, rustfmt (1.8.0-stable) split the changed line for me, upon > saving the file, I'm not sure if it will do that for everyone. Just > a heads up. If `rustfmt` happens to behave differently for different people (in the Rust versions we support), then that is a (quite painful) bug for us, so please let me know! Cheers, Miguel ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 2/2] rust: workqueue: add creation of workqueues 2025-11-13 10:01 [PATCH v2 0/2] Creation of workqueues in Rust Alice Ryhl 2025-11-13 10:01 ` [PATCH v2 1/2] rust: workqueue: restrict delayed work to global wqs Alice Ryhl @ 2025-11-13 10:01 ` Alice Ryhl 2025-11-13 19:52 ` Boqun Feng ` (2 more replies) 1 sibling, 3 replies; 12+ messages in thread From: Alice Ryhl @ 2025-11-13 10:01 UTC (permalink / raw) To: Tejun Heo, Miguel Ojeda Cc: Lai Jiangshan, Boqun Feng, Gary Guo, Björn Roy Baron, Andreas Hindborg, Trevor Gross, Danilo Krummrich, Daniel Almeida, John Hubbard, Philipp Stanner, Tamir Duberstein, rust-for-linux, linux-kernel, Alice Ryhl, Benno Lossin 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. A wrapper type Flags is provided for workqueue flags. It allows you to build any valid flag combination, while using a type-level marker for whether WQ_BH is used to prevent invalid flag combinations. The Flags wrapper also forces you to explicitly pick one of percpu, unbound, or bh. Signed-off-by: Alice Ryhl <aliceryhl@google.com> --- rust/helpers/workqueue.c | 6 ++ rust/kernel/workqueue.rs | 185 ++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 188 insertions(+), 3 deletions(-) diff --git a/rust/helpers/workqueue.c b/rust/helpers/workqueue.c index b2b82753509bf5dbd0f4ddebb96a95a51e5976b1..a67ed284b062b29937f09303cb516e6322cc961a 100644 --- a/rust/helpers/workqueue.c +++ b/rust/helpers/workqueue.c @@ -12,3 +12,9 @@ void rust_helper_init_work_with_key(struct work_struct *work, work_func_t func, INIT_LIST_HEAD(&work->entry); work->func = func; } + +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 901102a8bca54c9fb58655d80fc9624b4dfe1dc1..313d897fe93ceb84844ce9b253edec837e60ba6d 100644 --- a/rust/kernel/workqueue.rs +++ b/rust/kernel/workqueue.rs @@ -186,7 +186,7 @@ //! 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 +194,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] @@ -333,7 +337,7 @@ pub fn enqueue_delayed<W, const ID: u64>(&'static self, w: W, delay: Jiffies) -> /// 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 { @@ -346,6 +350,181 @@ pub fn try_spawn<T: 'static + Send + FnOnce()>( } } +/// Workqueue flags. +/// +/// 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`. +#[repr(transparent)] +#[derive(Copy, Clone)] +pub struct Flags<const BH: bool>(bindings::wq_flags); + +// BH only methods +impl Flags<true> { + /// Execute in bottom half (softirq) context. + #[inline] + pub const fn bh() -> Flags<true> { + Flags(bindings::wq_flags_WQ_BH) + } +} + +// Non-BH only methods +impl Flags<false> { + /// Not bound to any cpu. + #[inline] + pub const fn unbound() -> Flags<false> { + Flags(bindings::wq_flags_WQ_UNBOUND) + } + + /// Bound to a specific cpu. + #[inline] + pub const fn percpu() -> Flags<false> { + Flags(bindings::wq_flags_WQ_PERCPU) + } + + /// Allow this workqueue to be frozen during suspend. + #[inline] + pub const fn freezable(self) -> Self { + Flags(self.0 | bindings::wq_flags_WQ_FREEZABLE) + } + + /// This workqueue may be used during memory reclaim. + #[inline] + pub const fn mem_reclaim(self) -> Self { + Flags(self.0 | bindings::wq_flags_WQ_MEM_RECLAIM) + } + + /// Mark this workqueue as cpu intensive. + #[inline] + pub const fn cpu_intensive(self) -> Self { + Flags(self.0 | bindings::wq_flags_WQ_CPU_INTENSIVE) + } + + /// Make this workqueue visible in sysfs. + #[inline] + pub const fn sysfs(self) -> Self { + Flags(self.0 | bindings::wq_flags_WQ_SYSFS) + } +} + +// Methods for BH and non-BH. +impl<const BH: bool> Flags<BH> { + /// High priority workqueue. + #[inline] + pub const fn highpri(self) -> Self { + Flags(self.0 | bindings::wq_flags_WQ_HIGHPRI) + } +} + +/// 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>, +} + +#[expect(clippy::manual_c_str_literals)] +impl OwnedQueue { + /// Allocates a new workqueue. + /// + /// The provided name is used verbatim as the workqueue name. + /// + /// # Examples + /// + /// ``` + /// use kernel::c_str; + /// use kernel::workqueue::{OwnedQueue, Flags}; + /// + /// let wq = OwnedQueue::new(c_str!("my-wq"), Flags::unbound().sysfs(), 0)?; + /// wq.try_spawn( + /// GFP_KERNEL, + /// || pr_warn!("Printing from my-wq"), + /// )?; + /// # Ok::<(), Error>(()) + /// ``` + #[inline] + pub fn new<const BH: bool>( + name: &CStr, + flags: Flags<BH>, + max_active: usize, + ) -> Result<OwnedQueue, AllocError> { + // SAFETY: + // * "%s\0" is compatible with passing the name as a c-string. + // * the flags argument does not include internal flags. + let ptr = unsafe { + bindings::alloc_workqueue( + b"%s\0".as_ptr(), + flags.0, + i32::try_from(max_active).unwrap_or(i32::MAX), + 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; + /// use kernel::workqueue::{OwnedQueue, Flags}; + /// + /// fn my_wq(num: u32) -> Result<OwnedQueue, AllocError> { + /// OwnedQueue::new_fmt(format_args!("my-wq-{num}"), Flags::percpu(), 0) + /// } + /// ``` + #[inline] + pub fn new_fmt<const BH: bool>( + name: core::fmt::Arguments<'_>, + flags: Flags<BH>, + max_active: usize, + ) -> Result<OwnedQueue, AllocError> { + // SAFETY: + // * "%pA\0" is compatible with passing an `Arguments` pointer. + // * the flags argument does not include internal flags. + let ptr = unsafe { + bindings::alloc_workqueue( + b"%pA\0".as_ptr(), + flags.0, + i32::try_from(max_active).unwrap_or(i32::MAX), + core::ptr::from_ref(&name).cast::<c_void>(), + ) + }; + + Ok(OwnedQueue { + queue: NonNull::new(ptr).ok_or(AllocError)?.cast(), + }) + } +} + +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.51.2.1041.gc1ab5b90ca-goog ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] rust: workqueue: add creation of workqueues 2025-11-13 10:01 ` [PATCH v2 2/2] rust: workqueue: add creation of workqueues Alice Ryhl @ 2025-11-13 19:52 ` Boqun Feng 2025-11-14 9:44 ` Alice Ryhl 2025-11-13 21:55 ` Danilo Krummrich 2025-11-14 0:26 ` John Hubbard 2 siblings, 1 reply; 12+ messages in thread From: Boqun Feng @ 2025-11-13 19:52 UTC (permalink / raw) To: Alice Ryhl Cc: Tejun Heo, Miguel Ojeda, Lai Jiangshan, Gary Guo, Björn Roy Baron, Andreas Hindborg, Trevor Gross, Danilo Krummrich, Daniel Almeida, John Hubbard, Philipp Stanner, Tamir Duberstein, rust-for-linux, linux-kernel, Benno Lossin On Thu, Nov 13, 2025 at 10:01:07AM +0000, 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. > > A wrapper type Flags is provided for workqueue flags. It allows you to > build any valid flag combination, while using a type-level marker for > whether WQ_BH is used to prevent invalid flag combinations. The Flags wrapper > also forces you to explicitly pick one of percpu, unbound, or bh. > > Signed-off-by: Alice Ryhl <aliceryhl@google.com> [...] > +/// 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>, I hope Owned/Ownable can make it just a Owned<Queue> here ;-) And that'll make Owned<Queue> automatically Send + Sync. I think it's not a rare ask for `OwnedQueue` to be Send + Sync. > +} > + > +#[expect(clippy::manual_c_str_literals)] > +impl OwnedQueue { > + /// Allocates a new workqueue. > + /// > + /// The provided name is used verbatim as the workqueue name. > + /// > + /// # Examples > + /// > + /// ``` > + /// use kernel::c_str; > + /// use kernel::workqueue::{OwnedQueue, Flags}; > + /// > + /// let wq = OwnedQueue::new(c_str!("my-wq"), Flags::unbound().sysfs(), 0)?; > + /// wq.try_spawn( > + /// GFP_KERNEL, > + /// || pr_warn!("Printing from my-wq"), > + /// )?; > + /// # Ok::<(), Error>(()) > + /// ``` > + #[inline] > + pub fn new<const BH: bool>( > + name: &CStr, > + flags: Flags<BH>, > + max_active: usize, Do we need to support `max_active` as `usize` when the underlying C code only support i32? Regards, Boqun > + ) -> Result<OwnedQueue, AllocError> { > + // SAFETY: > + // * "%s\0" is compatible with passing the name as a c-string. > + // * the flags argument does not include internal flags. > + let ptr = unsafe { > + bindings::alloc_workqueue( > + b"%s\0".as_ptr(), > + flags.0, > + i32::try_from(max_active).unwrap_or(i32::MAX), > + name.as_char_ptr().cast::<c_void>(), > + ) > + }; > + > + Ok(OwnedQueue { > + queue: NonNull::new(ptr).ok_or(AllocError)?.cast(), > + }) > + } [...] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] rust: workqueue: add creation of workqueues 2025-11-13 19:52 ` Boqun Feng @ 2025-11-14 9:44 ` Alice Ryhl 0 siblings, 0 replies; 12+ messages in thread From: Alice Ryhl @ 2025-11-14 9:44 UTC (permalink / raw) To: Boqun Feng Cc: Tejun Heo, Miguel Ojeda, Lai Jiangshan, Gary Guo, Björn Roy Baron, Andreas Hindborg, Trevor Gross, Danilo Krummrich, Daniel Almeida, John Hubbard, Philipp Stanner, Tamir Duberstein, rust-for-linux, linux-kernel, Benno Lossin On Thu, Nov 13, 2025 at 11:52:17AM -0800, Boqun Feng wrote: > On Thu, Nov 13, 2025 at 10:01:07AM +0000, 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. > > > > A wrapper type Flags is provided for workqueue flags. It allows you to > > build any valid flag combination, while using a type-level marker for > > whether WQ_BH is used to prevent invalid flag combinations. The Flags wrapper > > also forces you to explicitly pick one of percpu, unbound, or bh. > > > > Signed-off-by: Alice Ryhl <aliceryhl@google.com> > [...] > > +/// 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>, > > I hope Owned/Ownable can make it just a Owned<Queue> here ;-) And > that'll make Owned<Queue> automatically Send + Sync. I think it's not a > rare ask for `OwnedQueue` to be Send + Sync. Oh yeah it should be Send/Sync. And hopefully we get Owned eventually. > > +} > > + > > +#[expect(clippy::manual_c_str_literals)] > > +impl OwnedQueue { > > + /// Allocates a new workqueue. > > + /// > > + /// The provided name is used verbatim as the workqueue name. > > + /// > > + /// # Examples > > + /// > > + /// ``` > > + /// use kernel::c_str; > > + /// use kernel::workqueue::{OwnedQueue, Flags}; > > + /// > > + /// let wq = OwnedQueue::new(c_str!("my-wq"), Flags::unbound().sysfs(), 0)?; > > + /// wq.try_spawn( > > + /// GFP_KERNEL, > > + /// || pr_warn!("Printing from my-wq"), > > + /// )?; > > + /// # Ok::<(), Error>(()) > > + /// ``` > > + #[inline] > > + pub fn new<const BH: bool>( > > + name: &CStr, > > + flags: Flags<BH>, > > + max_active: usize, > > Do we need to support `max_active` as `usize` when the underlying C > code only support i32? Negative values don't really make sense. But maybe it's a good idea, yeah. Alice ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] rust: workqueue: add creation of workqueues 2025-11-13 10:01 ` [PATCH v2 2/2] rust: workqueue: add creation of workqueues Alice Ryhl 2025-11-13 19:52 ` Boqun Feng @ 2025-11-13 21:55 ` Danilo Krummrich 2025-11-14 0:26 ` John Hubbard 2 siblings, 0 replies; 12+ messages in thread From: Danilo Krummrich @ 2025-11-13 21:55 UTC (permalink / raw) To: Alice Ryhl Cc: Tejun Heo, Miguel Ojeda, Lai Jiangshan, Boqun Feng, Gary Guo, Björn Roy Baron, Andreas Hindborg, Trevor Gross, Daniel Almeida, John Hubbard, Philipp Stanner, Tamir Duberstein, rust-for-linux, linux-kernel, Benno Lossin On Thu Nov 13, 2025 at 9:01 PM AEDT, Alice Ryhl wrote: > Creating workqueues is needed by various GPU drivers. Indeed, and of course lots of other drivers need this as well. :) > 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. Exactly, but for this case we should fence the workqueue with devres, such that we can guarantee a scope for a &Device<Bound> for all work items scheduled on such a workqueue, i.e. provide a &Device<Bound> in run(). Otherwise drivers have to either access device resources with try_access() or get themselves a &Device<Bound> with the unsafe Device::as_bound() method from within the run() callback. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] rust: workqueue: add creation of workqueues 2025-11-13 10:01 ` [PATCH v2 2/2] rust: workqueue: add creation of workqueues Alice Ryhl 2025-11-13 19:52 ` Boqun Feng 2025-11-13 21:55 ` Danilo Krummrich @ 2025-11-14 0:26 ` John Hubbard 2 siblings, 0 replies; 12+ messages in thread From: John Hubbard @ 2025-11-14 0:26 UTC (permalink / raw) To: Alice Ryhl, Tejun Heo, Miguel Ojeda Cc: Lai Jiangshan, Boqun Feng, Gary Guo, Björn Roy Baron, Andreas Hindborg, Trevor Gross, Danilo Krummrich, Daniel Almeida, Philipp Stanner, Tamir Duberstein, rust-for-linux, linux-kernel, Benno Lossin On 11/13/25 2:01 AM, 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. Hi Alice, Thanks for doing this! I am not seeing any large issues, other than the Device<Bound> point that Danilo requested. > > A wrapper type Flags is provided for workqueue flags. It allows you to > build any valid flag combination, while using a type-level marker for > whether WQ_BH is used to prevent invalid flag combinations. The Flags wrapper > also forces you to explicitly pick one of percpu, unbound, or bh. > > Signed-off-by: Alice Ryhl <aliceryhl@google.com> > --- > rust/helpers/workqueue.c | 6 ++ > rust/kernel/workqueue.rs | 185 ++++++++++++++++++++++++++++++++++++++++++++++- > 2 files changed, 188 insertions(+), 3 deletions(-) > > diff --git a/rust/helpers/workqueue.c b/rust/helpers/workqueue.c > index b2b82753509bf5dbd0f4ddebb96a95a51e5976b1..a67ed284b062b29937f09303cb516e6322cc961a 100644 > --- a/rust/helpers/workqueue.c > +++ b/rust/helpers/workqueue.c > @@ -12,3 +12,9 @@ void rust_helper_init_work_with_key(struct work_struct *work, work_func_t func, > INIT_LIST_HEAD(&work->entry); > work->func = func; > } > + > +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 901102a8bca54c9fb58655d80fc9624b4dfe1dc1..313d897fe93ceb84844ce9b253edec837e60ba6d 100644 > --- a/rust/kernel/workqueue.rs > +++ b/rust/kernel/workqueue.rs > @@ -186,7 +186,7 @@ > //! C header: [`include/linux/workqueue.h`](srctree/include/linux/workqueue.h) > > use crate::{ > - alloc::{AllocError, Flags}, > + alloc::{self, AllocError}, Nit: that should be: alloc::{ self, AllocError, // }, > container_of, > prelude::*, > sync::Arc, > @@ -194,7 +194,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] > @@ -333,7 +337,7 @@ pub fn enqueue_delayed<W, const ID: u64>(&'static self, w: W, delay: Jiffies) -> > /// 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 { > @@ -346,6 +350,181 @@ pub fn try_spawn<T: 'static + Send + FnOnce()>( > } > } > > +/// Workqueue flags. > +/// > +/// A valid combination of workqueue flags contains one of the base flags (`WQ_UNBOUND`, `WQ_BH`, Another tiny tweak: "contains exactly one", just to be extra clear. > +/// 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`. > +#[repr(transparent)] > +#[derive(Copy, Clone)] > +pub struct Flags<const BH: bool>(bindings::wq_flags); > + > +// BH only methods > +impl Flags<true> { > + /// Execute in bottom half (softirq) context. > + #[inline] > + pub const fn bh() -> Flags<true> { > + Flags(bindings::wq_flags_WQ_BH) > + } > +} > + > +// Non-BH only methods > +impl Flags<false> { > + /// Not bound to any cpu. > + #[inline] > + pub const fn unbound() -> Flags<false> { > + Flags(bindings::wq_flags_WQ_UNBOUND) > + } > + > + /// Bound to a specific cpu. > + #[inline] > + pub const fn percpu() -> Flags<false> { > + Flags(bindings::wq_flags_WQ_PERCPU) > + } > + It seems like the one public-facing item that's missing is WQ_POWER_EFFICIENT. Should we add it? It provides some nice flexibility for things like laptops. Although I see that it has not made its way into Documentation/core-api/workqueue.rst (!). Also, I think WQ_CPU_INTENSIVE might also be a desirable addition. > + /// Allow this workqueue to be frozen during suspend. > + #[inline] > + pub const fn freezable(self) -> Self { > + Flags(self.0 | bindings::wq_flags_WQ_FREEZABLE) > + } > + > + /// This workqueue may be used during memory reclaim. > + #[inline] > + pub const fn mem_reclaim(self) -> Self { > + Flags(self.0 | bindings::wq_flags_WQ_MEM_RECLAIM) > + } > + > + /// Mark this workqueue as cpu intensive. > + #[inline] > + pub const fn cpu_intensive(self) -> Self { > + Flags(self.0 | bindings::wq_flags_WQ_CPU_INTENSIVE) > + } > + > + /// Make this workqueue visible in sysfs. > + #[inline] > + pub const fn sysfs(self) -> Self { > + Flags(self.0 | bindings::wq_flags_WQ_SYSFS) > + } > +} > + > +// Methods for BH and non-BH. > +impl<const BH: bool> Flags<BH> { > + /// High priority workqueue. > + #[inline] > + pub const fn highpri(self) -> Self { > + Flags(self.0 | bindings::wq_flags_WQ_HIGHPRI) > + } > +} > + > +/// 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>, > +} > + > +#[expect(clippy::manual_c_str_literals)] Any reason not to move to c"" strings now? I suspect this is an older patch that you've revived, and since then the new approach showed up. > +impl OwnedQueue { > + /// Allocates a new workqueue. > + /// > + /// The provided name is used verbatim as the workqueue name. "The provided name is used, verbatim, as the workqueue name." Or simply: "The provided name is used as the workqueue name." > + /// > + /// # Examples > + /// > + /// ``` > + /// use kernel::c_str; > + /// use kernel::workqueue::{OwnedQueue, Flags}; > + /// > + /// let wq = OwnedQueue::new(c_str!("my-wq"), Flags::unbound().sysfs(), 0)?; > + /// wq.try_spawn( > + /// GFP_KERNEL, > + /// || pr_warn!("Printing from my-wq"), > + /// )?; > + /// # Ok::<(), Error>(()) > + /// ``` > + #[inline] > + pub fn new<const BH: bool>( > + name: &CStr, > + flags: Flags<BH>, > + max_active: usize, > + ) -> Result<OwnedQueue, AllocError> { > + // SAFETY: > + // * "%s\0" is compatible with passing the name as a c-string. > + // * the flags argument does not include internal flags. > + let ptr = unsafe { > + bindings::alloc_workqueue( > + b"%s\0".as_ptr(), > + flags.0, > + i32::try_from(max_active).unwrap_or(i32::MAX), Or just make max_active an i32. > + 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; > + /// use kernel::workqueue::{OwnedQueue, Flags}; > + /// > + /// fn my_wq(num: u32) -> Result<OwnedQueue, AllocError> { > + /// OwnedQueue::new_fmt(format_args!("my-wq-{num}"), Flags::percpu(), 0) > + /// } > + /// ``` > + #[inline] > + pub fn new_fmt<const BH: bool>( > + name: core::fmt::Arguments<'_>, > + flags: Flags<BH>, > + max_active: usize, > + ) -> Result<OwnedQueue, AllocError> { > + // SAFETY: > + // * "%pA\0" is compatible with passing an `Arguments` pointer. > + // * the flags argument does not include internal flags. > + let ptr = unsafe { > + bindings::alloc_workqueue( > + b"%pA\0".as_ptr(), > + flags.0, > + i32::try_from(max_active).unwrap_or(i32::MAX), > + core::ptr::from_ref(&name).cast::<c_void>(), > + ) > + }; > + > + Ok(OwnedQueue { > + queue: NonNull::new(ptr).ok_or(AllocError)?.cast(), > + }) > + } > +} > + > +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 > thanks, -- John Hubbard ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2025-11-14 9:45 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-11-13 10:01 [PATCH v2 0/2] Creation of workqueues in Rust Alice Ryhl 2025-11-13 10:01 ` [PATCH v2 1/2] rust: workqueue: restrict delayed work to global wqs Alice Ryhl 2025-11-13 10:06 ` Miguel Ojeda 2025-11-13 10:08 ` Alice Ryhl 2025-11-13 11:58 ` Miguel Ojeda 2025-11-13 20:40 ` John Hubbard 2025-11-13 21:06 ` Miguel Ojeda 2025-11-13 10:01 ` [PATCH v2 2/2] rust: workqueue: add creation of workqueues Alice Ryhl 2025-11-13 19:52 ` Boqun Feng 2025-11-14 9:44 ` Alice Ryhl 2025-11-13 21:55 ` Danilo Krummrich 2025-11-14 0:26 ` John Hubbard
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).