[PATCH] rust: time: add Ktime

[PATCH] 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, see [2].

Link: https://lore.kernel.org/lkml/5ac8c0d09392290be789423f0dd78a520b830fab.1682333709.git.zhangchuang3@xiaomi.com/ [1]
Link: https://r.android.com/3004103
Signed-off-by: Alice Ryhl <aliceryhl@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..ac8b35f662af 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`.
+        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] rust: time: add Ktime

[Thomas Gleixner]

On Wed, Mar 20 2024 at 10:08, Alice Ryhl wrote:
> +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`.

That's not entirely correct. ktime_get() cannot be safely invoked from
NMI context. That won't matter for driver writers obviously.

Looks sensible otherwise.

Thanks,

        tglx

Re: [PATCH] rust: time: add Ktime

[Alice Ryhl]

On Wed, Mar 20, 2024 at 2:18 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> On Wed, Mar 20 2024 at 10:08, Alice Ryhl wrote:
> > +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`.
>
> That's not entirely correct. ktime_get() cannot be safely invoked from
> NMI context. That won't matter for driver writers obviously.
>
> Looks sensible otherwise.

Thanks for pointing that out. I will send a v2 with a reworded comment soon.

Alice

Re: [PATCH] rust: time: add Ktime

[Boqun Feng]

Hi,

On Wed, Mar 20, 2024 at 10:08:45AM +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].
> 

I wonder whether Lina's previous patch also works for your case?

	https://lore.kernel.org/rust-for-linux/20230714-rust-time-v2-1-f5aed84218c4@asahilina.net/

You don't need to implement all the `Clock`s since you only need
MONOTONIC time. But maybe `Duration::as_nanos` and `Duration::as_millis`
return `u128` is problematic?

Regards,
Boqun

> For a usage example, see [2].
> 
> Link: https://lore.kernel.org/lkml/5ac8c0d09392290be789423f0dd78a520b830fab.1682333709.git.zhangchuang3@xiaomi.com/ [1]
> Link: https://r.android.com/3004103
> Signed-off-by: Alice Ryhl <aliceryhl@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..ac8b35f662af 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`.
> +        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] rust: time: add Ktime

[Alice Ryhl]

On Wed, Mar 20, 2024 at 4:30 PM Boqun Feng <boqun.feng@gmail.com> wrote:
>
> Hi,
>
> On Wed, Mar 20, 2024 at 10:08:45AM +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].
> >
>
> I wonder whether Lina's previous patch also works for your case?
>
>         https://lore.kernel.org/rust-for-linux/20230714-rust-time-v2-1-f5aed84218c4@asahilina.net/
>
> You don't need to implement all the `Clock`s since you only need
> MONOTONIC time. But maybe `Duration::as_nanos` and `Duration::as_millis`
> return `u128` is problematic?

It would work for me too, but adds more than what I need. As for
`u128`, I don't really know what the status on that is. I need to be
able to print it as a base-10 integer.

Adding an API with separate types that distinguish between Instant and
Duration and different clock sources so that you don't mix them up is
reasonable, but a bit overkill for my needs. I am spending enough API
design cycles on my uaccess, file, and linked list patchsets. Someone
else can submit patches to add a more well-typed time API in the
future.

Alice

Re: [PATCH] rust: time: add Ktime

[Boqun Feng]

On Fri, Mar 22, 2024 at 08:50:18AM +0100, Alice Ryhl wrote:
> On Wed, Mar 20, 2024 at 4:30 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> >
> > Hi,
> >
> > On Wed, Mar 20, 2024 at 10:08:45AM +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].
> > >
> >
> > I wonder whether Lina's previous patch also works for your case?
> >
> >         https://lore.kernel.org/rust-for-linux/20230714-rust-time-v2-1-f5aed84218c4@asahilina.net/
> >
> > You don't need to implement all the `Clock`s since you only need
> > MONOTONIC time. But maybe `Duration::as_nanos` and `Duration::as_millis`
> > return `u128` is problematic?
> 
> It would work for me too, but adds more than what I need. As for
> `u128`, I don't really know what the status on that is. I need to be
> able to print it as a base-10 integer.
> 
> Adding an API with separate types that distinguish between Instant and
> Duration and different clock sources so that you don't mix them up is
> reasonable, but a bit overkill for my needs. I am spending enough API
> design cycles on my uaccess, file, and linked list patchsets. Someone

I totally understand that! But the requirement was brought up a while
ago by Thomas:

	https://lore.kernel.org/rust-for-linux/87h6vfnh0f.ffs@tglx/

> else can submit patches to add a more well-typed time API in the
> future.
> 

Would it work, if I create a new patch based on yours? The work for me
won't be too much since you, Lina and Heghedus already did the hard work
along with some feedback from Thomas.

Regards,
Boqun

> Alice