[PATCH v13 0/5] rust: Add IO polling

[PATCH v13 0/5] rust: Add IO polling

[FUJITA Tomonori]

Add two new types, Instant and Delta, which represent a specific point
in time and a span of time, respectively, with Rust version of
fsleep().

I dropped patches related with read_poll_timeout() in this version,
which we haven't reached agreement on yet. There are other potential
uses for the Instant and Delta types, so it's better to upstream them
first. Note that I haven't changed the subject to avoid confusion.

Unlike the old rust time branch, this adds a wrapper for fsleep()
instead of msleep(). fsleep() automatically chooses the best sleep
method based on a duration.

v13
- fix typo in MAINTAINERS file
v12: https://lore.kernel.org/lkml/20250406013445.124688-1-fujita.tomonori@gmail.com/
- drop #1, #6, and #7 patches, which we haven't reached agreement on yet
- adjust hrtimer code to use Instance for the removal of Ktime
v11: https://lore.kernel.org/lkml/20250220070611.214262-1-fujita.tomonori@gmail.com/
- use file_len arg name in __might_resched_precision() instead of len for clarity
- remove unnecessary strlen in __might_resched(); just use a large value for the precision
- add more doc and example for read_poll_timeout()
- fix read_poll_timeout() to call __might_sleep() only with CONFIG_DEBUG_ATOMIC_SLEEP enabled
- call might_sleep() instead of __might_sleep() in read_poll_timeout() to match the C version
- Add new sections for the abstractions in MAINTAINERS instead of adding rust files to the existing sections
v10: https://lore.kernel.org/lkml/20250207132623.168854-1-fujita.tomonori@gmail.com/
- rebased on rust-next
- use Option type for timeout argument for read_poll_timeout()
- remove obsoleted comment on read_poll_timeout()
v9: https://lore.kernel.org/lkml/20250125101854.112261-1-fujita.tomonori@gmail.com/
- make the might_sleep() changes into as a separate patch
- add as_millis() method to Delta for Binder driver
- make Delta's as_*() methods const (useful in some use cases)
- add Delta::ZERO const; used in fsleep()
- fix typos
- use intra-doc links
- place the #[inline] marker before the documentation
- remove Instant's from_raw() method
- add Invariants to Instant type
- improve Delta's methods documents
- fix fsleep() SAFETY comment
- improve fsleep() documents
- lift T:Copy restriction in read_poll_timeout()
- use MutFn for Cond in read_poll_timeout() instead of Fn
- fix might_sleep() call in read_poll_timeout()
- simplify read_poll_timeout() logic
v8: https://lore.kernel.org/lkml/20250116044100.80679-1-fujita.tomonori@gmail.com/
- fix compile warnings
v7: https://lore.kernel.org/lkml/20241220061853.2782878-1-fujita.tomonori@gmail.com/
- rebased on rust-next
- use crate::ffi instead of core::ffi
v6: https://lore.kernel.org/lkml/20241114070234.116329-1-fujita.tomonori@gmail.com/
- use super::Delta in delay.rs
- improve the comments
- add Delta's is_negative() method
- rename processor.rs to cpu.rs for cpu_relax()
- add __might_sleep_precision() taking pointer to a string with the length
- implement read_poll_timeout as normal function instead of macro
v5: https://lore.kernel.org/lkml/20241101010121.69221-1-fujita.tomonori@gmail.com/
- set the range of Delta for fsleep function
- update comments
v4: https://lore.kernel.org/lkml/20241025033118.44452-1-fujita.tomonori@gmail.com/
- rebase on the tip tree's timers/core
- add Instant instead of using Ktime
- remove unused basic methods
- add Delta as_micros_ceil method
- use const fn for Delta from_* methods
- add more comments based on the feedback
- add a safe wrapper for cpu_relax()
- add __might_sleep() macro
v3: https://lore.kernel.org/lkml/20241016035214.2229-1-fujita.tomonori@gmail.com/
- Update time::Delta methods (use i64 for everything)
- Fix read_poll_timeout to show the proper debug info (file and line)
- Move fsleep to rust/kernel/time/delay.rs
- Round up delta for fsleep
- Access directly ktime_t instead of using ktime APIs
- Add Eq and Ord with PartialEq and PartialOrd
v2: https://lore.kernel.org/lkml/20241005122531.20298-1-fujita.tomonori@gmail.com/
- Introduce time::Delta instead of core::time::Duration
- Add some trait to Ktime for calculating timeout
- Use read_poll_timeout in QT2025 driver instead of using fsleep directly
v1: https://lore.kernel.org/netdev/20241001112512.4861-1-fujita.tomonori@gmail.com/

FUJITA Tomonori (5):
  rust: time: Add PartialEq/Eq/PartialOrd/Ord trait to Ktime
  rust: time: Introduce Delta type
  rust: time: Introduce Instant type
  rust: time: Add wrapper for fsleep() function
  MAINTAINERS: rust: Add a new section for all of the time stuff

 MAINTAINERS                         |  11 +-
 rust/helpers/helpers.c              |   1 +
 rust/helpers/time.c                 |   8 ++
 rust/kernel/time.rs                 | 165 ++++++++++++++++++++++------
 rust/kernel/time/delay.rs           |  49 +++++++++
 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 +-
 10 files changed, 210 insertions(+), 54 deletions(-)
 create mode 100644 rust/helpers/time.c
 create mode 100644 rust/kernel/time/delay.rs


base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
-- 
2.43.0

[PATCH v13 1/5] rust: time: Add PartialEq/Eq/PartialOrd/Ord trait to Ktime

[FUJITA Tomonori]

Add PartialEq/Eq/PartialOrd/Ord trait to Ktime so two Ktime instances
can be compared to determine whether a timeout is met or not.

Use the derive implements; we directly touch C's ktime_t rather than
using the C's accessors because it is more efficient and we already do
in the existing code (Ktime::sub).

Reviewed-by: Trevor Gross <tmgross@umich.edu>
Reviewed-by: Alice Ryhl <aliceryhl@google.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 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
index f509cb0eb71e..9d57e8a5552a 100644
--- a/rust/kernel/time.rs
+++ b/rust/kernel/time.rs
@@ -29,7 +29,7 @@ pub fn msecs_to_jiffies(msecs: Msecs) -> Jiffies {
 
 /// A Rust wrapper around a `ktime_t`.
 #[repr(transparent)]
-#[derive(Copy, Clone)]
+#[derive(Copy, Clone, PartialEq, PartialOrd, Eq, Ord)]
 pub struct Ktime {
     inner: bindings::ktime_t,
 }
-- 
2.43.0

[PATCH v13 2/5] rust: time: Introduce Delta type

[FUJITA Tomonori]

Introduce a type representing a span of time. Define our own type
because `core::time::Duration` is large and could panic during
creation.

time::Ktime could be also used for time duration but timestamp and
timedelta are different so better to use a new type.

i64 is used instead of u64 to represent a span of time; some C drivers
uses negative Deltas and i64 is more compatible with Ktime using i64
too (e.g., ktime_[us|ms]_delta() APIs return i64 so we create Delta
object without type conversion.

i64 is used instead of bindings::ktime_t because when the ktime_t
type is used as timestamp, it represents values from 0 to
KTIME_MAX, which is different from Delta.

as_millis() method isn't used in this patchset. It's planned to be
used in Binder driver.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Alice Ryhl <aliceryhl@google.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 | 88 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 88 insertions(+)

diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
index 9d57e8a5552a..e00b9a853e6a 100644
--- a/rust/kernel/time.rs
+++ b/rust/kernel/time.rs
@@ -10,9 +10,15 @@
 
 pub mod hrtimer;
 
+/// The number of nanoseconds per microsecond.
+pub const NSEC_PER_USEC: i64 = bindings::NSEC_PER_USEC as i64;
+
 /// The number of nanoseconds per millisecond.
 pub const NSEC_PER_MSEC: i64 = bindings::NSEC_PER_MSEC as i64;
 
+/// The number of nanoseconds per second.
+pub const NSEC_PER_SEC: i64 = bindings::NSEC_PER_SEC as i64;
+
 /// The time unit of Linux kernel. One jiffy equals (1/HZ) second.
 pub type Jiffies = crate::ffi::c_ulong;
 
@@ -149,3 +155,85 @@ fn into_c(self) -> bindings::clockid_t {
         self as bindings::clockid_t
     }
 }
+
+/// A span of time.
+///
+/// This struct represents a span of time, with its value stored as nanoseconds.
+/// The value can represent any valid i64 value, including negative, zero, and
+/// positive numbers.
+#[derive(Copy, Clone, PartialEq, PartialOrd, Eq, Ord, Debug)]
+pub struct Delta {
+    nanos: i64,
+}
+
+impl Delta {
+    /// A span of time equal to zero.
+    pub const ZERO: Self = Self { nanos: 0 };
+
+    /// Create a new [`Delta`] from a number of microseconds.
+    ///
+    /// The `micros` can range from -9_223_372_036_854_775 to 9_223_372_036_854_775.
+    /// If `micros` is outside this range, `i64::MIN` is used for negative values,
+    /// and `i64::MAX` is used for positive values due to saturation.
+    #[inline]
+    pub const fn from_micros(micros: i64) -> Self {
+        Self {
+            nanos: micros.saturating_mul(NSEC_PER_USEC),
+        }
+    }
+
+    /// Create a new [`Delta`] from a number of milliseconds.
+    ///
+    /// The `millis` can range from -9_223_372_036_854 to 9_223_372_036_854.
+    /// If `millis` is outside this range, `i64::MIN` is used for negative values,
+    /// and `i64::MAX` is used for positive values due to saturation.
+    #[inline]
+    pub const fn from_millis(millis: i64) -> Self {
+        Self {
+            nanos: millis.saturating_mul(NSEC_PER_MSEC),
+        }
+    }
+
+    /// Create a new [`Delta`] from a number of seconds.
+    ///
+    /// The `secs` can range from -9_223_372_036 to 9_223_372_036.
+    /// If `secs` is outside this range, `i64::MIN` is used for negative values,
+    /// and `i64::MAX` is used for positive values due to saturation.
+    #[inline]
+    pub const fn from_secs(secs: i64) -> Self {
+        Self {
+            nanos: secs.saturating_mul(NSEC_PER_SEC),
+        }
+    }
+
+    /// Return `true` if the [`Delta`] spans no time.
+    #[inline]
+    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 {
+        self.as_nanos() < 0
+    }
+
+    /// Return the number of nanoseconds in the [`Delta`].
+    #[inline]
+    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 const fn as_micros_ceil(self) -> i64 {
+        self.as_nanos().saturating_add(NSEC_PER_USEC - 1) / NSEC_PER_USEC
+    }
+
+    /// Return the number of milliseconds in the [`Delta`].
+    #[inline]
+    pub const fn as_millis(self) -> i64 {
+        self.as_nanos() / NSEC_PER_MSEC
+    }
+}
-- 
2.43.0

[PATCH v13 3/5] rust: time: Introduce Instant type

[FUJITA Tomonori]

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

Re: [PATCH v13 3/5] rust: time: Introduce Instant type

[Boqun Feng]

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

>  }
>  
[...]

Re: [PATCH v13 3/5] rust: time: Introduce Instant type

[Andreas Hindborg]

"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

Re: [PATCH v13 3/5] rust: time: Introduce Instant type

[FUJITA Tomonori]

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).

Re: [PATCH v13 3/5] rust: time: Introduce Instant type

[Boqun Feng]

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).
> 

Re: [PATCH v13 3/5] rust: time: Introduce Instant type

[FUJITA Tomonori]

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

Re: [PATCH v13 3/5] rust: time: Introduce Instant type

[Andreas Hindborg]

"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

Re: [PATCH v13 3/5] rust: time: Introduce Instant type

[FUJITA Tomonori]

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.

[PATCH v13 4/5] rust: time: Add wrapper for fsleep() function

[FUJITA Tomonori]

Add a wrapper for fsleep(), flexible sleep functions in
include/linux/delay.h which typically deals with hardware delays.

The kernel supports several sleep functions to handle various lengths
of delay. This adds fsleep(), automatically chooses the best sleep
method based on a duration.

sleep functions including fsleep() belongs to TIMERS, not
TIMEKEEPING. They are maintained separately. rust/kernel/time.rs is an
abstraction for TIMEKEEPING. To make Rust abstractions match the C
side, add rust/kernel/time/delay.rs for this wrapper.

fsleep() can only be used in a nonatomic context. This requirement is
not checked by these abstractions, but it is intended that klint [1]
or a similar tool will be used to check it in the future.

Link: https://rust-for-linux.com/klint [1]
Reviewed-by: Gary Guo <gary@garyguo.net>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
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/helpers/helpers.c    |  1 +
 rust/helpers/time.c       |  8 +++++++
 rust/kernel/time.rs       |  1 +
 rust/kernel/time/delay.rs | 49 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 59 insertions(+)
 create mode 100644 rust/helpers/time.c
 create mode 100644 rust/kernel/time/delay.rs

diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index e1c21eba9b15..48143cdd26b3 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -33,6 +33,7 @@
 #include "spinlock.c"
 #include "sync.c"
 #include "task.c"
+#include "time.c"
 #include "uaccess.c"
 #include "vmalloc.c"
 #include "wait.c"
diff --git a/rust/helpers/time.c b/rust/helpers/time.c
new file mode 100644
index 000000000000..7ae64ad8141d
--- /dev/null
+++ b/rust/helpers/time.c
@@ -0,0 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/delay.h>
+
+void rust_helper_fsleep(unsigned long usecs)
+{
+	fsleep(usecs);
+}
diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
index bc5082c01152..8d6aa88724ad 100644
--- a/rust/kernel/time.rs
+++ b/rust/kernel/time.rs
@@ -24,6 +24,7 @@
 //! C header: [`include/linux/jiffies.h`](srctree/include/linux/jiffies.h).
 //! C header: [`include/linux/ktime.h`](srctree/include/linux/ktime.h).
 
+pub mod delay;
 pub mod hrtimer;
 
 /// The number of nanoseconds per microsecond.
diff --git a/rust/kernel/time/delay.rs b/rust/kernel/time/delay.rs
new file mode 100644
index 000000000000..02b8731433c7
--- /dev/null
+++ b/rust/kernel/time/delay.rs
@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Delay and sleep primitives.
+//!
+//! This module contains the kernel APIs related to delay and sleep that
+//! have been ported or wrapped for usage by Rust code in the kernel.
+//!
+//! C header: [`include/linux/delay.h`](srctree/include/linux/delay.h).
+
+use super::Delta;
+use crate::ffi::c_ulong;
+
+/// Sleeps for a given duration at least.
+///
+/// Equivalent to the C side [`fsleep()`], flexible sleep function,
+/// which automatically chooses the best sleep method based on a duration.
+///
+/// `delta` must be within `[0, i32::MAX]` microseconds;
+/// otherwise, it is erroneous behavior. That is, it is considered a bug
+/// to call this function with an out-of-range value, in which case the function
+/// will sleep for at least the maximum value in the range and may warn
+/// in the future.
+///
+/// The behavior above differs from the C side [`fsleep()`] for which out-of-range
+/// values mean "infinite timeout" instead.
+///
+/// This function can only be used in a nonatomic context.
+///
+/// [`fsleep`]: https://docs.kernel.org/timers/delay_sleep_functions.html#c.fsleep
+pub fn fsleep(delta: Delta) {
+    // The maximum value is set to `i32::MAX` microseconds to prevent integer
+    // overflow inside fsleep, which could lead to unintentional infinite sleep.
+    const MAX_DELTA: Delta = Delta::from_micros(i32::MAX as i64);
+
+    let delta = if (Delta::ZERO..=MAX_DELTA).contains(&delta) {
+        delta
+    } else {
+        // TODO: Add WARN_ONCE() when it's supported.
+        MAX_DELTA
+    };
+
+    // SAFETY: It is always safe to call `fsleep()` with any duration.
+    unsafe {
+        // Convert the duration to microseconds and round up to preserve
+        // the guarantee; `fsleep()` sleeps for at least the provided duration,
+        // but that it may sleep for longer under some circumstances.
+        bindings::fsleep(delta.as_micros_ceil() as c_ulong)
+    }
+}
-- 
2.43.0

[PATCH v13 5/5] MAINTAINERS: rust: Add a new section for all of the time stuff

[FUJITA Tomonori]

Add a new section for all of the time stuff to MAINTAINERS file, with
the existing hrtimer entry fold.

Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 MAINTAINERS | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 96b827049501..104cec84146f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10583,20 +10583,23 @@ F:	kernel/time/timer_list.c
 F:	kernel/time/timer_migration.*
 F:	tools/testing/selftests/timers/
 
-HIGH-RESOLUTION TIMERS [RUST]
+DELAY, SLEEP, TIMEKEEPING, TIMERS [RUST]
 M:	Andreas Hindborg <a.hindborg@kernel.org>
 R:	Boqun Feng <boqun.feng@gmail.com>
+R:	FUJITA Tomonori <fujita.tomonori@gmail.com>
 R:	Frederic Weisbecker <frederic@kernel.org>
 R:	Lyude Paul <lyude@redhat.com>
 R:	Thomas Gleixner <tglx@linutronix.de>
 R:	Anna-Maria Behnsen <anna-maria@linutronix.de>
+R:	John Stultz <jstultz@google.com>
+R:	Stephen Boyd <sboyd@kernel.org>
 L:	rust-for-linux@vger.kernel.org
 S:	Supported
 W:	https://rust-for-linux.com
 B:	https://github.com/Rust-for-Linux/linux/issues
-T:	git https://github.com/Rust-for-Linux/linux.git hrtimer-next
-F:	rust/kernel/time/hrtimer.rs
-F:	rust/kernel/time/hrtimer/
+T:	git https://github.com/Rust-for-Linux/linux.git rust-timekeeping-next
+F:	rust/kernel/time.rs
+F:	rust/kernel/time/
 
 HIGH-SPEED SCC DRIVER FOR AX.25
 L:	linux-hams@vger.kernel.org
-- 
2.43.0