[<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!(),
> + )
> + };
> +}
[...]
Powered by blists - more mailing lists