Message-ID: <Zf2kio8NYG5DEgyY@tardis>
Date: Fri, 22 Mar 2024 08:32:26 -0700
From: Boqun Feng <boqun.feng@...il.com>
To: Alice Ryhl <aliceryhl@...gle.com>
Cc: benno.lossin@...ton.me, a.hindborg@...sung.com, alex.gaynor@...il.com,
	bjorn3_gh@...tonmail.com, gary@...yguo.net, jstultz@...gle.com,
	linux-kernel@...r.kernel.org, ojeda@...nel.org,
	rust-for-linux@...r.kernel.org, sboyd@...nel.org,
	tglx@...utronix.de, wedsonaf@...il.com, lina@...hilina.net,
	heghedus.razvan@...tonmail.com
Subject: Re: [PATCH v2] rust: time: add Ktime

On Fri, Mar 22, 2024 at 10:18:02AM +0000, Alice Ryhl wrote:
> Benno Lossin <benno.lossin@...ton.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@...tonmail.com>
Originally-by: Asahi Lina <lina@...hilina.net>
Signed-off-by: Alice Ryhl <aliceryhl@...gle.com>
Signed-off-by: Boqun Feng <boqun.feng@...il.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

Powered by blists - more mailing lists