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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aF7lVYl7p33kByoK@tardis.local>
Date: Fri, 27 Jun 2025 11:39:17 -0700
From: Boqun Feng <boqun.feng@...il.com>
To: Alice Ryhl <aliceryhl@...gle.com>
Cc: Tejun Heo <tj@...nel.org>, Miguel Ojeda <ojeda@...nel.org>,
	Alex Gaynor <alex.gaynor@...il.com>,
	Lai Jiangshan <jiangshanlai@...il.com>, Gary Guo <gary@...yguo.net>,
	Björn Roy Baron <bjorn3_gh@...tonmail.com>,
	Andreas Hindborg <a.hindborg@...nel.org>,
	Trevor Gross <tmgross@...ch.edu>,
	Danilo Krummrich <dakr@...nel.org>,
	Daniel Almeida <daniel.almeida@...labora.com>,
	Tamir Duberstein <tamird@...il.com>, rust-for-linux@...r.kernel.org,
	linux-kernel@...r.kernel.org, Benno Lossin <lossin@...nel.org>
Subject: Re: [PATCH v2] workqueue: rust: add delayed work items

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.

With the above fixed, feel free to add:

Reviewed-by: Boqun Feng <boqun.feng@...il.com>

Regards,
Boqun

> +            $crate::static_lock_class!(),
> +        )
> +    };
> +    ($name:literal) => {
> +        $crate::workqueue::DelayedWork::new(
> +            $crate::c_str!($name),
> +            $crate::static_lock_class!(),
> +            $crate::c_str!(::core::concat!($name, "_timer")),
> +            $crate::static_lock_class!(),
> +        )
> +    };
> +}
[...]