Introduce a type representing a specific point in time. We could use
the Ktime type but C's ktime_t is used for both timestamp and
timedelta. To avoid confusion, introduce a new Instant type for
timestamp.

Rename Ktime to Instant and modify their methods for timestamp.

Implement the subtraction operator for Instant:

Delta = Instant A - Instant B

Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
Reviewed-by: Gary Guo <gary@garyguo.net>
Reviewed-by: Fiona Behrens <me@kloenk.dev>
Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 rust/kernel/time.rs                 | 74 ++++++++++++++++-------------
 rust/kernel/time/hrtimer.rs         | 14 +++---
 rust/kernel/time/hrtimer/arc.rs     |  4 +-
 rust/kernel/time/hrtimer/pin.rs     |  4 +-
 rust/kernel/time/hrtimer/pin_mut.rs |  4 +-
 rust/kernel/time/hrtimer/tbox.rs    |  4 +-
 6 files changed, 55 insertions(+), 49 deletions(-)

diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
index e00b9a853e6a..bc5082c01152 100644
--- a/rust/kernel/time.rs
+++ b/rust/kernel/time.rs
@@ -5,6 +5,22 @@
 //! This module contains the kernel APIs related to time and timers that
 //! have been ported or wrapped for usage by Rust code in the kernel.
 //!
+//! There are two types in this module:
+//!
+//! - The [`Instant`] type represents a specific point in time.
+//! - The [`Delta`] type represents a span of time.
+//!
+//! Note that the C side uses `ktime_t` type to represent both. However, timestamp
+//! and timedelta are different. To avoid confusion, we use two different types.
+//!
+//! A [`Instant`] object can be created by calling the [`Instant::now()`] function.
+//! It represents a point in time at which the object was created.
+//! By calling the [`Instant::elapsed()`] method, a [`Delta`] object representing
+//! the elapsed time can be created. The [`Delta`] object can also be created
+//! by subtracting two [`Instant`] objects.
+//!
+//! A [`Delta`] type supports methods to retrieve the duration in various units.
+//!
 //! C header: [`include/linux/jiffies.h`](srctree/include/linux/jiffies.h).
 //! C header: [`include/linux/ktime.h`](srctree/include/linux/ktime.h).
 
@@ -33,59 +49,49 @@ pub fn msecs_to_jiffies(msecs: Msecs) -> Jiffies {
     unsafe { bindings::__msecs_to_jiffies(msecs) }
 }
 
-/// A Rust wrapper around a `ktime_t`.
+/// A specific point in time.
+///
+/// # Invariants
+///
+/// The `inner` value is in the range from 0 to `KTIME_MAX`.
 #[repr(transparent)]
 #[derive(Copy, Clone, PartialEq, PartialOrd, Eq, Ord)]
-pub struct Ktime {
+pub struct Instant {
     inner: bindings::ktime_t,
 }
 
-impl Ktime {
-    /// Create a `Ktime` from a raw `ktime_t`.
-    #[inline]
-    pub fn from_raw(inner: bindings::ktime_t) -> Self {
-        Self { inner }
-    }
-
+impl Instant {
     /// Get the current time using `CLOCK_MONOTONIC`.
     #[inline]
-    pub fn ktime_get() -> Self {
-        // SAFETY: It is always safe to call `ktime_get` outside of NMI context.
-        Self::from_raw(unsafe { bindings::ktime_get() })
+    pub fn now() -> Self {
+        // INVARIANT: The `ktime_get()` function returns a value in the range
+        // from 0 to `KTIME_MAX`.
+        Self {
+            // SAFETY: It is always safe to call `ktime_get()` outside of NMI context.
+            inner: unsafe { bindings::ktime_get() },
+        }
     }
 
-    /// Divide the number of nanoseconds by a compile-time constant.
+    /// Return the amount of time elapsed since the [`Instant`].
     #[inline]
-    fn divns_constant<const DIV: i64>(self) -> i64 {
-        self.to_ns() / DIV
+    pub fn elapsed(&self) -> Delta {
+        Self::now() - *self
     }
 
-    /// Returns the number of nanoseconds.
     #[inline]
-    pub fn to_ns(self) -> i64 {
+    pub(crate) fn as_nanos(self) -> i64 {
         self.inner
     }
-
-    /// Returns the number of milliseconds.
-    #[inline]
-    pub fn to_ms(self) -> i64 {
-        self.divns_constant::<NSEC_PER_MSEC>()
-    }
 }
 
-/// Returns the number of milliseconds between two ktimes.
-#[inline]
-pub fn ktime_ms_delta(later: Ktime, earlier: Ktime) -> i64 {
-    (later - earlier).to_ms()
-}
-
-impl core::ops::Sub for Ktime {
-    type Output = Ktime;
+impl core::ops::Sub for Instant {
+    type Output = Delta;
 
+    // By the type invariant, it never overflows.
     #[inline]
-    fn sub(self, other: Ktime) -> Ktime {
-        Self {
-            inner: self.inner - other.inner,
+    fn sub(self, other: Instant) -> Delta {
+        Delta {
+            nanos: self.inner - other.inner,
         }
     }
 }
diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
index ce53f8579d18..27243eaaf8ed 100644
--- a/rust/kernel/time/hrtimer.rs
+++ b/rust/kernel/time/hrtimer.rs
@@ -68,7 +68,7 @@
 //! `start` operation.
 
 use super::ClockId;
-use crate::{prelude::*, time::Ktime, types::Opaque};
+use crate::{prelude::*, time::Instant, types::Opaque};
 use core::marker::PhantomData;
 use pin_init::PinInit;
 
@@ -189,7 +189,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: Instant) -> Self::TimerHandle;
 }
 
 /// Unsafe version of [`HrTimerPointer`] for situations where leaking the
@@ -220,7 +220,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: Instant) -> Self::TimerHandle;
 }
 
 /// A trait for stack allocated timers.
@@ -232,7 +232,7 @@ pub unsafe trait UnsafeHrTimerPointer: Sync + Sized {
 pub unsafe trait ScopedHrTimerPointer {
     /// 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: Instant, f: F) -> T
     where
         F: FnOnce() -> T;
 }
@@ -244,7 +244,7 @@ unsafe impl<T> ScopedHrTimerPointer for T
 where
     T: UnsafeHrTimerPointer,
 {
-    fn start_scoped<U, F>(self, expires: Ktime, f: F) -> U
+    fn start_scoped<U, F>(self, expires: Instant, f: F) -> U
     where
         F: FnOnce() -> U,
     {
@@ -366,12 +366,12 @@ 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: Instant) {
         // 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.into_c(),
             );
diff --git a/rust/kernel/time/hrtimer/arc.rs b/rust/kernel/time/hrtimer/arc.rs
index 4a984d85b4a1..acc70a0ea1be 100644
--- a/rust/kernel/time/hrtimer/arc.rs
+++ b/rust/kernel/time/hrtimer/arc.rs
@@ -8,7 +8,7 @@
 use super::RawHrTimerCallback;
 use crate::sync::Arc;
 use crate::sync::ArcBorrow;
-use crate::time::Ktime;
+use crate::time::Instant;
 
 /// A handle for an `Arc<HasHrTimer<T>>` returned by a call to
 /// [`HrTimerPointer::start`].
@@ -56,7 +56,7 @@ impl<T> HrTimerPointer for Arc<T>
 {
     type TimerHandle = ArcHrTimerHandle<T>;
 
-    fn start(self, expires: Ktime) -> ArcHrTimerHandle<T> {
+    fn start(self, expires: Instant) -> 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 f760db265c7b..dba22d11a95f 100644
--- a/rust/kernel/time/hrtimer/pin.rs
+++ b/rust/kernel/time/hrtimer/pin.rs
@@ -6,7 +6,7 @@
 use super::HrTimerHandle;
 use super::RawHrTimerCallback;
 use super::UnsafeHrTimerPointer;
-use crate::time::Ktime;
+use crate::time::Instant;
 use core::pin::Pin;
 
 /// A handle for a `Pin<&HasHrTimer>`. When the handle exists, the timer might be
@@ -56,7 +56,7 @@ unsafe impl<'a, T> UnsafeHrTimerPointer for Pin<&'a T>
 {
     type TimerHandle = PinHrTimerHandle<'a, T>;
 
-    unsafe fn start(self, expires: Ktime) -> Self::TimerHandle {
+    unsafe fn start(self, expires: Instant) -> 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 90c0351d62e4..aeff8e102e1d 100644
--- a/rust/kernel/time/hrtimer/pin_mut.rs
+++ b/rust/kernel/time/hrtimer/pin_mut.rs
@@ -3,7 +3,7 @@
 use super::{
     HasHrTimer, HrTimer, HrTimerCallback, HrTimerHandle, RawHrTimerCallback, UnsafeHrTimerPointer,
 };
-use crate::time::Ktime;
+use crate::time::Instant;
 use core::{marker::PhantomData, pin::Pin, ptr::NonNull};
 
 /// A handle for a `Pin<&mut HasHrTimer>`. When the handle exists, the timer might
@@ -54,7 +54,7 @@ unsafe impl<'a, T> UnsafeHrTimerPointer for Pin<&'a mut T>
 {
     type TimerHandle = PinMutHrTimerHandle<'a, T>;
 
-    unsafe fn start(mut self, expires: Ktime) -> Self::TimerHandle {
+    unsafe fn start(mut self, expires: Instant) -> 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 2071cae07234..3df4e359e9bb 100644
--- a/rust/kernel/time/hrtimer/tbox.rs
+++ b/rust/kernel/time/hrtimer/tbox.rs
@@ -7,7 +7,7 @@
 use super::HrTimerPointer;
 use super::RawHrTimerCallback;
 use crate::prelude::*;
-use crate::time::Ktime;
+use crate::time::Instant;
 use core::ptr::NonNull;
 
 /// A handle for a [`Box<HasHrTimer<T>>`] returned by a call to
@@ -66,7 +66,7 @@ impl<T, A> HrTimerPointer for Pin<Box<T, A>>
 {
     type TimerHandle = BoxHrTimerHandle<T, A>;
 
-    fn start(self, expires: Ktime) -> Self::TimerHandle {
+    fn start(self, expires: Instant) -> 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

On Sun, Apr 13, 2025 at 07:43:08PM +0900, FUJITA Tomonori wrote:
> Introduce a type representing a specific point in time. We could use
> the Ktime type but C's ktime_t is used for both timestamp and
> timedelta. To avoid confusion, introduce a new Instant type for
> timestamp.
> 
> Rename Ktime to Instant and modify their methods for timestamp.
> 
> Implement the subtraction operator for Instant:
> 
> Delta = Instant A - Instant B
> 
> Reviewed-by: Boqun Feng <boqun.feng@gmail.com>

I probably need to drop my Reviewed-by because of something below:

> Reviewed-by: Gary Guo <gary@garyguo.net>
> Reviewed-by: Fiona Behrens <me@kloenk.dev>
> Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
> Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> ---
[...]
> diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
> index ce53f8579d18..27243eaaf8ed 100644
> --- a/rust/kernel/time/hrtimer.rs
> +++ b/rust/kernel/time/hrtimer.rs
> @@ -68,7 +68,7 @@
>  //! `start` operation.
>  
>  use super::ClockId;
> -use crate::{prelude::*, time::Ktime, types::Opaque};
> +use crate::{prelude::*, time::Instant, types::Opaque};
>  use core::marker::PhantomData;
>  use pin_init::PinInit;
>  
> @@ -189,7 +189,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: Instant) -> Self::TimerHandle;

We should be able to use what I suggested:

	https://lore.kernel.org/rust-for-linux/Z_ALZsnwN53ZPBrB@boqun-archlinux/	

to make different timer modes (rel or abs) choose different expire type.

I don't think we can merge this patch as it is, unfortunately, because
it doesn't make sense for a relative timer to take an Instant as expires
value.

Regards,
Boqun

>  }
>  
[...]

"Boqun Feng" <boqun.feng@gmail.com> writes:

> On Sun, Apr 13, 2025 at 07:43:08PM +0900, FUJITA Tomonori wrote:
>> Introduce a type representing a specific point in time. We could use
>> the Ktime type but C's ktime_t is used for both timestamp and
>> timedelta. To avoid confusion, introduce a new Instant type for
>> timestamp.
>>
>> Rename Ktime to Instant and modify their methods for timestamp.
>>
>> Implement the subtraction operator for Instant:
>>
>> Delta = Instant A - Instant B
>>
>> Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
>
> I probably need to drop my Reviewed-by because of something below:
>
>> Reviewed-by: Gary Guo <gary@garyguo.net>
>> Reviewed-by: Fiona Behrens <me@kloenk.dev>
>> Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
>> Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
>> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
>> ---
> [...]
>> diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
>> index ce53f8579d18..27243eaaf8ed 100644
>> --- a/rust/kernel/time/hrtimer.rs
>> +++ b/rust/kernel/time/hrtimer.rs
>> @@ -68,7 +68,7 @@
>>  //! `start` operation.
>>
>>  use super::ClockId;
>> -use crate::{prelude::*, time::Ktime, types::Opaque};
>> +use crate::{prelude::*, time::Instant, types::Opaque};
>>  use core::marker::PhantomData;
>>  use pin_init::PinInit;
>>
>> @@ -189,7 +189,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: Instant) -> Self::TimerHandle;
>
> We should be able to use what I suggested:
>
> 	https://lore.kernel.org/rust-for-linux/Z_ALZsnwN53ZPBrB@boqun-archlinux/
>
> to make different timer modes (rel or abs) choose different expire type.
>
> I don't think we can merge this patch as it is, unfortunately, because
> it doesn't make sense for a relative timer to take an Instant as expires
> value.

I told Tomo he could use `Instant` in this location and either he or I
would fix it up later [1].

I don't want to block the series on this since the new API is not worse
than the old one where Ktime is overloaded for both uses.


Best regards,
Andreas Hindborg



[1] https://lore.kernel.org/all/877c41v7kf.fsf@kernel.org

On Mon, 14 Apr 2025 09:04:14 +0200
Andreas Hindborg <a.hindborg@kernel.org> wrote:

> "Boqun Feng" <boqun.feng@gmail.com> writes:
> 
>> On Sun, Apr 13, 2025 at 07:43:08PM +0900, FUJITA Tomonori wrote:
>>> Introduce a type representing a specific point in time. We could use
>>> the Ktime type but C's ktime_t is used for both timestamp and
>>> timedelta. To avoid confusion, introduce a new Instant type for
>>> timestamp.
>>>
>>> Rename Ktime to Instant and modify their methods for timestamp.
>>>
>>> Implement the subtraction operator for Instant:
>>>
>>> Delta = Instant A - Instant B
>>>
>>> Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
>>
>> I probably need to drop my Reviewed-by because of something below:
>>
>>> Reviewed-by: Gary Guo <gary@garyguo.net>
>>> Reviewed-by: Fiona Behrens <me@kloenk.dev>
>>> Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
>>> Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
>>> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
>>> ---
>> [...]
>>> diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
>>> index ce53f8579d18..27243eaaf8ed 100644
>>> --- a/rust/kernel/time/hrtimer.rs
>>> +++ b/rust/kernel/time/hrtimer.rs
>>> @@ -68,7 +68,7 @@
>>>  //! `start` operation.
>>>
>>>  use super::ClockId;
>>> -use crate::{prelude::*, time::Ktime, types::Opaque};
>>> +use crate::{prelude::*, time::Instant, types::Opaque};
>>>  use core::marker::PhantomData;
>>>  use pin_init::PinInit;
>>>
>>> @@ -189,7 +189,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: Instant) -> Self::TimerHandle;
>>
>> We should be able to use what I suggested:
>>
>> 	https://lore.kernel.org/rust-for-linux/Z_ALZsnwN53ZPBrB@boqun-archlinux/
>>
>> to make different timer modes (rel or abs) choose different expire type.
>>
>> I don't think we can merge this patch as it is, unfortunately, because
>> it doesn't make sense for a relative timer to take an Instant as expires
>> value.
> 
> I told Tomo he could use `Instant` in this location and either he or I
> would fix it up later [1].
> 
> I don't want to block the series on this since the new API is not worse
> than the old one where Ktime is overloaded for both uses.

Here's the fix that I've worked on. As Boqun suggested, I added
`HrTimerExpireMode` trait since `HrTimerMode` is already used. It
compiles, but I'm not sure if it's what everyone had in mind.

Since many parts need to be made generic, I think the changes will be
complicated. Rather than including this in the instant and duration
patchset, I think it would be better to review this change separately.


diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
index 27243eaaf8ed..db3f99662222 100644
--- a/rust/kernel/time/hrtimer.rs
+++ b/rust/kernel/time/hrtimer.rs
@@ -68,10 +68,16 @@
 //! `start` operation.
 
 use super::ClockId;
-use crate::{prelude::*, time::Instant, types::Opaque};
+use crate::{prelude::*, types::Opaque};
 use core::marker::PhantomData;
 use pin_init::PinInit;
 
+pub trait HrTimerExpireMode {
+    type Expires; /* either Delta or Instant */
+
+    fn as_nanos(expires: Self::Expires) -> bindings::ktime_t;
+}
+
 /// A timer backed by a C `struct hrtimer`.
 ///
 /// # Invariants
@@ -176,7 +182,7 @@ pub(crate) unsafe fn raw_cancel(this: *const Self) -> bool {
 /// that more than one [`HrTimerHandle`] associated with a [`HrTimerPointer`] may
 /// exist. A timer can be manipulated through any of the handles, and a handle
 /// may represent a cancelled timer.
-pub trait HrTimerPointer: Sync + Sized {
+pub trait HrTimerPointer<Mode: HrTimerExpireMode>: Sync + Sized {
     /// A handle representing a started or restarted timer.
     ///
     /// If the timer is running or if the timer callback is executing when the
@@ -189,7 +195,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: Instant) -> Self::TimerHandle;
+    fn start(self, expires: Mode::Expires) -> Self::TimerHandle;
 }
 
 /// Unsafe version of [`HrTimerPointer`] for situations where leaking the
@@ -203,7 +209,7 @@ pub trait HrTimerPointer: Sync + Sized {
 /// Implementers of this trait must ensure that instances of types implementing
 /// [`UnsafeHrTimerPointer`] outlives any associated [`HrTimerPointer::TimerHandle`]
 /// instances.
-pub unsafe trait UnsafeHrTimerPointer: Sync + Sized {
+pub unsafe trait UnsafeHrTimerPointer<Mode: HrTimerExpireMode>: Sync + Sized {
     /// A handle representing a running timer.
     ///
     /// # Safety
@@ -220,7 +226,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: Instant) -> Self::TimerHandle;
+    unsafe fn start(self, expires: Mode::Expires) -> Self::TimerHandle;
 }
 
 /// A trait for stack allocated timers.
@@ -229,10 +235,10 @@ 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 {
+pub unsafe trait ScopedHrTimerPointer<Mode: HrTimerExpireMode> {
     /// 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: Instant, f: F) -> T
+    fn start_scoped<T, F>(self, expires: Mode::Expires, f: F) -> T
     where
         F: FnOnce() -> T;
 }
@@ -240,11 +246,12 @@ fn start_scoped<T, F>(self, expires: Instant, f: F) -> T
 // SAFETY: By the safety requirement of [`UnsafeHrTimerPointer`], dropping the
 // handle returned by [`UnsafeHrTimerPointer::start`] ensures that the timer is
 // killed.
-unsafe impl<T> ScopedHrTimerPointer for T
+unsafe impl<T, Mode> ScopedHrTimerPointer<Mode> for T
 where
-    T: UnsafeHrTimerPointer,
+    T: UnsafeHrTimerPointer<Mode>,
+    Mode: HrTimerExpireMode,
 {
-    fn start_scoped<U, F>(self, expires: Instant, f: F) -> U
+    fn start_scoped<U, F>(self, expires: Mode::Expires, f: F) -> U
     where
         F: FnOnce() -> U,
     {
@@ -319,6 +326,7 @@ pub unsafe trait HrTimerHandle {
 /// their documentation. All the methods of this trait must operate on the same
 /// field.
 pub unsafe trait HasHrTimer<T> {
+    type Mode: HrTimerExpireMode;
     /// Return a pointer to the [`HrTimer`] within `Self`.
     ///
     /// This function is useful to get access to the value without creating
@@ -366,12 +374,15 @@ 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: Instant) {
+    unsafe fn start(
+        this: *const Self,
+        expires: <<Self as HasHrTimer<T>>::Mode as HrTimerExpireMode>::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.as_nanos(),
+                Self::Mode::as_nanos(expires),
                 0,
                 (*Self::raw_get_timer(this)).mode.into_c(),
             );
diff --git a/rust/kernel/time/hrtimer/arc.rs b/rust/kernel/time/hrtimer/arc.rs
index acc70a0ea1be..90cf0edf4509 100644
--- a/rust/kernel/time/hrtimer/arc.rs
+++ b/rust/kernel/time/hrtimer/arc.rs
@@ -3,12 +3,12 @@
 use super::HasHrTimer;
 use super::HrTimer;
 use super::HrTimerCallback;
+use super::HrTimerExpireMode;
 use super::HrTimerHandle;
 use super::HrTimerPointer;
 use super::RawHrTimerCallback;
 use crate::sync::Arc;
 use crate::sync::ArcBorrow;
-use crate::time::Instant;
 
 /// A handle for an `Arc<HasHrTimer<T>>` returned by a call to
 /// [`HrTimerPointer::start`].
@@ -47,7 +47,7 @@ fn drop(&mut self) {
     }
 }
 
-impl<T> HrTimerPointer for Arc<T>
+impl<T> HrTimerPointer<<T as HasHrTimer<T>>::Mode> for Arc<T>
 where
     T: 'static,
     T: Send + Sync,
@@ -56,7 +56,10 @@ impl<T> HrTimerPointer for Arc<T>
 {
     type TimerHandle = ArcHrTimerHandle<T>;
 
-    fn start(self, expires: Instant) -> ArcHrTimerHandle<T> {
+    fn start(
+        self,
+        expires: <<T as HasHrTimer<T>>::Mode as HrTimerExpireMode>::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 dba22d11a95f..5b79cbcaca3f 100644
--- a/rust/kernel/time/hrtimer/pin.rs
+++ b/rust/kernel/time/hrtimer/pin.rs
@@ -3,6 +3,7 @@
 use super::HasHrTimer;
 use super::HrTimer;
 use super::HrTimerCallback;
+use super::HrTimerExpireMode;
 use super::HrTimerHandle;
 use super::RawHrTimerCallback;
 use super::UnsafeHrTimerPointer;
@@ -48,7 +49,7 @@ fn drop(&mut self) {
 
 // SAFETY: We capture the lifetime of `Self` when we create a `PinHrTimerHandle`,
 // so `Self` will outlive the handle.
-unsafe impl<'a, T> UnsafeHrTimerPointer for Pin<&'a T>
+unsafe impl<'a, T> UnsafeHrTimerPointer<<T as HasHrTimer<T>>::Mode> for Pin<&'a T>
 where
     T: Send + Sync,
     T: HasHrTimer<T>,
@@ -56,7 +57,10 @@ unsafe impl<'a, T> UnsafeHrTimerPointer for Pin<&'a T>
 {
     type TimerHandle = PinHrTimerHandle<'a, T>;
 
-    unsafe fn start(self, expires: Instant) -> Self::TimerHandle {
+    unsafe fn start(
+        self,
+        expires: <<T as HasHrTimer<T>>::Mode as HrTimerExpireMode>::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 aeff8e102e1d..82d7ecdbbfb6 100644
--- a/rust/kernel/time/hrtimer/pin_mut.rs
+++ b/rust/kernel/time/hrtimer/pin_mut.rs
@@ -1,7 +1,8 @@
 // SPDX-License-Identifier: GPL-2.0
 
 use super::{
-    HasHrTimer, HrTimer, HrTimerCallback, HrTimerHandle, RawHrTimerCallback, UnsafeHrTimerPointer,
+    HasHrTimer, HrTimer, HrTimerCallback, HrTimerExpireMode, HrTimerHandle, RawHrTimerCallback,
+    UnsafeHrTimerPointer,
 };
 use crate::time::Instant;
 use core::{marker::PhantomData, pin::Pin, ptr::NonNull};
@@ -46,7 +47,7 @@ fn drop(&mut self) {
 
 // SAFETY: We capture the lifetime of `Self` when we create a
 // `PinMutHrTimerHandle`, so `Self` will outlive the handle.
-unsafe impl<'a, T> UnsafeHrTimerPointer for Pin<&'a mut T>
+unsafe impl<'a, T> UnsafeHrTimerPointer<<T as HasHrTimer<T>>::Mode> for Pin<&'a mut T>
 where
     T: Send + Sync,
     T: HasHrTimer<T>,
@@ -54,7 +55,10 @@ unsafe impl<'a, T> UnsafeHrTimerPointer for Pin<&'a mut T>
 {
     type TimerHandle = PinMutHrTimerHandle<'a, T>;
 
-    unsafe fn start(mut self, expires: Instant) -> Self::TimerHandle {
+    unsafe fn start(
+        mut self,
+        expires: <<T as HasHrTimer<T>>::Mode as HrTimerExpireMode>::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 3df4e359e9bb..21aa0aa61cc8 100644
--- a/rust/kernel/time/hrtimer/tbox.rs
+++ b/rust/kernel/time/hrtimer/tbox.rs
@@ -3,6 +3,7 @@
 use super::HasHrTimer;
 use super::HrTimer;
 use super::HrTimerCallback;
+use super::HrTimerExpireMode;
 use super::HrTimerHandle;
 use super::HrTimerPointer;
 use super::RawHrTimerCallback;
@@ -56,7 +57,7 @@ fn drop(&mut self) {
     }
 }
 
-impl<T, A> HrTimerPointer for Pin<Box<T, A>>
+impl<T, A> HrTimerPointer<<T as HasHrTimer<T>>::Mode> for Pin<Box<T, A>>
 where
     T: 'static,
     T: Send + Sync,
@@ -66,7 +67,10 @@ impl<T, A> HrTimerPointer for Pin<Box<T, A>>
 {
     type TimerHandle = BoxHrTimerHandle<T, A>;
 
-    fn start(self, expires: Instant) -> Self::TimerHandle {
+    fn start(
+        self,
+        expires: <<T as HasHrTimer<T>>::Mode as HrTimerExpireMode>::Expires,
+    ) -> Self::TimerHandle {
         // SAFETY:
         //  - We will not move out of this box during timer callback (we pass an
         //    immutable reference to the callback).

On Mon, Apr 14, 2025 at 08:59:54PM +0900, FUJITA Tomonori wrote:
> On Mon, 14 Apr 2025 09:04:14 +0200
> Andreas Hindborg <a.hindborg@kernel.org> wrote:
> 
> > "Boqun Feng" <boqun.feng@gmail.com> writes:
> > 
> >> On Sun, Apr 13, 2025 at 07:43:08PM +0900, FUJITA Tomonori wrote:
> >>> Introduce a type representing a specific point in time. We could use
> >>> the Ktime type but C's ktime_t is used for both timestamp and
> >>> timedelta. To avoid confusion, introduce a new Instant type for
> >>> timestamp.
> >>>
> >>> Rename Ktime to Instant and modify their methods for timestamp.
> >>>
> >>> Implement the subtraction operator for Instant:
> >>>
> >>> Delta = Instant A - Instant B
> >>>
> >>> Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
> >>
> >> I probably need to drop my Reviewed-by because of something below:
> >>
> >>> Reviewed-by: Gary Guo <gary@garyguo.net>
> >>> Reviewed-by: Fiona Behrens <me@kloenk.dev>
> >>> Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
> >>> Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
> >>> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> >>> ---
> >> [...]
> >>> diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
> >>> index ce53f8579d18..27243eaaf8ed 100644
> >>> --- a/rust/kernel/time/hrtimer.rs
> >>> +++ b/rust/kernel/time/hrtimer.rs
> >>> @@ -68,7 +68,7 @@
> >>>  //! `start` operation.
> >>>
> >>>  use super::ClockId;
> >>> -use crate::{prelude::*, time::Ktime, types::Opaque};
> >>> +use crate::{prelude::*, time::Instant, types::Opaque};
> >>>  use core::marker::PhantomData;
> >>>  use pin_init::PinInit;
> >>>
> >>> @@ -189,7 +189,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: Instant) -> Self::TimerHandle;
> >>
> >> We should be able to use what I suggested:
> >>
> >> 	https://lore.kernel.org/rust-for-linux/Z_ALZsnwN53ZPBrB@boqun-archlinux/
> >>
> >> to make different timer modes (rel or abs) choose different expire type.
> >>
> >> I don't think we can merge this patch as it is, unfortunately, because
> >> it doesn't make sense for a relative timer to take an Instant as expires
> >> value.
> > 
> > I told Tomo he could use `Instant` in this location and either he or I
> > would fix it up later [1].
> > 

I saw that, however, I don't think we can put `Instant` as the parameter
for HrTimerPointer::start() because we don't yet know how long would the
fix-it-up-later take. And it would confuse users if they need a put an
Instant for relative time.

> > I don't want to block the series on this since the new API is not worse
> > than the old one where Ktime is overloaded for both uses.

How about we keep Ktime? That is HrTimerPointer::start() still uses
Ktime, until we totally finish the refactoring as Tomo show below?
`Ktime` is much better here because it at least matches C API behavior,
we can remove `Ktime` once the dust is settled. Thoughts?

Regards,
Boqun

> 
> Here's the fix that I've worked on. As Boqun suggested, I added
> `HrTimerExpireMode` trait since `HrTimerMode` is already used. It
> compiles, but I'm not sure if it's what everyone had in mind.
> 
> Since many parts need to be made generic, I think the changes will be
> complicated. Rather than including this in the instant and duration
> patchset, I think it would be better to review this change separately.
> 
> 
> diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
> index 27243eaaf8ed..db3f99662222 100644
> --- a/rust/kernel/time/hrtimer.rs
> +++ b/rust/kernel/time/hrtimer.rs
> @@ -68,10 +68,16 @@
>  //! `start` operation.
>  
>  use super::ClockId;
> -use crate::{prelude::*, time::Instant, types::Opaque};
> +use crate::{prelude::*, types::Opaque};
>  use core::marker::PhantomData;
>  use pin_init::PinInit;
>  
> +pub trait HrTimerExpireMode {
> +    type Expires; /* either Delta or Instant */
> +
> +    fn as_nanos(expires: Self::Expires) -> bindings::ktime_t;
> +}
> +
>  /// A timer backed by a C `struct hrtimer`.
>  ///
>  /// # Invariants
> @@ -176,7 +182,7 @@ pub(crate) unsafe fn raw_cancel(this: *const Self) -> bool {
>  /// that more than one [`HrTimerHandle`] associated with a [`HrTimerPointer`] may
>  /// exist. A timer can be manipulated through any of the handles, and a handle
>  /// may represent a cancelled timer.
> -pub trait HrTimerPointer: Sync + Sized {
> +pub trait HrTimerPointer<Mode: HrTimerExpireMode>: Sync + Sized {
>      /// A handle representing a started or restarted timer.
>      ///
>      /// If the timer is running or if the timer callback is executing when the
> @@ -189,7 +195,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: Instant) -> Self::TimerHandle;
> +    fn start(self, expires: Mode::Expires) -> Self::TimerHandle;
>  }
>  
>  /// Unsafe version of [`HrTimerPointer`] for situations where leaking the
> @@ -203,7 +209,7 @@ pub trait HrTimerPointer: Sync + Sized {
>  /// Implementers of this trait must ensure that instances of types implementing
>  /// [`UnsafeHrTimerPointer`] outlives any associated [`HrTimerPointer::TimerHandle`]
>  /// instances.
> -pub unsafe trait UnsafeHrTimerPointer: Sync + Sized {
> +pub unsafe trait UnsafeHrTimerPointer<Mode: HrTimerExpireMode>: Sync + Sized {
>      /// A handle representing a running timer.
>      ///
>      /// # Safety
> @@ -220,7 +226,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: Instant) -> Self::TimerHandle;
> +    unsafe fn start(self, expires: Mode::Expires) -> Self::TimerHandle;
>  }
>  
>  /// A trait for stack allocated timers.
> @@ -229,10 +235,10 @@ 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 {
> +pub unsafe trait ScopedHrTimerPointer<Mode: HrTimerExpireMode> {
>      /// 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: Instant, f: F) -> T
> +    fn start_scoped<T, F>(self, expires: Mode::Expires, f: F) -> T
>      where
>          F: FnOnce() -> T;
>  }
> @@ -240,11 +246,12 @@ fn start_scoped<T, F>(self, expires: Instant, f: F) -> T
>  // SAFETY: By the safety requirement of [`UnsafeHrTimerPointer`], dropping the
>  // handle returned by [`UnsafeHrTimerPointer::start`] ensures that the timer is
>  // killed.
> -unsafe impl<T> ScopedHrTimerPointer for T
> +unsafe impl<T, Mode> ScopedHrTimerPointer<Mode> for T
>  where
> -    T: UnsafeHrTimerPointer,
> +    T: UnsafeHrTimerPointer<Mode>,
> +    Mode: HrTimerExpireMode,
>  {
> -    fn start_scoped<U, F>(self, expires: Instant, f: F) -> U
> +    fn start_scoped<U, F>(self, expires: Mode::Expires, f: F) -> U
>      where
>          F: FnOnce() -> U,
>      {
> @@ -319,6 +326,7 @@ pub unsafe trait HrTimerHandle {
>  /// their documentation. All the methods of this trait must operate on the same
>  /// field.
>  pub unsafe trait HasHrTimer<T> {
> +    type Mode: HrTimerExpireMode;
>      /// Return a pointer to the [`HrTimer`] within `Self`.
>      ///
>      /// This function is useful to get access to the value without creating
> @@ -366,12 +374,15 @@ 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: Instant) {
> +    unsafe fn start(
> +        this: *const Self,
> +        expires: <<Self as HasHrTimer<T>>::Mode as HrTimerExpireMode>::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.as_nanos(),
> +                Self::Mode::as_nanos(expires),
>                  0,
>                  (*Self::raw_get_timer(this)).mode.into_c(),
>              );
> diff --git a/rust/kernel/time/hrtimer/arc.rs b/rust/kernel/time/hrtimer/arc.rs
> index acc70a0ea1be..90cf0edf4509 100644
> --- a/rust/kernel/time/hrtimer/arc.rs
> +++ b/rust/kernel/time/hrtimer/arc.rs
> @@ -3,12 +3,12 @@
>  use super::HasHrTimer;
>  use super::HrTimer;
>  use super::HrTimerCallback;
> +use super::HrTimerExpireMode;
>  use super::HrTimerHandle;
>  use super::HrTimerPointer;
>  use super::RawHrTimerCallback;
>  use crate::sync::Arc;
>  use crate::sync::ArcBorrow;
> -use crate::time::Instant;
>  
>  /// A handle for an `Arc<HasHrTimer<T>>` returned by a call to
>  /// [`HrTimerPointer::start`].
> @@ -47,7 +47,7 @@ fn drop(&mut self) {
>      }
>  }
>  
> -impl<T> HrTimerPointer for Arc<T>
> +impl<T> HrTimerPointer<<T as HasHrTimer<T>>::Mode> for Arc<T>
>  where
>      T: 'static,
>      T: Send + Sync,
> @@ -56,7 +56,10 @@ impl<T> HrTimerPointer for Arc<T>
>  {
>      type TimerHandle = ArcHrTimerHandle<T>;
>  
> -    fn start(self, expires: Instant) -> ArcHrTimerHandle<T> {
> +    fn start(
> +        self,
> +        expires: <<T as HasHrTimer<T>>::Mode as HrTimerExpireMode>::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 dba22d11a95f..5b79cbcaca3f 100644
> --- a/rust/kernel/time/hrtimer/pin.rs
> +++ b/rust/kernel/time/hrtimer/pin.rs
> @@ -3,6 +3,7 @@
>  use super::HasHrTimer;
>  use super::HrTimer;
>  use super::HrTimerCallback;
> +use super::HrTimerExpireMode;
>  use super::HrTimerHandle;
>  use super::RawHrTimerCallback;
>  use super::UnsafeHrTimerPointer;
> @@ -48,7 +49,7 @@ fn drop(&mut self) {
>  
>  // SAFETY: We capture the lifetime of `Self` when we create a `PinHrTimerHandle`,
>  // so `Self` will outlive the handle.
> -unsafe impl<'a, T> UnsafeHrTimerPointer for Pin<&'a T>
> +unsafe impl<'a, T> UnsafeHrTimerPointer<<T as HasHrTimer<T>>::Mode> for Pin<&'a T>
>  where
>      T: Send + Sync,
>      T: HasHrTimer<T>,
> @@ -56,7 +57,10 @@ unsafe impl<'a, T> UnsafeHrTimerPointer for Pin<&'a T>
>  {
>      type TimerHandle = PinHrTimerHandle<'a, T>;
>  
> -    unsafe fn start(self, expires: Instant) -> Self::TimerHandle {
> +    unsafe fn start(
> +        self,
> +        expires: <<T as HasHrTimer<T>>::Mode as HrTimerExpireMode>::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 aeff8e102e1d..82d7ecdbbfb6 100644
> --- a/rust/kernel/time/hrtimer/pin_mut.rs
> +++ b/rust/kernel/time/hrtimer/pin_mut.rs
> @@ -1,7 +1,8 @@
>  // SPDX-License-Identifier: GPL-2.0
>  
>  use super::{
> -    HasHrTimer, HrTimer, HrTimerCallback, HrTimerHandle, RawHrTimerCallback, UnsafeHrTimerPointer,
> +    HasHrTimer, HrTimer, HrTimerCallback, HrTimerExpireMode, HrTimerHandle, RawHrTimerCallback,
> +    UnsafeHrTimerPointer,
>  };
>  use crate::time::Instant;
>  use core::{marker::PhantomData, pin::Pin, ptr::NonNull};
> @@ -46,7 +47,7 @@ fn drop(&mut self) {
>  
>  // SAFETY: We capture the lifetime of `Self` when we create a
>  // `PinMutHrTimerHandle`, so `Self` will outlive the handle.
> -unsafe impl<'a, T> UnsafeHrTimerPointer for Pin<&'a mut T>
> +unsafe impl<'a, T> UnsafeHrTimerPointer<<T as HasHrTimer<T>>::Mode> for Pin<&'a mut T>
>  where
>      T: Send + Sync,
>      T: HasHrTimer<T>,
> @@ -54,7 +55,10 @@ unsafe impl<'a, T> UnsafeHrTimerPointer for Pin<&'a mut T>
>  {
>      type TimerHandle = PinMutHrTimerHandle<'a, T>;
>  
> -    unsafe fn start(mut self, expires: Instant) -> Self::TimerHandle {
> +    unsafe fn start(
> +        mut self,
> +        expires: <<T as HasHrTimer<T>>::Mode as HrTimerExpireMode>::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 3df4e359e9bb..21aa0aa61cc8 100644
> --- a/rust/kernel/time/hrtimer/tbox.rs
> +++ b/rust/kernel/time/hrtimer/tbox.rs
> @@ -3,6 +3,7 @@
>  use super::HasHrTimer;
>  use super::HrTimer;
>  use super::HrTimerCallback;
> +use super::HrTimerExpireMode;
>  use super::HrTimerHandle;
>  use super::HrTimerPointer;
>  use super::RawHrTimerCallback;
> @@ -56,7 +57,7 @@ fn drop(&mut self) {
>      }
>  }
>  
> -impl<T, A> HrTimerPointer for Pin<Box<T, A>>
> +impl<T, A> HrTimerPointer<<T as HasHrTimer<T>>::Mode> for Pin<Box<T, A>>
>  where
>      T: 'static,
>      T: Send + Sync,
> @@ -66,7 +67,10 @@ impl<T, A> HrTimerPointer for Pin<Box<T, A>>
>  {
>      type TimerHandle = BoxHrTimerHandle<T, A>;
>  
> -    fn start(self, expires: Instant) -> Self::TimerHandle {
> +    fn start(
> +        self,
> +        expires: <<T as HasHrTimer<T>>::Mode as HrTimerExpireMode>::Expires,
> +    ) -> Self::TimerHandle {
>          // SAFETY:
>          //  - We will not move out of this box during timer callback (we pass an
>          //    immutable reference to the callback).
>

On Tue, 15 Apr 2025 11:01:30 -0700
Boqun Feng <boqun.feng@gmail.com> wrote:

> On Mon, Apr 14, 2025 at 08:59:54PM +0900, FUJITA Tomonori wrote:
>> On Mon, 14 Apr 2025 09:04:14 +0200
>> Andreas Hindborg <a.hindborg@kernel.org> wrote:
>> 
>> > "Boqun Feng" <boqun.feng@gmail.com> writes:
>> > 
>> >> On Sun, Apr 13, 2025 at 07:43:08PM +0900, FUJITA Tomonori wrote:
>> >>> Introduce a type representing a specific point in time. We could use
>> >>> the Ktime type but C's ktime_t is used for both timestamp and
>> >>> timedelta. To avoid confusion, introduce a new Instant type for
>> >>> timestamp.
>> >>>
>> >>> Rename Ktime to Instant and modify their methods for timestamp.
>> >>>
>> >>> Implement the subtraction operator for Instant:
>> >>>
>> >>> Delta = Instant A - Instant B
>> >>>
>> >>> Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
>> >>
>> >> I probably need to drop my Reviewed-by because of something below:
>> >>
>> >>> Reviewed-by: Gary Guo <gary@garyguo.net>
>> >>> Reviewed-by: Fiona Behrens <me@kloenk.dev>
>> >>> Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
>> >>> Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
>> >>> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
>> >>> ---
>> >> [...]
>> >>> diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
>> >>> index ce53f8579d18..27243eaaf8ed 100644
>> >>> --- a/rust/kernel/time/hrtimer.rs
>> >>> +++ b/rust/kernel/time/hrtimer.rs
>> >>> @@ -68,7 +68,7 @@
>> >>>  //! `start` operation.
>> >>>
>> >>>  use super::ClockId;
>> >>> -use crate::{prelude::*, time::Ktime, types::Opaque};
>> >>> +use crate::{prelude::*, time::Instant, types::Opaque};
>> >>>  use core::marker::PhantomData;
>> >>>  use pin_init::PinInit;
>> >>>
>> >>> @@ -189,7 +189,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: Instant) -> Self::TimerHandle;
>> >>
>> >> We should be able to use what I suggested:
>> >>
>> >> 	https://lore.kernel.org/rust-for-linux/Z_ALZsnwN53ZPBrB@boqun-archlinux/
>> >>
>> >> to make different timer modes (rel or abs) choose different expire type.
>> >>
>> >> I don't think we can merge this patch as it is, unfortunately, because
>> >> it doesn't make sense for a relative timer to take an Instant as expires
>> >> value.
>> > 
>> > I told Tomo he could use `Instant` in this location and either he or I
>> > would fix it up later [1].
>> > 
> 
> I saw that, however, I don't think we can put `Instant` as the parameter
> for HrTimerPointer::start() because we don't yet know how long would the
> fix-it-up-later take. And it would confuse users if they need a put an
> Instant for relative time.
> 
>> > I don't want to block the series on this since the new API is not worse
>> > than the old one where Ktime is overloaded for both uses.
> 
> How about we keep Ktime? That is HrTimerPointer::start() still uses
> Ktime, until we totally finish the refactoring as Tomo show below?
> `Ktime` is much better here because it at least matches C API behavior,
> we can remove `Ktime` once the dust is settled. Thoughts?

Either is fine with me. I'll leave it to Andreas' judgment.

Andreas, if you like Boqun's approach, I'll replace the third patch
with the following one and send v14.

I added Ktime struct to hrtimer.rs so the well-reviewed changes to
time.rs remain unchanged.

---
rust: time: Introduce Instant type

Introduce a type representing a specific point in time. We could use
the Ktime type but C's ktime_t is used for both timestamp and
timedelta. To avoid confusion, introduce a new Instant type for
timestamp.

Rename Ktime to Instant and modify their methods for timestamp.

Implement the subtraction operator for Instant:

Delta = Instant A - Instant B

Note that hrtimer still uses the Ktime type for now. It will be
replaced with Instant and Delta types later.

Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
Reviewed-by: Gary Guo <gary@garyguo.net>
Reviewed-by: Fiona Behrens <me@kloenk.dev>
Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 rust/kernel/time.rs                 | 77 +++++++++++++++--------------
 rust/kernel/time/hrtimer.rs         | 17 ++++++-
 rust/kernel/time/hrtimer/arc.rs     |  2 +-
 rust/kernel/time/hrtimer/pin.rs     |  2 +-
 rust/kernel/time/hrtimer/pin_mut.rs |  4 +-
 rust/kernel/time/hrtimer/tbox.rs    |  2 +-
 6 files changed, 60 insertions(+), 44 deletions(-)

diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
index e00b9a853e6a..a8089a98da9e 100644
--- a/rust/kernel/time.rs
+++ b/rust/kernel/time.rs
@@ -5,6 +5,22 @@
 //! This module contains the kernel APIs related to time and timers that
 //! have been ported or wrapped for usage by Rust code in the kernel.
 //!
+//! There are two types in this module:
+//!
+//! - The [`Instant`] type represents a specific point in time.
+//! - The [`Delta`] type represents a span of time.
+//!
+//! Note that the C side uses `ktime_t` type to represent both. However, timestamp
+//! and timedelta are different. To avoid confusion, we use two different types.
+//!
+//! A [`Instant`] object can be created by calling the [`Instant::now()`] function.
+//! It represents a point in time at which the object was created.
+//! By calling the [`Instant::elapsed()`] method, a [`Delta`] object representing
+//! the elapsed time can be created. The [`Delta`] object can also be created
+//! by subtracting two [`Instant`] objects.
+//!
+//! A [`Delta`] type supports methods to retrieve the duration in various units.
+//!
 //! C header: [`include/linux/jiffies.h`](srctree/include/linux/jiffies.h).
 //! C header: [`include/linux/ktime.h`](srctree/include/linux/ktime.h).
 
@@ -33,59 +49,44 @@ pub fn msecs_to_jiffies(msecs: Msecs) -> Jiffies {
     unsafe { bindings::__msecs_to_jiffies(msecs) }
 }
 
-/// A Rust wrapper around a `ktime_t`.
+/// A specific point in time.
+///
+/// # Invariants
+///
+/// The `inner` value is in the range from 0 to `KTIME_MAX`.
 #[repr(transparent)]
 #[derive(Copy, Clone, PartialEq, PartialOrd, Eq, Ord)]
-pub struct Ktime {
+pub struct Instant {
     inner: bindings::ktime_t,
 }
 
-impl Ktime {
-    /// Create a `Ktime` from a raw `ktime_t`.
-    #[inline]
-    pub fn from_raw(inner: bindings::ktime_t) -> Self {
-        Self { inner }
-    }
-
+impl Instant {
     /// Get the current time using `CLOCK_MONOTONIC`.
     #[inline]
-    pub fn ktime_get() -> Self {
-        // SAFETY: It is always safe to call `ktime_get` outside of NMI context.
-        Self::from_raw(unsafe { bindings::ktime_get() })
-    }
-
-    /// Divide the number of nanoseconds by a compile-time constant.
-    #[inline]
-    fn divns_constant<const DIV: i64>(self) -> i64 {
-        self.to_ns() / DIV
-    }
-
-    /// Returns the number of nanoseconds.
-    #[inline]
-    pub fn to_ns(self) -> i64 {
-        self.inner
+    pub fn now() -> Self {
+        // INVARIANT: The `ktime_get()` function returns a value in the range
+        // from 0 to `KTIME_MAX`.
+        Self {
+            // SAFETY: It is always safe to call `ktime_get()` outside of NMI context.
+            inner: unsafe { bindings::ktime_get() },
+        }
     }
 
-    /// Returns the number of milliseconds.
+    /// Return the amount of time elapsed since the [`Instant`].
     #[inline]
-    pub fn to_ms(self) -> i64 {
-        self.divns_constant::<NSEC_PER_MSEC>()
+    pub fn elapsed(&self) -> Delta {
+        Self::now() - *self
     }
 }
 
-/// Returns the number of milliseconds between two ktimes.
-#[inline]
-pub fn ktime_ms_delta(later: Ktime, earlier: Ktime) -> i64 {
-    (later - earlier).to_ms()
-}
-
-impl core::ops::Sub for Ktime {
-    type Output = Ktime;
+impl core::ops::Sub for Instant {
+    type Output = Delta;
 
+    // By the type invariant, it never overflows.
     #[inline]
-    fn sub(self, other: Ktime) -> Ktime {
-        Self {
-            inner: self.inner - other.inner,
+    fn sub(self, other: Instant) -> Delta {
+        Delta {
+            nanos: self.inner - other.inner,
         }
     }
 }
diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
index ce53f8579d18..f46b41d3c31e 100644
--- a/rust/kernel/time/hrtimer.rs
+++ b/rust/kernel/time/hrtimer.rs
@@ -68,10 +68,25 @@
 //! `start` operation.
 
 use super::ClockId;
-use crate::{prelude::*, time::Ktime, types::Opaque};
+use crate::{prelude::*, types::Opaque};
 use core::marker::PhantomData;
 use pin_init::PinInit;
 
+/// A Rust wrapper around a `ktime_t`.
+#[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
diff --git a/rust/kernel/time/hrtimer/arc.rs b/rust/kernel/time/hrtimer/arc.rs
index 4a984d85b4a1..ccf1e66e5b2d 100644
--- a/rust/kernel/time/hrtimer/arc.rs
+++ b/rust/kernel/time/hrtimer/arc.rs
@@ -5,10 +5,10 @@
 use super::HrTimerCallback;
 use super::HrTimerHandle;
 use super::HrTimerPointer;
+use super::Ktime;
 use super::RawHrTimerCallback;
 use crate::sync::Arc;
 use crate::sync::ArcBorrow;
-use crate::time::Ktime;
 
 /// A handle for an `Arc<HasHrTimer<T>>` returned by a call to
 /// [`HrTimerPointer::start`].
diff --git a/rust/kernel/time/hrtimer/pin.rs b/rust/kernel/time/hrtimer/pin.rs
index f760db265c7b..293ca9cf058c 100644
--- a/rust/kernel/time/hrtimer/pin.rs
+++ b/rust/kernel/time/hrtimer/pin.rs
@@ -4,9 +4,9 @@
 use super::HrTimer;
 use super::HrTimerCallback;
 use super::HrTimerHandle;
+use super::Ktime;
 use super::RawHrTimerCallback;
 use super::UnsafeHrTimerPointer;
-use crate::time::Ktime;
 use core::pin::Pin;
 
 /// A handle for a `Pin<&HasHrTimer>`. When the handle exists, the timer might be
diff --git a/rust/kernel/time/hrtimer/pin_mut.rs b/rust/kernel/time/hrtimer/pin_mut.rs
index 90c0351d62e4..6033572d35ad 100644
--- a/rust/kernel/time/hrtimer/pin_mut.rs
+++ b/rust/kernel/time/hrtimer/pin_mut.rs
@@ -1,9 +1,9 @@
 // SPDX-License-Identifier: GPL-2.0
 
 use super::{
-    HasHrTimer, HrTimer, HrTimerCallback, HrTimerHandle, RawHrTimerCallback, UnsafeHrTimerPointer,
+    HasHrTimer, HrTimer, HrTimerCallback, HrTimerHandle, Ktime, RawHrTimerCallback,
+    UnsafeHrTimerPointer,
 };
-use crate::time::Ktime;
 use core::{marker::PhantomData, pin::Pin, ptr::NonNull};
 
 /// A handle for a `Pin<&mut HasHrTimer>`. When the handle exists, the timer might
diff --git a/rust/kernel/time/hrtimer/tbox.rs b/rust/kernel/time/hrtimer/tbox.rs
index 2071cae07234..29526a5da203 100644
--- a/rust/kernel/time/hrtimer/tbox.rs
+++ b/rust/kernel/time/hrtimer/tbox.rs
@@ -5,9 +5,9 @@
 use super::HrTimerCallback;
 use super::HrTimerHandle;
 use super::HrTimerPointer;
+use super::Ktime;
 use super::RawHrTimerCallback;
 use crate::prelude::*;
-use crate::time::Ktime;
 use core::ptr::NonNull;
 
 /// A handle for a [`Box<HasHrTimer<T>>`] returned by a call to
-- 
2.43.0

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

> On Tue, 15 Apr 2025 11:01:30 -0700
> Boqun Feng <boqun.feng@gmail.com> wrote:
>
>> On Mon, Apr 14, 2025 at 08:59:54PM +0900, FUJITA Tomonori wrote:
>>> On Mon, 14 Apr 2025 09:04:14 +0200
>>> Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>>
>>> > "Boqun Feng" <boqun.feng@gmail.com> writes:
>>> >
>>> >> On Sun, Apr 13, 2025 at 07:43:08PM +0900, FUJITA Tomonori wrote:
>>> >>> Introduce a type representing a specific point in time. We could use
>>> >>> the Ktime type but C's ktime_t is used for both timestamp and
>>> >>> timedelta. To avoid confusion, introduce a new Instant type for
>>> >>> timestamp.
>>> >>>
>>> >>> Rename Ktime to Instant and modify their methods for timestamp.
>>> >>>
>>> >>> Implement the subtraction operator for Instant:
>>> >>>
>>> >>> Delta = Instant A - Instant B
>>> >>>
>>> >>> Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
>>> >>
>>> >> I probably need to drop my Reviewed-by because of something below:
>>> >>
>>> >>> Reviewed-by: Gary Guo <gary@garyguo.net>
>>> >>> Reviewed-by: Fiona Behrens <me@kloenk.dev>
>>> >>> Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
>>> >>> Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
>>> >>> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
>>> >>> ---
>>> >> [...]
>>> >>> diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
>>> >>> index ce53f8579d18..27243eaaf8ed 100644
>>> >>> --- a/rust/kernel/time/hrtimer.rs
>>> >>> +++ b/rust/kernel/time/hrtimer.rs
>>> >>> @@ -68,7 +68,7 @@
>>> >>>  //! `start` operation.
>>> >>>
>>> >>>  use super::ClockId;
>>> >>> -use crate::{prelude::*, time::Ktime, types::Opaque};
>>> >>> +use crate::{prelude::*, time::Instant, types::Opaque};
>>> >>>  use core::marker::PhantomData;
>>> >>>  use pin_init::PinInit;
>>> >>>
>>> >>> @@ -189,7 +189,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: Instant) -> Self::TimerHandle;
>>> >>
>>> >> We should be able to use what I suggested:
>>> >>
>>> >> 	https://lore.kernel.org/rust-for-linux/Z_ALZsnwN53ZPBrB@boqun-archlinux/
>>> >>
>>> >> to make different timer modes (rel or abs) choose different expire type.
>>> >>
>>> >> I don't think we can merge this patch as it is, unfortunately, because
>>> >> it doesn't make sense for a relative timer to take an Instant as expires
>>> >> value.
>>> >
>>> > I told Tomo he could use `Instant` in this location and either he or I
>>> > would fix it up later [1].
>>> >
>>
>> I saw that, however, I don't think we can put `Instant` as the parameter
>> for HrTimerPointer::start() because we don't yet know how long would the
>> fix-it-up-later take. And it would confuse users if they need a put an
>> Instant for relative time.
>>
>>> > I don't want to block the series on this since the new API is not worse
>>> > than the old one where Ktime is overloaded for both uses.
>>
>> How about we keep Ktime? That is HrTimerPointer::start() still uses
>> Ktime, until we totally finish the refactoring as Tomo show below?
>> `Ktime` is much better here because it at least matches C API behavior,
>> we can remove `Ktime` once the dust is settled. Thoughts?
>
> Either is fine with me. I'll leave it to Andreas' judgment.
>
> Andreas, if you like Boqun's approach, I'll replace the third patch
> with the following one and send v14.
>
> I added Ktime struct to hrtimer.rs so the well-reviewed changes to
> time.rs remain unchanged.

OK, Let's keep Ktime for hrtimer for now. I am OK with you putting
`Ktime` in `hrtimer.rs` but you could also put it in time.rs. If you
don't want to modify the patches that already has reviews, you can add
it back in a separate patch.

Either way we should add a `// NOTE: Ktime is going to be removed when
hrtimer is converted to Instant/Duration`.


Best regards,
Andreas Hindborg

On Tue, 22 Apr 2025 12:07:02 +0200
Andreas Hindborg <a.hindborg@kernel.org> wrote:

> OK, Let's keep Ktime for hrtimer for now. I am OK with you putting
> `Ktime` in `hrtimer.rs` but you could also put it in time.rs. If you
> don't want to modify the patches that already has reviews, you can add
> it back in a separate patch.

Understood.

I added a separate patch that temporarily adds Ktime to hrtimer as the
first patch in the patch set. Each patch can build and run correctly
and there are no changes to the patches that have already been
reviewed.

> Either way we should add a `// NOTE: Ktime is going to be removed when
> hrtimer is converted to Instant/Duration`.

Added. Hopefully, I can finish it soon.