[PATCH v2] rust: time: add Ktime

[PATCH v2] rust: time: add Ktime

[Alice Ryhl]

Introduce a wrapper around `ktime_t` with a few different useful
methods.

Rust Binder will use these bindings to compute how many milliseconds a
transaction has been active for when dumping the current state of the
Binder driver. This replicates the logic in C Binder [1].

For a usage example in Rust Binder, see [2].

The `ktime_get` method cannot be safely called in NMI context. This
requirement is not checked by these abstractions, but it is intended
that klint [3] or a similar tool will be used to check it in the future.

Link: https://lore.kernel.org/lkml/5ac8c0d09392290be789423f0dd78a520b830fab.1682333709.git.zhangchuang3@xiaomi.com/ [1]
Link: https://r.android.com/3004103 [2]
Link: https://rust-for-linux.com/klint [3]
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
Changes in v2:
- Mention that ktime_get cannot be safely called in NMI context.
- Link to v1: https://lore.kernel.org/r/20240320-rust-ktime_ms_delta-v1-1-ccb8672a0941@google.com
---
 rust/kernel/time.rs | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
index 25a896eed468..6811d5cadbd4 100644
--- a/rust/kernel/time.rs
+++ b/rust/kernel/time.rs
@@ -5,6 +5,9 @@
 //! 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.
 
+/// The number of nanoseconds per millisecond.
+pub const NSEC_PER_MSEC: i64 = bindings::NSEC_PER_MSEC as i64;
+
 /// The time unit of Linux kernel. One jiffy equals (1/HZ) second.
 pub type Jiffies = core::ffi::c_ulong;
 
@@ -18,3 +21,60 @@ pub fn msecs_to_jiffies(msecs: Msecs) -> Jiffies {
     // matter what the argument is.
     unsafe { bindings::__msecs_to_jiffies(msecs) }
 }
+
+/// A Rust wrapper around a `ktime_t`.
+#[repr(transparent)]
+#[derive(Copy, Clone)]
+pub struct Ktime {
+    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 }
+    }
+
+    /// 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
+    }
+
+    /// 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;
+
+    #[inline]
+    fn sub(self, other: Ktime) -> Ktime {
+        Self {
+            inner: self.inner - other.inner,
+        }
+    }
+}

---
base-commit: 768409cff6cc89fe1194da880537a09857b6e4db
change-id: 20240320-rust-ktime_ms_delta-74b00c9ab872

Best regards,
-- 
Alice Ryhl <aliceryhl@google.com>

Re: [PATCH v2] rust: time: add Ktime

[Benno Lossin]

On 3/22/24 09:59, Alice Ryhl wrote:
> Introduce a wrapper around `ktime_t` with a few different useful
> methods.
> 
> Rust Binder will use these bindings to compute how many milliseconds a
> transaction has been active for when dumping the current state of the
> Binder driver. This replicates the logic in C Binder [1].
> 
> For a usage example in Rust Binder, see [2].
> 
> The `ktime_get` method cannot be safely called in NMI context. This
> requirement is not checked by these abstractions, but it is intended
> that klint [3] or a similar tool will be used to check it in the future.
> 
> Link: https://lore.kernel.org/lkml/5ac8c0d09392290be789423f0dd78a520b830fab.1682333709.git.zhangchuang3@xiaomi.com/ [1]
> Link: https://r.android.com/3004103 [2]
> Link: https://rust-for-linux.com/klint [3]
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>

I have one comment below, I don't mind leaving it as-is:

Reviewed-by: Benno Lossin <benno.lossin@proton.me>

> ---
> Changes in v2:
> - Mention that ktime_get cannot be safely called in NMI context.
> - Link to v1: https://lore.kernel.org/r/20240320-rust-ktime_ms_delta-v1-1-ccb8672a0941@google.com
> ---
>   rust/kernel/time.rs | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 60 insertions(+)
> 
> diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
> index 25a896eed468..6811d5cadbd4 100644
> --- a/rust/kernel/time.rs
> +++ b/rust/kernel/time.rs
> @@ -5,6 +5,9 @@
>   //! 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.
> 
> +/// The number of nanoseconds per millisecond.
> +pub const NSEC_PER_MSEC: i64 = bindings::NSEC_PER_MSEC as i64;
> +
>   /// The time unit of Linux kernel. One jiffy equals (1/HZ) second.
>   pub type Jiffies = core::ffi::c_ulong;
> 
> @@ -18,3 +21,60 @@ pub fn msecs_to_jiffies(msecs: Msecs) -> Jiffies {
>       // matter what the argument is.
>       unsafe { bindings::__msecs_to_jiffies(msecs) }
>   }
> +
> +/// A Rust wrapper around a `ktime_t`.
> +#[repr(transparent)]
> +#[derive(Copy, Clone)]
> +pub struct Ktime {
> +    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 }
> +    }
> +
> +    /// 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
> +    }
> +
> +    /// 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()
> +}

Is there a reason for this function being standalone?

-- 
Cheers,
Benno

> +
> +impl core::ops::Sub for Ktime {
> +    type Output = Ktime;
> +
> +    #[inline]
> +    fn sub(self, other: Ktime) -> Ktime {
> +        Self {
> +            inner: self.inner - other.inner,
> +        }
> +    }
> +}
> 
> ---
> base-commit: 768409cff6cc89fe1194da880537a09857b6e4db
> change-id: 20240320-rust-ktime_ms_delta-74b00c9ab872
> 
> Best regards,
> --
> Alice Ryhl <aliceryhl@google.com>
> 

Re: [PATCH v2] rust: time: add Ktime

[Alice Ryhl]

Benno Lossin <benno.lossin@proton.me> wrote:
> On 3/22/24 09:59, Alice Ryhl wrote:
>> +/// Returns the number of milliseconds between two ktimes.
>> +#[inline]
>> +pub fn ktime_ms_delta(later: Ktime, earlier: Ktime) -> i64 {
>> +    (later - earlier).to_ms()
>> +}
> 
> Is there a reason for this function being standalone?

I think for a Rust time API, we should make one of two choices:

* Match the C ktime_t API as closely as possible.
* Match the Rust standard library std::time API as closely as possible.

This patchset has made the former choice, and that is why I went with
this design.

In the future it could make sense to add a more "Rusty" API, but even
then I think it could make sense to have both and implement the latter
in terms of the former. That way, only the API that closely matches the
C ktime_t API needs to concern itself with unsafely calling into C.

Alice

Re: [PATCH v2] rust: time: add Ktime

[Boqun Feng]

On Fri, Mar 22, 2024 at 10:18:02AM +0000, Alice Ryhl wrote:
> Benno Lossin <benno.lossin@proton.me> wrote:
> > On 3/22/24 09:59, Alice Ryhl wrote:
> >> +/// Returns the number of milliseconds between two ktimes.
> >> +#[inline]
> >> +pub fn ktime_ms_delta(later: Ktime, earlier: Ktime) -> i64 {
> >> +    (later - earlier).to_ms()
> >> +}
> > 
> > Is there a reason for this function being standalone?
> 
> I think for a Rust time API, we should make one of two choices:
> 
> * Match the C ktime_t API as closely as possible.
> * Match the Rust standard library std::time API as closely as possible.
> 
> This patchset has made the former choice, and that is why I went with
> this design.
> 
> In the future it could make sense to add a more "Rusty" API, but even
> then I think it could make sense to have both and implement the latter
> in terms of the former. That way, only the API that closely matches the
> C ktime_t API needs to concern itself with unsafely calling into C.
> 

So I create the following one based on this patch and the previous we
have. I changed the title a bit, did a s/Ktime/KTime and add the part of
`Instant`, please take a look, I think the binder usage is still
covered.

Benno, I dropped your Reviewed-by since the patch has been changed.
Please take annother look.

Thomas, I tried to resolve a few comments you had for the previous
version, please let me know whether this version looks OK to you.

Regards,
Boqun

---------------------------->8
Subject: [PATCH] rust: time: Add clock source reading functionality

Introduce wrappers around `ktime_t` with a time duration type `KTime`
and a timestamp type `Instant`.

Rust Binder will use these bindings to compute how many milliseconds a
transaction has been active for when dumping the current state of the
Binder driver. This replicates the logic in C Binder [1].

For a usage example in Rust Binder, see [2].

The `ktime_get` method cannot be safely called in NMI context. This
requirement is not checked by these abstractions, but it is intended
that klint [3] or a similar tool will be used to check it in the future.

Link: https://lore.kernel.org/lkml/5ac8c0d09392290be789423f0dd78a520b830fab.1682333709.git.zhangchuang3@xiaomi.com/ [1]
Link: https://r.android.com/3004103 [2]
Link: https://rust-for-linux.com/klint [3]
Originally-by: Heghedus Razvan <heghedus.razvan@protonmail.com>
Originally-by: Asahi Lina <lina@asahilina.net>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
---
 rust/kernel/time.rs | 158 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 158 insertions(+)

diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
index 25a896eed468..50cc063aa9b4 100644
--- a/rust/kernel/time.rs
+++ b/rust/kernel/time.rs
@@ -4,6 +4,15 @@
 //!
 //! 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.
+//!
+//! C header: [`include/linux/jiffies.h`](srctree/include/linux/jiffies.h).
+//! C header: [`include/linux/ktime.h`](srctree/include/linux/ktime.h).
+
+use crate::pr_err;
+use core::marker::PhantomData;
+
+/// The number of nanoseconds per millisecond.
+pub const NSEC_PER_MSEC: i64 = bindings::NSEC_PER_MSEC as i64;
 
 /// The time unit of Linux kernel. One jiffy equals (1/HZ) second.
 pub type Jiffies = core::ffi::c_ulong;
@@ -18,3 +27,152 @@ pub fn msecs_to_jiffies(msecs: Msecs) -> Jiffies {
     // matter what the argument is.
     unsafe { bindings::__msecs_to_jiffies(msecs) }
 }
+
+/// A kernel time duration.
+///
+/// This type basically wraps the `ktime_t` with one restriction: it should only be used for
+/// representing a time duration, in other words, it's not the type for timestamps.
+#[repr(transparent)]
+#[derive(Debug, Copy, Clone, PartialEq, PartialOrd)]
+pub struct KTime {
+    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 }
+    }
+
+    /// 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
+    }
+
+    /// Returns the number of milliseconds.
+    #[inline]
+    pub fn to_ms(self) -> i64 {
+        self.divns_constant::<NSEC_PER_MSEC>()
+    }
+}
+
+impl core::ops::Sub for KTime {
+    type Output = KTime;
+
+    #[inline]
+    fn sub(self, other: KTime) -> KTime {
+        Self {
+            inner: self.inner - other.inner,
+        }
+    }
+}
+
+/// Represents a clock, that is, a unique time source and it can be queried for the current time.
+pub trait Clock: Sized {
+    /// Returns the current time for this clock.
+    fn now() -> Instant<Self>;
+}
+
+/// Marker trait for clock sources that are guaranteed to be monotonic.
+pub trait Monotonic {}
+
+/// An instant in time associated with a given clock source.
+#[derive(Debug)]
+pub struct Instant<T: Clock> {
+    ktime: KTime,
+    _type: PhantomData<T>,
+}
+
+impl<T: Clock> Clone for Instant<T> {
+    fn clone(&self) -> Self {
+        *self
+    }
+}
+
+impl<T: Clock> Copy for Instant<T> {}
+
+impl<T: Clock> Instant<T> {
+    fn new(ktime: KTime) -> Self {
+        Instant {
+            ktime,
+            _type: PhantomData,
+        }
+    }
+
+    /// Returns the time elapsed since an earlier [`Instant`], or None if the argument is a later
+    /// Instant.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use kernel::time::{Clock, clock::KernelTime};
+    ///
+    /// let a = KernelTime::now();
+    /// let b = KernelTime::now();
+    ///
+    /// // `KernelTime` is monotonic.
+    /// assert_eq!(a.since(b), None);
+    /// assert_eq!(b.since(a).map(|d| d.to_ns() >= 0), Some(true));
+    ///
+    /// ```
+    pub fn since(&self, earlier: Instant<T>) -> Option<KTime> {
+        if self.ktime < earlier.ktime {
+            None
+        } else {
+            Some(self.ktime - earlier.ktime)
+        }
+    }
+}
+
+impl<T: Clock + Monotonic> Instant<T> {
+    /// Returns the time elapsed since this [`Instant`].
+    ///
+    /// This is guaranteed to return a non-negative result, since it is only implemented for
+    /// monotonic clocks.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use kernel::time::{Clock, clock::KernelTime};
+    ///
+    /// let a = KernelTime::now();
+    ///
+    /// // `KernelTime` is monotonic.
+    /// assert!(a.elapsed().to_ns() >= 0);
+    ///
+    /// ```
+    pub fn elapsed(&self) -> KTime {
+        self.since(T::now()).unwrap_or_else(|| {
+            pr_err!(
+                "Monotonic clock {} went backwards!",
+                core::any::type_name::<T>()
+            );
+            KTime::from_raw(0)
+        })
+    }
+}
+
+/// Contains the various clock source types available to the kernel.
+pub mod clock {
+    use super::*;
+
+    /// A clock representing the default kernel time source (`CLOCK_MONOTONIC`).
+    pub struct KernelTime;
+
+    impl Monotonic for KernelTime {}
+    impl Clock for KernelTime {
+        #[inline]
+        fn now() -> Instant<Self> {
+            // SAFETY: It is always safe to call `ktime_get` outside of NMI context.
+            Instant::<Self>::new(KTime::from_raw(unsafe { bindings::ktime_get() }))
+        }
+    }
+}
-- 
2.43.0

Re: [PATCH v2] rust: time: add Ktime

[Valentin Obst]

> Subject: [PATCH] rust: time: Add clock source reading functionality
>
> Introduce wrappers around `ktime_t` with a time duration type `KTime`
> and a timestamp type `Instant`.
>
> Rust Binder will use these bindings to compute how many milliseconds a
> transaction has been active for when dumping the current state of the
> Binder driver. This replicates the logic in C Binder [1].
>
> For a usage example in Rust Binder, see [2].
>
> The `ktime_get` method cannot be safely called in NMI context. This
> requirement is not checked by these abstractions, but it is intended
> that klint [3] or a similar tool will be used to check it in the future.
>
> Link: https://lore.kernel.org/lkml/5ac8c0d09392290be789423f0dd78a520b830fab.1682333709.git.zhangchuang3@xiaomi.com/ [1]
> Link: https://r.android.com/3004103 [2]
> Link: https://rust-for-linux.com/klint [3]
> Originally-by: Heghedus Razvan <heghedus.razvan@protonmail.com>
> Originally-by: Asahi Lina <lina@asahilina.net>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> ---
>  rust/kernel/time.rs | 158 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 158 insertions(+)
>
> diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
> index 25a896eed468..50cc063aa9b4 100644
> --- a/rust/kernel/time.rs
> +++ b/rust/kernel/time.rs
> @@ -4,6 +4,15 @@
>  //!
>  //! 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.
> +//!
> +//! C header: [`include/linux/jiffies.h`](srctree/include/linux/jiffies.h).
> +//! C header: [`include/linux/ktime.h`](srctree/include/linux/ktime.h).
> +
> +use crate::pr_err;
> +use core::marker::PhantomData;
> +
> +/// The number of nanoseconds per millisecond.
> +pub const NSEC_PER_MSEC: i64 = bindings::NSEC_PER_MSEC as i64;
>
>  /// The time unit of Linux kernel. One jiffy equals (1/HZ) second.
>  pub type Jiffies = core::ffi::c_ulong;
> @@ -18,3 +27,152 @@ pub fn msecs_to_jiffies(msecs: Msecs) -> Jiffies {
>      // matter what the argument is.
>      unsafe { bindings::__msecs_to_jiffies(msecs) }
>  }
> +
> +/// A kernel time duration.
> +///
> +/// This type basically wraps the `ktime_t` with one restriction: it should only be used for
> +/// representing a time duration, in other words, it's not the type for timestamps.
> +#[repr(transparent)]
> +#[derive(Debug, Copy, Clone, PartialEq, PartialOrd)]
> +pub struct KTime {
> +    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 }
> +    }

Eventually we might want to be able to create instances of types that
represent durations in const contexts, e.g., for fixed thresholds or
fixed offsets to relative timers. Would it make sense to '/fn/const fn/'
for the `KTime` (or `Ktime`) methods that support it?

[For that use case the naming/signature `from_raw(inner: bindings::ktime_t)`
could maybe also be changed to something like `new(duration: i64)`, i.e.,
make it sound less like an internal API.]

	- Best Valentin

> +
> +    /// 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
> +    }
> +
> +    /// Returns the number of milliseconds.
> +    #[inline]
> +    pub fn to_ms(self) -> i64 {
> +        self.divns_constant::<NSEC_PER_MSEC>()
> +    }
> +}
> +
> +impl core::ops::Sub for KTime {
> +    type Output = KTime;
> +
> +    #[inline]
> +    fn sub(self, other: KTime) -> KTime {
> +        Self {
> +            inner: self.inner - other.inner,
> +        }
> +    }
> +}
> +
> +/// Represents a clock, that is, a unique time source and it can be queried for the current time.
> +pub trait Clock: Sized {
> +    /// Returns the current time for this clock.
> +    fn now() -> Instant<Self>;
> +}
> +
> +/// Marker trait for clock sources that are guaranteed to be monotonic.
> +pub trait Monotonic {}
> +
> +/// An instant in time associated with a given clock source.
> +#[derive(Debug)]
> +pub struct Instant<T: Clock> {
> +    ktime: KTime,
> +    _type: PhantomData<T>,
> +}
> +
> +impl<T: Clock> Clone for Instant<T> {
> +    fn clone(&self) -> Self {
> +        *self
> +    }
> +}
> +
> +impl<T: Clock> Copy for Instant<T> {}
> +
> +impl<T: Clock> Instant<T> {
> +    fn new(ktime: KTime) -> Self {
> +        Instant {
> +            ktime,
> +            _type: PhantomData,
> +        }
> +    }
> +
> +    /// Returns the time elapsed since an earlier [`Instant`], or None if the argument is a later
> +    /// Instant.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// use kernel::time::{Clock, clock::KernelTime};
> +    ///
> +    /// let a = KernelTime::now();
> +    /// let b = KernelTime::now();
> +    ///
> +    /// // `KernelTime` is monotonic.
> +    /// assert_eq!(a.since(b), None);
> +    /// assert_eq!(b.since(a).map(|d| d.to_ns() >= 0), Some(true));
> +    ///
> +    /// ```
> +    pub fn since(&self, earlier: Instant<T>) -> Option<KTime> {
> +        if self.ktime < earlier.ktime {
> +            None
> +        } else {
> +            Some(self.ktime - earlier.ktime)
> +        }
> +    }
> +}
> +
> +impl<T: Clock + Monotonic> Instant<T> {
> +    /// Returns the time elapsed since this [`Instant`].
> +    ///
> +    /// This is guaranteed to return a non-negative result, since it is only implemented for
> +    /// monotonic clocks.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// use kernel::time::{Clock, clock::KernelTime};
> +    ///
> +    /// let a = KernelTime::now();
> +    ///
> +    /// // `KernelTime` is monotonic.
> +    /// assert!(a.elapsed().to_ns() >= 0);
> +    ///
> +    /// ```
> +    pub fn elapsed(&self) -> KTime {
> +        self.since(T::now()).unwrap_or_else(|| {
> +            pr_err!(
> +                "Monotonic clock {} went backwards!",
> +                core::any::type_name::<T>()
> +            );
> +            KTime::from_raw(0)
> +        })
> +    }
> +}
> +
> +/// Contains the various clock source types available to the kernel.
> +pub mod clock {
> +    use super::*;
> +
> +    /// A clock representing the default kernel time source (`CLOCK_MONOTONIC`).
> +    pub struct KernelTime;
> +
> +    impl Monotonic for KernelTime {}

nit: blank line missing

> +    impl Clock for KernelTime {
> +        #[inline]
> +        fn now() -> Instant<Self> {
> +            // SAFETY: It is always safe to call `ktime_get` outside of NMI context.
> +            Instant::<Self>::new(KTime::from_raw(unsafe { bindings::ktime_get() }))
> +        }
> +    }
> +}

Re: [PATCH v2] rust: time: add Ktime

[Boqun Feng]

On Sun, Mar 24, 2024 at 10:40:23AM +0100, Valentin Obst wrote:
> > Subject: [PATCH] rust: time: Add clock source reading functionality
> >
> > Introduce wrappers around `ktime_t` with a time duration type `KTime`
> > and a timestamp type `Instant`.
> >
> > Rust Binder will use these bindings to compute how many milliseconds a
> > transaction has been active for when dumping the current state of the
> > Binder driver. This replicates the logic in C Binder [1].
> >
> > For a usage example in Rust Binder, see [2].
> >
> > The `ktime_get` method cannot be safely called in NMI context. This
> > requirement is not checked by these abstractions, but it is intended
> > that klint [3] or a similar tool will be used to check it in the future.
> >
> > Link: https://lore.kernel.org/lkml/5ac8c0d09392290be789423f0dd78a520b830fab.1682333709.git.zhangchuang3@xiaomi.com/ [1]
> > Link: https://r.android.com/3004103 [2]
> > Link: https://rust-for-linux.com/klint [3]
> > Originally-by: Heghedus Razvan <heghedus.razvan@protonmail.com>
> > Originally-by: Asahi Lina <lina@asahilina.net>
> > Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> > Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
> > ---
> >  rust/kernel/time.rs | 158 ++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 158 insertions(+)
> >
> > diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
> > index 25a896eed468..50cc063aa9b4 100644
> > --- a/rust/kernel/time.rs
> > +++ b/rust/kernel/time.rs
> > @@ -4,6 +4,15 @@
> >  //!
> >  //! 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.
> > +//!
> > +//! C header: [`include/linux/jiffies.h`](srctree/include/linux/jiffies.h).
> > +//! C header: [`include/linux/ktime.h`](srctree/include/linux/ktime.h).
> > +
> > +use crate::pr_err;
> > +use core::marker::PhantomData;
> > +
> > +/// The number of nanoseconds per millisecond.
> > +pub const NSEC_PER_MSEC: i64 = bindings::NSEC_PER_MSEC as i64;
> >
> >  /// The time unit of Linux kernel. One jiffy equals (1/HZ) second.
> >  pub type Jiffies = core::ffi::c_ulong;
> > @@ -18,3 +27,152 @@ pub fn msecs_to_jiffies(msecs: Msecs) -> Jiffies {
> >      // matter what the argument is.
> >      unsafe { bindings::__msecs_to_jiffies(msecs) }
> >  }
> > +
> > +/// A kernel time duration.
> > +///
> > +/// This type basically wraps the `ktime_t` with one restriction: it should only be used for
> > +/// representing a time duration, in other words, it's not the type for timestamps.
> > +#[repr(transparent)]
> > +#[derive(Debug, Copy, Clone, PartialEq, PartialOrd)]
> > +pub struct KTime {
> > +    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 }
> > +    }
> 
> Eventually we might want to be able to create instances of types that
> represent durations in const contexts, e.g., for fixed thresholds or
> fixed offsets to relative timers. Would it make sense to '/fn/const fn/'
> for the `KTime` (or `Ktime`) methods that support it?
> 

Yeah, that makes sense. Besides, I'm going to rename `KTime` as
`Duration`, since it really represents a duration of time, and...

> [For that use case the naming/signature `from_raw(inner: bindings::ktime_t)`
> could maybe also be changed to something like `new(duration: i64)`, i.e.,
> make it sound less like an internal API.]
> 

...it makes more sense for a `Duration::new()` instead of a
`KTime::from_raw()`. I will send an updated version soon, since I also
find a few problems:

> 	- Best Valentin
> 
> > +
> > +    /// 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
> > +    }
> > +
> > +    /// Returns the number of milliseconds.
> > +    #[inline]
> > +    pub fn to_ms(self) -> i64 {
> > +        self.divns_constant::<NSEC_PER_MSEC>()
> > +    }
> > +}
> > +
> > +impl core::ops::Sub for KTime {
> > +    type Output = KTime;
> > +
> > +    #[inline]
> > +    fn sub(self, other: KTime) -> KTime {
> > +        Self {
> > +            inner: self.inner - other.inner,

This doesn't handle the overflow cases.

> > +        }
> > +    }
> > +}
> > +
> > +/// Represents a clock, that is, a unique time source and it can be queried for the current time.
> > +pub trait Clock: Sized {
> > +    /// Returns the current time for this clock.
> > +    fn now() -> Instant<Self>;
> > +}
> > +
> > +/// Marker trait for clock sources that are guaranteed to be monotonic.
> > +pub trait Monotonic {}
> > +
> > +/// An instant in time associated with a given clock source.
> > +#[derive(Debug)]
> > +pub struct Instant<T: Clock> {
> > +    ktime: KTime,
> > +    _type: PhantomData<T>,
> > +}
> > +
> > +impl<T: Clock> Clone for Instant<T> {
> > +    fn clone(&self) -> Self {
> > +        *self
> > +    }
> > +}
> > +
> > +impl<T: Clock> Copy for Instant<T> {}
> > +
> > +impl<T: Clock> Instant<T> {
> > +    fn new(ktime: KTime) -> Self {
> > +        Instant {
> > +            ktime,
> > +            _type: PhantomData,
> > +        }
> > +    }
> > +
> > +    /// Returns the time elapsed since an earlier [`Instant`], or None if the argument is a later
> > +    /// Instant.
> > +    ///
> > +    /// # Examples
> > +    ///
> > +    /// ```
> > +    /// use kernel::time::{Clock, clock::KernelTime};
> > +    ///
> > +    /// let a = KernelTime::now();
> > +    /// let b = KernelTime::now();
> > +    ///
> > +    /// // `KernelTime` is monotonic.
> > +    /// assert_eq!(a.since(b), None);
> > +    /// assert_eq!(b.since(a).map(|d| d.to_ns() >= 0), Some(true));
> > +    ///
> > +    /// ```
> > +    pub fn since(&self, earlier: Instant<T>) -> Option<KTime> {
> > +        if self.ktime < earlier.ktime {
> > +            None
> > +        } else {
> > +            Some(self.ktime - earlier.ktime)
> > +        }
> > +    }
> > +}
> > +
> > +impl<T: Clock + Monotonic> Instant<T> {
> > +    /// Returns the time elapsed since this [`Instant`].
> > +    ///
> > +    /// This is guaranteed to return a non-negative result, since it is only implemented for
> > +    /// monotonic clocks.
> > +    ///

And this is not quite right, since if we implement `Add` trait for
`Instant` (`Instant` + `Duration`, i.e. the "wrapper" for
ktime_add_safe()), we could have an `Instant` of monotonic clocks that
points to the future (i.e. now() is earlier than `self`). I will remove
the `pr_err()` below (since it's not an error anymore). But the bound of
`Monotonic` will still be remained, since it's mostly a convenient
interface for monotonic clocks.

> > +    /// # Examples
> > +    ///
> > +    /// ```
> > +    /// use kernel::time::{Clock, clock::KernelTime};
> > +    ///
> > +    /// let a = KernelTime::now();
> > +    ///
> > +    /// // `KernelTime` is monotonic.
> > +    /// assert!(a.elapsed().to_ns() >= 0);
> > +    ///
> > +    /// ```
> > +    pub fn elapsed(&self) -> KTime {
> > +        self.since(T::now()).unwrap_or_else(|| {
> > +            pr_err!(
> > +                "Monotonic clock {} went backwards!",
> > +                core::any::type_name::<T>()
> > +            );
> > +            KTime::from_raw(0)
> > +        })
> > +    }
> > +}
> > +
> > +/// Contains the various clock source types available to the kernel.
> > +pub mod clock {
> > +    use super::*;
> > +
> > +    /// A clock representing the default kernel time source (`CLOCK_MONOTONIC`).
> > +    pub struct KernelTime;
> > +
> > +    impl Monotonic for KernelTime {}
> 
> nit: blank line missing
> 

Thanks, I will add it in a new version.

Regards,
Boqun

> > +    impl Clock for KernelTime {
> > +        #[inline]
> > +        fn now() -> Instant<Self> {
> > +            // SAFETY: It is always safe to call `ktime_get` outside of NMI context.
> > +            Instant::<Self>::new(KTime::from_raw(unsafe { bindings::ktime_get() }))
> > +        }
> > +    }
> > +}

Re: [PATCH v2] rust: time: add Ktime

[Thomas Gleixner]

On Fri, Mar 22 2024 at 08:59, Alice Ryhl wrote:
> Introduce a wrapper around `ktime_t` with a few different useful
> methods.
>
> Rust Binder will use these bindings to compute how many milliseconds a
> transaction has been active for when dumping the current state of the
> Binder driver. This replicates the logic in C Binder [1].
>
> For a usage example in Rust Binder, see [2].
>
> The `ktime_get` method cannot be safely called in NMI context. This
> requirement is not checked by these abstractions, but it is intended
> that klint [3] or a similar tool will be used to check it in the future.
>
> Link: https://lore.kernel.org/lkml/5ac8c0d09392290be789423f0dd78a520b830fab.1682333709.git.zhangchuang3@xiaomi.com/ [1]
> Link: https://r.android.com/3004103 [2]
> Link: https://rust-for-linux.com/klint [3]
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>

Reviewed-by: Thomas Gleixner <tglx@linutronix.de>

Re: [PATCH v2] rust: time: add Ktime

[Miguel Ojeda]

On Wed, Apr 10, 2024 at 6:57 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> Reviewed-by: Thomas Gleixner <tglx@linutronix.de>

Thanks Thomas -- if you pick it up through timers/core, please feel free to add:

Acked-by: Miguel Ojeda <ojeda@kernel.org>

Cheers,
Miguel

Re: [PATCH v2] rust: time: add Ktime

[Boqun Feng]

On Fri, Mar 22, 2024 at 08:59:38AM +0000, Alice Ryhl wrote:
> Introduce a wrapper around `ktime_t` with a few different useful
> methods.
> 
> Rust Binder will use these bindings to compute how many milliseconds a
> transaction has been active for when dumping the current state of the
> Binder driver. This replicates the logic in C Binder [1].
> 
> For a usage example in Rust Binder, see [2].
> 
> The `ktime_get` method cannot be safely called in NMI context. This
> requirement is not checked by these abstractions, but it is intended
> that klint [3] or a similar tool will be used to check it in the future.
> 
> Link: https://lore.kernel.org/lkml/5ac8c0d09392290be789423f0dd78a520b830fab.1682333709.git.zhangchuang3@xiaomi.com/ [1]
> Link: https://r.android.com/3004103 [2]
> Link: https://rust-for-linux.com/klint [3]
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> Changes in v2:
> - Mention that ktime_get cannot be safely called in NMI context.
> - Link to v1: https://lore.kernel.org/r/20240320-rust-ktime_ms_delta-v1-1-ccb8672a0941@google.com
> ---
>  rust/kernel/time.rs | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 60 insertions(+)
> 
> diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
> index 25a896eed468..6811d5cadbd4 100644
> --- a/rust/kernel/time.rs
> +++ b/rust/kernel/time.rs
> @@ -5,6 +5,9 @@
>  //! 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.
>  
> +/// The number of nanoseconds per millisecond.
> +pub const NSEC_PER_MSEC: i64 = bindings::NSEC_PER_MSEC as i64;
> +
>  /// The time unit of Linux kernel. One jiffy equals (1/HZ) second.
>  pub type Jiffies = core::ffi::c_ulong;
>  
> @@ -18,3 +21,60 @@ pub fn msecs_to_jiffies(msecs: Msecs) -> Jiffies {
>      // matter what the argument is.
>      unsafe { bindings::__msecs_to_jiffies(msecs) }
>  }
> +
> +/// A Rust wrapper around a `ktime_t`.
> +#[repr(transparent)]
> +#[derive(Copy, Clone)]
> +pub struct Ktime {
> +    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 }
> +    }
> +
> +    /// 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
> +    }
> +
> +    /// 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;
> +
> +    #[inline]
> +    fn sub(self, other: Ktime) -> Ktime {
> +        Self {
> +            inner: self.inner - other.inner,

Nit: although we use "Release mode" to compile Rust code in kernel, so
i64 substraction behaves as 2's complement wrap, I think it's better we
use wrapping_sub here:
		
        self.inner.wrapping_sub(other.inner)

however it's not a correctness issue for now, so with or without it,

Reviewed-by: Boqun Feng <boqun.feng@gmail.com>

Regards,
Boqun

> +        }
> +    }
> +}
> 
> ---
> base-commit: 768409cff6cc89fe1194da880537a09857b6e4db
> change-id: 20240320-rust-ktime_ms_delta-74b00c9ab872
> 
> Best regards,
> -- 
> Alice Ryhl <aliceryhl@google.com>
> 

Re: [PATCH v2] rust: time: add Ktime

[Alice Ryhl]

On Thu, Apr 11, 2024 at 5:57 PM Boqun Feng <boqun.feng@gmail.com> wrote:
>
> On Fri, Mar 22, 2024 at 08:59:38AM +0000, Alice Ryhl wrote:
> > +    /// 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;
> > +
> > +    #[inline]
> > +    fn sub(self, other: Ktime) -> Ktime {
> > +        Self {
> > +            inner: self.inner - other.inner,
>
> Nit: although we use "Release mode" to compile Rust code in kernel, so
> i64 substraction behaves as 2's complement wrap, I think it's better we
> use wrapping_sub here:
>
>         self.inner.wrapping_sub(other.inner)
>
> however it's not a correctness issue for now, so with or without it,

We enable overflow checks even on release mode right now. But I don't
understand this nit because we only have an overflow condition if the
two ktimes differ by more than 2^31, and if that happens then that's a
*legitimate* overflow that we would want to catch. Or is there
something I am missing?

Alice

Re: [PATCH v2] rust: time: add Ktime

[Boqun Feng]

On Thu, Apr 11, 2024 at 06:21:43PM +0200, Alice Ryhl wrote:
> On Thu, Apr 11, 2024 at 5:57 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> >
> > On Fri, Mar 22, 2024 at 08:59:38AM +0000, Alice Ryhl wrote:
> > > +    /// 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;
> > > +
> > > +    #[inline]
> > > +    fn sub(self, other: Ktime) -> Ktime {
> > > +        Self {
> > > +            inner: self.inner - other.inner,
> >
> > Nit: although we use "Release mode" to compile Rust code in kernel, so
> > i64 substraction behaves as 2's complement wrap, I think it's better we
> > use wrapping_sub here:
> >
> >         self.inner.wrapping_sub(other.inner)
> >
> > however it's not a correctness issue for now, so with or without it,
> 
> We enable overflow checks even on release mode right now. But I don't

Oh, I was missing that, then we actually have to skip the overflow
checking with wrapping_sub() to mirror what C side does, for performance
reasons and for avoiding panics.

Regards,
Boqun

> understand this nit because we only have an overflow condition if the
> two ktimes differ by more than 2^31, and if that happens then that's a
> *legitimate* overflow that we would want to catch. Or is there
> something I am missing?
> 
> Alice