Re: [PATCH] rust: time: New module for timekeeping functions

[Boqun Feng]

On Wed, Feb 22, 2023 at 01:45:58PM +0900, Asahi Lina wrote:
> On 22/02/2023 11.54, Boqun Feng wrote:
> > On Wed, Feb 22, 2023 at 01:24:53AM +0100, Thomas Gleixner wrote:
> >> Miguel!
> >>
> >> On Tue, Feb 21 2023 at 23:29, Miguel Ojeda wrote:
> >>> On Tue, Feb 21, 2023 at 7:45 PM Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:
> >>>>
> >>>> But xb abd xr are the same datatype because they represent a time delta.
> >>>
> >>> In principle, one could also have different duration types too. For
> >>> instance, C++'s `std::chrono::duration` type is parametrized on the
> >>> representation type and the tick period, and thus an operation between
> >>> two time points like t1 - t0 returns a duration type that depends on
> >>> the type of the time points, i.e. which clock they were obtained from.
> >>
> >> Correct, but for practical purposes I'd assume that the timestamps
> >> retrieved via ktime_get*() have the same granularity, i.e. 1ns.
> >>
> >> TBH, that's not entirely correct because:
> >>
> >>     - the underlying hardware clocksource might not have a 1ns
> >>       resolution
> >>
> >>     - the CLOCK_*_COARSE implementations are only advanced once per
> >>       tick, but are executing significantly faster because they avoid
> >>       the hardware counter access.
> >>
> >> But that's an assumption which has proven to be workable and correct
> >> with the full zoo of hardware supported by the kernel.
> >>
> >> The point is that all CLOCK_* variants, except CLOCK_REALTIME and
> >> CLOCK_TAI are guaranteed to never go backwards.
> >>
> >> CLOCK_REALTIME and CLOCK_TAI are special as they can be set by user
> >> space and CLOCK_REALTIME has the extra oddities of leap seconds.  But
> >> that's a well understood issue and is not specific to the kernel.
> >>
> >> Back to time deltas (or duration types). Independent of the above it
> >> might make sense to be type strict about these as well. Especially if we
> >> go one step further and have timers based on CLOCK_* which need to be
> >> armed by either timestamps for absolute expiry or time deltas for
> >> relative to now expiry. I definitely can see a point for requiring
> >> matching time delta types there.
> >>
> >> That said, I have no strong opinions about this particular detail and
> >> leave it to the Rusties to agree on something sensible.
> >>
> > 
> > I'd like to propose something below to make thing forward quickly:
> > 
> > Given Lina only uses CLOCK_BOOTTIME and CLOCK_MONOTONIC, I'd say we
> > reuse core::time::Duration and probably remain its ">=0" semantics even
> > in the future we change its internal representation to u64.
> > 
> > For timestamp type, use Instant semantics and use different types for
> > different clocks, i.e. similar to the implementation from Heghedus (much
> > better than mine!). But we can avoid implementing a fully version of
> > Instant, and focus on just the piece that Lina needs, which I believe
> > it's elapsed()?
> > 
> > For the future, if we were to support non-monotonic timestamp, maybe we
> > use the different type name like TimeStamp and TimeDelta.
> > 
> > In short:
> > 
> > *	For monotonic clocks, Instant + Duration, and keep them similar
> > 	to std semantics.
> > 
> > *	For non-monotonic clocks, don't worry it right now, and
> > 	probably different types for both stamps and deltas.
> > 
> > Thoughts?
> 
> I actually only used CLOCK_MONOTONIC in the end, so I could even leave
> CLOCK_BOOTTIME for later, though I like the idea of having scaffolding
> for several clock types even if we only implement one initially.
> 
> This works for me, if you're happy with the idea I'll give it a spin
> based on Heghedus' example. Heghedus, is it okay if I put you down as
> Co-developed-by and can I get a signoff? ^^
> 
> For the actual Instant type, I was thinking it makes sense to just
> internally represent it as a newtype of Duration as well. Then all the
> math becomes trivial based on Duration operations, and when we replace
> Duration with a new u64 type it'll all work out the same. Fundamentally
> that means Instant types are internally stored as the Duration between
> the epoch (e.g. system boot) subject to the way that clock ticks, which
> I think is a reasonable internal representation? (In other words, it's

Sounds even better ;-) It means Instant and Duration have the exact same
behavior about sub and so on.

Regards,
Boqun

> the same as my original patch behind the scenes, but wrapped in type
> safety).
> 
> ~~ Lina