[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230714100525.2192448-1-aliceryhl@google.com>
Date: Fri, 14 Jul 2023 10:05:25 +0000
From: Alice Ryhl <aliceryhl@...gle.com>
To: lina@...hilina.net
Cc: alex.gaynor@...il.com, asahi@...ts.linux.dev,
bjorn3_gh@...tonmail.com, boqun.feng@...il.com, gary@...yguo.net,
gbs@...ishe.com, heghedus.razvan@...tonmail.com,
jistone@...hat.com, 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
Subject: Re: [PATCH v2] rust: time: New module for timekeeping functions
Asahi Lina <lina@...hilina.net> writes:
> +/// Marker trait for clock sources that represent a calendar (wall clock)
> +/// relative to the UNIX epoch.
> +pub trait WallTime {}
What's the purpose of this trait? Perhaps it should have a method to get
the UNIX epoch as an Instant?
> + /// Returns the time elapsed since an earlier Instant<t>, or
> + /// None if the argument is a later Instant.
> + pub fn since(&self, earlier: Instant<T>) -> Option<Duration> {
> + if earlier.nanoseconds > self.nanoseconds {
> + None
> + } else {
> + // Casting to u64 and subtracting is guaranteed to give the right
> + // result for all inputs, as long as the condition we checked above
> + // holds.
> + Some(Duration::from_nanos(
> + self.nanoseconds as u64 - earlier.nanoseconds as u64,
> + ))
It looks like you intend to use wrapping semantics for this subtraction
so that self=1,earlier=-1 results in a difference of two.
In that case, you should explicitly use `.wrapping_sub` instead to
convey your intent.
I guess you could also use `abs_diff`, which takes two i64s and returns
an u64.
> +/// Contains the various clock source types available to the kernel.
> +pub mod clock {
> + use super::*;
> +
> + /// A clock representing the default kernel time source.
> + ///
> + /// This is `CLOCK_MONOTONIC` (though it is not the only
> + /// monotonic clock) and also the default clock used by
> + /// `ktime_get()` in the C API.
> + ///
> + /// This is like `BootTime`, but does not include time
> + /// spent sleeping.
> +
> + pub struct KernelTime;
> +
> + impl Clock for KernelTime {}
> + impl Monotonic for KernelTime {}
> + impl Now for KernelTime {
> + fn now() -> Instant<Self> {
> + Instant::<Self>::new(unsafe { bindings::ktime_get() })
> + }
> + }
We usually add a SAFETY comment even if it is trivial.
// SAFETY: Just an FFI call without any safety requirements.
Alice
Powered by blists - more mailing lists