rust/kernel/workqueue.rs | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)
`try_spawn()` could use an example to demonstrate the usage, and
arguably it's the most simple usage of workqueue in case someone needs a
deferred work, so add it.
Cc: Joel Fernandes <joelagnelf@nvidia.com>
Cc: "Paul E. McKenney" <paulmck@kernel.org>
Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
---
Miguel, Alice and Tejun, while I'm at it, should we also rename the
function to `spawn()` because of the motivation mentioned here [1]?
[1]: https://rust-for-linux.zulipchat.com/#narrow/channel/291566-Library/topic/.60new.60.20or.20.60try_new.60.3F/near/529533317
Also I find the `{ <clone> || { } }` is really good if I only need to
clone the Arc for passing to a callback closure, but I'm not sure how
people feel about it, so criticism is welcome ;-)
rust/kernel/workqueue.rs | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
index b9343d5bc00f..59c1a5e14d12 100644
--- a/rust/kernel/workqueue.rs
+++ b/rust/kernel/workqueue.rs
@@ -331,6 +331,33 @@ pub fn enqueue_delayed<W, const ID: u64>(&self, w: W, delay: Jiffies) -> W::Enqu
/// Tries to spawn the given function or closure as a work item.
///
/// This method can fail because it allocates memory to store the work item.
+ ///
+ /// # Examples
+ ///
+ /// ```
+ /// use kernel::{alloc::flags, sync::{Arc, Completion, new_spinlock}, workqueue};
+ ///
+ /// let work_done = Arc::pin_init(Completion::new(), flags::GFP_KERNEL)?;
+ /// let data = Arc::pin_init(new_spinlock!(0), flags::GFP_KERNEL)?;
+ ///
+ /// workqueue::system().try_spawn(
+ /// flags::GFP_KERNEL,
+ /// {
+ /// let work_done = work_done.clone();
+ /// let data = data.clone();
+ /// move || {
+ /// *data.lock() = 42;
+ /// work_done.complete_all();
+ /// }
+ /// }
+ /// )?;
+ ///
+ /// work_done.wait_for_completion();
+ ///
+ /// // `work_done` being completed implies the observation of the write of `data` in the work.
+ /// assert_eq!(*data.lock(), 42);
+ /// # Ok::<(), Error>(())
+ /// ```
pub fn try_spawn<T: 'static + Send + FnOnce()>(
&self,
flags: Flags,
--
2.39.5 (Apple Git-154)On Wed, Jul 30, 2025 at 09:34:39AM -0700, Boqun Feng wrote:
> `try_spawn()` could use an example to demonstrate the usage, and
> arguably it's the most simple usage of workqueue in case someone needs a
> deferred work, so add it.
>
> Cc: Joel Fernandes <joelagnelf@nvidia.com>
> Cc: "Paul E. McKenney" <paulmck@kernel.org>
> Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> ---
> Miguel, Alice and Tejun, while I'm at it, should we also rename the
> function to `spawn()` because of the motivation mentioned here [1]?
>
> [1]: https://rust-for-linux.zulipchat.com/#narrow/channel/291566-Library/topic/.60new.60.20or.20.60try_new.60.3F/near/529533317
Renaming sounds good to me.
> Also I find the `{ <clone> || { } }` is really good if I only need to
> clone the Arc for passing to a callback closure, but I'm not sure how
> people feel about it, so criticism is welcome ;-)
It's a neat trick, but I think it risks confusing people. I would
probably not include it in an example like this one.
> rust/kernel/workqueue.rs | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
> index b9343d5bc00f..59c1a5e14d12 100644
> --- a/rust/kernel/workqueue.rs
> +++ b/rust/kernel/workqueue.rs
> @@ -331,6 +331,33 @@ pub fn enqueue_delayed<W, const ID: u64>(&self, w: W, delay: Jiffies) -> W::Enqu
> /// Tries to spawn the given function or closure as a work item.
> ///
> /// This method can fail because it allocates memory to store the work item.
> + ///
> + /// # Examples
> + ///
> + /// ```
> + /// use kernel::{alloc::flags, sync::{Arc, Completion, new_spinlock}, workqueue};
> + ///
> + /// let work_done = Arc::pin_init(Completion::new(), flags::GFP_KERNEL)?;
> + /// let data = Arc::pin_init(new_spinlock!(0), flags::GFP_KERNEL)?;
> + ///
> + /// workqueue::system().try_spawn(
> + /// flags::GFP_KERNEL,
> + /// {
> + /// let work_done = work_done.clone();
> + /// let data = data.clone();
> + /// move || {
> + /// *data.lock() = 42;
> + /// work_done.complete_all();
> + /// }
> + /// }
> + /// )?;
> + ///
> + /// work_done.wait_for_completion();
> + ///
> + /// // `work_done` being completed implies the observation of the write of `data` in the work.
> + /// assert_eq!(*data.lock(), 42);
> + /// # Ok::<(), Error>(())
> + /// ```
> pub fn try_spawn<T: 'static + Send + FnOnce()>(
> &self,
> flags: Flags,
> --
> 2.39.5 (Apple Git-154)
>
On Wed Jul 30, 2025 at 6:34 PM CEST, Boqun Feng wrote:
> `try_spawn()` could use an example to demonstrate the usage, and
> arguably it's the most simple usage of workqueue in case someone needs a
> deferred work, so add it.
>
> Cc: Joel Fernandes <joelagnelf@nvidia.com>
> Cc: "Paul E. McKenney" <paulmck@kernel.org>
> Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> ---
> Miguel, Alice and Tejun, while I'm at it, should we also rename the
> function to `spawn()` because of the motivation mentioned here [1]?
>
> [1]: https://rust-for-linux.zulipchat.com/#narrow/channel/291566-Library/topic/.60new.60.20or.20.60try_new.60.3F/near/529533317
>
> Also I find the `{ <clone> || { } }` is really good if I only need to
> clone the Arc for passing to a callback closure, but I'm not sure how
> people feel about it, so criticism is welcome ;-)
I'm not so sure, see below :)
> rust/kernel/workqueue.rs | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
> index b9343d5bc00f..59c1a5e14d12 100644
> --- a/rust/kernel/workqueue.rs
> +++ b/rust/kernel/workqueue.rs
> @@ -331,6 +331,33 @@ pub fn enqueue_delayed<W, const ID: u64>(&self, w: W, delay: Jiffies) -> W::Enqu
> /// Tries to spawn the given function or closure as a work item.
> ///
> /// This method can fail because it allocates memory to store the work item.
> + ///
> + /// # Examples
> + ///
> + /// ```
> + /// use kernel::{alloc::flags, sync::{Arc, Completion, new_spinlock}, workqueue};
> + ///
> + /// let work_done = Arc::pin_init(Completion::new(), flags::GFP_KERNEL)?;
> + /// let data = Arc::pin_init(new_spinlock!(0), flags::GFP_KERNEL)?;
> + ///
> + /// workqueue::system().try_spawn(
> + /// flags::GFP_KERNEL,
> + /// {
> + /// let work_done = work_done.clone();
> + /// let data = data.clone();
> + /// move || {
> + /// *data.lock() = 42;
> + /// work_done.complete_all();
> + /// }
> + /// }
> + /// )?;
Not doing your pattern and instead adding a `2` postfix we get:
let work_done2 = work_done.clone();
let data2 = data.clone();
workqueue::system().try_spawn(flags::GFP_KERNEL, move || {
*data2.lock() = 42;
work_done2.complete_all();
})?;
There are some discussions of introducing some better syntax for (cheap)
cloning, so maybe we can use that in the future.
---
Cheers,
Benno
> + ///
> + /// work_done.wait_for_completion();
> + ///
> + /// // `work_done` being completed implies the observation of the write of `data` in the work.
> + /// assert_eq!(*data.lock(), 42);
> + /// # Ok::<(), Error>(())
> + /// ```
> pub fn try_spawn<T: 'static + Send + FnOnce()>(
> &self,
> flags: Flags,
On Wed, Jul 30, 2025 at 09:28:05PM +0200, Benno Lossin wrote:
> On Wed Jul 30, 2025 at 6:34 PM CEST, Boqun Feng wrote:
> > `try_spawn()` could use an example to demonstrate the usage, and
> > arguably it's the most simple usage of workqueue in case someone needs a
> > deferred work, so add it.
> >
> > Cc: Joel Fernandes <joelagnelf@nvidia.com>
> > Cc: "Paul E. McKenney" <paulmck@kernel.org>
> > Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> > ---
> > Miguel, Alice and Tejun, while I'm at it, should we also rename the
> > function to `spawn()` because of the motivation mentioned here [1]?
> >
> > [1]: https://rust-for-linux.zulipchat.com/#narrow/channel/291566-Library/topic/.60new.60.20or.20.60try_new.60.3F/near/529533317
> >
> > Also I find the `{ <clone> || { } }` is really good if I only need to
> > clone the Arc for passing to a callback closure, but I'm not sure how
> > people feel about it, so criticism is welcome ;-)
>
> I'm not so sure, see below :)
>
> > rust/kernel/workqueue.rs | 27 +++++++++++++++++++++++++++
> > 1 file changed, 27 insertions(+)
> >
> > diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
> > index b9343d5bc00f..59c1a5e14d12 100644
> > --- a/rust/kernel/workqueue.rs
> > +++ b/rust/kernel/workqueue.rs
> > @@ -331,6 +331,33 @@ pub fn enqueue_delayed<W, const ID: u64>(&self, w: W, delay: Jiffies) -> W::Enqu
> > /// Tries to spawn the given function or closure as a work item.
> > ///
> > /// This method can fail because it allocates memory to store the work item.
> > + ///
> > + /// # Examples
> > + ///
> > + /// ```
> > + /// use kernel::{alloc::flags, sync::{Arc, Completion, new_spinlock}, workqueue};
> > + ///
> > + /// let work_done = Arc::pin_init(Completion::new(), flags::GFP_KERNEL)?;
> > + /// let data = Arc::pin_init(new_spinlock!(0), flags::GFP_KERNEL)?;
> > + ///
> > + /// workqueue::system().try_spawn(
> > + /// flags::GFP_KERNEL,
> > + /// {
> > + /// let work_done = work_done.clone();
> > + /// let data = data.clone();
> > + /// move || {
> > + /// *data.lock() = 42;
> > + /// work_done.complete_all();
> > + /// }
> > + /// }
> > + /// )?;
>
> Not doing your pattern and instead adding a `2` postfix we get:
>
> let work_done2 = work_done.clone();
> let data2 = data.clone();
>
Yeah, the thing I want to achieve with my pattern is: make it clear that
the work and the task that queues the work are sharing the same
`work_done` and `data` (well, no the same `Arc` exactly, but the `Arc`s
that are pointing to the same object). This pattern here doesn't show
that clearly imo.
That said, I'm not really against using `work_done2` and `data2`, just
I'm afraid that may be more confusing.
> workqueue::system().try_spawn(flags::GFP_KERNEL, move || {
> *data2.lock() = 42;
> work_done2.complete_all();
> })?;
>
> There are some discussions of introducing some better syntax for (cheap)
> cloning, so maybe we can use that in the future.
Do you have links to these discussions.
Regards,
Boqun
>
> ---
> Cheers,
> Benno
>
> > + ///
> > + /// work_done.wait_for_completion();
> > + ///
> > + /// // `work_done` being completed implies the observation of the write of `data` in the work.
> > + /// assert_eq!(*data.lock(), 42);
> > + /// # Ok::<(), Error>(())
> > + /// ```
> > pub fn try_spawn<T: 'static + Send + FnOnce()>(
> > &self,
> > flags: Flags,
>
On Wed Jul 30, 2025 at 9:38 PM CEST, Boqun Feng wrote:
> On Wed, Jul 30, 2025 at 09:28:05PM +0200, Benno Lossin wrote:
>> On Wed Jul 30, 2025 at 6:34 PM CEST, Boqun Feng wrote:
>> > + /// workqueue::system().try_spawn(
>> > + /// flags::GFP_KERNEL,
>> > + /// {
>> > + /// let work_done = work_done.clone();
>> > + /// let data = data.clone();
>> > + /// move || {
>> > + /// *data.lock() = 42;
>> > + /// work_done.complete_all();
>> > + /// }
>> > + /// }
>> > + /// )?;
>>
>> Not doing your pattern and instead adding a `2` postfix we get:
>>
>> let work_done2 = work_done.clone();
>> let data2 = data.clone();
>>
>
> Yeah, the thing I want to achieve with my pattern is: make it clear that
> the work and the task that queues the work are sharing the same
> `work_done` and `data` (well, no the same `Arc` exactly, but the `Arc`s
> that are pointing to the same object). This pattern here doesn't show
> that clearly imo.
I think it's fine, that pattern is often used for that. Not heavily
opposed to doing it your way, but I feel like the code looks a bit weird
& my instinct is to move the let bindings out (which would produce code
that doesn't compile).
> That said, I'm not really against using `work_done2` and `data2`, just
> I'm afraid that may be more confusing.
I don't think that's a problem.
>> workqueue::system().try_spawn(flags::GFP_KERNEL, move || {
>> *data2.lock() = 42;
>> work_done2.complete_all();
>> })?;
>>
>> There are some discussions of introducing some better syntax for (cheap)
>> cloning, so maybe we can use that in the future.
>
> Do you have links to these discussions.
It's an RFC:
https://github.com/rust-lang/rfcs/pull/368
There probably are more discussions on zulip, but I haven't read those.
The RFC also has a project goal:
https://rust-lang.github.io/rust-project-goals/2025h2/ergonomic-rc.html
---
Cheers,
Benno
On Thu, Jul 31, 2025 at 11:30:10AM +0200, Benno Lossin wrote:
> On Wed Jul 30, 2025 at 9:38 PM CEST, Boqun Feng wrote:
> > On Wed, Jul 30, 2025 at 09:28:05PM +0200, Benno Lossin wrote:
> >> On Wed Jul 30, 2025 at 6:34 PM CEST, Boqun Feng wrote:
> >> > + /// workqueue::system().try_spawn(
> >> > + /// flags::GFP_KERNEL,
> >> > + /// {
> >> > + /// let work_done = work_done.clone();
> >> > + /// let data = data.clone();
> >> > + /// move || {
> >> > + /// *data.lock() = 42;
> >> > + /// work_done.complete_all();
> >> > + /// }
> >> > + /// }
> >> > + /// )?;
> >>
> >> Not doing your pattern and instead adding a `2` postfix we get:
> >>
> >> let work_done2 = work_done.clone();
> >> let data2 = data.clone();
> >>
> >
> > Yeah, the thing I want to achieve with my pattern is: make it clear that
> > the work and the task that queues the work are sharing the same
> > `work_done` and `data` (well, no the same `Arc` exactly, but the `Arc`s
> > that are pointing to the same object). This pattern here doesn't show
> > that clearly imo.
>
> I think it's fine, that pattern is often used for that. Not heavily
> opposed to doing it your way, but I feel like the code looks a bit weird
Ok, I will drop my style and use work_done2 and data2, because it'll be
at the general documentation, but I might keep using my pattern in other
code because it looks reasonable to me ;-)
> & my instinct is to move the let bindings out (which would produce code
> that doesn't compile).
>
> > That said, I'm not really against using `work_done2` and `data2`, just
> > I'm afraid that may be more confusing.
>
> I don't think that's a problem.
>
> >> workqueue::system().try_spawn(flags::GFP_KERNEL, move || {
> >> *data2.lock() = 42;
> >> work_done2.complete_all();
> >> })?;
> >>
> >> There are some discussions of introducing some better syntax for (cheap)
> >> cloning, so maybe we can use that in the future.
> >
> > Do you have links to these discussions.
>
> It's an RFC:
>
> https://github.com/rust-lang/rfcs/pull/368
>
> There probably are more discussions on zulip, but I haven't read those.
> The RFC also has a project goal:
>
> https://rust-lang.github.io/rust-project-goals/2025h2/ergonomic-rc.html
Thanks for the pointer.
Regards,
Boqun
>
> ---
> Cheers,
> Benno
On Fri Aug 1, 2025 at 3:15 AM CEST, Boqun Feng wrote:
> On Thu, Jul 31, 2025 at 11:30:10AM +0200, Benno Lossin wrote:
>> On Wed Jul 30, 2025 at 9:38 PM CEST, Boqun Feng wrote:
>> > On Wed, Jul 30, 2025 at 09:28:05PM +0200, Benno Lossin wrote:
>> >> On Wed Jul 30, 2025 at 6:34 PM CEST, Boqun Feng wrote:
>> >> > + /// workqueue::system().try_spawn(
>> >> > + /// flags::GFP_KERNEL,
>> >> > + /// {
>> >> > + /// let work_done = work_done.clone();
>> >> > + /// let data = data.clone();
>> >> > + /// move || {
>> >> > + /// *data.lock() = 42;
>> >> > + /// work_done.complete_all();
>> >> > + /// }
>> >> > + /// }
>> >> > + /// )?;
>> >>
>> >> Not doing your pattern and instead adding a `2` postfix we get:
>> >>
>> >> let work_done2 = work_done.clone();
>> >> let data2 = data.clone();
>> >>
>> >
>> > Yeah, the thing I want to achieve with my pattern is: make it clear that
>> > the work and the task that queues the work are sharing the same
>> > `work_done` and `data` (well, no the same `Arc` exactly, but the `Arc`s
>> > that are pointing to the same object). This pattern here doesn't show
>> > that clearly imo.
>>
>> I think it's fine, that pattern is often used for that. Not heavily
>> opposed to doing it your way, but I feel like the code looks a bit weird
>
> Ok, I will drop my style and use work_done2 and data2, because it'll be
> at the general documentation, but I might keep using my pattern in other
> code because it looks reasonable to me ;-)
It is reasonable :) If you do want to use the same name for them, then I
personally think this looks better than moving the let bindings inside
of the expression:
{
let work_done = work_done.clone();
let data = data.clone();
workqueue::system().try_spawn(flags::GFP_KERNEL, move || {
*data.lock() = 42;
work_done.complete_all();
})?;
}
But that gives the `try_spawn` call extra indentation which also isn't
ideal... I'd be best for the language feature to exist :)
---
Cheers,
Benno
On 7/30/2025 12:34 PM, Boqun Feng wrote:
> `try_spawn()` could use an example to demonstrate the usage, and
> arguably it's the most simple usage of workqueue in case someone needs a
> deferred work, so add it.
>
> Cc: Joel Fernandes <joelagnelf@nvidia.com>
> Cc: "Paul E. McKenney" <paulmck@kernel.org>
> Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
Makes sense, LGTM.
Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>
thanks,
- Joel
> ---
> Miguel, Alice and Tejun, while I'm at it, should we also rename the
> function to `spawn()` because of the motivation mentioned here [1]?
>
> [1]: https://rust-for-linux.zulipchat.com/#narrow/channel/291566-Library/topic/.60new.60.20or.20.60try_new.60.3F/near/529533317
>
> Also I find the `{ <clone> || { } }` is really good if I only need to
> clone the Arc for passing to a callback closure, but I'm not sure how
> people feel about it, so criticism is welcome ;-)
>
> rust/kernel/workqueue.rs | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
> index b9343d5bc00f..59c1a5e14d12 100644
> --- a/rust/kernel/workqueue.rs
> +++ b/rust/kernel/workqueue.rs
> @@ -331,6 +331,33 @@ pub fn enqueue_delayed<W, const ID: u64>(&self, w: W, delay: Jiffies) -> W::Enqu
> /// Tries to spawn the given function or closure as a work item.
> ///
> /// This method can fail because it allocates memory to store the work item.
> + ///
> + /// # Examples
> + ///
> + /// ```
> + /// use kernel::{alloc::flags, sync::{Arc, Completion, new_spinlock}, workqueue};
> + ///
> + /// let work_done = Arc::pin_init(Completion::new(), flags::GFP_KERNEL)?;
> + /// let data = Arc::pin_init(new_spinlock!(0), flags::GFP_KERNEL)?;
> + ///
> + /// workqueue::system().try_spawn(
> + /// flags::GFP_KERNEL,
> + /// {
> + /// let work_done = work_done.clone();
> + /// let data = data.clone();
> + /// move || {
> + /// *data.lock() = 42;
> + /// work_done.complete_all();
> + /// }
> + /// }
> + /// )?;
> + ///
> + /// work_done.wait_for_completion();
> + ///
> + /// // `work_done` being completed implies the observation of the write of `data` in the work.
> + /// assert_eq!(*data.lock(), 42);
> + /// # Ok::<(), Error>(())
> + /// ```
> pub fn try_spawn<T: 'static + Send + FnOnce()>(
> &self,
> flags: Flags,
On Wed, Jul 30, 2025 at 6:34 PM Boqun Feng <boqun.feng@gmail.com> wrote: > > Miguel, Alice and Tejun, while I'm at it, should we also rename the > function to `spawn()` because of the motivation mentioned here [1]? Sounds good to me. Cheers, Miguel
On Wed, Jul 30, 2025 at 06:38:46PM +0200, Miguel Ojeda wrote: > On Wed, Jul 30, 2025 at 6:34 PM Boqun Feng <boqun.feng@gmail.com> wrote: > > > > Miguel, Alice and Tejun, while I'm at it, should we also rename the > > function to `spawn()` because of the motivation mentioned here [1]? > > Sounds good to me. > Thanks, I will add it in v2 (probably as a separate patch) if no one shows any objection. Regards, Boqun > Cheers, > Miguel
© 2016 - 2026 Red Hat, Inc.