All of lore.kernel.org
 help / color / mirror / Atom feed
From: Boqun Feng <boqun.feng@gmail.com>
To: Alice Ryhl <aliceryhl@google.com>
Cc: benno.lossin@proton.me, a.hindborg@samsung.com,
	alex.gaynor@gmail.com, bjorn3_gh@protonmail.com,
	gary@garyguo.net, jstultz@google.com,
	linux-kernel@vger.kernel.org, ojeda@kernel.org,
	rust-for-linux@vger.kernel.org, sboyd@kernel.org,
	tglx@linutronix.de, wedsonaf@gmail.com, lina@asahilina.net,
	heghedus.razvan@protonmail.com
Subject: Re: [PATCH v2] rust: time: add Ktime
Date: Fri, 22 Mar 2024 08:32:26 -0700	[thread overview]
Message-ID: <Zf2kio8NYG5DEgyY@tardis> (raw)
In-Reply-To: <20240322101803.400735-1-aliceryhl@google.com>

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


  reply	other threads:[~2024-03-22 15:32 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-22  8:59 [PATCH v2] rust: time: add Ktime Alice Ryhl
2024-03-22  9:56 ` Benno Lossin
2024-03-22 10:18   ` Alice Ryhl
2024-03-22 15:32     ` Boqun Feng [this message]
2024-03-24  9:40       ` Valentin Obst
2024-03-24 20:52         ` Boqun Feng
2024-04-10 16:57 ` Thomas Gleixner
2024-04-11 15:39   ` Miguel Ojeda
2024-04-11 15:56 ` Boqun Feng
2024-04-11 16:21   ` Alice Ryhl
2024-04-11 18:19     ` Boqun Feng
2024-04-11 21:29 ` [tip: timers/core] rust: time: Add Ktime tip-bot2 for Alice Ryhl

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Zf2kio8NYG5DEgyY@tardis \
    --to=boqun.feng@gmail.com \
    --cc=a.hindborg@samsung.com \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=benno.lossin@proton.me \
    --cc=bjorn3_gh@protonmail.com \
    --cc=gary@garyguo.net \
    --cc=heghedus.razvan@protonmail.com \
    --cc=jstultz@google.com \
    --cc=lina@asahilina.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=sboyd@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=wedsonaf@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.