lkml.org 
[lkml]   [2023]   [Feb]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] rust: time: New module for timekeeping functions
On Tue, Feb 21, 2023 at 01:32:51PM +0100, Thomas Gleixner wrote:
> On Tue, Feb 21 2023 at 16:06, Asahi Lina wrote:
> > +
> > +use crate::bindings;
> > +use core::time::Duration;
> > +
> > +/// Returns the kernel time elapsed since boot, excluding time spent sleeping, as a [`Duration`].
> > +pub fn ktime_get() -> Duration {
> > + // SAFETY: Function has no side effects and no inputs.
> > + Duration::from_nanos(unsafe { bindings::ktime_get() }.try_into().unwrap())
>
> Why is this a Duration? From the spec:
>

I agree that returning a Duration may not be ideal, but..

> Duration
>
> A Duration type to represent a span of time, typically used for
> system timeouts.
>
> Instant
>
> A measurement of a monotonically nondecreasing clock. Opaque and
> useful only with Duration.
>
> In my understanding 'Duration' is a time span between two points, while
> ktime_get() and ktime_get_boottime() return the current time of
> monotonically nondecreasing clocks, i.e. they fall into the 'Instant'
> category.
>
> Now the problem is that 'Instant' in it's specification is bound to
> CLOCK_MONOTONIC and there is no way to express CLOCK_BOOTTIME, but
> that's a shortcoming of the spec which ignores CLOCK_BOOTTIME
> completely. IOW, that's also a problem for user space.
>
> This makes sense vs. the other representation:
>
> SystemTime
>
> A measurement of the system clock, useful for talking to
> external entities like the file system or other processes.
>
> This maps to CLOCK_REALTIME and CLOCK_TAI, i.e. ktime_get_real_ns() and
> ktime_get_clocktai().
>
> Similar to 'Instant' 'SystemTime' is strictly bound to CLOCK_REALTIME
> by specification and there is no way to read CLOCK_TAI.
>

..'Instant' and 'SystemTime' are in Rust std, we cannot use them
directly, similar as we cannot use userspace libc. To me, there seems
two options to provide Rust types for kernel time management:

* Use KTime which maps to ktime_t, then we have the similar
semantics around it: sometimes it's a duration, sometimes it's
a point of time.. but I know "this is a safe language, you
should do more" ;-)

* Introduce kernel's own types, e.g. BootTime, RawTime, TAI,
RealTime, and make them play with Duration (actually I'd prefer
we have own Duration, because Rust core::time::Duration takes
more than u64), something like below:


pub struct BootTime {
d: Duration
}

impl BootTime {
fn now() -> Self {
unsafe { BootTime { d: ktime_to_duration(ktime_get_boottime())} }
}
fn add(self, d: Duration) -> Self {
<Add a duration, similar to ktime_add>
}
fn sub(self, other: Self) -> Duration {
...
}
...
}

Thoughts?

Regards,
Boqun

> Please fix this in the spec and do not try to work around that by
> pretending that a clock read is a 'Duration'.
>
> > +}
> > +
> > +/// Returns the kernel time elapsed since boot, including time spent sleeping, as a [`Duration`].
> > +pub 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) }
>
> No. Please use ktime_get_boottime() and not the timekeeping internal function.
>
> Thanks,
>
> tglx

\
 
 \ /
  Last update: 2023-03-27 00:30    [W:0.110 / U:0.968 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site