Message-ID: <CA+CK2bAMSspkLmvypqEgO-4mzNvE7JzjkkTWv7Pq-+DanZR+xw@mail.gmail.com>
Date:   Fri, 4 Jan 2019 15:54:20 -0500
From:   Pavel Tatashin <pasha.tatashin@...een.com>
To:     Marc Zyngier <marc.zyngier@....com>
Cc:     catalin.marinas@....com, Will Deacon <will.deacon@....com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        rppt@...ux.vnet.ibm.com, Michal Hocko <mhocko@...e.com>,
        Ard Biesheuvel <ard.biesheuvel@...aro.org>,
        andrew.murray@....com, james.morse@....com, sboyd@...nel.org,
        linux-arm-kernel@...ts.infradead.org,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 3/3] arm64: Early boot time stamps

> > sched_clock() will still be strictly monotonic. During switch over we
> > will guarantee to continue from where the early clock left.
>
> Not quite. There is at least one broken integration that results in
> large, spurious jumps ahead. If one of these jumps happens during the
> "unstable" phase, we'll only return old_clock. At some point, we switch
> early_unstable_clock to be false, as we've now properly initialized the
> timer and found the appropriate workaround. We'll now return a much
> smaller value. sched_clock continuity doesn't seem to apply here, as
> you're not registering a new sched_clock (or at least that's not how I
> understand your code above).
>
> >> What I'm proposing is that we allow architectures to override the hard
> >> tie between local_clock/sched_clock and kernel log time stamping, with
> >> the default being of course what we have today. This gives a clean
> >> separation between the two when the architecture needs to delay the
> >> availability of sched_clock until implementation requirements are
> >> discovered. It also keep sched_clock simple and efficient.
> >>
> >> To illustrate what I'm trying to argue for, I've pushed out a couple
> >> of proof of concept patches here[1]. I've briefly tested them in a
> >> guest, and things seem to work OK.
> >
> > What I am worried is that decoupling time stamps from the
> > sched_clock() will cause uptime and other commands that show boot time
> > not to correlate with timestamps in dmesg with these changes. For them
> > to correlate we would still have to have a switch back to
> > local_clock() in timestamp_clock() after we are done with early boot,
> > which brings us back to using a temporarily unstable clock that I
> > proposed above but without adding an architectural hook for it. Again,
> > we would need to solve the problem of time continuity during switch
> > over, which is not a hard problem to solve, as we do it already in
> > sched_clock.c, and everytime clocksource changes.
> >
> > During early boot time stamps project for x86 we were extra careful to
> > make sure that they stay the same.
>
> I can see two ways to achieve this requirement:
>
> - we allow timestamp_clock to fall-back to sched_clock once it becomes
> non-zero. It has the drawback of resetting the time stamping in the
> middle of the boot, which isn't great.

Right, I'd like those timestamps to be continuous.

>
> - we allow sched_clock to inherit the timestamp_clock value instead of
> starting at zero like it does now. Not sure if that breaks anything, but
> that's worth trying (it should be a matter of setting new_epoch to zero
> in sched_clock_register).

This is what I am proposing above with my approach. Inherit the last
value of unstable sched_clock before switching to permanent. Please
see [1] how I implemented it, and we can discuss what is better
whether to use timestamp hook in printk or what I am suggestion.

[1] https://github.com/soleen/time_arm64/commits/time
sched_clock: generic unstable clock is a new patch, the other patches
are the ones sent out in this series. Because we use sched_clock() as
the last value calculating epoch in sched_clock_register() we
guarantee monotonicity during clock change.

Thank you,
Pasha

Powered by blists - more mailing lists