Re: [PATCH] timekeeping: move multigrain ctime floor handling into timekeeper

["Arnd Bergmann" <arnd@kernel.org>]

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