[PATCH v1 0/5] rust: time: Convert hrtimer to use Instant and Delta

[PATCH v1 0/5] rust: time: Convert hrtimer to use Instant and Delta

[FUJITA Tomonori]

Convert hrtimer to use `Instant` and `Delta`; remove the use of
`Ktime` from the hrtimer code, which was originally introduced as a
temporary workaround.

hrtimer uses either an `Instant` or a `Delta` as its expiration value,
depending on the mode specified at creation time. This patchset
replaces `HrTimerMode` enum with a trait-based abstraction and
associates each mode with either an `Instant` or a `Delta`. By
leveraging Rust's type system, this change enables `HrTimer` to be
statically associated with a specific `HrTimerMode`, the corresponding
`Instant` or `Delta`, and a `ClockSource`.

The `impl_has_hr_timer` macro is extended to allow specifying the
`HrTimerMode`. In the following example, it guarantees that the
`start()` method for `Foo` only accepts `Instant<Monotonic>`. Using a
`Delta` or an `Instant` with a different clock source will result in a
compile-time error:

struct Foo {
    #[pin]
    timer: HrTimer<Self>,

}

impl_has_hr_timer! {
    impl HasHrTimer<Self> for Foo {
        mode = AbsoluteMode<Monotonic>,
        self.timer
    }
}

This design eliminates runtime mismatches between expires types and
clock sources, and enables stronger type-level guarantees throughout
hrtimer.

This patchset can be applied on top of the typed clock sources patchset:

https://lore.kernel.org/lkml/20250504042436.237756-1-fujita.tomonori@gmail.com/


FUJITA Tomonori (5):
  rust: time: Change Delta methods to take &self instead of self
  rust: timer: Replace HrTimerMode enum with trait-based mode types
  rust: time: Add HrTimerExpires trait
  rust: time: Make HasHrTimer generic over HrTimerMode
  rust: time: Remove Ktime in hrtimer

 rust/kernel/time.rs                 |  15 +-
 rust/kernel/time/hrtimer.rs         | 288 ++++++++++++++++++----------
 rust/kernel/time/hrtimer/arc.rs     |   8 +-
 rust/kernel/time/hrtimer/pin.rs     |   8 +-
 rust/kernel/time/hrtimer/pin_mut.rs |   8 +-
 rust/kernel/time/hrtimer/tbox.rs    |   8 +-
 6 files changed, 225 insertions(+), 110 deletions(-)


base-commit: 7b33a2d9c12023a73b7ba105a3eed77c3527a94e
-- 
2.43.0

[PATCH v1 1/5] rust: time: Change Delta methods to take &self instead of self

[FUJITA Tomonori]

Change several methods of the `Delta` type in Rust to take `&self`
instead of `self`. These methods do not mutate or consume the `Delta`
value and are more idiomatically expressed as taking a shared
reference. This change improves consistency with common Rust practice
and allows calling these methods on references without requiring an
explicit copy or move of the value.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 rust/kernel/time.rs | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
index 1be5ecd814d3..deca2999ced6 100644
--- a/rust/kernel/time.rs
+++ b/rust/kernel/time.rs
@@ -260,26 +260,26 @@ pub const fn from_secs(secs: i64) -> Self {
 
     /// Return `true` if the [`Delta`] spans no time.
     #[inline]
-    pub fn is_zero(self) -> bool {
+    pub fn is_zero(&self) -> bool {
         self.as_nanos() == 0
     }
 
     /// Return `true` if the [`Delta`] spans a negative amount of time.
     #[inline]
-    pub fn is_negative(self) -> bool {
+    pub fn is_negative(&self) -> bool {
         self.as_nanos() < 0
     }
 
     /// Return the number of nanoseconds in the [`Delta`].
     #[inline]
-    pub const fn as_nanos(self) -> i64 {
+    pub const fn as_nanos(&self) -> i64 {
         self.nanos
     }
 
     /// Return the smallest number of microseconds greater than or equal
     /// to the value in the [`Delta`].
     #[inline]
-    pub fn as_micros_ceil(self) -> i64 {
+    pub fn as_micros_ceil(&self) -> i64 {
         #[cfg(CONFIG_64BIT)]
         {
             self.as_nanos().saturating_add(NSEC_PER_USEC - 1) / NSEC_PER_USEC
@@ -294,7 +294,7 @@ pub fn as_micros_ceil(self) -> i64 {
 
     /// Return the number of milliseconds in the [`Delta`].
     #[inline]
-    pub fn as_millis(self) -> i64 {
+    pub fn as_millis(&self) -> i64 {
         #[cfg(CONFIG_64BIT)]
         {
             self.as_nanos() / NSEC_PER_MSEC
-- 
2.43.0

Re: [PATCH v1 1/5] rust: time: Change Delta methods to take &self instead of self

[Alice Ryhl]

On Sun, May 04, 2025 at 01:59:54PM +0900, FUJITA Tomonori wrote:
> Change several methods of the `Delta` type in Rust to take `&self`
> instead of `self`. These methods do not mutate or consume the `Delta`
> value and are more idiomatically expressed as taking a shared
> reference. This change improves consistency with common Rust practice
> and allows calling these methods on references without requiring an
> explicit copy or move of the value.

For small values that can be freely copied, I actualy think that using
`self` is more common Rust practice.

Alice

Re: [PATCH v1 1/5] rust: time: Change Delta methods to take &self instead of self

[Andreas Hindborg]

"Alice Ryhl" <aliceryhl@google.com> writes:

> On Sun, May 04, 2025 at 01:59:54PM +0900, FUJITA Tomonori wrote:
>> Change several methods of the `Delta` type in Rust to take `&self`
>> instead of `self`. These methods do not mutate or consume the `Delta`
>> value and are more idiomatically expressed as taking a shared
>> reference. This change improves consistency with common Rust practice
>> and allows calling these methods on references without requiring an
>> explicit copy or move of the value.
>
> For small values that can be freely copied, I actualy think that using
> `self` is more common Rust practice.

Besides best practice, the value will pass in a register. There is no
benefit at all from passing a reference here and no improved ergonomics
from using a reference.


Best regards,
Andreas Hindborg

Re: [PATCH v1 1/5] rust: time: Change Delta methods to take &self instead of self

[FUJITA Tomonori]

On Mon, 02 Jun 2025 14:19:38 +0200
Andreas Hindborg <a.hindborg@kernel.org> wrote:

> "Alice Ryhl" <aliceryhl@google.com> writes:
> 
>> On Sun, May 04, 2025 at 01:59:54PM +0900, FUJITA Tomonori wrote:
>>> Change several methods of the `Delta` type in Rust to take `&self`
>>> instead of `self`. These methods do not mutate or consume the `Delta`
>>> value and are more idiomatically expressed as taking a shared
>>> reference. This change improves consistency with common Rust practice
>>> and allows calling these methods on references without requiring an
>>> explicit copy or move of the value.
>>
>> For small values that can be freely copied, I actualy think that using
>> `self` is more common Rust practice.
> 
> Besides best practice, the value will pass in a register. There is no
> benefit at all from passing a reference here and no improved ergonomics
> from using a reference.

Yeah, but somehow I thought that using &self is more common practice
in Rust.

I'll drop this patch in the next version.

[PATCH v1 2/5] rust: timer: Replace HrTimerMode enum with trait-based mode types

[FUJITA Tomonori]

Replace the `HrTimerMode` enum with a trait-based approach that uses
zero-sized types to represent each mode of operation. Each mode now
implements the `HrTimerMode` trait.

This refactoring is a preparation for replacing raw `Ktime` in HrTimer
with the `Instant` and `Delta` types, and for making `HrTimer` generic
over a `ClockSource`.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 rust/kernel/time/hrtimer.rs | 164 ++++++++++++++++++++----------------
 1 file changed, 90 insertions(+), 74 deletions(-)

diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
index 380712d4302a..24d013e47c7b 100644
--- a/rust/kernel/time/hrtimer.rs
+++ b/rust/kernel/time/hrtimer.rs
@@ -98,7 +98,7 @@ pub fn to_ns(self) -> i64 {
 pub struct HrTimer<T> {
     #[pin]
     timer: Opaque<bindings::hrtimer>,
-    mode: HrTimerMode,
+    mode: bindings::hrtimer_mode,
     _t: PhantomData<T>,
 }
 
@@ -112,7 +112,7 @@ unsafe impl<T> Sync for HrTimer<T> {}
 
 impl<T> HrTimer<T> {
     /// Return an initializer for a new timer instance.
-    pub fn new<U: ClockSource>(mode: HrTimerMode) -> impl PinInit<Self>
+    pub fn new<U: ClockSource, M: HrTimerMode>() -> impl PinInit<Self>
     where
         T: HrTimerCallback,
     {
@@ -127,11 +127,11 @@ pub fn new<U: ClockSource>(mode: HrTimerMode) -> impl PinInit<Self>
                         place,
                         Some(T::Pointer::run),
                         U::ID,
-                        mode.into_c(),
+                        M::C_MODE,
                     );
                 }
             }),
-            mode: mode,
+            mode: M::C_MODE,
             _t: PhantomData,
         })
     }
@@ -389,7 +389,7 @@ unsafe fn start(this: *const Self, expires: Ktime) {
                 Self::c_timer_ptr(this).cast_mut(),
                 expires.to_ns(),
                 0,
-                (*Self::raw_get_timer(this)).mode.into_c(),
+                (*Self::raw_get_timer(this)).mode,
             );
         }
     }
@@ -414,77 +414,93 @@ fn into_c(self) -> bindings::hrtimer_restart {
 }
 
 /// Operational mode of [`HrTimer`].
-// NOTE: Some of these have the same encoding on the C side, so we keep
-// `repr(Rust)` and convert elsewhere.
-#[derive(Clone, Copy, PartialEq, Eq, Debug)]
-pub enum HrTimerMode {
-    /// Timer expires at the given expiration time.
-    Absolute,
-    /// Timer expires after the given expiration time interpreted as a duration from now.
-    Relative,
-    /// Timer does not move between CPU cores.
-    Pinned,
-    /// Timer handler is executed in soft irq context.
-    Soft,
-    /// Timer handler is executed in hard irq context.
-    Hard,
-    /// Timer expires at the given expiration time.
-    /// Timer does not move between CPU cores.
-    AbsolutePinned,
-    /// Timer expires after the given expiration time interpreted as a duration from now.
-    /// Timer does not move between CPU cores.
-    RelativePinned,
-    /// Timer expires at the given expiration time.
-    /// Timer handler is executed in soft irq context.
-    AbsoluteSoft,
-    /// Timer expires after the given expiration time interpreted as a duration from now.
-    /// Timer handler is executed in soft irq context.
-    RelativeSoft,
-    /// Timer expires at the given expiration time.
-    /// Timer does not move between CPU cores.
-    /// Timer handler is executed in soft irq context.
-    AbsolutePinnedSoft,
-    /// Timer expires after the given expiration time interpreted as a duration from now.
-    /// Timer does not move between CPU cores.
-    /// Timer handler is executed in soft irq context.
-    RelativePinnedSoft,
-    /// Timer expires at the given expiration time.
-    /// Timer handler is executed in hard irq context.
-    AbsoluteHard,
-    /// Timer expires after the given expiration time interpreted as a duration from now.
-    /// Timer handler is executed in hard irq context.
-    RelativeHard,
-    /// Timer expires at the given expiration time.
-    /// Timer does not move between CPU cores.
-    /// Timer handler is executed in hard irq context.
-    AbsolutePinnedHard,
-    /// Timer expires after the given expiration time interpreted as a duration from now.
-    /// Timer does not move between CPU cores.
-    /// Timer handler is executed in hard irq context.
-    RelativePinnedHard,
+pub trait HrTimerMode {
+    /// The C representation of hrtimer mode.
+    const C_MODE: bindings::hrtimer_mode;
 }
 
-impl HrTimerMode {
-    fn into_c(self) -> bindings::hrtimer_mode {
-        use bindings::*;
-        match self {
-            HrTimerMode::Absolute => hrtimer_mode_HRTIMER_MODE_ABS,
-            HrTimerMode::Relative => hrtimer_mode_HRTIMER_MODE_REL,
-            HrTimerMode::Pinned => hrtimer_mode_HRTIMER_MODE_PINNED,
-            HrTimerMode::Soft => hrtimer_mode_HRTIMER_MODE_SOFT,
-            HrTimerMode::Hard => hrtimer_mode_HRTIMER_MODE_HARD,
-            HrTimerMode::AbsolutePinned => hrtimer_mode_HRTIMER_MODE_ABS_PINNED,
-            HrTimerMode::RelativePinned => hrtimer_mode_HRTIMER_MODE_REL_PINNED,
-            HrTimerMode::AbsoluteSoft => hrtimer_mode_HRTIMER_MODE_ABS_SOFT,
-            HrTimerMode::RelativeSoft => hrtimer_mode_HRTIMER_MODE_REL_SOFT,
-            HrTimerMode::AbsolutePinnedSoft => hrtimer_mode_HRTIMER_MODE_ABS_PINNED_SOFT,
-            HrTimerMode::RelativePinnedSoft => hrtimer_mode_HRTIMER_MODE_REL_PINNED_SOFT,
-            HrTimerMode::AbsoluteHard => hrtimer_mode_HRTIMER_MODE_ABS_HARD,
-            HrTimerMode::RelativeHard => hrtimer_mode_HRTIMER_MODE_REL_HARD,
-            HrTimerMode::AbsolutePinnedHard => hrtimer_mode_HRTIMER_MODE_ABS_PINNED_HARD,
-            HrTimerMode::RelativePinnedHard => hrtimer_mode_HRTIMER_MODE_REL_PINNED_HARD,
-        }
-    }
+/// Timer that expires at a fixed point in time.
+pub struct AbsoluteMode;
+
+impl HrTimerMode for AbsoluteMode {
+    const C_MODE: bindings::hrtimer_mode = bindings::hrtimer_mode_HRTIMER_MODE_ABS;
+}
+
+/// Timer that expires after a delay from now.
+pub struct RelativeMode;
+
+impl HrTimerMode for RelativeMode {
+    const C_MODE: bindings::hrtimer_mode = bindings::hrtimer_mode_HRTIMER_MODE_REL;
+}
+
+/// Timer with absolute expiration time, pinned to its current CPU.
+pub struct AbsolutePinnedMode;
+
+impl HrTimerMode for AbsolutePinnedMode {
+    const C_MODE: bindings::hrtimer_mode = bindings::hrtimer_mode_HRTIMER_MODE_ABS_PINNED;
+}
+
+/// Timer with relative expiration time, pinned to its current CPU.
+pub struct RelativePinnedMode;
+
+impl HrTimerMode for RelativePinnedMode {
+    const C_MODE: bindings::hrtimer_mode = bindings::hrtimer_mode_HRTIMER_MODE_REL_PINNED;
+}
+
+/// Timer with absolute expiration, handled in soft irq context.
+pub struct AbsoluteSoftMode;
+
+impl HrTimerMode for AbsoluteSoftMode {
+    const C_MODE: bindings::hrtimer_mode = bindings::hrtimer_mode_HRTIMER_MODE_ABS_SOFT;
+}
+
+/// Timer with relative expiration, handled in soft irq context.
+pub struct RelativeSoftMode;
+
+impl HrTimerMode for RelativeSoftMode {
+    const C_MODE: bindings::hrtimer_mode = bindings::hrtimer_mode_HRTIMER_MODE_REL_SOFT;
+}
+
+/// Timer with absolute expiration, pinned to CPU and handled in soft irq context.
+pub struct AbsolutePinnedSoftMode;
+
+impl HrTimerMode for AbsolutePinnedSoftMode {
+    const C_MODE: bindings::hrtimer_mode = bindings::hrtimer_mode_HRTIMER_MODE_ABS_PINNED_SOFT;
+}
+
+/// Timer with relative expiration, pinned to CPU and handled in soft irq context.
+pub struct RelativePinnedSoftMode;
+
+impl HrTimerMode for RelativePinnedSoftMode {
+    const C_MODE: bindings::hrtimer_mode = bindings::hrtimer_mode_HRTIMER_MODE_REL_PINNED_SOFT;
+}
+
+/// Timer with absolute expiration, handled in hard irq context.
+pub struct AbsoluteHardMode;
+
+impl HrTimerMode for AbsoluteHardMode {
+    const C_MODE: bindings::hrtimer_mode = bindings::hrtimer_mode_HRTIMER_MODE_ABS_HARD;
+}
+
+/// Timer with relative expiration, handled in hard irq context.
+pub struct RelativeHardMode;
+
+impl HrTimerMode for RelativeHardMode {
+    const C_MODE: bindings::hrtimer_mode = bindings::hrtimer_mode_HRTIMER_MODE_REL_HARD;
+}
+
+/// Timer with absolute expiration, pinned to CPU and handled in hard irq context.
+pub struct AbsolutePinnedHardMode;
+
+impl HrTimerMode for AbsolutePinnedHardMode {
+    const C_MODE: bindings::hrtimer_mode = bindings::hrtimer_mode_HRTIMER_MODE_ABS_PINNED_HARD;
+}
+
+/// Timer with relative expiration, pinned to CPU and handled in hard irq context.
+pub struct RelativePinnedHardMode;
+
+impl HrTimerMode for RelativePinnedHardMode {
+    const C_MODE: bindings::hrtimer_mode = bindings::hrtimer_mode_HRTIMER_MODE_REL_PINNED_HARD;
 }
 
 /// Use to implement the [`HasHrTimer<T>`] trait.
-- 
2.43.0

Re: [PATCH v1 2/5] rust: timer: Replace HrTimerMode enum with trait-based mode types

[Andreas Hindborg]

"FUJITA Tomonori" <fujita.tomonori@gmail.com> writes:

> Replace the `HrTimerMode` enum with a trait-based approach that uses
> zero-sized types to represent each mode of operation. Each mode now
> implements the `HrTimerMode` trait.
>
> This refactoring is a preparation for replacing raw `Ktime` in HrTimer
> with the `Instant` and `Delta` types, and for making `HrTimer` generic
> over a `ClockSource`.
>
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>


Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>


Best regards,
Andreas Hindborg

[PATCH v1 3/5] rust: time: Add HrTimerExpires trait

[FUJITA Tomonori]

Introduce the `HrTimerExpires` trait to represent types that can be
used as expiration values for high-resolution timers. Define a
required method, `as_nanos()`, which returns the expiration time as a
raw nanosecond value suitable for use with C's hrtimer APIs.

Also extend the `HrTimerMode` to use the `HrTimerExpires` trait.

This refactoring is a preparation for enabling hrtimer code to work
uniformly with both absolute and relative expiration modes.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 rust/kernel/time.rs         |   5 +
 rust/kernel/time/hrtimer.rs | 181 ++++++++++++++++++++++++------------
 2 files changed, 128 insertions(+), 58 deletions(-)

diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
index deca2999ced6..ac9551fca14f 100644
--- a/rust/kernel/time.rs
+++ b/rust/kernel/time.rs
@@ -194,6 +194,11 @@ pub fn now() -> Self {
     pub fn elapsed(&self) -> Delta {
         Self::now() - *self
     }
+
+    #[inline]
+    pub(crate) fn as_nanos(&self) -> i64 {
+        self.inner
+    }
 }
 
 impl<C: ClockSource> core::ops::Sub for Instant<C> {
diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
index 24d013e47c7b..55e1825425b6 100644
--- a/rust/kernel/time/hrtimer.rs
+++ b/rust/kernel/time/hrtimer.rs
@@ -67,7 +67,7 @@
 //! A `restart` operation on a timer in the **stopped** state is equivalent to a
 //! `start` operation.
 
-use super::ClockSource;
+use super::{ClockSource, Delta, Instant};
 use crate::{prelude::*, types::Opaque};
 use core::marker::PhantomData;
 use pin_init::PinInit;
@@ -413,94 +413,159 @@ fn into_c(self) -> bindings::hrtimer_restart {
     }
 }
 
+/// Time representations that can be used as expiration values in [`HrTimer`].
+pub trait HrTimerExpires {
+    /// Converts the expiration time into a nanosecond representation.
+    ///
+    /// This value corresponds to a raw ktime_t value, suitable for passing to kernel
+    /// timer functions. The interpretation (absolute vs relative) depends on the
+    /// associated [HrTimerMode] in use.
+    fn as_nanos(&self) -> i64;
+}
+
+impl<C: ClockSource> HrTimerExpires for Instant<C> {
+    fn as_nanos(&self) -> i64 {
+        Instant::<C>::as_nanos(self)
+    }
+}
+
+impl HrTimerExpires for Delta {
+    fn as_nanos(&self) -> i64 {
+        Delta::as_nanos(self)
+    }
+}
+
 /// Operational mode of [`HrTimer`].
 pub trait HrTimerMode {
     /// The C representation of hrtimer mode.
     const C_MODE: bindings::hrtimer_mode;
-}
 
-/// Timer that expires at a fixed point in time.
-pub struct AbsoluteMode;
+    /// Type representing the clock source.
+    type Clock: ClockSource;
 
-impl HrTimerMode for AbsoluteMode {
-    const C_MODE: bindings::hrtimer_mode = bindings::hrtimer_mode_HRTIMER_MODE_ABS;
+    /// Type representing the expiration specification (absolute or relative time).
+    type Expires: HrTimerExpires;
 }
 
-/// Timer that expires after a delay from now.
-pub struct RelativeMode;
-
-impl HrTimerMode for RelativeMode {
-    const C_MODE: bindings::hrtimer_mode = bindings::hrtimer_mode_HRTIMER_MODE_REL;
-}
+/// Defines a new `HrTimerMode` implementation with a given expiration type and C mode.
+#[doc(hidden)]
+macro_rules! define_hrtimer_mode {
+    (
+        $(#[$meta:meta])*
+        $vis:vis struct $name:ident<$clock:ident> {
+            c = $mode:ident,
+            expires = $expires:ty
+        }
+    ) => {
+        $(#[$meta])*
+        $vis struct $name<$clock: $crate::time::ClockSource>(
+            ::core::marker::PhantomData<$clock>
+        );
 
-/// Timer with absolute expiration time, pinned to its current CPU.
-pub struct AbsolutePinnedMode;
+        impl<$clock: $crate::time::ClockSource> $crate::time::hrtimer::HrTimerMode for $name<$clock> {
+            const C_MODE: $crate::bindings::hrtimer_mode =
+                $crate::macros::paste! {$crate::bindings::[<hrtimer_mode_ $mode>]};
 
-impl HrTimerMode for AbsolutePinnedMode {
-    const C_MODE: bindings::hrtimer_mode = bindings::hrtimer_mode_HRTIMER_MODE_ABS_PINNED;
+            type Clock = $clock;
+            type Expires = $expires;
+        }
+    };
 }
 
-/// Timer with relative expiration time, pinned to its current CPU.
-pub struct RelativePinnedMode;
-
-impl HrTimerMode for RelativePinnedMode {
-    const C_MODE: bindings::hrtimer_mode = bindings::hrtimer_mode_HRTIMER_MODE_REL_PINNED;
+define_hrtimer_mode! {
+    /// Timer that expires at a fixed point in time.
+    pub struct AbsoluteMode<C> {
+        c = HRTIMER_MODE_ABS,
+        expires = Instant<C>
+    }
 }
 
-/// Timer with absolute expiration, handled in soft irq context.
-pub struct AbsoluteSoftMode;
-
-impl HrTimerMode for AbsoluteSoftMode {
-    const C_MODE: bindings::hrtimer_mode = bindings::hrtimer_mode_HRTIMER_MODE_ABS_SOFT;
+define_hrtimer_mode! {
+    /// Timer that expires after a delay from now.
+    pub struct RelativeMode<C> {
+        c = HRTIMER_MODE_REL,
+        expires = Delta
+    }
 }
 
-/// Timer with relative expiration, handled in soft irq context.
-pub struct RelativeSoftMode;
-
-impl HrTimerMode for RelativeSoftMode {
-    const C_MODE: bindings::hrtimer_mode = bindings::hrtimer_mode_HRTIMER_MODE_REL_SOFT;
+define_hrtimer_mode! {
+    /// Timer with absolute expiration time, pinned to its current CPU.
+    pub struct AbsolutePinnedMode<C> {
+        c = HRTIMER_MODE_ABS_PINNED,
+        expires = Instant<C>
+    }
 }
 
-/// Timer with absolute expiration, pinned to CPU and handled in soft irq context.
-pub struct AbsolutePinnedSoftMode;
-
-impl HrTimerMode for AbsolutePinnedSoftMode {
-    const C_MODE: bindings::hrtimer_mode = bindings::hrtimer_mode_HRTIMER_MODE_ABS_PINNED_SOFT;
+define_hrtimer_mode! {
+    /// Timer with relative expiration time, pinned to its current CPU.
+    pub struct RelativePinnedMode<C> {
+        c = HRTIMER_MODE_REL_PINNED,
+        expires = Delta
+    }
 }
 
-/// Timer with relative expiration, pinned to CPU and handled in soft irq context.
-pub struct RelativePinnedSoftMode;
-
-impl HrTimerMode for RelativePinnedSoftMode {
-    const C_MODE: bindings::hrtimer_mode = bindings::hrtimer_mode_HRTIMER_MODE_REL_PINNED_SOFT;
+define_hrtimer_mode! {
+    /// Timer with absolute expiration, handled in soft irq context.
+    pub struct AbsoluteSoftMode<C> {
+        c = HRTIMER_MODE_ABS_SOFT,
+        expires = Instant<C>
+    }
 }
 
-/// Timer with absolute expiration, handled in hard irq context.
-pub struct AbsoluteHardMode;
-
-impl HrTimerMode for AbsoluteHardMode {
-    const C_MODE: bindings::hrtimer_mode = bindings::hrtimer_mode_HRTIMER_MODE_ABS_HARD;
+define_hrtimer_mode! {
+    /// Timer with relative expiration, handled in soft irq context.
+    pub struct RelativeSoftMode<C> {
+        c = HRTIMER_MODE_REL_SOFT,
+        expires = Delta
+    }
 }
 
-/// Timer with relative expiration, handled in hard irq context.
-pub struct RelativeHardMode;
+define_hrtimer_mode! {
+    /// Timer with absolute expiration, pinned to CPU and handled in soft irq context.
+    pub struct AbsolutePinnedSoftMode<C> {
+        c = HRTIMER_MODE_ABS_PINNED_SOFT,
+        expires = Instant<C>
+    }
+}
 
-impl HrTimerMode for RelativeHardMode {
-    const C_MODE: bindings::hrtimer_mode = bindings::hrtimer_mode_HRTIMER_MODE_REL_HARD;
+define_hrtimer_mode! {
+    /// Timer with absolute expiration, pinned to CPU and handled in soft irq context.
+    pub struct RelativePinnedSoftMode<C> {
+        c = HRTIMER_MODE_REL_PINNED_SOFT,
+        expires = Delta
+    }
 }
 
-/// Timer with absolute expiration, pinned to CPU and handled in hard irq context.
-pub struct AbsolutePinnedHardMode;
+define_hrtimer_mode! {
+    /// Timer with absolute expiration, handled in hard irq context.
+    pub struct AbsoluteHardMode<C> {
+        c = HRTIMER_MODE_ABS_HARD,
+        expires = Instant<C>
+    }
+}
 
-impl HrTimerMode for AbsolutePinnedHardMode {
-    const C_MODE: bindings::hrtimer_mode = bindings::hrtimer_mode_HRTIMER_MODE_ABS_PINNED_HARD;
+define_hrtimer_mode! {
+    /// Timer with relative expiration, handled in hard irq context.
+    pub struct RelativeHardMode<C> {
+        c = HRTIMER_MODE_REL_HARD,
+        expires = Delta
+    }
 }
 
-/// Timer with relative expiration, pinned to CPU and handled in hard irq context.
-pub struct RelativePinnedHardMode;
+define_hrtimer_mode! {
+    /// Timer with absolute expiration, pinned to CPU and handled in hard irq context.
+    pub struct AbsolutePinnedHardMode<C> {
+        c = HRTIMER_MODE_ABS_PINNED_HARD,
+        expires = Instant<C>
+    }
+}
 
-impl HrTimerMode for RelativePinnedHardMode {
-    const C_MODE: bindings::hrtimer_mode = bindings::hrtimer_mode_HRTIMER_MODE_REL_PINNED_HARD;
+define_hrtimer_mode! {
+    /// Timer with relative expiration, pinned to CPU and handled in hard irq context.
+    pub struct RelativePinnedHardMode<C> {
+        c = HRTIMER_MODE_REL_PINNED_HARD,
+        expires = Delta
+    }
 }
 
 /// Use to implement the [`HasHrTimer<T>`] trait.
-- 
2.43.0

Re: [PATCH v1 3/5] rust: time: Add HrTimerExpires trait

[Andreas Hindborg]

Hi Tomonori,

Thanks for fixing this.

FUJITA Tomonori <fujita.tomonori@gmail.com> writes:

> Introduce the `HrTimerExpires` trait to represent types that can be
> used as expiration values for high-resolution timers. Define a
> required method, `as_nanos()`, which returns the expiration time as a
> raw nanosecond value suitable for use with C's hrtimer APIs.
>
> Also extend the `HrTimerMode` to use the `HrTimerExpires` trait.
>
> This refactoring is a preparation for enabling hrtimer code to work
> uniformly with both absolute and relative expiration modes.
>
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> ---
>  rust/kernel/time.rs         |   5 +
>  rust/kernel/time/hrtimer.rs | 181 ++++++++++++++++++++++++------------
>  2 files changed, 128 insertions(+), 58 deletions(-)
>
> diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
> index deca2999ced6..ac9551fca14f 100644
> --- a/rust/kernel/time.rs
> +++ b/rust/kernel/time.rs
> @@ -194,6 +194,11 @@ pub fn now() -> Self {
>      pub fn elapsed(&self) -> Delta {
>          Self::now() - *self
>      }
> +
> +    #[inline]
> +    pub(crate) fn as_nanos(&self) -> i64 {
> +        self.inner
> +    }
>  }
>  
>  impl<C: ClockSource> core::ops::Sub for Instant<C> {
> diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
> index 24d013e47c7b..55e1825425b6 100644
> --- a/rust/kernel/time/hrtimer.rs
> +++ b/rust/kernel/time/hrtimer.rs

<cut>

> +/// Defines a new `HrTimerMode` implementation with a given expiration type and C mode.
> +#[doc(hidden)]
> +macro_rules! define_hrtimer_mode {
> +    (
> +        $(#[$meta:meta])*
> +        $vis:vis struct $name:ident<$clock:ident> {
> +            c = $mode:ident,
> +            expires = $expires:ty
> +        }
> +    ) => {
> +        $(#[$meta])*
> +        $vis struct $name<$clock: $crate::time::ClockSource>(
> +            ::core::marker::PhantomData<$clock>
> +        );

I think a macro is too much here. The code would be easier to read
without the macro, and the macro does not remove much code here.

Could you try to do the trait implementations without the macro?


Best regards,
Andreas Hindborg

Re: [PATCH v1 3/5] rust: time: Add HrTimerExpires trait

[FUJITA Tomonori]

On Fri, 30 May 2025 15:04:00 +0200
Andreas Hindborg <a.hindborg@kernel.org> wrote:

>> +/// Defines a new `HrTimerMode` implementation with a given expiration type and C mode.
>> +#[doc(hidden)]
>> +macro_rules! define_hrtimer_mode {
>> +    (
>> +        $(#[$meta:meta])*
>> +        $vis:vis struct $name:ident<$clock:ident> {
>> +            c = $mode:ident,
>> +            expires = $expires:ty
>> +        }
>> +    ) => {
>> +        $(#[$meta])*
>> +        $vis struct $name<$clock: $crate::time::ClockSource>(
>> +            ::core::marker::PhantomData<$clock>
>> +        );
> 
> I think a macro is too much here. The code would be easier to read
> without the macro, and the macro does not remove much code here.
> 
> Could you try to do the trait implementations without the macro?

Something like the following, right? If so, I'll do in the next
version. I'm also fine with that way.

/// Timer that expires at a fixed point in time.
pub struct AbsoluteMode<C: ClockSource>(PhantomData<C>);

impl<C: ClockSource> HrTimerMode for AbsoluteMode<C> {
    const C_MODE: bindings::hrtimer_mode = bindings::hrtimer_mode_HRTIMER_MODE_ABS;

    type Clock = C;
    type Expires = Instant<C>;
}

instead of

define_hrtimer_mode! {
    /// Timer that expires at a fixed point in time.
    pub struct AbsoluteMode<C> {
        c = HRTIMER_MODE_ABS,
        expires = Instant<C>
    }
}

Re: [PATCH v1 3/5] rust: time: Add HrTimerExpires trait

[Andreas Hindborg]

"FUJITA Tomonori" <fujita.tomonori@gmail.com> writes:

> On Fri, 30 May 2025 15:04:00 +0200
> Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
>>> +/// Defines a new `HrTimerMode` implementation with a given expiration type and C mode.
>>> +#[doc(hidden)]
>>> +macro_rules! define_hrtimer_mode {
>>> +    (
>>> +        $(#[$meta:meta])*
>>> +        $vis:vis struct $name:ident<$clock:ident> {
>>> +            c = $mode:ident,
>>> +            expires = $expires:ty
>>> +        }
>>> +    ) => {
>>> +        $(#[$meta])*
>>> +        $vis struct $name<$clock: $crate::time::ClockSource>(
>>> +            ::core::marker::PhantomData<$clock>
>>> +        );
>>
>> I think a macro is too much here. The code would be easier to read
>> without the macro, and the macro does not remove much code here.
>>
>> Could you try to do the trait implementations without the macro?
>
> Something like the following, right? If so, I'll do in the next
> version. I'm also fine with that way.
>
> /// Timer that expires at a fixed point in time.
> pub struct AbsoluteMode<C: ClockSource>(PhantomData<C>);
>
> impl<C: ClockSource> HrTimerMode for AbsoluteMode<C> {
>     const C_MODE: bindings::hrtimer_mode = bindings::hrtimer_mode_HRTIMER_MODE_ABS;
>
>     type Clock = C;
>     type Expires = Instant<C>;
> }

OK, let's do that then 👍


Best regards,
Andreas Hindborg

[PATCH v1 4/5] rust: time: Make HasHrTimer generic over HrTimerMode

[FUJITA Tomonori]

Add a `TimerMode` associated type to the `HasHrTimer` trait to
represent the operational mode of the timer, such as absolute or
relative expiration.  This new type must implement the `HrTimerMode`
trait, which defines how expiration values are interpreted.

Update the `start()` method to accept an `expires` parameter of type
`<Self::TimerMode as HrTimerMode>::Expires` instead of the fixed `Ktime`.
This enables different timer modes to provide strongly typed expiration
values, such as `Instant<C>` or `Delta`.

The `impl_has_hr_timer` macro is also extended to allow specifying the
`HrTimerMode`. In the following example, it guarantees that the
`start()` method for `Foo` only accepts `Instant<Monotonic>`. Using a
`Delta` or an `Instant` with a different clock source will result in a
compile-time error:

struct Foo {
    #[pin]
    timer: HrTimer<Self>,

}

impl_has_hr_timer! {
    impl HasHrTimer<Self> for Foo {
        mode = AbsoluteMode<Monotonic>,
        self.timer
    }
}

This design eliminates runtime mismatches between expires types and
clock sources, and enables stronger type-level guarantees throughout
hrtimer.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 rust/kernel/time/hrtimer.rs         | 55 ++++++++++++++++++++++-------
 rust/kernel/time/hrtimer/arc.rs     |  8 +++--
 rust/kernel/time/hrtimer/pin.rs     |  8 +++--
 rust/kernel/time/hrtimer/pin_mut.rs |  8 +++--
 rust/kernel/time/hrtimer/tbox.rs    |  8 +++--
 5 files changed, 66 insertions(+), 21 deletions(-)

diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
index 55e1825425b6..3355ae6fe76d 100644
--- a/rust/kernel/time/hrtimer.rs
+++ b/rust/kernel/time/hrtimer.rs
@@ -98,7 +98,6 @@ pub fn to_ns(self) -> i64 {
 pub struct HrTimer<T> {
     #[pin]
     timer: Opaque<bindings::hrtimer>,
-    mode: bindings::hrtimer_mode,
     _t: PhantomData<T>,
 }
 
@@ -112,9 +111,10 @@ unsafe impl<T> Sync for HrTimer<T> {}
 
 impl<T> HrTimer<T> {
     /// Return an initializer for a new timer instance.
-    pub fn new<U: ClockSource, M: HrTimerMode>() -> impl PinInit<Self>
+    pub fn new() -> impl PinInit<Self>
     where
         T: HrTimerCallback,
+        T: HasHrTimer<T>,
     {
         pin_init!(Self {
             // INVARIANT: We initialize `timer` with `hrtimer_setup` below.
@@ -126,12 +126,11 @@ pub fn new<U: ClockSource, M: HrTimerMode>() -> impl PinInit<Self>
                     bindings::hrtimer_setup(
                         place,
                         Some(T::Pointer::run),
-                        U::ID,
-                        M::C_MODE,
+                        <<T as HasHrTimer<T>>::TimerMode as HrTimerMode>::Clock::ID,
+                        <T as HasHrTimer<T>>::TimerMode::C_MODE,
                     );
                 }
             }),
-            mode: M::C_MODE,
             _t: PhantomData,
         })
     }
@@ -193,6 +192,11 @@ pub(crate) unsafe fn raw_cancel(this: *const Self) -> bool {
 /// exist. A timer can be manipulated through any of the handles, and a handle
 /// may represent a cancelled timer.
 pub trait HrTimerPointer: Sync + Sized {
+    /// The operational mode associated with this timer.
+    ///
+    /// This defines how the expiration value is interpreted.
+    type TimerMode: HrTimerMode;
+
     /// A handle representing a started or restarted timer.
     ///
     /// If the timer is running or if the timer callback is executing when the
@@ -205,7 +209,7 @@ pub trait HrTimerPointer: Sync + Sized {
 
     /// Start the timer with expiry after `expires` time units. If the timer was
     /// already running, it is restarted with the new expiry time.
-    fn start(self, expires: Ktime) -> Self::TimerHandle;
+    fn start(self, expires: <Self::TimerMode as HrTimerMode>::Expires) -> Self::TimerHandle;
 }
 
 /// Unsafe version of [`HrTimerPointer`] for situations where leaking the
@@ -220,6 +224,11 @@ pub trait HrTimerPointer: Sync + Sized {
 /// [`UnsafeHrTimerPointer`] outlives any associated [`HrTimerPointer::TimerHandle`]
 /// instances.
 pub unsafe trait UnsafeHrTimerPointer: Sync + Sized {
+    /// The operational mode associated with this timer.
+    ///
+    /// This defines how the expiration value is interpreted.
+    type TimerMode: HrTimerMode;
+
     /// A handle representing a running timer.
     ///
     /// # Safety
@@ -236,7 +245,7 @@ pub unsafe trait UnsafeHrTimerPointer: Sync + Sized {
     ///
     /// Caller promises keep the timer structure alive until the timer is dead.
     /// Caller can ensure this by not leaking the returned [`Self::TimerHandle`].
-    unsafe fn start(self, expires: Ktime) -> Self::TimerHandle;
+    unsafe fn start(self, expires: <Self::TimerMode as HrTimerMode>::Expires) -> Self::TimerHandle;
 }
 
 /// A trait for stack allocated timers.
@@ -246,9 +255,14 @@ pub unsafe trait UnsafeHrTimerPointer: Sync + Sized {
 /// Implementers must ensure that `start_scoped` does not return until the
 /// timer is dead and the timer handler is not running.
 pub unsafe trait ScopedHrTimerPointer {
+    /// The operational mode associated with this timer.
+    ///
+    /// This defines how the expiration value is interpreted.
+    type TimerMode: HrTimerMode;
+
     /// Start the timer to run after `expires` time units and immediately
     /// after call `f`. When `f` returns, the timer is cancelled.
-    fn start_scoped<T, F>(self, expires: Ktime, f: F) -> T
+    fn start_scoped<T, F>(self, expires: <Self::TimerMode as HrTimerMode>::Expires, f: F) -> T
     where
         F: FnOnce() -> T;
 }
@@ -260,7 +274,13 @@ unsafe impl<T> ScopedHrTimerPointer for T
 where
     T: UnsafeHrTimerPointer,
 {
-    fn start_scoped<U, F>(self, expires: Ktime, f: F) -> U
+    type TimerMode = T::TimerMode;
+
+    fn start_scoped<U, F>(
+        self,
+        expires: <<T as UnsafeHrTimerPointer>::TimerMode as HrTimerMode>::Expires,
+        f: F,
+    ) -> U
     where
         F: FnOnce() -> U,
     {
@@ -335,6 +355,11 @@ pub unsafe trait HrTimerHandle {
 /// their documentation. All the methods of this trait must operate on the same
 /// field.
 pub unsafe trait HasHrTimer<T> {
+    /// The operational mode associated with this timer.
+    ///
+    /// This defines how the expiration value is interpreted.
+    type TimerMode: HrTimerMode;
+
     /// Return a pointer to the [`HrTimer`] within `Self`.
     ///
     /// This function is useful to get access to the value without creating
@@ -382,14 +407,14 @@ unsafe fn c_timer_ptr(this: *const Self) -> *const bindings::hrtimer {
     /// - `this` must point to a valid `Self`.
     /// - Caller must ensure that the pointee of `this` lives until the timer
     ///   fires or is canceled.
-    unsafe fn start(this: *const Self, expires: Ktime) {
+    unsafe fn start(this: *const Self, expires: <Self::TimerMode as HrTimerMode>::Expires) {
         // SAFETY: By function safety requirement, `this` is a valid `Self`.
         unsafe {
             bindings::hrtimer_start_range_ns(
                 Self::c_timer_ptr(this).cast_mut(),
-                expires.to_ns(),
+                expires.as_nanos(),
                 0,
-                (*Self::raw_get_timer(this)).mode,
+                <Self::TimerMode as HrTimerMode>::Clock::ID as u32,
             );
         }
     }
@@ -579,12 +604,16 @@ macro_rules! impl_has_hr_timer {
         impl$({$($generics:tt)*})?
             HasHrTimer<$timer_type:ty>
             for $self:ty
-        { self.$field:ident }
+        {
+            mode = $mode:ty,
+            self.$field:ident
+        }
         $($rest:tt)*
     ) => {
         // SAFETY: This implementation of `raw_get_timer` only compiles if the
         // field has the right type.
         unsafe impl$(<$($generics)*>)? $crate::time::hrtimer::HasHrTimer<$timer_type> for $self {
+            type TimerMode = $mode;
 
             #[inline]
             unsafe fn raw_get_timer(
diff --git a/rust/kernel/time/hrtimer/arc.rs b/rust/kernel/time/hrtimer/arc.rs
index ccf1e66e5b2d..ed490a7a8950 100644
--- a/rust/kernel/time/hrtimer/arc.rs
+++ b/rust/kernel/time/hrtimer/arc.rs
@@ -4,8 +4,8 @@
 use super::HrTimer;
 use super::HrTimerCallback;
 use super::HrTimerHandle;
+use super::HrTimerMode;
 use super::HrTimerPointer;
-use super::Ktime;
 use super::RawHrTimerCallback;
 use crate::sync::Arc;
 use crate::sync::ArcBorrow;
@@ -54,9 +54,13 @@ impl<T> HrTimerPointer for Arc<T>
     T: HasHrTimer<T>,
     T: for<'a> HrTimerCallback<Pointer<'a> = Self>,
 {
+    type TimerMode = <T as HasHrTimer<T>>::TimerMode;
     type TimerHandle = ArcHrTimerHandle<T>;
 
-    fn start(self, expires: Ktime) -> ArcHrTimerHandle<T> {
+    fn start(
+        self,
+        expires: <<T as HasHrTimer<T>>::TimerMode as HrTimerMode>::Expires,
+    ) -> ArcHrTimerHandle<T> {
         // SAFETY:
         //  - We keep `self` alive by wrapping it in a handle below.
         //  - Since we generate the pointer passed to `start` from a valid
diff --git a/rust/kernel/time/hrtimer/pin.rs b/rust/kernel/time/hrtimer/pin.rs
index 293ca9cf058c..550aad28d987 100644
--- a/rust/kernel/time/hrtimer/pin.rs
+++ b/rust/kernel/time/hrtimer/pin.rs
@@ -4,7 +4,7 @@
 use super::HrTimer;
 use super::HrTimerCallback;
 use super::HrTimerHandle;
-use super::Ktime;
+use super::HrTimerMode;
 use super::RawHrTimerCallback;
 use super::UnsafeHrTimerPointer;
 use core::pin::Pin;
@@ -54,9 +54,13 @@ unsafe impl<'a, T> UnsafeHrTimerPointer for Pin<&'a T>
     T: HasHrTimer<T>,
     T: HrTimerCallback<Pointer<'a> = Self>,
 {
+    type TimerMode = <T as HasHrTimer<T>>::TimerMode;
     type TimerHandle = PinHrTimerHandle<'a, T>;
 
-    unsafe fn start(self, expires: Ktime) -> Self::TimerHandle {
+    unsafe fn start(
+        self,
+        expires: <<T as HasHrTimer<T>>::TimerMode as HrTimerMode>::Expires,
+    ) -> Self::TimerHandle {
         // Cast to pointer
         let self_ptr: *const T = self.get_ref();
 
diff --git a/rust/kernel/time/hrtimer/pin_mut.rs b/rust/kernel/time/hrtimer/pin_mut.rs
index 6033572d35ad..bacd3d5d972a 100644
--- a/rust/kernel/time/hrtimer/pin_mut.rs
+++ b/rust/kernel/time/hrtimer/pin_mut.rs
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 
 use super::{
-    HasHrTimer, HrTimer, HrTimerCallback, HrTimerHandle, Ktime, RawHrTimerCallback,
+    HasHrTimer, HrTimer, HrTimerCallback, HrTimerHandle, HrTimerMode, RawHrTimerCallback,
     UnsafeHrTimerPointer,
 };
 use core::{marker::PhantomData, pin::Pin, ptr::NonNull};
@@ -52,9 +52,13 @@ unsafe impl<'a, T> UnsafeHrTimerPointer for Pin<&'a mut T>
     T: HasHrTimer<T>,
     T: HrTimerCallback<Pointer<'a> = Self>,
 {
+    type TimerMode = <T as HasHrTimer<T>>::TimerMode;
     type TimerHandle = PinMutHrTimerHandle<'a, T>;
 
-    unsafe fn start(mut self, expires: Ktime) -> Self::TimerHandle {
+    unsafe fn start(
+        mut self,
+        expires: <<T as HasHrTimer<T>>::TimerMode as HrTimerMode>::Expires,
+    ) -> Self::TimerHandle {
         // SAFETY:
         // - We promise not to move out of `self`. We only pass `self`
         //   back to the caller as a `Pin<&mut self>`.
diff --git a/rust/kernel/time/hrtimer/tbox.rs b/rust/kernel/time/hrtimer/tbox.rs
index 29526a5da203..ec08303315f2 100644
--- a/rust/kernel/time/hrtimer/tbox.rs
+++ b/rust/kernel/time/hrtimer/tbox.rs
@@ -4,8 +4,8 @@
 use super::HrTimer;
 use super::HrTimerCallback;
 use super::HrTimerHandle;
+use super::HrTimerMode;
 use super::HrTimerPointer;
-use super::Ktime;
 use super::RawHrTimerCallback;
 use crate::prelude::*;
 use core::ptr::NonNull;
@@ -64,9 +64,13 @@ impl<T, A> HrTimerPointer for Pin<Box<T, A>>
     T: for<'a> HrTimerCallback<Pointer<'a> = Pin<Box<T, A>>>,
     A: crate::alloc::Allocator,
 {
+    type TimerMode = <T as HasHrTimer<T>>::TimerMode;
     type TimerHandle = BoxHrTimerHandle<T, A>;
 
-    fn start(self, expires: Ktime) -> Self::TimerHandle {
+    fn start(
+        self,
+        expires: <<T as HasHrTimer<T>>::TimerMode as HrTimerMode>::Expires,
+    ) -> Self::TimerHandle {
         // SAFETY:
         //  - We will not move out of this box during timer callback (we pass an
         //    immutable reference to the callback).
-- 
2.43.0

Re: [PATCH v1 4/5] rust: time: Make HasHrTimer generic over HrTimerMode

[Andreas Hindborg]

"FUJITA Tomonori" <fujita.tomonori@gmail.com> writes:

> Add a `TimerMode` associated type to the `HasHrTimer` trait to
> represent the operational mode of the timer, such as absolute or
> relative expiration.  This new type must implement the `HrTimerMode`
> trait, which defines how expiration values are interpreted.
>
> Update the `start()` method to accept an `expires` parameter of type
> `<Self::TimerMode as HrTimerMode>::Expires` instead of the fixed `Ktime`.
> This enables different timer modes to provide strongly typed expiration
> values, such as `Instant<C>` or `Delta`.
>
> The `impl_has_hr_timer` macro is also extended to allow specifying the
> `HrTimerMode`. In the following example, it guarantees that the
> `start()` method for `Foo` only accepts `Instant<Monotonic>`. Using a
> `Delta` or an `Instant` with a different clock source will result in a
> compile-time error:
>
> struct Foo {
>     #[pin]
>     timer: HrTimer<Self>,
>
> }
>
> impl_has_hr_timer! {
>     impl HasHrTimer<Self> for Foo {
>         mode = AbsoluteMode<Monotonic>,
>         self.timer
>     }
> }
>
> This design eliminates runtime mismatches between expires types and
> clock sources, and enables stronger type-level guarantees throughout
> hrtimer.
>
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> ---
>  rust/kernel/time/hrtimer.rs         | 55 ++++++++++++++++++++++-------
>  rust/kernel/time/hrtimer/arc.rs     |  8 +++--
>  rust/kernel/time/hrtimer/pin.rs     |  8 +++--
>  rust/kernel/time/hrtimer/pin_mut.rs |  8 +++--
>  rust/kernel/time/hrtimer/tbox.rs    |  8 +++--
>  5 files changed, 66 insertions(+), 21 deletions(-)
>
> diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
> index 55e1825425b6..3355ae6fe76d 100644
> --- a/rust/kernel/time/hrtimer.rs
> +++ b/rust/kernel/time/hrtimer.rs
> @@ -98,7 +98,6 @@ pub fn to_ns(self) -> i64 {
>  pub struct HrTimer<T> {
>      #[pin]
>      timer: Opaque<bindings::hrtimer>,
> -    mode: bindings::hrtimer_mode,
>      _t: PhantomData<T>,
>  }
>
> @@ -112,9 +111,10 @@ unsafe impl<T> Sync for HrTimer<T> {}
>
>  impl<T> HrTimer<T> {
>      /// Return an initializer for a new timer instance.
> -    pub fn new<U: ClockSource, M: HrTimerMode>() -> impl PinInit<Self>
> +    pub fn new() -> impl PinInit<Self>
>      where
>          T: HrTimerCallback,
> +        T: HasHrTimer<T>,
>      {
>          pin_init!(Self {
>              // INVARIANT: We initialize `timer` with `hrtimer_setup` below.
> @@ -126,12 +126,11 @@ pub fn new<U: ClockSource, M: HrTimerMode>() -> impl PinInit<Self>
>                      bindings::hrtimer_setup(
>                          place,
>                          Some(T::Pointer::run),
> -                        U::ID,
> -                        M::C_MODE,
> +                        <<T as HasHrTimer<T>>::TimerMode as HrTimerMode>::Clock::ID,
> +                        <T as HasHrTimer<T>>::TimerMode::C_MODE,
>                      );
>                  }
>              }),
> -            mode: M::C_MODE,
>              _t: PhantomData,
>          })
>      }
> @@ -193,6 +192,11 @@ pub(crate) unsafe fn raw_cancel(this: *const Self) -> bool {
>  /// exist. A timer can be manipulated through any of the handles, and a handle
>  /// may represent a cancelled timer.
>  pub trait HrTimerPointer: Sync + Sized {
> +    /// The operational mode associated with this timer.
> +    ///
> +    /// This defines how the expiration value is interpreted.
> +    type TimerMode: HrTimerMode;
> +
>      /// A handle representing a started or restarted timer.
>      ///
>      /// If the timer is running or if the timer callback is executing when the
> @@ -205,7 +209,7 @@ pub trait HrTimerPointer: Sync + Sized {
>
>      /// Start the timer with expiry after `expires` time units. If the timer was
>      /// already running, it is restarted with the new expiry time.
> -    fn start(self, expires: Ktime) -> Self::TimerHandle;
> +    fn start(self, expires: <Self::TimerMode as HrTimerMode>::Expires) -> Self::TimerHandle;
>  }
>
>  /// Unsafe version of [`HrTimerPointer`] for situations where leaking the
> @@ -220,6 +224,11 @@ pub trait HrTimerPointer: Sync + Sized {
>  /// [`UnsafeHrTimerPointer`] outlives any associated [`HrTimerPointer::TimerHandle`]
>  /// instances.
>  pub unsafe trait UnsafeHrTimerPointer: Sync + Sized {
> +    /// The operational mode associated with this timer.
> +    ///
> +    /// This defines how the expiration value is interpreted.
> +    type TimerMode: HrTimerMode;
> +
>      /// A handle representing a running timer.
>      ///
>      /// # Safety
> @@ -236,7 +245,7 @@ pub unsafe trait UnsafeHrTimerPointer: Sync + Sized {
>      ///
>      /// Caller promises keep the timer structure alive until the timer is dead.
>      /// Caller can ensure this by not leaking the returned [`Self::TimerHandle`].
> -    unsafe fn start(self, expires: Ktime) -> Self::TimerHandle;
> +    unsafe fn start(self, expires: <Self::TimerMode as HrTimerMode>::Expires) -> Self::TimerHandle;
>  }
>
>  /// A trait for stack allocated timers.
> @@ -246,9 +255,14 @@ pub unsafe trait UnsafeHrTimerPointer: Sync + Sized {
>  /// Implementers must ensure that `start_scoped` does not return until the
>  /// timer is dead and the timer handler is not running.
>  pub unsafe trait ScopedHrTimerPointer {
> +    /// The operational mode associated with this timer.
> +    ///
> +    /// This defines how the expiration value is interpreted.
> +    type TimerMode: HrTimerMode;
> +
>      /// Start the timer to run after `expires` time units and immediately
>      /// after call `f`. When `f` returns, the timer is cancelled.
> -    fn start_scoped<T, F>(self, expires: Ktime, f: F) -> T
> +    fn start_scoped<T, F>(self, expires: <Self::TimerMode as HrTimerMode>::Expires, f: F) -> T
>      where
>          F: FnOnce() -> T;
>  }
> @@ -260,7 +274,13 @@ unsafe impl<T> ScopedHrTimerPointer for T
>  where
>      T: UnsafeHrTimerPointer,
>  {
> -    fn start_scoped<U, F>(self, expires: Ktime, f: F) -> U
> +    type TimerMode = T::TimerMode;
> +
> +    fn start_scoped<U, F>(
> +        self,
> +        expires: <<T as UnsafeHrTimerPointer>::TimerMode as HrTimerMode>::Expires,
> +        f: F,
> +    ) -> U
>      where
>          F: FnOnce() -> U,
>      {
> @@ -335,6 +355,11 @@ pub unsafe trait HrTimerHandle {
>  /// their documentation. All the methods of this trait must operate on the same
>  /// field.
>  pub unsafe trait HasHrTimer<T> {
> +    /// The operational mode associated with this timer.
> +    ///
> +    /// This defines how the expiration value is interpreted.
> +    type TimerMode: HrTimerMode;
> +
>      /// Return a pointer to the [`HrTimer`] within `Self`.
>      ///
>      /// This function is useful to get access to the value without creating
> @@ -382,14 +407,14 @@ unsafe fn c_timer_ptr(this: *const Self) -> *const bindings::hrtimer {
>      /// - `this` must point to a valid `Self`.
>      /// - Caller must ensure that the pointee of `this` lives until the timer
>      ///   fires or is canceled.
> -    unsafe fn start(this: *const Self, expires: Ktime) {
> +    unsafe fn start(this: *const Self, expires: <Self::TimerMode as HrTimerMode>::Expires) {
>          // SAFETY: By function safety requirement, `this` is a valid `Self`.
>          unsafe {
>              bindings::hrtimer_start_range_ns(
>                  Self::c_timer_ptr(this).cast_mut(),
> -                expires.to_ns(),
> +                expires.as_nanos(),
>                  0,
> -                (*Self::raw_get_timer(this)).mode,
> +                <Self::TimerMode as HrTimerMode>::Clock::ID as u32,
>              );
>          }
>      }
> @@ -579,12 +604,16 @@ macro_rules! impl_has_hr_timer {
>          impl$({$($generics:tt)*})?
>              HasHrTimer<$timer_type:ty>
>              for $self:ty
> -        { self.$field:ident }
> +        {
> +            mode = $mode:ty,
> +            self.$field:ident

How about:

  mode = $mode:ty,
  field = self.$field:ident

So that there is some sort of red line when calling this. We could also
consider adopting another syntax for association:

  mode: $mode:ty,
  field: self.$field:ident

or something else like `<-` or `->` ?


Best regards,
Andreas Hindborg

Re: [PATCH v1 4/5] rust: time: Make HasHrTimer generic over HrTimerMode

[FUJITA Tomonori]

On Mon, 02 Jun 2025 14:41:10 +0200
Andreas Hindborg <a.hindborg@kernel.org> wrote:

>> @@ -579,12 +604,16 @@ macro_rules! impl_has_hr_timer {
>>          impl$({$($generics:tt)*})?
>>              HasHrTimer<$timer_type:ty>
>>              for $self:ty
>> -        { self.$field:ident }
>> +        {
>> +            mode = $mode:ty,
>> +            self.$field:ident
> 
> How about:
> 
>   mode = $mode:ty,
>   field = self.$field:ident

Works fo me.

> So that there is some sort of red line when calling this. We could also
> consider adopting another syntax for association:
> 
>   mode: $mode:ty,
>   field: self.$field:ident

Looks fine too.

> or something else like `<-` or `->` ?

I personally prefer one of the two options above, but I'm also ok with
`<-` or `->`.

Re: [PATCH v1 4/5] rust: time: Make HasHrTimer generic over HrTimerMode

[Andreas Hindborg]

"FUJITA Tomonori" <fujita.tomonori@gmail.com> writes:

> On Mon, 02 Jun 2025 14:41:10 +0200
> Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
>>> @@ -579,12 +604,16 @@ macro_rules! impl_has_hr_timer {
>>>          impl$({$($generics:tt)*})?
>>>              HasHrTimer<$timer_type:ty>
>>>              for $self:ty
>>> -        { self.$field:ident }
>>> +        {
>>> +            mode = $mode:ty,
>>> +            self.$field:ident
>>
>> How about:
>>
>>   mode = $mode:ty,
>>   field = self.$field:ident
>
> Works fo me.
>
>> So that there is some sort of red line when calling this. We could also
>> consider adopting another syntax for association:
>>
>>   mode: $mode:ty,
>>   field: self.$field:ident
>
> Looks fine too.
>
>> or something else like `<-` or `->` ?
>
> I personally prefer one of the two options above, but I'm also ok with
> `<-` or `->`.

OK, let's go with struct initializer syntax then (:). We can always
change it later if someone has a different opinion.


Best regards,
Andreas Hindborg

[PATCH v1 5/5] rust: time: Remove Ktime in hrtimer

[FUJITA Tomonori]

Remove the use of `Ktime` from the hrtimer code, which was originally
introduced as a temporary workaround. The hrtimer has now been fully
converted to use the `Instant` and `Delta` types instead.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 rust/kernel/time/hrtimer.rs | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
index 3355ae6fe76d..e396c27e0e1a 100644
--- a/rust/kernel/time/hrtimer.rs
+++ b/rust/kernel/time/hrtimer.rs
@@ -72,22 +72,6 @@
 use core::marker::PhantomData;
 use pin_init::PinInit;
 
-/// A Rust wrapper around a `ktime_t`.
-// NOTE: Ktime is going to be removed when hrtimer is converted to Instant/Delta.
-#[repr(transparent)]
-#[derive(Copy, Clone, PartialEq, PartialOrd, Eq, Ord)]
-pub struct Ktime {
-    inner: bindings::ktime_t,
-}
-
-impl Ktime {
-    /// Returns the number of nanoseconds.
-    #[inline]
-    pub fn to_ns(self) -> i64 {
-        self.inner
-    }
-}
-
 /// A timer backed by a C `struct hrtimer`.
 ///
 /// # Invariants
-- 
2.43.0

Re: [PATCH v1 5/5] rust: time: Remove Ktime in hrtimer

[Andreas Hindborg]

"FUJITA Tomonori" <fujita.tomonori@gmail.com> writes:

> Remove the use of `Ktime` from the hrtimer code, which was originally
> introduced as a temporary workaround. The hrtimer has now been fully
> converted to use the `Instant` and `Delta` types instead.
>
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>

Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>


Best regards,
Andreas Hindborg