Message-ID: <03e9af69-3fea-032d-6dc3-ac0ea8e57471@asahilina.net>
Date:   Wed, 22 Feb 2023 13:45:58 +0900
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.9.1
Subject: Re: [PATCH] rust: time: New module for timekeeping functions
Content-Language: en-US
To:     Boqun Feng <boqun.feng@gmail.com>,
        Thomas Gleixner <tglx@linutronix.de>
Cc:     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>,
        =?UTF-8?Q?Bj=c3=b6rn_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>
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>
From:   Asahi Lina <lina@asahilina.net>
In-Reply-To: <Y/WDz013yBIfQPDq@boqun-archlinux>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

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
the same as my original patch behind the scenes, but wrapped in type
safety).

~~ Lina