[v2] rust: time: New module for timekeeping functions

Message ID 20230714-rust-time-v2-1-f5aed84218c4@asahilina.net
State New
Headers
Series [v2] rust: time: New module for timekeeping functions |

Commit Message

Asahi Lina July 14, 2023, 7:55 a.m. UTC
  This module is intended to contain functions related to kernel
timekeeping and time.

Initially, this implements an abstraction for a time Instant (analogous
to the Rust std::time::Instant) that represents an opaque instant in
time. Unlike the std Instant, this is a generic type that is bound to a
specific clock source, so that only Instants from the same clock source
can be subtracted/compared.

Then we implement the relevant clocks available to the kernel:
KernelTime (CLOCK_MONOTONIC), BootTime (CLOCK_BOOTTIME),
RealTime (CLOCK_REALTIME), and TaiTime.

Co-developed-by: Heghedus Razvan <heghedus.razvan@protonmail.com>
Signed-off-by: Asahi Lina <lina@asahilina.net>
---

Based on the feedback to v1, now we have proper type checking for kernel
time. I decided to implement marker traits for monotonic vs. wallclock
time sources, since it's useful to be able to safely implement different
semantics conditional on that, but that left me with a name conflict of
the Monotonic trait with the CLOCK_MONOTONIC / "default ktime" clock
source. I ended up calling it KernelTime since it's the most fundamental
kernel timesource, but suggestions welcome!

Heghedus: I think I need a signoff on this since this is based on the
playground demo you wrote in the feedback to v1. Can you provide that? I
can fold it into v3 (if there is one, otherwise Miguel can probably just
add it when he applies it). Thanks!
---
 rust/bindings/bindings_helper.h |   2 +
 rust/helpers.c                  |  16 +++++
 rust/kernel/lib.rs              |   1 +
 rust/kernel/time.rs             | 150 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 169 insertions(+)


---
base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5
change-id: 20230714-rust-time-10dd9ed25333

Thank you,
~~ Lina
  

Comments

Alice Ryhl July 14, 2023, 10:05 a.m. UTC | #1
Asahi Lina <lina@asahilina.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
  
Martin Rodriguez Reboredo July 14, 2023, 1:58 p.m. UTC | #2
On 7/14/23 04:55, Asahi Lina wrote:
> This module is intended to contain functions related to kernel
> timekeeping and time.
> 
> Initially, this implements an abstraction for a time Instant (analogous
> to the Rust std::time::Instant) that represents an opaque instant in
> time. Unlike the std Instant, this is a generic type that is bound to a
> specific clock source, so that only Instants from the same clock source
> can be subtracted/compared.
> 
> Then we implement the relevant clocks available to the kernel:
> KernelTime (CLOCK_MONOTONIC), BootTime (CLOCK_BOOTTIME),
> RealTime (CLOCK_REALTIME), and TaiTime.
> 
> Co-developed-by: Heghedus Razvan <heghedus.razvan@protonmail.com>
> Signed-off-by: Asahi Lina <lina@asahilina.net>
> ---
> [...]

Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
  
Boqun Feng July 14, 2023, 5:15 p.m. UTC | #3
On Fri, Jul 14, 2023 at 04:55:01PM +0900, Asahi Lina wrote:
> This module is intended to contain functions related to kernel
> timekeeping and time.
> 
> Initially, this implements an abstraction for a time Instant (analogous
> to the Rust std::time::Instant) that represents an opaque instant in
> time. Unlike the std Instant, this is a generic type that is bound to a
> specific clock source, so that only Instants from the same clock source
> can be subtracted/compared.
> 
> Then we implement the relevant clocks available to the kernel:
> KernelTime (CLOCK_MONOTONIC), BootTime (CLOCK_BOOTTIME),
> RealTime (CLOCK_REALTIME), and TaiTime.
> 
> Co-developed-by: Heghedus Razvan <heghedus.razvan@protonmail.com>
> Signed-off-by: Asahi Lina <lina@asahilina.net>
> ---
> 
> Based on the feedback to v1, now we have proper type checking for kernel
> time. I decided to implement marker traits for monotonic vs. wallclock
> time sources, since it's useful to be able to safely implement different
> semantics conditional on that, but that left me with a name conflict of
> the Monotonic trait with the CLOCK_MONOTONIC / "default ktime" clock
> source. I ended up calling it KernelTime since it's the most fundamental
> kernel timesource, but suggestions welcome!
> 
> Heghedus: I think I need a signoff on this since this is based on the
> playground demo you wrote in the feedback to v1. Can you provide that? I
> can fold it into v3 (if there is one, otherwise Miguel can probably just
> add it when he applies it). Thanks!
> ---
>  rust/bindings/bindings_helper.h |   2 +
>  rust/helpers.c                  |  16 +++++
>  rust/kernel/lib.rs              |   1 +
>  rust/kernel/time.rs             | 150 ++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 169 insertions(+)
> 
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index 3e601ce2548d..eddfdf887364 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -8,9 +8,11 @@
>  
>  #include <linux/errname.h>
>  #include <linux/slab.h>
> +#include <linux/ktime.h>
>  #include <linux/refcount.h>
>  #include <linux/wait.h>
>  #include <linux/sched.h>
> +#include <linux/timekeeping.h>
>  
>  /* `bindgen` gets confused at certain things. */
>  const gfp_t BINDINGS_GFP_KERNEL = GFP_KERNEL;
> diff --git a/rust/helpers.c b/rust/helpers.c
> index bb594da56137..eff092302e23 100644
> --- a/rust/helpers.c
> +++ b/rust/helpers.c
> @@ -26,6 +26,7 @@
>  #include <linux/mutex.h>
>  #include <linux/spinlock.h>
>  #include <linux/sched/signal.h>
> +#include <linux/timekeeping.h>
>  #include <linux/wait.h>
>  
>  __noreturn void rust_helper_BUG(void)
> @@ -135,6 +136,21 @@ void rust_helper_put_task_struct(struct task_struct *t)
>  }
>  EXPORT_SYMBOL_GPL(rust_helper_put_task_struct);
>  
> +ktime_t rust_helper_ktime_get_real(void) {
> +	return ktime_get_real();
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_ktime_get_real);
> +
> +ktime_t rust_helper_ktime_get_boottime(void) {
> +	return ktime_get_boottime();
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_ktime_get_boottime);
> +
> +ktime_t rust_helper_ktime_get_clocktai(void) {
> +	return ktime_get_clocktai();
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_ktime_get_clocktai);
> +
>  /*
>   * We use `bindgen`'s `--size_t-is-usize` option to bind the C `size_t` type
>   * as the Rust `usize` type, so we can use it in contexts where Rust
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 85b261209977..52c91484c5d8 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -42,6 +42,7 @@
>  pub mod str;
>  pub mod sync;
>  pub mod task;
> +pub mod time;
>  pub mod types;
>  
>  #[doc(hidden)]
> diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
> new file mode 100644
> index 000000000000..f3bfeed20145
> --- /dev/null
> +++ b/rust/kernel/time.rs
> @@ -0,0 +1,150 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Timekeeping functions.
> +//!
> +//! C header: [`include/linux/ktime.h`](../../../../include/linux/ktime.h)
> +//! C header: [`include/linux/timekeeping.h`](../../../../include/linux/timekeeping.h)
> +
> +use crate::{bindings, pr_err};
> +use core::marker::PhantomData;
> +use core::time::Duration;
> +
> +/// Represents a clock, that is, a unique time source.
> +pub trait Clock: Sized {}
> +
> +/// A time source that can be queried for the current time.
> +pub trait Now: Clock {

Is there a `Clock` that we cannot get the current time? ;-)

> +    /// 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 {}
> +
> +/// Marker trait for clock sources that represent a calendar (wall clock)
> +/// relative to the UNIX epoch.
> +pub trait WallTime {}
> +
> +/// An instant in time associated with a given clock source.
> +#[derive(Debug)]
> +pub struct Instant<T: Clock> {
> +    nanoseconds: i64,
> +    _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(nanoseconds: i64) -> Self {
> +        Instant {
> +            nanoseconds,
> +            _type: PhantomData,
> +        }
> +    }
> +
> +    /// 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,
> +            ))
> +        }
> +    }
> +}
> +
> +impl<T: Clock + Now + Monotonic> Instant<T> {
> +    /// Returns the time elapsed since this Instant<T>.
> +    ///
> +    /// This is guaranteed to return a positive result, since
> +    /// it is only implemented for monotonic clocks.
> +    pub fn elapsed(&self) -> Duration {
> +        T::now().since(*self).unwrap_or_else(|| {
> +            pr_err!(
> +                "Monotonic clock {} went backwards!",
> +                core::any::type_name::<T>()
> +            );
> +            Duration::ZERO
> +        })
> +    }
> +}
> +
> +/// 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.
> +

Could you add one example for each `*Time` type? Not only they can show
people how to use the API, but also we can use them to generate unit
tests:

	https://lore.kernel.org/rust-for-linux/20230614180837.630180-1-ojeda@kernel.orge 

If you want to test your examples, you can either apply that patchset or
use the rust-dev branch:

	https://github.com/Rust-for-Linux/linux.git rust-dev

I use the following command to run tests:

	./tools/testing/kunit/kunit.py run --make_options LLVM=1 --arch x86_64 --kconfig_add CONFIG_RUST=y

Thanks!

Regards,
Boqun

> +    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() })
> +        }
> +    }
> +
> +    /// A clock representing the time elapsed since boot.
> +    ///
> +    /// 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 `KernelTime`, but does include time
> +    /// spent sleeping.
> +    pub struct BootTime;
> +
> +    impl Clock for BootTime {}
> +    impl Monotonic for BootTime {}
> +    impl Now for BootTime {
> +        fn now() -> Instant<Self> {
> +            Instant::<Self>::new(unsafe { bindings::ktime_get_boottime() })
> +        }
> +    }
> +
> +    /// A clock representing TAI time.
> +    ///
> +    /// This clock is not monotonic and can be changed from userspace.
> +    /// However, it is not affected by leap seconds.
> +    pub struct TaiTime;
> +
> +    impl Clock for TaiTime {}
> +    impl WallTime for TaiTime {}
> +    impl Now for TaiTime {
> +        fn now() -> Instant<Self> {
> +            Instant::<Self>::new(unsafe { bindings::ktime_get_clocktai() })
> +        }
> +    }
> +
> +    /// A clock representing wall clock time.
> +    ///
> +    /// This clock is not monotonic and can be changed from userspace.
> +    pub struct RealTime;
> +
> +    impl Clock for RealTime {}
> +    impl WallTime for RealTime {}
> +    impl Now for RealTime {
> +        fn now() -> Instant<Self> {
> +            Instant::<Self>::new(unsafe { bindings::ktime_get_real() })
> +        }
> +    }
> +}
> 
> ---
> base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5
> change-id: 20230714-rust-time-10dd9ed25333
> 
> Thank you,
> ~~ Lina
>
  
Thomas Gleixner July 15, 2023, 1:17 a.m. UTC | #4
Lina!

On Fri, Jul 14 2023 at 16:55, Asahi Lina wrote:
> +ktime_t rust_helper_ktime_get_real(void) {
> +	return ktime_get_real();
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_ktime_get_real);

Colour me confused. But why does this need another export?

This just creates yet another layer of duct tape. If it's unsafe from
the rust perspective then wrapping it into rust_"unsafe_function" does not
make it any better.

Why on earth can't you use the actual C interfaces diretly which are
already exported?

> +use crate::{bindings, pr_err};
> +use core::marker::PhantomData;
> +use core::time::Duration;
> +
> +/// Represents a clock, that is, a unique time source.
> +pub trait Clock: Sized {}
> +
> +/// A time source that can be queried for the current time.

I doubt you can read anything else than current time from a time
source. At least the C side does not provide time traveling interfaces
unless the underlying hardware goes south.

> +pub trait Now: Clock {
> +impl<T: Clock> Instant<T> {
> +    fn new(nanoseconds: i64) -> Self {
> +        Instant {
> +            nanoseconds,
> +            _type: PhantomData,
> +        }
> +    }
> +
> +    /// 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,

Clever, but any timestamp greater than KTIME_MAX or less than 0 for such
a comparison is invalid. I'm too lazy to do the math for you..

> +            ))
> +        }
> +    }
> +}
> +
> +impl<T: Clock + Now + Monotonic> Instant<T> {
> +    /// Returns the time elapsed since this Instant<T>.
> +    ///
> +    /// This is guaranteed to return a positive result, since
> +    /// it is only implemented for monotonic clocks.
> +    pub fn elapsed(&self) -> Duration {
> +        T::now().since(*self).unwrap_or_else(|| {
> +            pr_err!(
> +                "Monotonic clock {} went backwards!",
> +                core::any::type_name::<T>()

Can you please write this in one line?

 +            pr_err!("Monotonic clock {} went backwards!", core::any::type_name::<T>()

The above is unreadable gunk for no reason.

> +/// 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 "(though it is not the only monotonic clock)" phrase is irritating
at best. CLOCK_MONOTONIC is well defined as the other CLOCK_* variants.

> +    ///
> +    /// 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() })
> +        }
> +    }
> +
> +    /// A clock representing the time elapsed since boot.
> +    ///
> +    /// 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.

The wonders of copy and pasta...

> +    ///
> +    /// This is like `KernelTime`, but does include time
> +    /// spent sleeping.

Can you please expand your editors line wrap limit to the year 2023
standards? This looks like a IBM 2260 terminal.

> +    /// A clock representing TAI time.
> +    ///
> +    /// This clock is not monotonic and can be changed from userspace.
> +    /// However, it is not affected by leap seconds.

I'm not impressed by this at all.

Lots of copy and pasta with zero content. I don't see how this is an
improvement over the admittedly lousy or non-existant kernel interface
documentations.

I thought Rust is set out to be better, but obviously it's just another
variant of copy & pasta and sloppy wrappers with useless documentation
around some admittedly not well documented, but well understood C
interfaces.

So the right approach to this is:

 1) Extend the kernel C-API documentations first if required

 2) Build your wrapper so that it can refer to the documentation which
    was either there already or got refined/added in #1

 3) Add one clock per patch with a proper changelog and not some
    wholesale drop it all.

It's well documented in Documentation/process/*, no?

Thanks,

        tglx
  
Alice Ryhl July 15, 2023, 8:14 a.m. UTC | #5
On 7/15/23 03:17, Thomas Gleixner wrote:
> On Fri, Jul 14 2023 at 16:55, Asahi Lina wrote:
>> +ktime_t rust_helper_ktime_get_real(void) {
>> +	return ktime_get_real();
>> +}
>> +EXPORT_SYMBOL_GPL(rust_helper_ktime_get_real);
> 
> Colour me confused. But why does this need another export?
> 
> This just creates yet another layer of duct tape. If it's unsafe from
> the rust perspective then wrapping it into rust_"unsafe_function" does not
> make it any better.
> 
> Why on earth can't you use the actual C interfaces diretly which are
> already exported?

Currently, you can't call inline C functions directly from Rust. So we 
wrap them in non-inline functions for now.

It's something we definitely want to fix, but it hasn't been fixed yet.

Alice
  
Gary Guo July 15, 2023, 3:33 p.m. UTC | #6
On Fri, 14 Jul 2023 16:55:01 +0900
Asahi Lina <lina@asahilina.net> wrote:

> This module is intended to contain functions related to kernel
> timekeeping and time.
> 
> Initially, this implements an abstraction for a time Instant (analogous
> to the Rust std::time::Instant) that represents an opaque instant in
> time. Unlike the std Instant, this is a generic type that is bound to a
> specific clock source, so that only Instants from the same clock source
> can be subtracted/compared.
> 
> Then we implement the relevant clocks available to the kernel:
> KernelTime (CLOCK_MONOTONIC), BootTime (CLOCK_BOOTTIME),
> RealTime (CLOCK_REALTIME), and TaiTime.
> 
> Co-developed-by: Heghedus Razvan <heghedus.razvan@protonmail.com>
> Signed-off-by: Asahi Lina <lina@asahilina.net>

The API seems much nicer. Comments inline below.

> ---
> 
> Based on the feedback to v1, now we have proper type checking for kernel
> time. I decided to implement marker traits for monotonic vs. wallclock
> time sources, since it's useful to be able to safely implement different
> semantics conditional on that, but that left me with a name conflict of
> the Monotonic trait with the CLOCK_MONOTONIC / "default ktime" clock
> source. I ended up calling it KernelTime since it's the most fundamental
> kernel timesource, but suggestions welcome!
> 
> Heghedus: I think I need a signoff on this since this is based on the
> playground demo you wrote in the feedback to v1. Can you provide that? I
> can fold it into v3 (if there is one, otherwise Miguel can probably just
> add it when he applies it). Thanks!
> ---
>  rust/bindings/bindings_helper.h |   2 +
>  rust/helpers.c                  |  16 +++++
>  rust/kernel/lib.rs              |   1 +
>  rust/kernel/time.rs             | 150 ++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 169 insertions(+)
> 
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index 3e601ce2548d..eddfdf887364 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -8,9 +8,11 @@
>  
>  #include <linux/errname.h>
>  #include <linux/slab.h>
> +#include <linux/ktime.h>
>  #include <linux/refcount.h>
>  #include <linux/wait.h>
>  #include <linux/sched.h>
> +#include <linux/timekeeping.h>
>  
>  /* `bindgen` gets confused at certain things. */
>  const gfp_t BINDINGS_GFP_KERNEL = GFP_KERNEL;
> diff --git a/rust/helpers.c b/rust/helpers.c
> index bb594da56137..eff092302e23 100644
> --- a/rust/helpers.c
> +++ b/rust/helpers.c
> @@ -26,6 +26,7 @@
>  #include <linux/mutex.h>
>  #include <linux/spinlock.h>
>  #include <linux/sched/signal.h>
> +#include <linux/timekeeping.h>
>  #include <linux/wait.h>
>  
>  __noreturn void rust_helper_BUG(void)
> @@ -135,6 +136,21 @@ void rust_helper_put_task_struct(struct task_struct *t)
>  }
>  EXPORT_SYMBOL_GPL(rust_helper_put_task_struct);
>  
> +ktime_t rust_helper_ktime_get_real(void) {
> +	return ktime_get_real();
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_ktime_get_real);
> +
> +ktime_t rust_helper_ktime_get_boottime(void) {
> +	return ktime_get_boottime();
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_ktime_get_boottime);
> +
> +ktime_t rust_helper_ktime_get_clocktai(void) {
> +	return ktime_get_clocktai();
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_ktime_get_clocktai);
> +
>  /*
>   * We use `bindgen`'s `--size_t-is-usize` option to bind the C `size_t` type
>   * as the Rust `usize` type, so we can use it in contexts where Rust
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 85b261209977..52c91484c5d8 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -42,6 +42,7 @@
>  pub mod str;
>  pub mod sync;
>  pub mod task;
> +pub mod time;
>  pub mod types;
>  
>  #[doc(hidden)]
> diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
> new file mode 100644
> index 000000000000..f3bfeed20145
> --- /dev/null
> +++ b/rust/kernel/time.rs
> @@ -0,0 +1,150 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Timekeeping functions.
> +//!
> +//! C header: [`include/linux/ktime.h`](../../../../include/linux/ktime.h)
> +//! C header: [`include/linux/timekeeping.h`](../../../../include/linux/timekeeping.h)
> +
> +use crate::{bindings, pr_err};
> +use core::marker::PhantomData;
> +use core::time::Duration;
> +
> +/// Represents a clock, that is, a unique time source.
> +pub trait Clock: Sized {}
> +
> +/// A time source that can be queried for the current time.
> +pub trait Now: Clock {
> +    /// 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 {}
> +
> +/// Marker trait for clock sources that represent a calendar (wall clock)
> +/// relative to the UNIX epoch.
> +pub trait WallTime {}
> +
> +/// An instant in time associated with a given clock source.
> +#[derive(Debug)]

We probably want a manual Debug impl, but that could be left to a
future patch.

> +pub struct Instant<T: Clock> {
> +    nanoseconds: i64,
> +    _type: PhantomData<T>,
> +}

Do we want to pick a default type parameter? The `KernelTime` could be
a good default, given that it's `ktime_get`.

> +
> +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(nanoseconds: i64) -> Self {
> +        Instant {
> +            nanoseconds,
> +            _type: PhantomData,
> +        }
> +    }
> +
> +    /// 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,

`wrapping_sub` as other people have already point out.

> +            ))
> +        }
> +    }
> +}
> +
> +impl<T: Clock + Now + Monotonic> Instant<T> {
> +    /// Returns the time elapsed since this Instant<T>.
> +    ///
> +    /// This is guaranteed to return a positive result, since
> +    /// it is only implemented for monotonic clocks.
> +    pub fn elapsed(&self) -> Duration {
> +        T::now().since(*self).unwrap_or_else(|| {
> +            pr_err!(
> +                "Monotonic clock {} went backwards!",
> +                core::any::type_name::<T>()
> +            );
> +            Duration::ZERO
> +        })
> +    }
> +}
> +
> +/// 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.
> +

Extra line

> +    pub struct KernelTime;

I feel that the struct names are a bit awkward, but couldn't think of
any better suggestions...

> +
> +    impl Clock for KernelTime {}
> +    impl Monotonic for KernelTime {}
> +    impl Now for KernelTime {
> +        fn now() -> Instant<Self> {
> +            Instant::<Self>::new(unsafe { bindings::ktime_get() })
> +        }
> +    }
> +
> +    /// A clock representing the time elapsed since boot.
> +    ///
> +    /// 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 `KernelTime`, but does include time
> +    /// spent sleeping.
> +    pub struct BootTime;
> +
> +    impl Clock for BootTime {}
> +    impl Monotonic for BootTime {}
> +    impl Now for BootTime {
> +        fn now() -> Instant<Self> {
> +            Instant::<Self>::new(unsafe { bindings::ktime_get_boottime() })
> +        }
> +    }
> +
> +    /// A clock representing TAI time.
> +    ///
> +    /// This clock is not monotonic and can be changed from userspace.
> +    /// However, it is not affected by leap seconds.
> +    pub struct TaiTime;
> +
> +    impl Clock for TaiTime {}
> +    impl WallTime for TaiTime {}
> +    impl Now for TaiTime {
> +        fn now() -> Instant<Self> {
> +            Instant::<Self>::new(unsafe { bindings::ktime_get_clocktai() })
> +        }
> +    }
> +
> +    /// A clock representing wall clock time.
> +    ///
> +    /// This clock is not monotonic and can be changed from userspace.
> +    pub struct RealTime;
> +
> +    impl Clock for RealTime {}
> +    impl WallTime for RealTime {}
> +    impl Now for RealTime {
> +        fn now() -> Instant<Self> {
> +            Instant::<Self>::new(unsafe { bindings::ktime_get_real() })
> +        }
> +    }
> +}
> 
> ---
> base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5
> change-id: 20230714-rust-time-10dd9ed25333
> 
> Thank you,
> ~~ Lina
>
  
Miguel Ojeda July 15, 2023, 4:59 p.m. UTC | #7
On Fri, Jul 14, 2023 at 12:05 PM Alice Ryhl <aliceryhl@google.com> wrote:
>
> In that case, you should explicitly use `.wrapping_sub` instead to
> convey your intent.

Also to prevent extra code and panics with `CONFIG_RUST_OVERFLOW_CHECKS=y`.

> I guess you could also use `abs_diff`, which takes two i64s and returns
> an u64.

I think `abs_diff` sounds best. It introduces an extra branch in its
implementation from what I see, though...

However, since it is the same branch that Lina does for returning
`None`, it should be getting removed. At least in a quick test, LLVM
generates the same code: https://godbolt.org/z/61rafzx71

Cheers,
Miguel
  
Asahi Lina July 16, 2023, 9:20 a.m. UTC | #8
On 15/07/2023 10.17, Thomas Gleixner wrote:
> Lina!
> 
> On Fri, Jul 14 2023 at 16:55, Asahi Lina wrote:
>> +ktime_t rust_helper_ktime_get_real(void) {
>> +	return ktime_get_real();
>> +}
>> +EXPORT_SYMBOL_GPL(rust_helper_ktime_get_real);
> 
> Colour me confused. But why does this need another export?

Static inline functions aren't exported, so Rust can't call them.

> This just creates yet another layer of duct tape. If it's unsafe from
> the rust perspective then wrapping it into rust_"unsafe_function" does not
> make it any better.
> 
> Why on earth can't you use the actual C interfaces diretly which are
> already exported?

Because Rust isn't C and can't compile static inline C code...

Bindgen very recently gained a feature to autogenerate these wrappers, 
but we're not on that version yet. But there's no reasonable way around 
the wrappers, whether they are automatically generated or not, unless 
someone wants to write a whole C->Rust transpiler...

>> +use crate::{bindings, pr_err};
>> +use core::marker::PhantomData;
>> +use core::time::Duration;
>> +
>> +/// Represents a clock, that is, a unique time source.
>> +pub trait Clock: Sized {}
>> +
>> +/// A time source that can be queried for the current time.
> 
> I doubt you can read anything else than current time from a time
> source. At least the C side does not provide time traveling interfaces
> unless the underlying hardware goes south.

My thought was that we might have time sources (think some kind of 
hardware clock) which might want to use these types but cannot be 
queried for the current time globally (e.g. they are bound to a specific 
device and need state to query the time, so you can't just have a global 
read function with no arguments). Those would probably be declared 
within other subsystems or drivers, so they don't belong here, but the 
idea that a clock source and the ability to query it statically at any 
time are distinct concepts does, to enable that use case with this 
common API.

>> +pub trait Now: Clock {
>> +impl<T: Clock> Instant<T> {
>> +    fn new(nanoseconds: i64) -> Self {
>> +        Instant {
>> +            nanoseconds,
>> +            _type: PhantomData,
>> +        }
>> +    }
>> +
>> +    /// 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,
> 
> Clever, but any timestamp greater than KTIME_MAX or less than 0 for such
> a comparison is invalid. I'm too lazy to do the math for you..

This is computing a Rust Duration (which is unsigned and always 
positive) from the difference between two ktimes. As long as the two 
ktimes have the right >= relationship, the difference will always lie 
within the representable range of an unsigned 64-bit integer, which is 
itself a subset of the representable range of a Rust Duration (which is 
u64 seconds + u32 nanoseconds). This is trivially true, since the types 
have the same size and the absolute difference between two values can 
never exceed the number of values representable in that number of bits, 
regardless of whether the types are signed or unsigned. Feel free to do 
the math ^^

Or are you saying ktime timestamps can never be negative anyway, or that 
those conditions should be special-cased somehow?

> 
>> +            ))
>> +        }
>> +    }
>> +}
>> +
>> +impl<T: Clock + Now + Monotonic> Instant<T> {
>> +    /// Returns the time elapsed since this Instant<T>.
>> +    ///
>> +    /// This is guaranteed to return a positive result, since
>> +    /// it is only implemented for monotonic clocks.
>> +    pub fn elapsed(&self) -> Duration {
>> +        T::now().since(*self).unwrap_or_else(|| {
>> +            pr_err!(
>> +                "Monotonic clock {} went backwards!",
>> +                core::any::type_name::<T>()
> 
> Can you please write this in one line?
> 
>   +            pr_err!("Monotonic clock {} went backwards!", core::any::type_name::<T>()
> 
> The above is unreadable gunk for no reason.

We use rustfmt style for all Rust code in the kernel, and this code 
follows that. See Documentation/rust/coding-guidelines.rst.

>> +/// 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 "(though it is not the only monotonic clock)" phrase is irritating
> at best. CLOCK_MONOTONIC is well defined as the other CLOCK_* variants.

The issue I ran into here is what to call this clock when "Monotonic" is 
both a trait that a clock can have and the name of the canonical default 
clock. CLOCK_BOOTTIME is also monotonic and therefore implements the 
Monotonic trait. That's why I called it KernelTime and why I made a 
point that, while this one is called CLOCK_MONOTONIC in C, it's not the 
*only* monotonic clock.

I'm open to suggestions for the naming, I just think we might as well 
try to make things clear since clock selection can be a confusing thing.

>> +    ///
>> +    /// 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() })
>> +        }
>> +    }
>> +
>> +    /// A clock representing the time elapsed since boot.
>> +    ///
>> +    /// 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.
> 
> The wonders of copy and pasta...

Oops, sorry... I'll fix it for v2.

> 
>> +    ///
>> +    /// This is like `KernelTime`, but does include time
>> +    /// spent sleeping.
> 
> Can you please expand your editors line wrap limit to the year 2023
> standards? This looks like a IBM 2260 terminal.

I think I manually wrapped this, I can rewrap it up to the rustfmt limit 
for v2.

>> +    /// A clock representing TAI time.
>> +    ///
>> +    /// This clock is not monotonic and can be changed from userspace.
>> +    /// However, it is not affected by leap seconds.
> 
> I'm not impressed by this at all.
> 
> Lots of copy and pasta with zero content. I don't see how this is an
> improvement over the admittedly lousy or non-existant kernel interface
> documentations.
> 
> I thought Rust is set out to be better, but obviously it's just another
> variant of copy & pasta and sloppy wrappers with useless documentation
> around some admittedly not well documented, but well understood C
> interfaces.

At least the API doesn't conflate all clock sources as well as intervals 
derived from them into a single type, like the C API does... I thought 
that was what we were aiming to fix here, based on the previous discussion.

I can definitely improve the docs, but I don't think it's fair to call 
this "copy & pasta sloppy wrappers"...

> So the right approach to this is:
> 
>   1) Extend the kernel C-API documentations first if required

I am not comfortable further documenting the Linux timekeeping 
subsystem, since I am not an expert in that area... if you think the 
docs should be improved across the board, I'm afraid you'll have to do 
that yourself first. It doesn't make any sense for me to become a 
timekeeping expert just to write some missing C docs.

I'm trying to upstream abstractions for a whole bunch of Linux 
subsystems, I can't become an expert in all of them to improve the C 
docs just because the people writing the C code didn't document it 
properly themselves... it's hard enough wrapping my head around the 
lifetime constraints and all that stuff already (which is almost never 
documented), just to make sure the Rust abstraction is safe. Thankfully 
that one isn't an issue for timekeeping, since there are no objects with 
lifetimes...

>   2) Build your wrapper so that it can refer to the documentation which
>      was either there already or got refined/added in #1

The Rust documentation needs to at least cover the Rust API. I can link 
to Documentation/core-api/timekeeping.rst for reference, but we still 
need module/function docs in the Rust code.

What I can certainly do is expand on the current docs with more details 
from that file, to try to get things more consistent. I'll look it over 
for v2.

>   3) Add one clock per patch with a proper changelog and not some
>      wholesale drop it all.

The whole file is 151 lines of code... do you really want me to break it 
up into 5 patches where the last 4 add 10 lines each? What does that 
accomplish? I don't get it... it's just a bunch of clock options, it's 
not like there's any logic to review or actual clock implementations here.

~~ Lina
  
Heghedus Razvan July 19, 2023, 12:17 p.m. UTC | #9
On Fri Jul 14, 2023 at 10:55 AM EEST, Asahi Lina wrote:
> This module is intended to contain functions related to kernel
> timekeeping and time.
>
> Initially, this implements an abstraction for a time Instant (analogous
> to the Rust std::time::Instant) that represents an opaque instant in
> time. Unlike the std Instant, this is a generic type that is bound to a
> specific clock source, so that only Instants from the same clock source
> can be subtracted/compared.
>
> Then we implement the relevant clocks available to the kernel:
> KernelTime (CLOCK_MONOTONIC), BootTime (CLOCK_BOOTTIME),
> RealTime (CLOCK_REALTIME), and TaiTime.
>
> Co-developed-by: Heghedus Razvan <heghedus.razvan@protonmail.com>
> Signed-off-by: Asahi Lina <lina@asahilina.net>

Signed-off-by: Heghedus Razvan <heghedus.razvan@protonmail.com>
> ---
>
> Based on the feedback to v1, now we have proper type checking for kernel
> time. I decided to implement marker traits for monotonic vs. wallclock
> time sources, since it's useful to be able to safely implement different
> semantics conditional on that, but that left me with a name conflict of
> the Monotonic trait with the CLOCK_MONOTONIC / "default ktime" clock
> source. I ended up calling it KernelTime since it's the most fundamental
> kernel timesource, but suggestions welcome!
>
> Heghedus: I think I need a signoff on this since this is based on the
> playground demo you wrote in the feedback to v1. Can you provide that? I
> can fold it into v3 (if there is one, otherwise Miguel can probably just
> add it when he applies it). Thanks!
> ---
>  rust/bindings/bindings_helper.h |   2 +
>  rust/helpers.c                  |  16 +++++
>  rust/kernel/lib.rs              |   1 +
>  rust/kernel/time.rs             | 150 ++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 169 insertions(+)
>
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index 3e601ce2548d..eddfdf887364 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -8,9 +8,11 @@
>
>  #include <linux/errname.h>
>  #include <linux/slab.h>
> +#include <linux/ktime.h>
>  #include <linux/refcount.h>
>  #include <linux/wait.h>
>  #include <linux/sched.h>
> +#include <linux/timekeeping.h>
>
>  /* `bindgen` gets confused at certain things. */
>  const gfp_t BINDINGS_GFP_KERNEL = GFP_KERNEL;
> diff --git a/rust/helpers.c b/rust/helpers.c
> index bb594da56137..eff092302e23 100644
> --- a/rust/helpers.c
> +++ b/rust/helpers.c
> @@ -26,6 +26,7 @@
>  #include <linux/mutex.h>
>  #include <linux/spinlock.h>
>  #include <linux/sched/signal.h>
> +#include <linux/timekeeping.h>
>  #include <linux/wait.h>
>
>  __noreturn void rust_helper_BUG(void)
> @@ -135,6 +136,21 @@ void rust_helper_put_task_struct(struct task_struct *t)
>  }
>  EXPORT_SYMBOL_GPL(rust_helper_put_task_struct);
>
> +ktime_t rust_helper_ktime_get_real(void) {
> +	return ktime_get_real();
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_ktime_get_real);
> +
> +ktime_t rust_helper_ktime_get_boottime(void) {
> +	return ktime_get_boottime();
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_ktime_get_boottime);
> +
> +ktime_t rust_helper_ktime_get_clocktai(void) {
> +	return ktime_get_clocktai();
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_ktime_get_clocktai);
> +
>  /*
>   * We use `bindgen`'s `--size_t-is-usize` option to bind the C `size_t` type
>   * as the Rust `usize` type, so we can use it in contexts where Rust
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 85b261209977..52c91484c5d8 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -42,6 +42,7 @@
>  pub mod str;
>  pub mod sync;
>  pub mod task;
> +pub mod time;
>  pub mod types;
>
>  #[doc(hidden)]
> diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
> new file mode 100644
> index 000000000000..f3bfeed20145
> --- /dev/null
> +++ b/rust/kernel/time.rs
> @@ -0,0 +1,150 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Timekeeping functions.
> +//!
> +//! C header: [`include/linux/ktime.h`](../../../../include/linux/ktime.h)
> +//! C header: [`include/linux/timekeeping.h`](../../../../include/linux/timekeeping.h)
> +
> +use crate::{bindings, pr_err};
> +use core::marker::PhantomData;
> +use core::time::Duration;
> +
> +/// Represents a clock, that is, a unique time source.
> +pub trait Clock: Sized {}
> +
> +/// A time source that can be queried for the current time.
> +pub trait Now: Clock {
> +    /// 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 {}
> +
> +/// Marker trait for clock sources that represent a calendar (wall clock)
> +/// relative to the UNIX epoch.
> +pub trait WallTime {}
> +
> +/// An instant in time associated with a given clock source.
> +#[derive(Debug)]
> +pub struct Instant<T: Clock> {
> +    nanoseconds: i64,
> +    _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(nanoseconds: i64) -> Self {
> +        Instant {
> +            nanoseconds,
> +            _type: PhantomData,
> +        }
> +    }
> +
> +    /// 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,
> +            ))
> +        }
> +    }
> +}
> +
> +impl<T: Clock + Now + Monotonic> Instant<T> {
> +    /// Returns the time elapsed since this Instant<T>.
> +    ///
> +    /// This is guaranteed to return a positive result, since
> +    /// it is only implemented for monotonic clocks.
> +    pub fn elapsed(&self) -> Duration {
> +        T::now().since(*self).unwrap_or_else(|| {
> +            pr_err!(
> +                "Monotonic clock {} went backwards!",
> +                core::any::type_name::<T>()
> +            );
> +            Duration::ZERO
> +        })
> +    }
> +}
> +
> +/// 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() })
> +        }
> +    }
> +
> +    /// A clock representing the time elapsed since boot.
> +    ///
> +    /// 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 `KernelTime`, but does include time
> +    /// spent sleeping.
> +    pub struct BootTime;
> +
> +    impl Clock for BootTime {}
> +    impl Monotonic for BootTime {}
> +    impl Now for BootTime {
> +        fn now() -> Instant<Self> {
> +            Instant::<Self>::new(unsafe { bindings::ktime_get_boottime() })
> +        }
> +    }
> +
> +    /// A clock representing TAI time.
> +    ///
> +    /// This clock is not monotonic and can be changed from userspace.
> +    /// However, it is not affected by leap seconds.
> +    pub struct TaiTime;
> +
> +    impl Clock for TaiTime {}
> +    impl WallTime for TaiTime {}
> +    impl Now for TaiTime {
> +        fn now() -> Instant<Self> {
> +            Instant::<Self>::new(unsafe { bindings::ktime_get_clocktai() })
> +        }
> +    }
> +
> +    /// A clock representing wall clock time.
> +    ///
> +    /// This clock is not monotonic and can be changed from userspace.
> +    pub struct RealTime;
> +
> +    impl Clock for RealTime {}
> +    impl WallTime for RealTime {}
> +    impl Now for RealTime {
> +        fn now() -> Instant<Self> {
> +            Instant::<Self>::new(unsafe { bindings::ktime_get_real() })
> +        }
> +    }
> +}
>
> ---
> base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5
> change-id: 20230714-rust-time-10dd9ed25333
>
> Thank you,
> ~~ Lina
  

Patch

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 3e601ce2548d..eddfdf887364 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -8,9 +8,11 @@ 
 
 #include <linux/errname.h>
 #include <linux/slab.h>
+#include <linux/ktime.h>
 #include <linux/refcount.h>
 #include <linux/wait.h>
 #include <linux/sched.h>
+#include <linux/timekeeping.h>
 
 /* `bindgen` gets confused at certain things. */
 const gfp_t BINDINGS_GFP_KERNEL = GFP_KERNEL;
diff --git a/rust/helpers.c b/rust/helpers.c
index bb594da56137..eff092302e23 100644
--- a/rust/helpers.c
+++ b/rust/helpers.c
@@ -26,6 +26,7 @@ 
 #include <linux/mutex.h>
 #include <linux/spinlock.h>
 #include <linux/sched/signal.h>
+#include <linux/timekeeping.h>
 #include <linux/wait.h>
 
 __noreturn void rust_helper_BUG(void)
@@ -135,6 +136,21 @@  void rust_helper_put_task_struct(struct task_struct *t)
 }
 EXPORT_SYMBOL_GPL(rust_helper_put_task_struct);
 
+ktime_t rust_helper_ktime_get_real(void) {
+	return ktime_get_real();
+}
+EXPORT_SYMBOL_GPL(rust_helper_ktime_get_real);
+
+ktime_t rust_helper_ktime_get_boottime(void) {
+	return ktime_get_boottime();
+}
+EXPORT_SYMBOL_GPL(rust_helper_ktime_get_boottime);
+
+ktime_t rust_helper_ktime_get_clocktai(void) {
+	return ktime_get_clocktai();
+}
+EXPORT_SYMBOL_GPL(rust_helper_ktime_get_clocktai);
+
 /*
  * We use `bindgen`'s `--size_t-is-usize` option to bind the C `size_t` type
  * as the Rust `usize` type, so we can use it in contexts where Rust
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 85b261209977..52c91484c5d8 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -42,6 +42,7 @@ 
 pub mod str;
 pub mod sync;
 pub mod task;
+pub mod time;
 pub mod types;
 
 #[doc(hidden)]
diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
new file mode 100644
index 000000000000..f3bfeed20145
--- /dev/null
+++ b/rust/kernel/time.rs
@@ -0,0 +1,150 @@ 
+// SPDX-License-Identifier: GPL-2.0
+
+//! Timekeeping functions.
+//!
+//! C header: [`include/linux/ktime.h`](../../../../include/linux/ktime.h)
+//! C header: [`include/linux/timekeeping.h`](../../../../include/linux/timekeeping.h)
+
+use crate::{bindings, pr_err};
+use core::marker::PhantomData;
+use core::time::Duration;
+
+/// Represents a clock, that is, a unique time source.
+pub trait Clock: Sized {}
+
+/// A time source that can be queried for the current time.
+pub trait Now: Clock {
+    /// 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 {}
+
+/// Marker trait for clock sources that represent a calendar (wall clock)
+/// relative to the UNIX epoch.
+pub trait WallTime {}
+
+/// An instant in time associated with a given clock source.
+#[derive(Debug)]
+pub struct Instant<T: Clock> {
+    nanoseconds: i64,
+    _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(nanoseconds: i64) -> Self {
+        Instant {
+            nanoseconds,
+            _type: PhantomData,
+        }
+    }
+
+    /// 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,
+            ))
+        }
+    }
+}
+
+impl<T: Clock + Now + Monotonic> Instant<T> {
+    /// Returns the time elapsed since this Instant<T>.
+    ///
+    /// This is guaranteed to return a positive result, since
+    /// it is only implemented for monotonic clocks.
+    pub fn elapsed(&self) -> Duration {
+        T::now().since(*self).unwrap_or_else(|| {
+            pr_err!(
+                "Monotonic clock {} went backwards!",
+                core::any::type_name::<T>()
+            );
+            Duration::ZERO
+        })
+    }
+}
+
+/// 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() })
+        }
+    }
+
+    /// A clock representing the time elapsed since boot.
+    ///
+    /// 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 `KernelTime`, but does include time
+    /// spent sleeping.
+    pub struct BootTime;
+
+    impl Clock for BootTime {}
+    impl Monotonic for BootTime {}
+    impl Now for BootTime {
+        fn now() -> Instant<Self> {
+            Instant::<Self>::new(unsafe { bindings::ktime_get_boottime() })
+        }
+    }
+
+    /// A clock representing TAI time.
+    ///
+    /// This clock is not monotonic and can be changed from userspace.
+    /// However, it is not affected by leap seconds.
+    pub struct TaiTime;
+
+    impl Clock for TaiTime {}
+    impl WallTime for TaiTime {}
+    impl Now for TaiTime {
+        fn now() -> Instant<Self> {
+            Instant::<Self>::new(unsafe { bindings::ktime_get_clocktai() })
+        }
+    }
+
+    /// A clock representing wall clock time.
+    ///
+    /// This clock is not monotonic and can be changed from userspace.
+    pub struct RealTime;
+
+    impl Clock for RealTime {}
+    impl WallTime for RealTime {}
+    impl Now for RealTime {
+        fn now() -> Instant<Self> {
+            Instant::<Self>::new(unsafe { bindings::ktime_get_real() })
+        }
+    }
+}