Hello Thomas,
On Friday, May 26, 2017 1:05 PM, Thomas Gleixner wrote:
> On Wed, 24 May 2017, Mirea, Bogdan-Stefan wrote:
> > I am thinking about using timekeeping_inject_sleeptime64(delta) hook to
>
> That's not a hook. It's a regular function. Please use proper technical
> terms.
>
> > add a delta time at boot to the CLOCK_BOOTTIME, but the problem that
> > arise here is that this hook is intended to be used on rtc_resume()
> > code (to add the time spent in suspend to CLOCK_BOOTTIME).
>
> It's not the intention. It _IS_ used for injecting the suspended time.
>
> > >From my point of view this will do the trick even if the
> > CONFIG_BOOT_TIME_PRESERVE will then depend on PM_SLEEP &&
> RTC_HCTOSYS
> > (needed by timekeeping_inject_sleeptime64 to work).
>
> First of all there is no point in having this extra CONFIG switch. This can
> be made unconditionally and depend on a kernel command line argument.
>
> What has this to do with PM_SLEEP and RTC_HCTOSYS? Just because that
> function is currently only available when these config switches are enabled
> it does not mean that it cannot be made available unconditionally.
>
> What you are trying to do is to cram that new functionality into the
> existing code without actually spending time on thinking about a proper
> design.
>
> Let's talk about the objective.
>
> 1) Insert time from system start (power on, hardware reset) to kernel start
> (timekeeping init) into CLOCK_BOOTTIME, if the system can read out this
> value.
>
> 2) Coordinate sched_clock and CLOCK_BOOTTIME
>
> #1 timekeeping_inject_sleeptime64() provides the required functionality
> already. We can rename that function to reflect that it's used for both
> (injecting suspend time and injecting system start time).
>
> The more interesting question is how to provide a generic mechanism to
> retrieve that value. There are two possibilities:
>
> A) Value is stored by the early boot code and retrieved from there
>
> B) Value is made available via a clocksource after initializing and
> registering it.
>
> #A can be solved by having a weak function
>
> u64 __weak read_system_starttime(void)
> {
> return 0;
> }
>
> and let architectures implementing it when required. Though I doubt
> that we need that right away.
>
> #B can be made generic in a very simple way
>
> Add a flag CLOCK_SOURCE_STARTTIME to the clocksource flags and let the
> core code use that flag exactly once to inject the system start time
> into CLOCK_BOOTTIME.
>
> #2 depends on #1
>
> When the timekeeping core injects the system start time it can tell the
> sched_clock core to use that offset as well.
>
> But, that does not solve the problem that sched_clock and CLOCK_BOOTTIME
> will drift apart over time which makes the whole thing moot.
>
> There was a patch set floating around, which made the clock used for
> printk selectable so it can actually use CLOCK_MONOTONIC or
> CLOCK_BOOTTIME via ktime_get_[mono|boot]_fast_ns(), but that never got
> merged.
>
> So instead of trying to provide a half correct solution which does not
> allow you to coordinate dmesg timestamps, uptime, tracer timestamps
> etc. reliably due to drift, I rather have that dmesg selectable
> timestamping patch merged and provide coordinated timestamps that way.
> (Cc'ed Prarit, who was working on that)
>
> Thanks,
>
> tglx
>
>
Thank you for your valuable and complete answer, I will consider everything you
said and update accordingly. I'll also notify you when this is ready.
Kind Regards,
Bogdan
