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

From: Boqun Feng <boqun.feng@gmail.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: "Miguel Ojeda" <miguel.ojeda.sandonis@gmail.com>,
	"Asahi Lina" <lina@asahilina.net>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Alex Gaynor" <alex.gaynor@gmail.com>,
	"Wedson Almeida Filho" <wedsonaf@gmail.com>,
	"Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"John Stultz" <jstultz@google.com>,
	"Stephen Boyd" <sboyd@kernel.org>,
	linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org,
	asahi@lists.linux.dev,
	"Heghedus Razvan" <heghedus.razvan@protonmail.com>
Subject: Re: [PATCH] rust: time: New module for timekeeping functions
Date: Tue, 21 Feb 2023 18:54:07 -0800	[thread overview]
Message-ID: <Y/WDz013yBIfQPDq@boqun-archlinux> (raw)
In-Reply-To: <87o7pmmtre.ffs@tglx>

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@linutronix.de> 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?

Regards,
Boqun

> Thanks,
> 
>         tglx