From: Pavel Tatashin <pasha.tatashin@soleen.com>
To: Marc Zyngier <marc.zyngier@arm.com>
Cc: Michal Hocko <mhocko@suse.com>,
Ard Biesheuvel <ard.biesheuvel@linaro.org>,
sboyd@kernel.org, catalin.marinas@arm.com,
Will Deacon <will.deacon@arm.com>,
LKML <linux-kernel@vger.kernel.org>,
rppt@linux.vnet.ibm.com, james.morse@arm.com,
andrew.murray@arm.com, Andrew Morton <akpm@linux-foundation.org>,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v3 3/3] arm64: Early boot time stamps
Date: Fri, 4 Jan 2019 15:54:20 -0500 [thread overview]
Message-ID: <CA+CK2bAMSspkLmvypqEgO-4mzNvE7JzjkkTWv7Pq-+DanZR+xw@mail.gmail.com> (raw)
In-Reply-To: <54caed52-2859-df94-1e3b-223397d42602@arm.com>
> > 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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
prev parent reply other threads:[~2019-01-04 20:54 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-12-26 16:45 [PATCH v3 0/3] Early boot time stamps for arm64 Pavel Tatashin
2018-12-26 16:45 ` [PATCH v3 1/3] arm_arch_timer: add macro for timer nbits Pavel Tatashin
2018-12-26 16:45 ` [PATCH v3 2/3] arm64: vdso: Use ARCH_TIMER_NBITS to calculate mask Pavel Tatashin
2018-12-26 16:45 ` [PATCH v3 3/3] arm64: Early boot time stamps Pavel Tatashin
2019-01-03 10:51 ` Marc Zyngier
2019-01-03 19:58 ` Pavel Tatashin
2019-01-04 15:39 ` Marc Zyngier
2019-01-04 16:23 ` Pavel Tatashin
2019-01-04 16:49 ` Marc Zyngier
2019-01-04 20:54 ` Pavel Tatashin [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CA+CK2bAMSspkLmvypqEgO-4mzNvE7JzjkkTWv7Pq-+DanZR+xw@mail.gmail.com \
--to=pasha.tatashin@soleen.com \
--cc=akpm@linux-foundation.org \
--cc=andrew.murray@arm.com \
--cc=ard.biesheuvel@linaro.org \
--cc=catalin.marinas@arm.com \
--cc=james.morse@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=marc.zyngier@arm.com \
--cc=mhocko@suse.com \
--cc=rppt@linux.vnet.ibm.com \
--cc=sboyd@kernel.org \
--cc=will.deacon@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).