Re: [PATCH] timekeeping: move multigrain ctime floor handling into timekeeper
From: Arnd Bergmann
Date: Thu Sep 12 2024 - 12:53:29 EST
On Thu, Sep 12, 2024, at 14:37, Jeff Layton wrote:
> On Thu, 2024-09-12 at 09:26 -0400, Jeff Layton wrote:
>> On Thu, 2024-09-12 at 13:17 +0000, Arnd Bergmann wrote:
>> > On Thu, Sep 12, 2024, at 11:34, Jeff Layton wrote:
>>
>> I'll plan to hack something together later today and see how it does.
>>
>
> Ok, already hit a couple of problems:
>
> First, moving the floor word into struct timekeeper is probably not a
> good idea. This is going to be updated more often than the rest of the
> timekeeper, and so its cacheline will be invalidated more. I think we
> need to keep the floor word on its own cacheline. It can be a static
> u64 though inside timekeeper.c.
Right.
> So, I think that we actually need an API like this:
>
> /* returns opaque cookie value */
> u64 ktime_get_coarse_real_ts64_mg(struct timespec64 *ts);
>
> /* accepts opaque cookie value from above function */
> void ktime_get_real_ts64_mg(struct timespec64 *ts, u64 cookie);
>
> The first function fills in @ts with the max of coarse time and floor,
> and returns an opaque cookie (a copy of the floor word). The second
> fetches a fine-grained timestamp and uses the floor cookie as the "old"
> value when doing the cmpxchg, and then fills in @ts with the result.
I think you lost me here, I'd need to look at the code in
more detail to understand it.
> Does that sound reasonable? If so, then the next question is around
> what the floor word should hold:
>
> IMO, just keeping it as a monotonic time value seems simplest. I'm
> struggling to understand where the "delta" portion would come from in
> your earlier proposal, and the fact that that value could overflow
> seems less than ideal.
I was thinking of the diffence between tk->xtime_nsec and the
computed nsecs in ktime_get_real_ts64().
The calculation is what is in timekeeping_cycles_to_ns(),
with the "+ tkr->xtime_nsec" left out, roughly
((tk_clock_read(tkr) - tkr->cycle_last) & tkr->mask) * \
tkr->mult >> tkr->shift
There are a few subtleties here, including the possible
1-bit rounding error from the shift.
Arnd