[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <C1C6DB90-202A-45FF-9D01-D7304A8DC8B0@collabora.com>
Date: Wed, 17 Dec 2025 11:03:48 -0300
From: Daniel Almeida <daniel.almeida@...labora.com>
To: Andreas Hindborg <a.hindborg@...nel.org>
Cc: Boqun Feng <boqun.feng@...il.com>,
FUJITA Tomonori <fujita.tomonori@...il.com>,
Frederic Weisbecker <frederic@...nel.org>,
Lyude Paul <lyude@...hat.com>,
Thomas Gleixner <tglx@...utronix.de>,
Anna-Maria Behnsen <anna-maria@...utronix.de>,
John Stultz <jstultz@...gle.com>,
Stephen Boyd <sboyd@...nel.org>,
Miguel Ojeda <ojeda@...nel.org>,
Gary Guo <gary@...yguo.net>,
Björn Roy Baron <bjorn3_gh@...tonmail.com>,
Benno Lossin <lossin@...nel.org>,
Alice Ryhl <aliceryhl@...gle.com>,
Trevor Gross <tmgross@...ch.edu>,
Danilo Krummrich <dakr@...nel.org>,
rust-for-linux@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] hrtimer: add usage examples to documentation
Hi Andreas,
> On 17 Dec 2025, at 07:30, Andreas Hindborg <a.hindborg@...nel.org> wrote:
>
> Add documentation examples showing various ways to use hrtimers:
>
> - Box-allocated timers with shared state in Arc.
> - Arc-allocated timers.
> - Stack-based timers for scoped usage.
> - Mutable stack-based timers with shared state.
>
> Signed-off-by: Andreas Hindborg <a.hindborg@...nel.org>
> ---
> rust/kernel/time/hrtimer.rs | 327 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 327 insertions(+)
>
> diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
> index 856d2d929a008..6ca198947b679 100644
> --- a/rust/kernel/time/hrtimer.rs
> +++ b/rust/kernel/time/hrtimer.rs
> @@ -66,6 +66,333 @@
> //!
> //! A `restart` operation on a timer in the **stopped** state is equivalent to a
> //! `start` operation.
> +//!
> +//! When a type implements both `HrTimerPointer` and `Clone`, it is possible to
> +//! issue the `start` operation while the timer is in the **started** state. In
> +//! this case the `start` operation is equivalent to the `restart` operation.
> +//!
> +//! # Examples
> +//!
> +//! ## Using an intrusive timer living in a [`Box`]
> +//!
> +//! ```
> +//! # use kernel::{
> +//! # alloc::flags,
> +//! # impl_has_hr_timer,
> +//! # prelude::*,
> +//! # sync::{
> +//! # atomic::{ordering, Atomic},
> +//! # completion::Completion,
> +//! # Arc,
> +//! # },
> +//! # time::{
> +//! # hrtimer::{
> +//! # RelativeMode, HrTimer, HrTimerCallback, HrTimerPointer,
> +//! # HrTimerRestart, HrTimerCallbackContext
> +//! # },
> +//! # Delta, Monotonic,
> +//! # },
> +//! # };
> +//!
> +//! #[pin_data]
> +//! struct Shared {
> +//! #[pin]
> +//! flag: Atomic<u64>,
> +//! #[pin]
> +//! cond: Completion,
> +//! }
> +//!
> +//! impl Shared {
> +//! fn new() -> impl PinInit<Self, kernel::error::Error> {
> +//! try_pin_init!(Self {
> +//! flag <- Atomic::new(0),
> +//! cond <- Completion::new(),
> +//! })
This doesn’t seem to be fallible? Can’t this use pin_init!() directly?
Ditto elsewhere.
> +//! }
> +//! }
> +//!
> +//! #[pin_data]
> +//! struct BoxIntrusiveHrTimer {
> +//! #[pin]
> +//! timer: HrTimer<Self>,
> +//! shared: Arc<Shared>,
> +//! }
> +//!
> +//! impl BoxIntrusiveHrTimer {
> +//! fn new() -> impl PinInit<Self, kernel::error::Error> {
> +//! try_pin_init!(Self {
> +//! timer <- HrTimer::new(),
> +//! shared: Arc::pin_init(Shared::new(), flags::GFP_KERNEL).unwrap(),
Why the unwrap() here instead of the "?" operator? I think that having unwrap() in examples encourages
people to panic the kernel.
> +//! })
> +//! }
> +//! }
> +//!
> +//! impl HrTimerCallback for BoxIntrusiveHrTimer {
> +//! type Pointer<'a> = Pin<KBox<Self>>;
> +//!
> +//! fn run(this: Pin<&mut Self>, _ctx: HrTimerCallbackContext<'_, Self>) -> HrTimerRestart {
> +//! pr_info!("Timer called\n");
Blanks here, ditto elsewhere.
> +//! let flag = this.shared.flag.fetch_add(1, ordering::Full);
> +//! this.shared.cond.complete_all();
> +//! if flag == 4 {
> +//! HrTimerRestart::NoRestart
> +//! } else {
> +//! HrTimerRestart::Restart
> +//! }
> +//! }
> +//! }
> +//!
> +//! impl_has_hr_timer! {
> +//! impl HasHrTimer<Self> for BoxIntrusiveHrTimer {
> +//! mode: RelativeMode<Monotonic>, field: self.timer
> +//! }
> +//! }
> +//!
> +//! let has_timer = Box::pin_init(BoxIntrusiveHrTimer::new(), GFP_KERNEL)?;
> +//! let shared = has_timer.shared.clone();
> +//! let _handle = has_timer.start(Delta::from_micros(200));
> +//!
> +//! while shared.flag.load(ordering::Relaxed) != 5 {
> +//! shared.cond.wait_for_completion();
> +//! }
> +//!
> +//! pr_info!("Counted to 5\n");
> +//! # Ok::<(), kernel::error::Error>(())
> +//! ```
> +//!
> +//! ## An intrusive timer in an [`Arc`]
This one is the only one that reads differently, FYI.
> +//!
> +//! ```
> +//! # use kernel::{
> +//! # alloc::flags,
> +//! # impl_has_hr_timer,
> +//! # prelude::*,
> +//! # sync::{
> +//! # atomic::{ordering, Atomic},
> +//! # completion::Completion,
> +//! # Arc, ArcBorrow,
> +//! # },
> +//! # time::{
> +//! # hrtimer::{
> +//! # RelativeMode, HrTimer, HrTimerCallback, HrTimerPointer, HrTimerRestart,
> +//! # HasHrTimer, HrTimerCallbackContext
> +//! # },
> +//! # Delta, Monotonic,
> +//! # },
> +//! # };
> +//!
> +//! #[pin_data]
> +//! struct ArcIntrusiveHrTimer {
> +//! #[pin]
> +//! timer: HrTimer<Self>,
> +//! #[pin]
> +//! flag: Atomic<u64>,
> +//! #[pin]
> +//! cond: Completion,
> +//! }
> +//!
> +//! impl ArcIntrusiveHrTimer {
> +//! fn new() -> impl PinInit<Self, kernel::error::Error> {
> +//! try_pin_init!(Self {
> +//! timer <- HrTimer::new(),
> +//! flag <- Atomic::new(0),
> +//! cond <- Completion::new(),
> +//! })
> +//! }
> +//! }
> +//!
> +//! impl HrTimerCallback for ArcIntrusiveHrTimer {
> +//! type Pointer<'a> = Arc<Self>;
> +//!
> +//! fn run(
> +//! this: ArcBorrow<'_, Self>,
> +//! _ctx: HrTimerCallbackContext<'_, Self>,
> +//! ) -> HrTimerRestart {
> +//! pr_info!("Timer called\n");
> +//! let flag = this.flag.fetch_add(1, ordering::Full);
> +//! this.cond.complete_all();
> +//! if flag == 4 {
> +//! HrTimerRestart::NoRestart
> +//! } else {
> +//! HrTimerRestart::Restart
> +//! }
> +//! }
Blanks as well ^
> +//! }
> +//!
> +//! impl_has_hr_timer! {
> +//! impl HasHrTimer<Self> for ArcIntrusiveHrTimer {
> +//! mode: RelativeMode<Monotonic>, field: self.timer
> +//! }
> +//! }
> +//!
> +//! let has_timer = Arc::pin_init(ArcIntrusiveHrTimer::new(), GFP_KERNEL)?;
> +//! let _handle = has_timer.clone().start(Delta::from_micros(200));
> +//!
> +//! while has_timer.flag.load(ordering::Relaxed) != 5 {
> +//! has_timer.cond.wait_for_completion();
> +//! }
> +//!
> +//! pr_info!("Counted to 5\n");
> +//! # Ok::<(), kernel::error::Error>(())
> +//! ```
> +//!
> +//! ## Using a stack-based timer
> +//!
> +//! ```
> +//! # use kernel::{
> +//! # impl_has_hr_timer,
> +//! # prelude::*,
> +//! # sync::{
> +//! # atomic::{ordering, Atomic},
> +//! # completion::Completion,
> +//! # },
> +//! # time::{
> +//! # hrtimer::{
> +//! # ScopedHrTimerPointer, HrTimer, HrTimerCallback, HrTimerPointer, HrTimerRestart,
> +//! # HasHrTimer, RelativeMode, HrTimerCallbackContext
> +//! # },
> +//! # Delta, Monotonic,
> +//! # },
> +//! # };
> +//! # use pin_init::stack_try_pin_init;
> +//!
> +//! #[pin_data]
> +//! struct IntrusiveHrTimer {
> +//! #[pin]
> +//! timer: HrTimer<Self>,
> +//! #[pin]
> +//! flag: Atomic<u64>,
> +//! #[pin]
> +//! cond: Completion,
> +//! }
> +//!
> +//! impl IntrusiveHrTimer {
> +//! fn new() -> impl PinInit<Self, kernel::error::Error> {
> +//! try_pin_init!(Self {
> +//! timer <- HrTimer::new(),
> +//! flag <- Atomic::new(0),
> +//! cond <- Completion::new(),
> +//! })
> +//! }
> +//! }
> +//!
> +//! impl HrTimerCallback for IntrusiveHrTimer {
> +//! type Pointer<'a> = Pin<&'a Self>;
> +//!
> +//! fn run(this: Pin<&Self>, _ctx: HrTimerCallbackContext<'_, Self>) -> HrTimerRestart {
> +//! pr_info!("Timer called\n");
> +//! this.flag.store(1, ordering::Release);
> +//! this.cond.complete_all();
> +//! HrTimerRestart::NoRestart
> +//! }
> +//! }
> +//!
> +//! impl_has_hr_timer! {
> +//! impl HasHrTimer<Self> for IntrusiveHrTimer {
> +//! mode: RelativeMode<Monotonic>, field: self.timer
> +//! }
> +//! }
> +//!
> +//! stack_try_pin_init!( let has_timer =? IntrusiveHrTimer::new() );
??? Benno, the syntax is getting more interesting with each passing day :)
> +//! has_timer.as_ref().start_scoped(Delta::from_micros(200), || {
> +//! while has_timer.flag.load(ordering::Relaxed) != 1 {
> +//! has_timer.cond.wait_for_completion();
> +//! }
> +//! });
> +//!
> +//! pr_info!("Flag raised\n");
> +//! # Ok::<(), kernel::error::Error>(())
> +//! ```
> +//!
> +//! ## Using a mutable stack-based timer
> +//!
> +//! ```
> +//! # use kernel::{
> +//! # alloc::flags,
> +//! # impl_has_hr_timer,
> +//! # prelude::*,
> +//! # sync::{
> +//! # atomic::{ordering, Atomic},
> +//! # completion::Completion,
> +//! # Arc,
> +//! # },
> +//! # time::{
> +//! # hrtimer::{
> +//! # ScopedHrTimerPointer, HrTimer, HrTimerCallback, HrTimerPointer, HrTimerRestart,
> +//! # HasHrTimer, RelativeMode, HrTimerCallbackContext
> +//! # },
> +//! # Delta, Monotonic,
> +//! # },
> +//! # };
> +//! # use pin_init::stack_try_pin_init;
> +//!
> +//! #[pin_data]
> +//! struct Shared {
> +//! #[pin]
> +//! flag: Atomic<u64>,
> +//! #[pin]
> +//! cond: Completion,
> +//! }
> +//!
> +//! impl Shared {
> +//! fn new() -> impl PinInit<Self, kernel::error::Error> {
> +//! try_pin_init!(Self {
> +//! flag <- Atomic::new(0),
> +//! cond <- Completion::new(),
> +//! })
> +//! }
> +//! }
> +//!
> +//! #[pin_data]
> +//! struct IntrusiveHrTimer {
> +//! #[pin]
> +//! timer: HrTimer<Self>,
> +//! shared: Arc<Shared>,
> +//! }
> +//!
> +//! impl IntrusiveHrTimer {
> +//! fn new() -> impl PinInit<Self, kernel::error::Error> {
> +//! try_pin_init!(Self {
> +//! timer <- HrTimer::new(),
> +//! shared: Arc::pin_init(Shared::new(), flags::GFP_KERNEL).unwrap(),
> +//! })
> +//! }
> +//! }
> +//!
> +//! impl HrTimerCallback for IntrusiveHrTimer {
> +//! type Pointer<'a> = Pin<&'a mut Self>;
> +//!
> +//! fn run(this: Pin<&mut Self>, _ctx: HrTimerCallbackContext<'_, Self>) -> HrTimerRestart {
> +//! pr_info!("Timer called\n");
> +//! let flag = this.shared.flag.fetch_add(1, ordering::Full);
> +//! this.shared.cond.complete_all();
> +//! if flag == 4 {
> +//! HrTimerRestart::NoRestart
> +//! } else {
> +//! HrTimerRestart::Restart
> +//! }
> +//! }
> +//! }
> +//!
> +//! impl_has_hr_timer! {
> +//! impl HasHrTimer<Self> for IntrusiveHrTimer {
> +//! mode: RelativeMode<Monotonic>, field: self.timer
> +//! }
> +//! }
> +//!
> +//! stack_try_pin_init!( let has_timer =? IntrusiveHrTimer::new() );
> +//! let shared = has_timer.shared.clone();
> +//! has_timer.as_mut().start_scoped(Delta::from_micros(200), || {
> +//! while shared.flag.load(ordering::Relaxed) != 5 {
> +//! shared.cond.wait_for_completion();
> +//! }
> +//! });
> +//!
> +//! pr_info!("Counted to 5\n");
> +//! # Ok::<(), kernel::error::Error>(())
> +//! ```
> +//!
> +//! [`Arc`]: kernel::sync::Arc
>
> use super::{ClockSource, Delta, Instant};
> use crate::{prelude::*, types::Opaque};
>
> ---
> base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
> change-id: 20251215-hrtimer-examples-v6-19-rc1-2658ce9bb5d0
>
> Best regards,
> --
> Andreas Hindborg <a.hindborg@...nel.org>
>
>
>
FWIW:
[10:52:46] [PASSED] rust_doctest_kernel_time_hrtimer_rs_0
[10:52:46] [PASSED] rust_doctest_kernel_time_hrtimer_rs_1
[10:52:46] [PASSED] rust_doctest_kernel_time_hrtimer_rs_2
[10:52:46] [PASSED] rust_doctest_kernel_time_hrtimer_rs_3
This looks good to me. The rendering looks ok as well.
With the nits addressed:
Tested-by: Daniel Almeida <daniel.almeida@...labora.com>
Reviewed-by: Daniel Almeida <daniel.almeida@...labora.com>
Powered by blists - more mailing lists