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
Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
Reviewed-by: Gary Guo <gary@garyguo.net>
Reviewed-by: Fiona Behrens <me@kloenk.dev>
Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
rust/kernel/time.rs | 77 +++++++++++++++++++++++----------------------
1 file changed, 39 insertions(+), 38 deletions(-)
diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
index 622cd01e24d7..d64a05a4f4d1 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).
@@ -31,59 +47,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,
}
}
}
--
2.43.0FUJITA Tomonori <fujita.tomonori@gmail.com> writes:
> 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
>
> Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
> Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
> Reviewed-by: Gary Guo <gary@garyguo.net>
> Reviewed-by: Fiona Behrens <me@kloenk.dev>
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
As Boqun mentioned, we should make this generic over `ClockId` when the
hrtimer patches land.
One question regarding overflow below.
> ---
> rust/kernel/time.rs | 77 +++++++++++++++++++++++----------------------
> 1 file changed, 39 insertions(+), 38 deletions(-)
>
> diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
> index 622cd01e24d7..d64a05a4f4d1 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).
>
> @@ -31,59 +47,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,
If this never overflows by invariant, would it make sense to use
`unchecked_sub` or `wraping_sub`? That would remove the overflow check.
Best regards,
Andreas Hindborg
On Sat, 22 Mar 2025 14:58:16 +0100
Andreas Hindborg <a.hindborg@kernel.org> wrote:
> FUJITA Tomonori <fujita.tomonori@gmail.com> writes:
>
>> 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
>>
>> Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
>> Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
>> Reviewed-by: Gary Guo <gary@garyguo.net>
>> Reviewed-by: Fiona Behrens <me@kloenk.dev>
>> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
>
>
> Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
>
>
> As Boqun mentioned, we should make this generic over `ClockId` when the
> hrtimer patches land.
Seems that I overlooked his mail. Can you give me a pointer?
I assume that you want the Instance type to vary depending on the
clock source.
>> -/// 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,
>
> If this never overflows by invariant, would it make sense to use
> `unchecked_sub` or `wraping_sub`? That would remove the overflow check.
Yeah, I think that it can. But I prefer to keep the code as is to
catch a bug.
"FUJITA Tomonori" <fujita.tomonori@gmail.com> writes: > On Sat, 22 Mar 2025 14:58:16 +0100 > Andreas Hindborg <a.hindborg@kernel.org> wrote: > >> FUJITA Tomonori <fujita.tomonori@gmail.com> writes: >> >>> 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 >>> >>> Tested-by: Daniel Almeida <daniel.almeida@collabora.com> >>> Reviewed-by: Boqun Feng <boqun.feng@gmail.com> >>> Reviewed-by: Gary Guo <gary@garyguo.net> >>> Reviewed-by: Fiona Behrens <me@kloenk.dev> >>> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com> >> >> >> Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org> >> >> >> As Boqun mentioned, we should make this generic over `ClockId` when the >> hrtimer patches land. > > Seems that I overlooked his mail. Can you give me a pointer? > > I assume that you want the Instance type to vary depending on the > clock source. Yea, basically it is only okay to subtract instants if they are derived from the same clock source. Boqun suggested here [1] before hrtimer patches landed I think. At any rate, now we have `kernel::time::ClockId`. It is an enum though, so I am not sure how to go about it in practice. But we would want `Instant<RealTime> - Instant<BootTime>` to give a compiler error. Best regards, Andreas Hindborg [1] https://lore.kernel.org/all/ZxwFyl0xIje5gv7J@Boquns-Mac-mini.local
On Thu, 03 Apr 2025 12:41:52 +0200 Andreas Hindborg <a.hindborg@kernel.org> wrote: >>> As Boqun mentioned, we should make this generic over `ClockId` when the >>> hrtimer patches land. >> >> Seems that I overlooked his mail. Can you give me a pointer? >> >> I assume that you want the Instance type to vary depending on the >> clock source. > > Yea, basically it is only okay to subtract instants if they are derived > from the same clock source. Boqun suggested here [1] before hrtimer > patches landed I think. > > At any rate, now we have `kernel::time::ClockId`. It is an enum though, > so I am not sure how to go about it in practice. But we would want > `Instant<RealTime> - Instant<BootTime>` to give a compiler error. Understood, thanks! If we need a compile error for this, I don't think that the enum works; I guess that we need a distinct type for each of RealTime, BootTime, and so on. Once the current patchset is merged, I'll work on that.
© 2016 - 2025 Red Hat, Inc.