`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