Message-ID: <Y/UgWqgRz2QxUT+t@boqun-archlinux>
Date:   Tue, 21 Feb 2023 11:49:46 -0800
From:   Boqun Feng <boqun.feng@...il.com>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     Asahi Lina <lina@...hilina.net>, Miguel Ojeda <ojeda@...nel.org>,
        Alex Gaynor <alex.gaynor@...il.com>,
        Wedson Almeida Filho <wedsonaf@...il.com>,
        Gary Guo <gary@...yguo.net>,
        Björn Roy Baron <bjorn3_gh@...tonmail.com>,
        John Stultz <jstultz@...gle.com>,
        Stephen Boyd <sboyd@...nel.org>, linux-kernel@...r.kernel.org,
        rust-for-linux@...r.kernel.org, asahi@...ts.linux.dev
Subject: Re: [PATCH] rust: time: New module for timekeeping functions

On Tue, Feb 21, 2023 at 08:00:52PM +0100, Thomas Gleixner wrote:
[...]
> > I figured we might get a better idea for what to do once a
> > second user comes along. For example, do we want straight methods like
> > that or std::ops trait impls? And do we make everything fallible or
> > panic on overflow or just wrap?
> >
> > It also means we basically have to reimplement all of
> > core::time::Duration if we want to offer an equally ergonomic API with a
> > 64-bit type (for reference, Duration is a struct with u64 secs and u32
> > nanoseconds).
> 
> As you said yourself: The kernel can't use Rust std lib. So you better
> implement sensible interfaces which are straight forward and simple to
> use in the context you are aiming for.
> 

Agreed!

Lina, my suggestion is just to go ahead and add the minimal timestamp
abstraction, surely you may make some bad decisions about APIs (e.g.
panic vs returning a Result), but kernel doesn't have a stable internal
API, so we can always fix things later.

Myself actually do something like below based on your patch, because
nothing like `now()` ;-)

I'm sure you can do better because as you said, you're the first user
;-)

Regards,
Boqun

---------->8
diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
index 02844db47d34..3398388de0e1 100644
--- a/rust/kernel/time.rs
+++ b/rust/kernel/time.rs
@@ -6,16 +6,61 @@
 //! C header: [`include/linux/timekeeping.h`](../../../../include/linux/timekeeping.h)
 
 use crate::bindings;
-use core::time::Duration;
+// Re-exports [`Duration`], so that it's easy to provide kernel's own implemention in the future.
+pub use core::time::Duration;
+
+/// A timestamp
+pub trait TimeStamp: Copy {
+    /// Gets the current stamp.
+    fn now() -> Self;
+
+    /// Calculates the passed duration since `another`.
+    fn duration_since(&self, another: Self) -> Duration;
+
+
+    /// Return the duration passed since this stamp was created.
+    fn elapsed(&self) -> Duration {
+        let created = self.clone();
+        self.duration_since(created)
+    }
+}
+
+/// CLOCK_MONOTONIC timestamps.
+#[derive(Clone, Copy)]
+pub struct MonoTime(Duration);
+
+impl TimeStamp for MonoTime {
+    fn now() -> Self {
+        Self(ktime_get())
+    }
+
+    fn duration_since(&self, another: Self) -> Duration {
+        ktime_get() - another.0
+    }
+}
+
+/// CLOCK_BOOTTIME timestamps.
+#[derive(Clone, Copy)]
+pub struct BootTime(Duration);
+
+impl TimeStamp for BootTime {
+    fn now() -> Self {
+        Self(ktime_get_boottime())
+    }
+
+    fn duration_since(&self, another: Self) -> Duration {
+        ktime_get_boottime() - another.0
+    }
+}
 
 /// Returns the kernel time elapsed since boot, excluding time spent sleeping, as a [`Duration`].
-pub fn ktime_get() -> Duration {
+fn ktime_get() -> Duration {
     // SAFETY: Function has no side effects and no inputs.
     Duration::from_nanos(unsafe { bindings::ktime_get() }.try_into().unwrap())
 }
 
 /// Returns the kernel time elapsed since boot, including time spent sleeping, as a [`Duration`].
-pub fn ktime_get_boottime() -> Duration {
+fn ktime_get_boottime() -> Duration {
     Duration::from_nanos(
         // SAFETY: Function has no side effects and no variable inputs.
         unsafe { bindings::ktime_get_with_offset(bindings::tk_offsets_TK_OFFS_BOOT) }

Powered by blists - more mailing lists