Re: [PATCH v8 3/7] rust: time: Introduce Instant type

On Thu, 16 Jan 2025 12:17:15 +0000
Tom Gundersen [off-list ref] wrote:

quoted

-/// A Rust wrapper around a `ktime_t`.
+/// A specific point in time.
 #[repr(transparent)]
 #[derive(Copy, Clone, PartialEq, PartialOrd, Eq, Ord)]
-pub struct Ktime {
+pub struct Instant {
+    // Range from 0 to `KTIME_MAX`.
     inner: bindings::ktime_t,
 }

-impl Ktime {
-    /// Create a `Ktime` from a raw `ktime_t`.
+impl Instant {
+    /// Create a `Instant` from a raw `ktime_t`.
     #[inline]
-    pub fn from_raw(inner: bindings::ktime_t) -> Self {
+    fn from_raw(inner: bindings::ktime_t) -> Self {

How do we know inner is between 0 and KTIME_MAX?

In my understanding, the kernel assumes that the range of the ktime_t
type is from 0 to KTIME_MAX. The ktime APIs guarantees to give a valid
ktime_t. The Rust abstraction creates ktime_t via ktime_get() so it's
fine now.

However, if we makes from_raw() public, a caller can create invalid
ktime_t by not using ktime APIs. Then from_raw() needs ceiling like
C's ktime_set(), I think.

         Self { inner }

quoted

     }

     /// Get the current time using `CLOCK_MONOTONIC`.
     #[inline]
-    pub fn ktime_get() -> Self {
+    pub fn now() -> Self {
         // SAFETY: It is always safe to call `ktime_get` outside of NMI
context.

Similarly, should there be a comment here about the range being guaranteed
to be correct?

         Self::from_raw(unsafe { bindings::ktime_get() })

We could add something like "The ktime API guarantees a valid
ktime_t". But adding similar comments to all the places where the
ktime API is called is redundant?

The comment on Instant struct must be improved instead, I think.