Re: [PATCH v2] workqueue: rust: add delayed work items

[Alice Ryhl <aliceryhl@google.com>]

On Fri, Jun 27, 2025 at 11:39:17AM -0700, Boqun Feng wrote:
> On Fri, Jun 27, 2025 at 09:38:42AM +0000, Alice Ryhl wrote:
> [...]
> > diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
> > index d092112d843f3225ea582e013581b39e36652a84..3a7ab7d078fc2091ad556bfde997b9f73f618773 100644
> > --- a/rust/kernel/workqueue.rs
> > +++ b/rust/kernel/workqueue.rs
> > @@ -131,10 +131,69 @@
> >  //! # print_2_later(MyStruct::new(41, 42).unwrap());
> >  //! ```
> >  //!
> > +//! This example shows how you can schedule delayed work items:
> > +//!
> > +//! ```
> > +//! use kernel::sync::Arc;
> > +//! use kernel::workqueue::{self, impl_has_delayed_work, new_delayed_work, DelayedWork, WorkItem};
> > +//!
> > +//! #[pin_data]
> > +//! struct MyStruct {
> > +//!     value: i32,
> > +//!     #[pin]
> > +//!     work: DelayedWork<MyStruct>,
> > +//! }
> > +//!
> > +//! impl_has_delayed_work! {
> > +//!     impl HasDelayedWork<Self> for MyStruct { self.work }
> > +//! }
> > +//!
> > +//! impl MyStruct {
> > +//!     fn new(value: i32) -> Result<Arc<Self>> {
> > +//!         Arc::pin_init(
> > +//!             pin_init!(MyStruct {
> > +//!                 value,
> > +//!                 work <- new_delayed_work!("MyStruct::work"),
> > +//!             }),
> > +//!             GFP_KERNEL,
> > +//!         )
> > +//!     }
> > +//! }
> > +//!
> > +//! impl WorkItem for MyStruct {
> > +//!     type Pointer = Arc<MyStruct>;
> > +//!
> > +//!     fn run(this: Arc<MyStruct>) {
> > +//!         pr_info!("The value is: {}\n", this.value);
> > +//!     }
> > +//! }
> > +//!
> > +//! /// This method will enqueue the struct for execution on the system workqueue, where its value
> > +//! /// will be printed 42 jiffies later.
> > +//! fn print_later(val: Arc<MyStruct>) {
> > +//!     let _ = workqueue::system().enqueue_delayed(val, 42);
> 
> Nit: maybe use a different jiffies value? Because you call
> MyStruct::new(42) later, and we want to demonstrate that the '42' in
> enqueue_delayed() is the value for delay, not the '42' in
> MyStruct::new() ;-)
> 
> Just use 10 jiffies, maybe?
> 
> > +//! }
> > +//!
> > +//! /// It is also possible to use the ordinary `enqueue` method together with `DelayedWork`. This
> > +//! /// is equivalent to calling `enqueue_delayed` with a delay of zero.
> > +//! fn print_now(val: Arc<MyStruct>) {
> > +//!     let _ = workqueue::system().enqueue(val);
> > +//! }
> > +//! # print_later(MyStruct::new(42).unwrap());
> > +//! # print_now(MyStruct::new(42).unwrap());
> > +//! ```
> > +//!
> >  //! C header: [`include/linux/workqueue.h`](srctree/include/linux/workqueue.h)
> >  
> > -use crate::alloc::{AllocError, Flags};
> > -use crate::{prelude::*, sync::Arc, sync::LockClassKey, types::Opaque};
> > +use crate::{
> > +    alloc::{AllocError, Flags},
> > +    container_of,
> > +    prelude::*,
> > +    sync::Arc,
> > +    sync::LockClassKey,
> > +    time::Jiffies,
> > +    types::Opaque,
> > +};
> >  use core::marker::PhantomData;
> >  
> >  /// Creates a [`Work`] initialiser with the given name and a newly-created lock class.
> > @@ -146,6 +205,33 @@ macro_rules! new_work {
> >  }
> >  pub use new_work;
> >  
> > +/// Creates a [`DelayedWork`] initialiser with the given name and a newly-created lock class.
> > +#[macro_export]
> > +macro_rules! new_delayed_work {
> > +    () => {
> > +        $crate::workqueue::DelayedWork::new(
> > +            $crate::c_str!(::core::concat!(::core::file!(), ":", ::core::line!())),
> 
> We can use `optional_name!()` for this.
> 
> > +            $crate::static_lock_class!(),
> > +            $crate::c_str!(::core::concat!(
> > +                ::core::file!(),
> > +                ":",
> > +                ::core::line!(),
> > +                "_timer"
> > +            )),
> 
> and maybe extend `optional_name!()` to support a suffix? Or we make a
> concat!() for `CStr`? Anyway I don't think this blocks the patch, if you
> don't have time, I will create an issue for this.

I'll use optional_name!() for the first one. We can add suffix support
separately.

Alice