Feedback-ID: iad51458e:Fastmail
Date:   Tue, 21 Feb 2023 21:20:10 -0800
From:   Boqun Feng <boqun.feng@gmail.com>
To:     Asahi Lina <lina@asahilina.net>
Cc:     Thomas Gleixner <tglx@linutronix.de>,
        Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>,
        Miguel Ojeda <ojeda@kernel.org>,
        Alex Gaynor <alex.gaynor@gmail.com>,
        Wedson Almeida Filho <wedsonaf@gmail.com>,
        Gary Guo <gary@garyguo.net>,
        =?iso-8859-1?Q?Bj=F6rn?= 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
Message-ID: <Y/WmCtTw2NPFdJBC@Boquns-Mac-mini.local>
References: <20230221-gpu-up-time-v1-1-bf8fe74b7f55@asahilina.net>
 <87v8jvnqq4.ffs@tglx>
 <Y/TP6as7qqwfcI42@boqun-archlinux>
 <87h6vfnh0f.ffs@tglx>
 <7b93bf74-abdc-f8c1-9a12-7c7f080f9e19@asahilina.net>
 <87edqioo1e.ffs@tglx>
 <CANiq72n-++roTv3yDNOA=Mi_sscBZX3xPdZ6RZ0Oxx+T82pf5A@mail.gmail.com>
 <87o7pmmtre.ffs@tglx>
 <Y/WDz013yBIfQPDq@boqun-archlinux>
 <03e9af69-3fea-032d-6dc3-ac0ea8e57471@asahilina.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <03e9af69-3fea-032d-6dc3-ac0ea8e57471@asahilina.net>
Precedence: bulk

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@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?
> 
> 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