* [PATCH v3 0/3] rust: time: Introduce typed clock sources and generalize Instant
@ 2025-06-09 1:04 FUJITA Tomonori
2025-06-09 1:04 ` [PATCH v3 1/3] rust: time: Replace ClockId enum with ClockSource trait FUJITA Tomonori
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: FUJITA Tomonori @ 2025-06-09 1:04 UTC (permalink / raw)
To: a.hindborg, alex.gaynor, ojeda
Cc: aliceryhl, anna-maria, bjorn3_gh, boqun.feng, dakr, frederic,
gary, jstultz, linux-kernel, lossin, lyude, rust-for-linux, sboyd,
tglx, tmgross
This patch series introduces a type-safe abstraction over clock
sources. The goal is to remove the need for runtime clock selection
(via ClockId) and instead leverage Rust's type system to statically
associate the Instant type with a specific clock.
This approach enables compile-time enforcement of clock correctness
across the APIs of Instant type.
v3:
- rebased on 6.16-rc1
v2: https://lore.kernel.org/rust-for-linux/20250504042436.237756-1-fujita.tomonori@gmail.com/
- removed most of changes to hrtimer code
v1: https://lore.kernel.org/rust-for-linux/20250413105629.162349-1-fujita.tomonori@gmail.com/
FUJITA Tomonori (3):
rust: time: Replace ClockId enum with ClockSource trait
rust: time: Make Instant generic over ClockSource
rust: time: Add ktime_get() to ClockSource trait
rust/helpers/helpers.c | 1 +
rust/helpers/time.c | 18 ++++
rust/kernel/time.rs | 201 ++++++++++++++++++++++--------------
rust/kernel/time/hrtimer.rs | 6 +-
4 files changed, 148 insertions(+), 78 deletions(-)
create mode 100644 rust/helpers/time.c
base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
--
2.43.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3 1/3] rust: time: Replace ClockId enum with ClockSource trait
2025-06-09 1:04 [PATCH v3 0/3] rust: time: Introduce typed clock sources and generalize Instant FUJITA Tomonori
@ 2025-06-09 1:04 ` FUJITA Tomonori
2025-06-09 1:04 ` [PATCH v3 2/3] rust: time: Make Instant generic over ClockSource FUJITA Tomonori
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: FUJITA Tomonori @ 2025-06-09 1:04 UTC (permalink / raw)
To: a.hindborg, alex.gaynor, ojeda
Cc: aliceryhl, anna-maria, bjorn3_gh, boqun.feng, dakr, frederic,
gary, jstultz, linux-kernel, lossin, lyude, rust-for-linux, sboyd,
tglx, tmgross
Replace the ClockId enum with a trait-based abstraction called
ClockSource. This change enables expressing clock sources as types and
leveraging the Rust type system to enforce clock correctness at
compile time.
This also sets the stage for future generic abstractions over Instant
types such as Instant<C>.
Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
rust/kernel/time.rs | 147 ++++++++++++++++++++----------------
rust/kernel/time/hrtimer.rs | 6 +-
2 files changed, 84 insertions(+), 69 deletions(-)
diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
index a8089a98da9e..efe68462b899 100644
--- a/rust/kernel/time.rs
+++ b/rust/kernel/time.rs
@@ -49,6 +49,87 @@ pub fn msecs_to_jiffies(msecs: Msecs) -> Jiffies {
unsafe { bindings::__msecs_to_jiffies(msecs) }
}
+/// Trait for clock sources.
+///
+/// Selection of the clock source depends on the use case. In some cases the usage of a
+/// particular clock is mandatory, e.g. in network protocols, filesystems. In other
+/// cases the user of the clock has to decide which clock is best suited for the
+/// purpose. In most scenarios clock [`Monotonic`] is the best choice as it
+/// provides a accurate monotonic notion of time (leap second smearing ignored).
+pub trait ClockSource {
+ /// The kernel clock ID associated with this clock source.
+ ///
+ /// This constant corresponds to the C side `clockid_t` value.
+ const ID: bindings::clockid_t;
+}
+
+/// A monotonically increasing clock.
+///
+/// A nonsettable system-wide clock that represents monotonic time since as
+/// described by POSIX, "some unspecified point in the past". On Linux, that
+/// point corresponds to the number of seconds that the system has been
+/// running since it was booted.
+///
+/// The CLOCK_MONOTONIC clock is not affected by discontinuous jumps in the
+/// CLOCK_REAL (e.g., if the system administrator manually changes the
+/// clock), but is affected by frequency adjustments. This clock does not
+/// count time that the system is suspended.
+pub struct Monotonic;
+
+impl ClockSource for Monotonic {
+ const ID: bindings::clockid_t = bindings::CLOCK_MONOTONIC as bindings::clockid_t;
+}
+
+/// A settable system-wide clock that measures real (i.e., wall-clock) time.
+///
+/// Setting this clock requires appropriate privileges. This clock is
+/// affected by discontinuous jumps in the system time (e.g., if the system
+/// administrator manually changes the clock), and by frequency adjustments
+/// performed by NTP and similar applications via adjtime(3), adjtimex(2),
+/// clock_adjtime(2), and ntp_adjtime(3). This clock normally counts the
+/// number of seconds since 1970-01-01 00:00:00 Coordinated Universal Time
+/// (UTC) except that it ignores leap seconds; near a leap second it may be
+/// adjusted by leap second smearing to stay roughly in sync with UTC. Leap
+/// second smearing applies frequency adjustments to the clock to speed up
+/// or slow down the clock to account for the leap second without
+/// discontinuities in the clock. If leap second smearing is not applied,
+/// the clock will experience discontinuity around leap second adjustment.
+pub struct RealTime;
+
+impl ClockSource for RealTime {
+ const ID: bindings::clockid_t = bindings::CLOCK_REALTIME as bindings::clockid_t;
+}
+
+/// A monotonic that ticks while system is suspended.
+///
+/// A nonsettable system-wide clock that is identical to CLOCK_MONOTONIC,
+/// except that it also includes any time that the system is suspended. This
+/// allows applications to get a suspend-aware monotonic clock without
+/// having to deal with the complications of CLOCK_REALTIME, which may have
+/// discontinuities if the time is changed using settimeofday(2) or similar.
+pub struct BootTime;
+
+impl ClockSource for BootTime {
+ const ID: bindings::clockid_t = bindings::CLOCK_BOOTTIME as bindings::clockid_t;
+}
+
+/// International Atomic Time.
+///
+/// A system-wide clock derived from wall-clock time but counting leap seconds.
+///
+/// This clock is coupled to CLOCK_REALTIME and will be set when CLOCK_REALTIME is
+/// set, or when the offset to CLOCK_REALTIME is changed via adjtimex(2). This
+/// usually happens during boot and **should** not happen during normal operations.
+/// However, if NTP or another application adjusts CLOCK_REALTIME by leap second
+/// smearing, this clock will not be precise during leap second smearing.
+///
+/// The acronym TAI refers to International Atomic Time.
+pub struct Tai;
+
+impl ClockSource for Tai {
+ const ID: bindings::clockid_t = bindings::CLOCK_TAI as bindings::clockid_t;
+}
+
/// A specific point in time.
///
/// # Invariants
@@ -91,72 +172,6 @@ fn sub(self, other: Instant) -> Delta {
}
}
-/// An identifier for a clock. Used when specifying clock sources.
-///
-///
-/// Selection of the clock depends on the use case. In some cases the usage of a
-/// particular clock is mandatory, e.g. in network protocols, filesystems.In other
-/// cases the user of the clock has to decide which clock is best suited for the
-/// purpose. In most scenarios clock [`ClockId::Monotonic`] is the best choice as it
-/// provides a accurate monotonic notion of time (leap second smearing ignored).
-#[derive(Clone, Copy, PartialEq, Eq, Debug)]
-#[repr(u32)]
-pub enum ClockId {
- /// A settable system-wide clock that measures real (i.e., wall-clock) time.
- ///
- /// Setting this clock requires appropriate privileges. This clock is
- /// affected by discontinuous jumps in the system time (e.g., if the system
- /// administrator manually changes the clock), and by frequency adjustments
- /// performed by NTP and similar applications via adjtime(3), adjtimex(2),
- /// clock_adjtime(2), and ntp_adjtime(3). This clock normally counts the
- /// number of seconds since 1970-01-01 00:00:00 Coordinated Universal Time
- /// (UTC) except that it ignores leap seconds; near a leap second it may be
- /// adjusted by leap second smearing to stay roughly in sync with UTC. Leap
- /// second smearing applies frequency adjustments to the clock to speed up
- /// or slow down the clock to account for the leap second without
- /// discontinuities in the clock. If leap second smearing is not applied,
- /// the clock will experience discontinuity around leap second adjustment.
- RealTime = bindings::CLOCK_REALTIME,
- /// A monotonically increasing clock.
- ///
- /// A nonsettable system-wide clock that represents monotonic time since—as
- /// described by POSIX—"some unspecified point in the past". On Linux, that
- /// point corresponds to the number of seconds that the system has been
- /// running since it was booted.
- ///
- /// The CLOCK_MONOTONIC clock is not affected by discontinuous jumps in the
- /// CLOCK_REAL (e.g., if the system administrator manually changes the
- /// clock), but is affected by frequency adjustments. This clock does not
- /// count time that the system is suspended.
- Monotonic = bindings::CLOCK_MONOTONIC,
- /// A monotonic that ticks while system is suspended.
- ///
- /// A nonsettable system-wide clock that is identical to CLOCK_MONOTONIC,
- /// except that it also includes any time that the system is suspended. This
- /// allows applications to get a suspend-aware monotonic clock without
- /// having to deal with the complications of CLOCK_REALTIME, which may have
- /// discontinuities if the time is changed using settimeofday(2) or similar.
- BootTime = bindings::CLOCK_BOOTTIME,
- /// International Atomic Time.
- ///
- /// A system-wide clock derived from wall-clock time but counting leap seconds.
- ///
- /// This clock is coupled to CLOCK_REALTIME and will be set when CLOCK_REALTIME is
- /// set, or when the offset to CLOCK_REALTIME is changed via adjtimex(2). This
- /// usually happens during boot and **should** not happen during normal operations.
- /// However, if NTP or another application adjusts CLOCK_REALTIME by leap second
- /// smearing, this clock will not be precise during leap second smearing.
- ///
- /// The acronym TAI refers to International Atomic Time.
- TAI = bindings::CLOCK_TAI,
-}
-
-impl ClockId {
- 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.
diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs
index 9df3dcd2fa39..280128d7e982 100644
--- a/rust/kernel/time/hrtimer.rs
+++ b/rust/kernel/time/hrtimer.rs
@@ -67,7 +67,7 @@
//! A `restart` operation on a timer in the **stopped** state is equivalent to a
//! `start` operation.
-use super::ClockId;
+use super::ClockSource;
use crate::{prelude::*, types::Opaque};
use core::marker::PhantomData;
use pin_init::PinInit;
@@ -112,7 +112,7 @@ unsafe impl<T> Sync for HrTimer<T> {}
impl<T> HrTimer<T> {
/// Return an initializer for a new timer instance.
- pub fn new(mode: HrTimerMode, clock: ClockId) -> impl PinInit<Self>
+ pub fn new<U: ClockSource>(mode: HrTimerMode) -> impl PinInit<Self>
where
T: HrTimerCallback,
{
@@ -126,7 +126,7 @@ pub fn new(mode: HrTimerMode, clock: ClockId) -> impl PinInit<Self>
bindings::hrtimer_setup(
place,
Some(T::Pointer::run),
- clock.into_c(),
+ U::ID,
mode.into_c(),
);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v3 2/3] rust: time: Make Instant generic over ClockSource
2025-06-09 1:04 [PATCH v3 0/3] rust: time: Introduce typed clock sources and generalize Instant FUJITA Tomonori
2025-06-09 1:04 ` [PATCH v3 1/3] rust: time: Replace ClockId enum with ClockSource trait FUJITA Tomonori
@ 2025-06-09 1:04 ` FUJITA Tomonori
2025-06-09 1:04 ` [PATCH v3 3/3] rust: time: Add ktime_get() to ClockSource trait FUJITA Tomonori
2025-06-09 2:55 ` [PATCH v3 0/3] rust: time: Introduce typed clock sources and generalize Instant Boqun Feng
3 siblings, 0 replies; 8+ messages in thread
From: FUJITA Tomonori @ 2025-06-09 1:04 UTC (permalink / raw)
To: a.hindborg, alex.gaynor, ojeda
Cc: aliceryhl, anna-maria, bjorn3_gh, boqun.feng, dakr, frederic,
gary, jstultz, linux-kernel, lossin, lyude, rust-for-linux, sboyd,
tglx, tmgross
Refactor the Instant type to be generic over a ClockSource type
parameter, enabling static enforcement of clock correctness across
APIs that deal with time. Previously, the clock source was implicitly
fixed (typically CLOCK_MONOTONIC), and developers had to ensure
compatibility manually.
This design eliminates runtime mismatches between clock sources, and
enables stronger type-level guarantees throughout the timer subsystem.
Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
rust/kernel/time.rs | 22 +++++++++++++++++-----
1 file changed, 17 insertions(+), 5 deletions(-)
diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
index efe68462b899..bba62c7f37e4 100644
--- a/rust/kernel/time.rs
+++ b/rust/kernel/time.rs
@@ -24,6 +24,8 @@
//! C header: [`include/linux/jiffies.h`](srctree/include/linux/jiffies.h).
//! C header: [`include/linux/ktime.h`](srctree/include/linux/ktime.h).
+use core::marker::PhantomData;
+
pub mod hrtimer;
/// The number of nanoseconds per microsecond.
@@ -136,12 +138,21 @@ impl ClockSource for Tai {
///
/// The `inner` value is in the range from 0 to `KTIME_MAX`.
#[repr(transparent)]
-#[derive(Copy, Clone, PartialEq, PartialOrd, Eq, Ord)]
-pub struct Instant {
+#[derive(PartialEq, PartialOrd, Eq, Ord)]
+pub struct Instant<C: ClockSource> {
inner: bindings::ktime_t,
+ _c: PhantomData<C>,
}
-impl Instant {
+impl<C: ClockSource> Clone for Instant<C> {
+ fn clone(&self) -> Self {
+ *self
+ }
+}
+
+impl<C: ClockSource> Copy for Instant<C> {}
+
+impl<C: ClockSource> Instant<C> {
/// Get the current time using `CLOCK_MONOTONIC`.
#[inline]
pub fn now() -> Self {
@@ -150,6 +161,7 @@ pub fn now() -> Self {
Self {
// SAFETY: It is always safe to call `ktime_get()` outside of NMI context.
inner: unsafe { bindings::ktime_get() },
+ _c: PhantomData,
}
}
@@ -160,12 +172,12 @@ pub fn elapsed(&self) -> Delta {
}
}
-impl core::ops::Sub for Instant {
+impl<C: ClockSource> core::ops::Sub for Instant<C> {
type Output = Delta;
// By the type invariant, it never overflows.
#[inline]
- fn sub(self, other: Instant) -> Delta {
+ fn sub(self, other: Instant<C>) -> Delta {
Delta {
nanos: self.inner - other.inner,
}
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v3 3/3] rust: time: Add ktime_get() to ClockSource trait
2025-06-09 1:04 [PATCH v3 0/3] rust: time: Introduce typed clock sources and generalize Instant FUJITA Tomonori
2025-06-09 1:04 ` [PATCH v3 1/3] rust: time: Replace ClockId enum with ClockSource trait FUJITA Tomonori
2025-06-09 1:04 ` [PATCH v3 2/3] rust: time: Make Instant generic over ClockSource FUJITA Tomonori
@ 2025-06-09 1:04 ` FUJITA Tomonori
2025-06-10 8:01 ` Andreas Hindborg
2025-06-09 2:55 ` [PATCH v3 0/3] rust: time: Introduce typed clock sources and generalize Instant Boqun Feng
3 siblings, 1 reply; 8+ messages in thread
From: FUJITA Tomonori @ 2025-06-09 1:04 UTC (permalink / raw)
To: a.hindborg, alex.gaynor, ojeda
Cc: aliceryhl, anna-maria, bjorn3_gh, boqun.feng, dakr, frederic,
gary, jstultz, linux-kernel, lossin, lyude, rust-for-linux, sboyd,
tglx, tmgross
Introduce the ktime_get() associated function to the ClockSource
trait, allowing each clock source to specify how it retrieves the
current time. This enables Instant::now() to be implemented
generically using the type-level ClockSource abstraction.
This change enhances the type safety and extensibility of timekeeping
by statically associating time retrieval mechanisms with their
respective clock types. It also reduces the reliance on hardcoded
clock logic within Instant.
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 | 18 ++++++++++++++++++
rust/kernel/time.rs | 32 ++++++++++++++++++++++++++++----
3 files changed, 47 insertions(+), 4 deletions(-)
create mode 100644 rust/helpers/time.c
diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index 0f1b5d115985..0613a849e05c 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -39,6 +39,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..9c296e93a560
--- /dev/null
+++ b/rust/helpers/time.c
@@ -0,0 +1,18 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/timekeeping.h>
+
+ktime_t rust_helper_ktime_get_real(void)
+{
+ return ktime_get_with_offset(TK_OFFS_REAL);
+}
+
+ktime_t rust_helper_ktime_get_boottime(void)
+{
+ return ktime_get_with_offset(TK_OFFS_BOOT);
+}
+
+ktime_t rust_helper_ktime_get_clocktai(void)
+{
+ return ktime_get_with_offset(TK_OFFS_TAI);
+}
diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
index bba62c7f37e4..9fd487276457 100644
--- a/rust/kernel/time.rs
+++ b/rust/kernel/time.rs
@@ -63,6 +63,11 @@ pub trait ClockSource {
///
/// This constant corresponds to the C side `clockid_t` value.
const ID: bindings::clockid_t;
+
+ /// Get the current time from the clock source.
+ ///
+ /// The function must return a value in the range from 0 to `KTIME_MAX`.
+ fn ktime_get() -> bindings::ktime_t;
}
/// A monotonically increasing clock.
@@ -80,6 +85,11 @@ pub trait ClockSource {
impl ClockSource for Monotonic {
const ID: bindings::clockid_t = bindings::CLOCK_MONOTONIC as bindings::clockid_t;
+
+ fn ktime_get() -> bindings::ktime_t {
+ // SAFETY: It is always safe to call `ktime_get()` outside of NMI context.
+ unsafe { bindings::ktime_get() }
+ }
}
/// A settable system-wide clock that measures real (i.e., wall-clock) time.
@@ -100,6 +110,11 @@ impl ClockSource for Monotonic {
impl ClockSource for RealTime {
const ID: bindings::clockid_t = bindings::CLOCK_REALTIME as bindings::clockid_t;
+
+ fn ktime_get() -> bindings::ktime_t {
+ // SAFETY: It is always safe to call `ktime_get_real()` outside of NMI context.
+ unsafe { bindings::ktime_get_real() }
+ }
}
/// A monotonic that ticks while system is suspended.
@@ -113,6 +128,11 @@ impl ClockSource for RealTime {
impl ClockSource for BootTime {
const ID: bindings::clockid_t = bindings::CLOCK_BOOTTIME as bindings::clockid_t;
+
+ fn ktime_get() -> bindings::ktime_t {
+ // SAFETY: It is always safe to call `ktime_get_boottime()` outside of NMI context.
+ unsafe { bindings::ktime_get_boottime() }
+ }
}
/// International Atomic Time.
@@ -130,6 +150,11 @@ impl ClockSource for BootTime {
impl ClockSource for Tai {
const ID: bindings::clockid_t = bindings::CLOCK_TAI as bindings::clockid_t;
+
+ fn ktime_get() -> bindings::ktime_t {
+ // SAFETY: It is always safe to call `ktime_get_tai()` outside of NMI context.
+ unsafe { bindings::ktime_get_clocktai() }
+ }
}
/// A specific point in time.
@@ -153,14 +178,13 @@ fn clone(&self) -> Self {
impl<C: ClockSource> Copy for Instant<C> {}
impl<C: ClockSource> Instant<C> {
- /// Get the current time using `CLOCK_MONOTONIC`.
+ /// Get the current time from the clock source.
#[inline]
pub fn now() -> Self {
- // INVARIANT: The `ktime_get()` function returns a value in the range
+ // INVARIANT: The `ClockSource::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() },
+ inner: C::ktime_get(),
_c: PhantomData,
}
}
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3 0/3] rust: time: Introduce typed clock sources and generalize Instant
2025-06-09 1:04 [PATCH v3 0/3] rust: time: Introduce typed clock sources and generalize Instant FUJITA Tomonori
` (2 preceding siblings ...)
2025-06-09 1:04 ` [PATCH v3 3/3] rust: time: Add ktime_get() to ClockSource trait FUJITA Tomonori
@ 2025-06-09 2:55 ` Boqun Feng
3 siblings, 0 replies; 8+ messages in thread
From: Boqun Feng @ 2025-06-09 2:55 UTC (permalink / raw)
To: FUJITA Tomonori
Cc: a.hindborg, alex.gaynor, ojeda, aliceryhl, anna-maria, bjorn3_gh,
dakr, frederic, gary, jstultz, linux-kernel, lossin, lyude,
rust-for-linux, sboyd, tglx, tmgross
On Mon, Jun 09, 2025 at 10:04:10AM +0900, FUJITA Tomonori wrote:
> This patch series introduces a type-safe abstraction over clock
> sources. The goal is to remove the need for runtime clock selection
> (via ClockId) and instead leverage Rust's type system to statically
> associate the Instant type with a specific clock.
>
> This approach enables compile-time enforcement of clock correctness
> across the APIs of Instant type.
>
Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
Thanks a lot!
Regards,
Boqun
> v3:
> - rebased on 6.16-rc1
> v2: https://lore.kernel.org/rust-for-linux/20250504042436.237756-1-fujita.tomonori@gmail.com/
> - removed most of changes to hrtimer code
> v1: https://lore.kernel.org/rust-for-linux/20250413105629.162349-1-fujita.tomonori@gmail.com/
>
> FUJITA Tomonori (3):
> rust: time: Replace ClockId enum with ClockSource trait
> rust: time: Make Instant generic over ClockSource
> rust: time: Add ktime_get() to ClockSource trait
>
> rust/helpers/helpers.c | 1 +
> rust/helpers/time.c | 18 ++++
> rust/kernel/time.rs | 201 ++++++++++++++++++++++--------------
> rust/kernel/time/hrtimer.rs | 6 +-
> 4 files changed, 148 insertions(+), 78 deletions(-)
> create mode 100644 rust/helpers/time.c
>
>
> base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 3/3] rust: time: Add ktime_get() to ClockSource trait
2025-06-09 1:04 ` [PATCH v3 3/3] rust: time: Add ktime_get() to ClockSource trait FUJITA Tomonori
@ 2025-06-10 8:01 ` Andreas Hindborg
2025-06-10 8:15 ` FUJITA Tomonori
0 siblings, 1 reply; 8+ messages in thread
From: Andreas Hindborg @ 2025-06-10 8:01 UTC (permalink / raw)
To: FUJITA Tomonori
Cc: alex.gaynor, ojeda, aliceryhl, anna-maria, bjorn3_gh, boqun.feng,
dakr, frederic, gary, jstultz, linux-kernel, lossin, lyude,
rust-for-linux, sboyd, tglx, tmgross
"FUJITA Tomonori" <fujita.tomonori@gmail.com> writes:
> Introduce the ktime_get() associated function to the ClockSource
> trait, allowing each clock source to specify how it retrieves the
> current time. This enables Instant::now() to be implemented
> generically using the type-level ClockSource abstraction.
>
> This change enhances the type safety and extensibility of timekeeping
> by statically associating time retrieval mechanisms with their
> respective clock types. It also reduces the reliance on hardcoded
> clock logic within Instant.
>
> 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 | 18 ++++++++++++++++++
> rust/kernel/time.rs | 32 ++++++++++++++++++++++++++++----
> 3 files changed, 47 insertions(+), 4 deletions(-)
> create mode 100644 rust/helpers/time.c
>
> diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
> index 0f1b5d115985..0613a849e05c 100644
> --- a/rust/helpers/helpers.c
> +++ b/rust/helpers/helpers.c
> @@ -39,6 +39,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..9c296e93a560
> --- /dev/null
> +++ b/rust/helpers/time.c
> @@ -0,0 +1,18 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/timekeeping.h>
> +
> +ktime_t rust_helper_ktime_get_real(void)
> +{
> + return ktime_get_with_offset(TK_OFFS_REAL);
> +}
> +
> +ktime_t rust_helper_ktime_get_boottime(void)
> +{
> + return ktime_get_with_offset(TK_OFFS_BOOT);
> +}
> +
> +ktime_t rust_helper_ktime_get_clocktai(void)
> +{
> + return ktime_get_with_offset(TK_OFFS_TAI);
> +}
This just caught my eye. I think policy is to make helpers as much 1:1 as
possible. We should not inject arguments here. Instead, we should have
just one function that passes the argument along.
Best regards,
Andreas Hindborg
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 3/3] rust: time: Add ktime_get() to ClockSource trait
2025-06-10 8:01 ` Andreas Hindborg
@ 2025-06-10 8:15 ` FUJITA Tomonori
2025-06-10 9:34 ` Andreas Hindborg
0 siblings, 1 reply; 8+ messages in thread
From: FUJITA Tomonori @ 2025-06-10 8:15 UTC (permalink / raw)
To: a.hindborg
Cc: fujita.tomonori, alex.gaynor, ojeda, aliceryhl, anna-maria,
bjorn3_gh, boqun.feng, dakr, frederic, gary, jstultz,
linux-kernel, lossin, lyude, rust-for-linux, sboyd, tglx, tmgross
On Tue, 10 Jun 2025 10:01:57 +0200
Andreas Hindborg <a.hindborg@kernel.org> wrote:
> "FUJITA Tomonori" <fujita.tomonori@gmail.com> writes:
>
>> Introduce the ktime_get() associated function to the ClockSource
>> trait, allowing each clock source to specify how it retrieves the
>> current time. This enables Instant::now() to be implemented
>> generically using the type-level ClockSource abstraction.
>>
>> This change enhances the type safety and extensibility of timekeeping
>> by statically associating time retrieval mechanisms with their
>> respective clock types. It also reduces the reliance on hardcoded
>> clock logic within Instant.
>>
>> 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 | 18 ++++++++++++++++++
>> rust/kernel/time.rs | 32 ++++++++++++++++++++++++++++----
>> 3 files changed, 47 insertions(+), 4 deletions(-)
>> create mode 100644 rust/helpers/time.c
>>
>> diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
>> index 0f1b5d115985..0613a849e05c 100644
>> --- a/rust/helpers/helpers.c
>> +++ b/rust/helpers/helpers.c
>> @@ -39,6 +39,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..9c296e93a560
>> --- /dev/null
>> +++ b/rust/helpers/time.c
>> @@ -0,0 +1,18 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +#include <linux/timekeeping.h>
>> +
>> +ktime_t rust_helper_ktime_get_real(void)
>> +{
>> + return ktime_get_with_offset(TK_OFFS_REAL);
>> +}
>> +
>> +ktime_t rust_helper_ktime_get_boottime(void)
>> +{
>> + return ktime_get_with_offset(TK_OFFS_BOOT);
>> +}
>> +
>> +ktime_t rust_helper_ktime_get_clocktai(void)
>> +{
>> + return ktime_get_with_offset(TK_OFFS_TAI);
>> +}
>
> This just caught my eye. I think policy is to make helpers as much 1:1 as
> possible. We should not inject arguments here. Instead, we should have
> just one function that passes the argument along.
Indeed, you're right. It should have been as follows:
+++ b/rust/helpers/time.c
@@ -0,0 +1,18 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/timekeeping.h>
+
+ktime_t rust_helper_ktime_get_real(void)
+{
+ return ktime_get_real();
+}
+
+ktime_t rust_helper_ktime_get_boottime(void)
+{
+ return ktime_get_boottime();
+}
+
+ktime_t rust_helper_ktime_get_clocktai(void)
+{
+ return ktime_get_clocktai();
+}
I'll send v4 shortly.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 3/3] rust: time: Add ktime_get() to ClockSource trait
2025-06-10 8:15 ` FUJITA Tomonori
@ 2025-06-10 9:34 ` Andreas Hindborg
0 siblings, 0 replies; 8+ messages in thread
From: Andreas Hindborg @ 2025-06-10 9:34 UTC (permalink / raw)
To: FUJITA Tomonori
Cc: alex.gaynor, ojeda, aliceryhl, anna-maria, bjorn3_gh, boqun.feng,
dakr, frederic, gary, jstultz, linux-kernel, lossin, lyude,
rust-for-linux, sboyd, tglx, tmgross
"FUJITA Tomonori" <fujita.tomonori@gmail.com> writes:
> On Tue, 10 Jun 2025 10:01:57 +0200
> Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
>> "FUJITA Tomonori" <fujita.tomonori@gmail.com> writes:
>>
>>> Introduce the ktime_get() associated function to the ClockSource
>>> trait, allowing each clock source to specify how it retrieves the
>>> current time. This enables Instant::now() to be implemented
>>> generically using the type-level ClockSource abstraction.
>>>
>>> This change enhances the type safety and extensibility of timekeeping
>>> by statically associating time retrieval mechanisms with their
>>> respective clock types. It also reduces the reliance on hardcoded
>>> clock logic within Instant.
>>>
>>> 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 | 18 ++++++++++++++++++
>>> rust/kernel/time.rs | 32 ++++++++++++++++++++++++++++----
>>> 3 files changed, 47 insertions(+), 4 deletions(-)
>>> create mode 100644 rust/helpers/time.c
>>>
>>> diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
>>> index 0f1b5d115985..0613a849e05c 100644
>>> --- a/rust/helpers/helpers.c
>>> +++ b/rust/helpers/helpers.c
>>> @@ -39,6 +39,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..9c296e93a560
>>> --- /dev/null
>>> +++ b/rust/helpers/time.c
>>> @@ -0,0 +1,18 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +
>>> +#include <linux/timekeeping.h>
>>> +
>>> +ktime_t rust_helper_ktime_get_real(void)
>>> +{
>>> + return ktime_get_with_offset(TK_OFFS_REAL);
>>> +}
>>> +
>>> +ktime_t rust_helper_ktime_get_boottime(void)
>>> +{
>>> + return ktime_get_with_offset(TK_OFFS_BOOT);
>>> +}
>>> +
>>> +ktime_t rust_helper_ktime_get_clocktai(void)
>>> +{
>>> + return ktime_get_with_offset(TK_OFFS_TAI);
>>> +}
>>
>> This just caught my eye. I think policy is to make helpers as much 1:1 as
>> possible. We should not inject arguments here. Instead, we should have
>> just one function that passes the argument along.
>
> Indeed, you're right. It should have been as follows:
>
> +++ b/rust/helpers/time.c
> @@ -0,0 +1,18 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/timekeeping.h>
> +
> +ktime_t rust_helper_ktime_get_real(void)
> +{
> + return ktime_get_real();
> +}
> +
> +ktime_t rust_helper_ktime_get_boottime(void)
> +{
> + return ktime_get_boottime();
> +}
> +
> +ktime_t rust_helper_ktime_get_clocktai(void)
> +{
> + return ktime_get_clocktai();
> +}
Yes, even better 👍
Best regards,
Andreas Hindborg
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-06-10 10:25 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-09 1:04 [PATCH v3 0/3] rust: time: Introduce typed clock sources and generalize Instant FUJITA Tomonori
2025-06-09 1:04 ` [PATCH v3 1/3] rust: time: Replace ClockId enum with ClockSource trait FUJITA Tomonori
2025-06-09 1:04 ` [PATCH v3 2/3] rust: time: Make Instant generic over ClockSource FUJITA Tomonori
2025-06-09 1:04 ` [PATCH v3 3/3] rust: time: Add ktime_get() to ClockSource trait FUJITA Tomonori
2025-06-10 8:01 ` Andreas Hindborg
2025-06-10 8:15 ` FUJITA Tomonori
2025-06-10 9:34 ` Andreas Hindborg
2025-06-09 2:55 ` [PATCH v3 0/3] rust: time: Introduce typed clock sources and generalize Instant Boqun Feng
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).