lkml.org 
[lkml]   [2017]   [May]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRE: [PATCH v3] Added "Preserve Boot Time Support"
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



\
 
 \ /
  Last update: 2017-05-26 12:05    [W:0.068 / U:1.948 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site