timekeeping: Introduce a fast boot clock derived from fast monotonic clock

Unlike monotonic clock, boot clock as a trace clock will account for time spent in suspend useful for tracing suspend/resume. This uses earlier introduced infrastructure for using the fast boot clock.

Cc Steven Rostedt <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc John Stultz <[email protected]>

Cc Ingo Molnar <[email protected]>

Signed-off-by Joel Fernandes <[email protected]>

---
 kernel/trace/trace.c | 1 +
 1 file changed, 1 insertion(+)

kernel/trace/trace.c +0 -0

index 8696ce6..f7b64db 100644

kernel/time/timekeeping.c static struct timekeeper shadow_timekeeper; · line 63

Quoted 9 lines · Joel Fernandes

kernel/time/timekeeping.c +5 -0

@@ -56,6 +56,12 @@ static struct timekeeper shadow_timekeeper;
 struct tk_fast {
      seqcount_t              seq;
      struct tk_read_base     base[2];
+
+     /*
+      * first dimension is based on lower seq bit,
+      * second dimension is for offset type (real, boot, tai)
+      */

Can you figure out why there are comments above the struct which explain the members in Kernel Doc format and not randomly formatted comments inside the struct definition?

kernel/time/timekeeping.c static struct timekeeper shadow_timekeeper; · line 64

Quoted 1 line

kernel/time/timekeeping.c +1 -0

+     ktime_t                 offsets[2][TK_OFFS_MAX];

This bloats fast_tk_raw for no value, along with the extra stores in the update function for fast_tk_raw which will never use that offset stuff.

Aside of that, I really have to ask the question whether it's really necessary to add this extra bloat in storage, update and readout code for something which is not used by most people.

What's wrong with adding a tracepoint into the boot offset update function and let perf or the tracer inject the value of the boot offset into the trace data when starting. The time adjustment can be done in postprocessing.

That should be sufficient for tracing suspend/resume behaviour. If there is not a really convincing reason for having that as a real trace clock then I prefer to avoid that extra stuff.

Hi Thomas,

Quoted 15 lines · Thomas Gleixner

kernel/time/timekeeping.c static struct timekeeper shadow_timekeeper; · line 63

Quoted 9 lines · Joel Fernandes

kernel/time/timekeeping.c +5 -0

@@ -56,6 +56,12 @@ static struct timekeeper shadow_timekeeper;
 struct tk_fast {
      seqcount_t              seq;
      struct tk_read_base     base[2];
+
+     /*
+      * first dimension is based on lower seq bit,
+      * second dimension is for offset type (real, boot, tai)
+      */

Can you figure out why there are comments above the struct which explain the members in Kernel Doc format and not randomly formatted comments inside the struct definition?

Ok sorry. I can move the comments before the function in the prescribed format.

Quoted 5 lines

kernel/time/timekeeping.c static struct timekeeper shadow_timekeeper; · line 64

Quoted 1 line

kernel/time/timekeeping.c +1 -0

+     ktime_t                 offsets[2][TK_OFFS_MAX];

This bloats fast_tk_raw for no value, along with the extra stores in the update function for fast_tk_raw which will never use that offset stuff.

Quoted 8 lines

Aside of that, I really have to ask the question whether it's really necessary to add this extra bloat in storage, update and readout code for something which is not used by most people.

What's wrong with adding a tracepoint into the boot offset update function and let perf or the tracer inject the value of the boot offset into the trace data when starting. The time adjustment can be done in postprocessing.

I agree we're bloating this and probably not very acceptable. tracepoint adds additional complexity and out of tree patches which we'd like to avoid. Would you be Ok if we added a relatively simple function like below that could do the job and not bloat the optimal case?

/*
* Fast and NMI safe access to boot time. It may be slightly out of date
* as we're accessing offset without seqcount writes, but is safe to access.

*/
u64 ktime_get_boot_fast_ns(void)

{
        struct timekeeper *tk = &tk_core.timekeeper;
        return __ktime_get_fast_ns(&tk_fast_mono) + tk->offs_boot;

}
EXPORT_SYMBOL_GPL(ktime_get_boot_fast_ns);

Quoted 3 lines

That should be sufficient for tracing suspend/resume behaviour. If there is not a really convincing reason for having that as a real trace clock then I prefer to avoid that extra stuff.

Several clocks are accessible just by simple writing of clock name to trace_clock in debugfs. This is really cool and doesn't require any out of tree patches or post processing complexity.

Thanks,
Joel

Quoted 4 lines · Thomas Gleixner

What's wrong with adding a tracepoint into the boot offset update function and let perf or the tracer inject the value of the boot offset into the trace data when starting. The time adjustment can be done in postprocessing.

I agree we're bloating this and probably not very acceptable. tracepoint adds additional complexity and out of tree patches which

we'd like to avoid. Would you be Ok if we added a relatively simple function like below that could do the job and not bloat the optimal case?

/*
* Fast and NMI safe access to boot time. It may be slightly out of date
* as we're accessing offset without seqcount writes, but is safe to access. */
u64 ktime_get_boot_fast_ns(void)

{
        struct timekeeper *tk = &tk_core.timekeeper;
        return __ktime_get_fast_ns(&tk_fast_mono) + tk->offs_boot;

Quoted 3 lines

That should be sufficient for tracing suspend/resume behaviour. If there is not a really convincing reason for having that as a real trace clock then I prefer to avoid that extra stuff.

Several clocks are accessible just by simple writing of clock name to trace_clock in debugfs. This is really cool and doesn't require any out of tree patches or post processing complexity.