workqueue: rust: add delayed work items
Alice Ryhl
reviewerPATCH v2 1/1 workqueue: rust: add delayed work items
This patch is being sent for use in the various Rust GPU drivers that are under development. It provides the additional feature of work items that are executed after a delay.
The design of the existing workqueue is rather extensible, as most of the logic is reused for delayed work items even though a different work item type is required. The new logic consists of:
* A new DelayedWork struct that wraps struct delayed_work.
* A new impl_has_delayed_work! macro that provides adjusted versions of the container_of logic, that is suitable with delayed work items.
* A enqueue_delayed method that can enqueue a delayed work item.
This patch does *not* rely on the fact that struct delayed_work contains struct work_struct at offset zero. It will continue to work even if the layout is changed to hold the work field at a different offset.
Please see the example introduced at the top of the file for example usage of delayed work items.
---
Changes in v2:
- Rebase on v6.16-rc2.
- There are no longer any dependencies as they were merged into
v6-16-rc1.
- Fix some compilation errors related to changes to container_of! and
lock class keys.
- Link to v1: https://lore.kernel.org/r/[email protected]
--- rust/kernel/workqueue.rs | 330 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 327 insertions(+), 3 deletions(-)
rust/kernel/workqueue.rs +327 -3
@@ -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); +//! } +//! +//! /// 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!())), + $crate::static_lock_class!(), + $crate::c_str!(::core::concat!( + ::core::file!(), +":" , + ::core::line!(), +"_timer" + )), + $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!(), + ) + }; +} +pub use new_delayed_work; +/// A kernel work queue. /// /// Wraps the kernel's C `struct workqueue_struct`.
@@ -206,6 +292,42 @@ pub fn enqueue<W, const ID: u64>(&self, w: W) -> W::EnqueueOutput } } +/// Enqueues a delayed work item. +/// +/// This may fail if the work item is already enqueued in a workqueue. +/// +/// The work item will be submitted using `WORK_CPU_UNBOUND`. +pub fn enqueue_delayed <W ,const ID :u64 >(&self, w:W , delay:Jiffies ) ->W ::EnqueueOutput +where +W :RawDelayedWorkItem <ID > +Send + 'static, + { +let queue_ptr = self.0 .get (); + +// SAFETY: We only return `false` if the `work_struct` is already in a workqueue. The other +// `__enqueue` requirements are not relevant since `W` is `Send` and static. +// +// The call to `bindings::queue_delayed_work_on` will dereference the provided raw pointer, +// which is ok because `__enqueue` guarantees that the pointer is valid for the duration of +// this closure, and the safety requirements of `RawDelayedWorkItem` expands this +// requirement to apply to the entire `delayed_work`. +// +// Furthermore, if the C workqueue code accesses the pointer after this call to +// `__enqueue`, then the work item was successfully enqueued, and +// `bindings::queue_delayed_work_on` will have returned true. In this case, `__enqueue` +// promises that the raw pointer will stay valid until we call the function pointer in the +// `work_struct`, so the access is ok. +unsafe { + w.__enqueue (move |work_ptr| { + bindings::queue_delayed_work_on ( + bindings::wq_misc_consts_WORK_CPU_UNBOUNDas _ , + queue_ptr, +container_of! (work_ptr, bindings::delayed_work, work), + delay, + ) + }) + } + } +/// 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.
@@ -298,6 +420,16 @@ unsafe fn __enqueue<F>(self, queue_work_on: F) -> Self::EnqueueOutput F: FnOnce(*mut bindings::work_struct) -> bool; } +/// A raw delayed work item. +/// +/// # Safety +/// +/// If the `__enqueue` method in the `RawWorkItem` implementation calls the closure, then the +/// provided pointer must point at the `work` field of a valid `delayed_work`, and the guarantees +/// that `__enqueue` provides about accessing the `work_struct` must also apply to the rest of the +/// `delayed_work` struct. +pub unsafe trait RawDelayedWorkItem <const ID :u64 >:RawWorkItem <ID > {} +/// Defines the method that should be called directly when a work item is executed. /// /// This trait is implemented by `Pin<KBox<T>>` and [`Arc<T>`], and is mainly intended to be
@@ -407,7 +539,7 @@ pub unsafe fn raw_get(ptr: *const Self) -> *mut bindings::work_struct { } } -/// Declares that a type has a [`Work<T, ID>`] field. +/// Declares that a type contains a [`Work<T, ID>`]. /// /// The intended way of using this trait is via the [`impl_has_work!`] macro. You can use the macro /// like this:
@@ -506,6 +638,178 @@ unsafe fn work_container_of( impl{T} HasWork<Self> for ClosureWork<T> { self.work } } +/// Links for a delayed work item. +/// +/// This struct contains a function pointer to the [`run`] function from the [`WorkItemPointer`] +/// trait, and defines the linked list pointers necessary to enqueue a work item in a workqueue in +/// a delayed manner. +/// +/// Wraps the kernel's C `struct delayed_work`. +/// +/// This is a helper type used to associate a `delayed_work` with the [`WorkItem`] that uses it. +/// +/// [`run`]: WorkItemPointer::run +#[pin_data] +#[repr(transparent)] +pub struct DelayedWork <T : ?Sized ,const ID :u64 =0 > { +#[pin] + dwork:Opaque <bindings::delayed_work >, + _inner:PhantomData <T >, +} + +// SAFETY: Kernel work items are usable from any thread. +// +// We do not need to constrain `T` since the work item does not actually contain a `T`. +unsafe impl <T : ?Sized ,const ID :u64 >Send for DelayedWork <T ,ID > {} +// SAFETY: Kernel work items are usable from any thread. +// +// We do not need to constrain `T` since the work item does not actually contain a `T`. +unsafe impl <T : ?Sized ,const ID :u64 >Sync for DelayedWork <T ,ID > {} + +impl <T : ?Sized ,const ID :u64 >DelayedWork <T ,ID > { +/// Creates a new instance of [`DelayedWork`]. +#[inline] +pub fn new ( + work_name: &'staticCStr , + work_key:Pin <&'staticLockClassKey >, + timer_name: &'staticCStr , + timer_key:Pin <&'staticLockClassKey >, + ) ->impl PinInit <Self > +where +T :WorkItem <ID >, + { +pin_init! (Self { + dwork <-Opaque ::ffi_init (|slot: *mut bindings::delayed_work | { +// SAFETY: The `WorkItemPointer` implementation promises that `run` can be used as +// the work item function. +unsafe { + bindings::init_work_with_key ( + core::ptr::addr_of_mut!((*slot).work), +Some (T ::Pointer ::run), +false , + work_name.as_char_ptr (), + work_key.as_ptr (), + ) + } + +// SAFETY: The `delayed_work_timer_fn` function pointer can be used here because +// the timer is embedded in a `struct delayed_work`, and only ever scheduled via +// the core workqueue code, and configured to run in irqsafe context. +unsafe { + bindings::timer_init_key ( + core::ptr::addr_of_mut!((*slot).timer), +Some (bindings::delayed_work_timer_fn), + bindings::TIMER_IRQSAFE , + timer_name.as_char_ptr (), + timer_key.as_ptr (), + ) + } + }), + _inner:PhantomData , + }) + } + +/// Get a pointer to the inner `delayed_work`. +/// +/// # Safety +/// +/// The provided pointer must not be dangling and must be properly aligned. (But the memory +/// need not be initialized.) +#[inline] +pub unsafe fn raw_as_work (ptr: *const Self ) -> *mut Work <T ,ID > { +// SAFETY: The caller promises that the pointer is aligned and not dangling. +let dw: *mut bindings::delayed_work = +unsafe {Opaque ::raw_get (core::ptr::addr_of!((*ptr).dwork)) }; +// SAFETY: The caller promises that the pointer is aligned and not dangling. +let wrk: *mut bindings::work_struct =unsafe { core::ptr::addr_of_mut!((*dw).work) }; +// CAST: Work and work_struct have compatible layouts. + wrk.cast () + } +} + +/// Declares that a type contains a [`DelayedWork<T, ID>`]. +/// +/// # Safety +/// +/// The `HasWork<T, ID>` implementation must return a `work_struct` that is stored in the `work` +/// field of a `delayed_work` with the same access rules as the `work_struct`. +pub unsafe trait HasDelayedWork <T ,const ID :u64 =0 >:HasWork <T ,ID > {} + +/// Used to safely implement the [`HasDelayedWork<T, ID>`] trait. +/// +/// This macro also implements the [`HasWork`] trait, so you do not need to use [`impl_has_work!`] +/// when using this macro. +/// +/// # Examples +/// +/// ``` +/// use kernel::sync::Arc; +/// use kernel::workqueue::{self, impl_has_delayed_work, DelayedWork}; +/// +/// struct MyStruct<'a, T, const N: usize> { +/// work_field: DelayedWork<MyStruct<'a, T, N>, 17>, +/// f: fn(&'a [T; N]), +/// } +/// +/// impl_has_delayed_work! { +/// impl{'a, T, const N: usize} HasDelayedWork<MyStruct<'a, T, N>, 17> +/// for MyStruct<'a, T, N> { self.work_field } +/// } +/// ``` +#[macro_export] +macro_rules! impl_has_delayed_work { + ($(impl $({$($generics:tt)*})? +HasDelayedWork <$work_type:ty $(, $id:tt)?> +for $self:ty + { self.$field:ident } + )*) => {$( +// SAFETY: The implementation of `raw_get_work` only compiles if the field has the right +// type. +unsafe impl $(<$($generics)+>)? + $crate::workqueue::HasDelayedWork <$work_type $(, $id)?>for $self {} + +// SAFETY: The implementation of `raw_get_work` only compiles if the field has the right +// type. +unsafe impl $(<$($generics)+>)? $crate::workqueue::HasWork <$work_type $(, $id)?>for $self { + #[inline ] +unsafe fn raw_get_work ( + ptr: *mut Self + ) -> *mut $crate::workqueue::Work <$work_type $(, $id)?> { +// SAFETY: The caller promises that the pointer is not dangling. +let ptr : *mut $crate::workqueue::DelayedWork <$work_type $(, $id)?> =unsafe { + ::core::ptr::addr_of_mut!((*ptr).$field) + }; + +// SAFETY: The caller promises that the pointer is not dangling. +unsafe { $crate::workqueue::DelayedWork ::raw_as_work (ptr ) } + } + + #[inline ] +unsafe fn work_container_of ( + ptr: *mut $crate::workqueue::Work <$work_type $(, $id)?>, + ) -> *mut Self { +// SAFETY: The caller promises that the pointer points at a field of the right type +// in the right kind of struct. +let ptr =unsafe { $crate::workqueue::Work ::raw_get (ptr ) }; + +// SAFETY: The caller promises that the pointer points at a field of the right type +// in the right kind of struct. +let delayed_work =unsafe { + $crate::container_of!(ptr, $crate ::bindings::delayed_work, work) + }; + +let delayed_work: *mut $crate::workqueue::DelayedWork <$work_type $(, $id)?> = + delayed_work.cast (); + +// SAFETY: The caller promises that the pointer points at a field of the right type +// in the right kind of struct. +unsafe { $crate::container_of!(delayed_work,Self , $field) } + } + } + )*}; +} +pub use impl_has_delayed_work; +// SAFETY: The `__enqueue` implementation in RawWorkItem uses a `work_struct` initialized with the // `run` method of this trait as the function pointer because: // - `__enqueue` gets the `work_struct` from the `Work` field, using `T::raw_get_work`.
@@ -567,6 +871,16 @@ unsafe fn __enqueue<F>(self, queue_work_on: F) -> Self::EnqueueOutput } } +// SAFETY: By the safety requirements of `HasDelayedWork`, the `work_struct` returned by methods in +// `HasWork` provides a `work_struct` that is the `work` field of a `delayed_work`, and the rest of +// the `delayed_work` has the same access rules as its `work` field. +unsafe impl <T ,const ID :u64 >RawDelayedWorkItem <ID >for Arc <T > +where +T :WorkItem <ID ,Pointer =Self >, +T :HasDelayedWork <T ,ID >, +{ +} +// SAFETY: TODO. unsafe impl <T ,const ID :u64 >WorkItemPointer <ID >for Pin <KBox <T >>where
@@ -617,6 +931,16 @@ unsafe fn __enqueue<F>(self, queue_work_on: F) -> Self::EnqueueOutput } } +// SAFETY: By the safety requirements of `HasDelayedWork`, the `work_struct` returned by methods in +// `HasWork` provides a `work_struct` that is the `work` field of a `delayed_work`, and the rest of +// the `delayed_work` has the same access rules as its `work` field. +unsafe impl <T ,const ID :u64 >RawDelayedWorkItem <ID >for Pin <KBox <T >> +where +T :WorkItem <ID ,Pointer =Self >, +T :HasDelayedWork <T ,ID >, +{ +} +/// Returns the system work queue (`system_wq`). /// /// It is the one used by `schedule[_delayed]_work[_on]()`. Multi-CPU multi-threaded. There are
---
base-commit: e04c78d86a9699d136910cfc0bdcf01087e3267e
change-id: 20250409-workqueue-delay-40eb0cd7c6d0
Best regards,
Signature · 1 line
Alice Ryhl <[email protected]>
Quoted 18 lines · Alice Ryhl
This patch is being sent for use in the various Rust GPU drivers that are under development. It provides the additional feature of work items that are executed after a delay.
The design of the existing workqueue is rather extensible, as most of the logic is reused for delayed work items even though a different work item type is required. The new logic consists of:
* A new DelayedWork struct that wraps struct
delayed_work.
* A newimpl_has_delayed_work! macro that provides adjusted versions of thecontainer_oflogic, that is suitable with delayed work items.
* Aenqueue_delayedmethod that can enqueue a delayed work item.This patch does *not* rely on the fact that
struct delayed_workcontainsstruct work_structat offset zero. It will continue to work even if the layout is changed to hold theworkfield at a different offset.
Quoted 4 lines
Please see the example introduced at the top of the file for example usage of delayed work items.
Signed-off-by Alice Ryhl <[email protected]>
Thanks.
Signature · 1 line
tejun
On Fri, Jun 27, 2025 at 09:38:42AM +0000, Alice Ryhl wrote:
[...]
rust/kernel/workqueue.rs unsafe fn work_container_of( · line 722
Quoted 11 lines
rust/kernel/workqueue.rs +11
-0+/// Get a pointer to the inner `delayed_work`. +/// +/// # Safety +/// +/// The provided pointer must not be dangling and must be properly aligned. (But the memory +/// need not be initialized.) +#[inline] +pub unsafe fn raw_as_work (ptr: *const Self ) -> *mut Work <T ,ID > { +// SAFETY: The caller promises that the pointer is aligned and not dangling. +let dw: *mut bindings::delayed_work = +unsafe {Opaque ::raw_get (core::ptr::addr_of!((*ptr).dwork)) };
This would conflict with your Opaque::cast_into() series ;-) So I suggest that we rebase this one onto:
https://lore.kernel.org/rust-for-linux/[email protected]/
and send them together. But it's up to you and maintainers, and even we do that let's wait for some reviews on this patch to save extra versions.
I'm going to review it, just figure this better be pointed out.
Btw, as a review comment, I believe we are in favor of &raw mut now, so this should be:
unsafe { Opaque::cast_from(&raw const (*ptr).dwork) };Review
Quoted 2 lines
(quoted diff) +2
-0+ // SAFETY: The caller promises that the pointer is aligned and not dangling. + let wrk: *mut bindings::work_struct = unsafe { core::ptr::addr_of_mut!((*dw).work) };
Ditto.
Regards,
Boqun
Quoted 4 lines
rust/kernel/workqueue.rs +4
-0+// CAST: Work and work_struct have compatible layouts. + wrk.cast () + } +}
[...]
On Fri, Jun 27, 2025 at 09:38:42AM +0000, Alice Ryhl wrote:
[...]
Quoted 39 lines
rust/kernel/workqueue.rs +31
-0@@ -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>;
rust/kernel/workqueue.rs line 174
Quoted 10 lines
rust/kernel/workqueue.rs +10
-0+//! +//! 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?
Quoted 28 lines
rust/kernel/workqueue.rs +20
-2+//! } +//! +//! /// 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.
rust/kernel/workqueue.rs macro_rules! new_work { · line 213
Quoted 9 lines
rust/kernel/workqueue.rs +6
-0@@ -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.
rust/kernel/workqueue.rs macro_rules! new_work { · line 220
Quoted 7 lines
rust/kernel/workqueue.rs +7
-0+ $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:
Regards,
Boqun
Quoted 12 lines
rust/kernel/workqueue.rs +12
-0+ $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!(), + ) + }; +}
[...]
Alice Ryhl
reviewerQuoted 14 lines · Boqun Feng
On Fri, Jun 27, 2025 at 09:38:42AM +0000, Alice Ryhl wrote:
[...]rust/kernel/workqueue.rs unsafe fn work_container_of( · line 722
Quoted 11 lines
rust/kernel/workqueue.rs +11
-0+/// Get a pointer to the inner `delayed_work`. +/// +/// # Safety +/// +/// The provided pointer must not be dangling and must be properly aligned. (But the memory +/// need not be initialized.) +#[inline] +pub unsafe fn raw_as_work (ptr: *const Self ) -> *mut Work <T ,ID > { +// SAFETY: The caller promises that the pointer is aligned and not dangling. +let dw: *mut bindings::delayed_work = +unsafe {Opaque ::raw_get (core::ptr::addr_of!((*ptr).dwork)) };
Quoted 8 lines
This would conflict with your
Opaque::cast_into()series ;-) So I suggest that we rebase this one onto:https://lore.kernel.org/rust-for-linux/[email protected]/and send them together. But it's up to you and maintainers, and even we do that let's wait for some reviews on this patch to save extra versions.
Will rebase.
Quoted 6 lines
I'm going to review it, just figure this better be pointed out.
Btw, as a review comment, I believe we are in favor of
&raw mutnow, so this should be:
Quoted 6 lines
unsafe { Opaque::cast_from(&raw const (*ptr).dwork) };Review
Quoted 2 lines
(quoted diff) +2
-0+ // SAFETY: The caller promises that the pointer is aligned and not dangling. + let wrk: *mut bindings::work_struct = unsafe { core::ptr::addr_of_mut!((*dw).work) };Ditto.
The rest of the file still uses addr_of!. I think we should stay consistent. We can update the entire file in one go.
Alice
Alice Ryhl
reviewerQuoted 107 lines · Boqun Feng
On Fri, Jun 27, 2025 at 09:38:42AM +0000, Alice Ryhl wrote:
[...]Quoted 39 lines
rust/kernel/workqueue.rs +31
-0@@ -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>; rust/kernel/workqueue.rs line 174
Quoted 10 lines
rust/kernel/workqueue.rs +10
-0+//! +//! 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' inenqueue_delayed()is the value for delay, not the '42' inMyStruct::new();-)Just use 10 jiffies, maybe?
Quoted 28 lines
rust/kernel/workqueue.rs +20
-2+//! } +//! +//! /// 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. rust/kernel/workqueue.rs macro_rules! new_work { · line 213
Quoted 9 lines
rust/kernel/workqueue.rs +6
-0@@ -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.rust/kernel/workqueue.rs macro_rules! new_work { · line 220
Quoted 7 lines
rust/kernel/workqueue.rs +7
-0+ $crate::static_lock_class!(), + $crate ::c_str!(::core::concat!( + ::core::file!(), +":" , + ::core::line!(), +"_timer" + )),
Quoted 3 lines
and maybe extend
optional_name!()to support a suffix? Or we make a concat!() forCStr? 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
On Thu, Jul 10, 2025 at 01:10:59PM +0000, Alice Ryhl wrote:
[...]
Quoted 4 lines
Quoted 4 lines
unsafe { Opaque::cast_from(&raw const (*ptr).dwork) };
Quoted 8 lines
Quoted 4 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+ // SAFETY: The caller promises that the pointer is aligned and not dangling. + let wrk: *mut bindings::work_struct = unsafe { core::ptr::addr_of_mut!((*dw).work) };Ditto.
The rest of the file still uses
addr_of!. I think we should stay consistent. We can update the entire file in one go.
Sounds good. Thanks!
Regards,
Boqun
Quoted 1 line
Alice