Messages in this thread Patch in this message | | | Date | Wed, 27 Mar 2024 10:49:49 -0700 | From | Boqun Feng <> | Subject | Re: [PATCH 3/5] rust: time: Introduce clock reading framework |
| |
On Wed, Mar 27, 2024 at 12:50:50PM +0000, Benno Lossin wrote: > On 26.03.24 20:19, Boqun Feng wrote: > > On Tue, Mar 26, 2024 at 05:00:39PM +0000, Benno Lossin wrote: > >> On 24.03.24 23:33, Boqun Feng wrote: > >>> To make sure Rust code doesn't mix timestamps from different clocks, a > >>> type safe clock reading framework is introduced. It includes: > >>> > >>> * A `Clock` trait that represents different clocks, to read a particular > >>> clock, one needs implement the `Clock::now()` function. > >>> > >>> * A `Instant<Clock>` type that represents timestamps of different > >>> clocks, whose implementation is just a `ktime_t`, so all the > >>> calculation on `ktime_t` should apply to it as well. > >>> > >>> Co-developed-by: Heghedus Razvan <heghedus.razvan@protonmail.com> > >>> Signed-off-by: Heghedus Razvan <heghedus.razvan@protonmail.com> > >>> Co-developed-by: Asahi Lina <lina@asahilina.net> > >>> Signed-off-by: Asahi Lina <lina@asahilina.net> > >>> Signed-off-by: Boqun Feng <boqun.feng@gmail.com> > >>> --- > >>> rust/kernel/time.rs | 49 +++++++++++++++++++++++++++++++++++++++++++++ > >>> 1 file changed, 49 insertions(+) > >>> > >>> diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs > >>> index b238b3a4e899..0f9f5605ed48 100644 > >>> --- a/rust/kernel/time.rs > >>> +++ b/rust/kernel/time.rs > >>> @@ -6,6 +6,9 @@ > >>> //! 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 core::marker::PhantomData; > >>> > >>> /// The number of nanoseconds per millisecond. > >>> pub const NSEC_PER_MSEC: i64 = bindings::NSEC_PER_MSEC as i64; > >>> @@ -64,3 +67,49 @@ pub fn to_ns(self) -> i64 { > >>> self.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 {} > >> > >> Why is this trait not an extension of `Clock`? > >> > > > > This was carried over from the old version, for myself, it doesn't make > > much difference between: > > > > trait A { .. } > > trait B { .. } > > > > impl<T: A + B> ... > > > > vs > > > > trait A { .. } > > trait B: A { .. } > > > > impl<T: B> ... > > > > hence I kept it as it is, but yes, it a `Monotonic` *`Clock`*, so I will > > change it in the next version. > > I think it would also make sense to rename it to `MonotonicClock`. > > > > >>> + > >>> +/// A timestamp of a given [`Clock`] > >> > >> Missing '.'. > >> > > > > Fixed locally. > > > >>> +#[repr(transparent)] > >>> +#[derive(Debug)] > >>> +pub struct Instant<T: Clock> { > >>> + inner: bindings::ktime_t, > >>> + clock: 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: bindings::ktime_t) -> Self { > >> > >> When compiling, this function is marked as dead-code in this patch. > >> > > > > Hmm... I cannot trigger any compile errors with this patch. If you see > > an error, could you share the build command? It's not a `pub` function > > though. > > I created a fresh config using `menuconfig` and enabled CONFIG_RUST. > Then I get this error: > > error: associated function `new` is never used > --> rust/kernel/time.rs:97:8 > | > 96 | impl<T: Clock> Instant<T> { > | ------------------------- associated function in this implementation > 97 | fn new(ktime: bindings::ktime_t) -> Self { > | ^^^ > | > = note: `-D dead-code` implied by `-D warnings` > = help: to override `-D warnings` add `#[allow(dead_code)]` > > error: aborting due to 1 previous error > > Note that the error vanishes in the next patch, since there you do have > a user for `new`. >
Seems I didn't select CONFIG_WERROR=y, so I missed it as a warning. and I can hit it now with that config. Will fix it in the next version. Thanks!
Regards
> -- > Cheers, > Benno
| |