lkml.org 
[lkml]   [2017]   [Aug]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    SubjectRe: [RFC][PATCH 4/4] time: Clean up CLOCK_MONOTONIC_RAW time handling
    Date
    Quoting John Stultz (2017-05-27 04:33:55)
    > Now that we fixed the sub-ns handling for CLOCK_MONOTONIC_RAW,
    > remove the duplicitive tk->raw_time.tv_nsec, which can be
    > stored in tk->tkr_raw.xtime_nsec (similarly to how its handled
    > for monotonic time).

    This patch breaks tkr_raw pretty badly. It is now accumulating 2x faster
    than tkr_mono, with the occasional wacky jump between two reads.

    e.g:

    @@ -838,6 +840,11 @@ ktime_t ktime_get_raw(void)

    } while (read_seqcount_retry(&tk_core.seq, seq));

    + pr_err("%s: raw.base=%llu, mono.base=%llu: nsecs=%llu, mono.nsecs=%llu\n",
    + __func__,
    + ktime_to_ns(base), ktime_to_ns(tk->tkr_mono.base),
    + nsecs, timekeeping_get_ns(&tk->tkr_mono));
    +

    gave
    ktime_get_raw: raw.base=40255680121, mono.base=39940532364: nsecs=261321742, mono.nsecs=318630514
    ktime_get_raw: raw.base=40339680124, mono.base=39940532364: nsecs=345836800, mono.nsecs=403109209

    https://bugs.freedesktop.org/show_bug.cgi?id=102336

    The patch still reverts cleanly and aiui was not part of the bugfixes,
    but a cleanup afterwards; so can be reapplied at leisure.

    > Cc: Thomas Gleixner <tglx@linutronix.de>
    > Cc: Ingo Molnar <mingo@kernel.org>
    > Cc: Miroslav Lichvar <mlichvar@redhat.com>
    > Cc: Richard Cochran <richardcochran@gmail.com>
    > Cc: Prarit Bhargava <prarit@redhat.com>
    > Cc: Stephen Boyd <stephen.boyd@linaro.org>
    > Cc: Kevin Brodsky <kevin.brodsky@arm.com>
    > Cc: Will Deacon <will.deacon@arm.com>
    > Cc: Daniel Mentz <danielmentz@google.com>
    > Signed-off-by: John Stultz <john.stultz@linaro.org>
    > ---
    > arch/arm64/kernel/vdso.c | 6 ++---
    > include/linux/timekeeper_internal.h | 4 ++--
    > kernel/time/timekeeping.c | 47 ++++++++++++++++++++-----------------
    > 3 files changed, 29 insertions(+), 28 deletions(-)
    >
    > diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
    > index d0cb007..7492d90 100644
    > --- a/arch/arm64/kernel/vdso.c
    > +++ b/arch/arm64/kernel/vdso.c
    > @@ -220,10 +220,8 @@ void update_vsyscall(struct timekeeper *tk)
    > if (!use_syscall) {
    > /* tkr_mono.cycle_last == tkr_raw.cycle_last */
    > vdso_data->cs_cycle_last = tk->tkr_mono.cycle_last;
    > - vdso_data->raw_time_sec = tk->raw_time.tv_sec;
    > - vdso_data->raw_time_nsec = (tk->raw_time.tv_nsec <<
    > - tk->tkr_raw.shift) +
    > - tk->tkr_raw.xtime_nsec;
    > + vdso_data->raw_time_sec = tk->raw_sec;
    > + vdso_data->raw_time_nsec = tk->tkr_raw.xtime_nsec;
    > vdso_data->xtime_clock_sec = tk->xtime_sec;
    > vdso_data->xtime_clock_nsec = tk->tkr_mono.xtime_nsec;
    > vdso_data->cs_mono_mult = tk->tkr_mono.mult;
    > diff --git a/include/linux/timekeeper_internal.h b/include/linux/timekeeper_internal.h
    > index 528cc86..8abf6df 100644
    > --- a/include/linux/timekeeper_internal.h
    > +++ b/include/linux/timekeeper_internal.h
    > @@ -52,7 +52,7 @@ struct tk_read_base {
    > * @clock_was_set_seq: The sequence number of clock was set events
    > * @cs_was_changed_seq: The sequence number of clocksource change events
    > * @next_leap_ktime: CLOCK_MONOTONIC time value of a pending leap-second
    > - * @raw_time: Monotonic raw base time in timespec64 format
    > + * @raw_sec: CLOCK_MONOTONIC_RAW time in seconds
    > * @cycle_interval: Number of clock cycles in one NTP interval
    > * @xtime_interval: Number of clock shifted nano seconds in one NTP
    > * interval.
    > @@ -94,7 +94,7 @@ struct timekeeper {
    > unsigned int clock_was_set_seq;
    > u8 cs_was_changed_seq;
    > ktime_t next_leap_ktime;
    > - struct timespec64 raw_time;
    > + u64 raw_sec;
    >
    > /* The following members are for timekeeping internal use */
    > u64 cycle_interval;
    > diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
    > index 35d3ba3..0c3b8c1 100644
    > --- a/kernel/time/timekeeping.c
    > +++ b/kernel/time/timekeeping.c
    > @@ -72,6 +72,10 @@ static inline void tk_normalize_xtime(struct timekeeper *tk)
    > tk->tkr_mono.xtime_nsec -= (u64)NSEC_PER_SEC << tk->tkr_mono.shift;
    > tk->xtime_sec++;
    > }
    > + while (tk->tkr_raw.xtime_nsec >= ((u64)NSEC_PER_SEC << tk->tkr_raw.shift)) {
    > + tk->tkr_raw.xtime_nsec -= (u64)NSEC_PER_SEC << tk->tkr_raw.shift;
    > + tk->raw_sec++;
    > + }
    > }
    >
    > static inline struct timespec64 tk_xtime(struct timekeeper *tk)
    > @@ -287,12 +291,14 @@ static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock)
    > /* if changing clocks, convert xtime_nsec shift units */
    > if (old_clock) {
    > int shift_change = clock->shift - old_clock->shift;
    > - if (shift_change < 0)
    > + if (shift_change < 0) {
    > tk->tkr_mono.xtime_nsec >>= -shift_change;
    > - else
    > + tk->tkr_raw.xtime_nsec >>= -shift_change;
    > + } else {
    > tk->tkr_mono.xtime_nsec <<= shift_change;
    > + tk->tkr_raw.xtime_nsec <<= shift_change;
    > + }
    > }
    > - tk->tkr_raw.xtime_nsec = 0;
    >
    > tk->tkr_mono.shift = clock->shift;
    > tk->tkr_raw.shift = clock->shift;
    > @@ -617,9 +623,6 @@ static inline void tk_update_ktime_data(struct timekeeper *tk)
    > nsec = (u32) tk->wall_to_monotonic.tv_nsec;
    > tk->tkr_mono.base = ns_to_ktime(seconds * NSEC_PER_SEC + nsec);
    >
    > - /* Update the monotonic raw base */
    > - tk->tkr_raw.base = timespec64_to_ktime(tk->raw_time);
    > -
    > /*
    > * The sum of the nanoseconds portions of xtime and
    > * wall_to_monotonic can be greater/equal one second. Take
    > @@ -629,6 +632,11 @@ static inline void tk_update_ktime_data(struct timekeeper *tk)
    > if (nsec >= NSEC_PER_SEC)
    > seconds++;
    > tk->ktime_sec = seconds;
    > +
    > + /* Update the monotonic raw base */
    > + seconds = tk->raw_sec;
    > + nsec = (u32)(tk->tkr_raw.xtime_nsec >> tk->tkr_raw.shift);
    > + tk->tkr_raw.base = ns_to_ktime(seconds * NSEC_PER_SEC + nsec);
    > }
    >
    > /* must hold timekeeper_lock */
    > @@ -670,7 +678,6 @@ static void timekeeping_update(struct timekeeper *tk, unsigned int action)
    > static void timekeeping_forward_now(struct timekeeper *tk)
    > {
    > u64 cycle_now, delta;
    > - u64 nsec;
    >
    > cycle_now = tk_clock_read(&tk->tkr_mono);
    > delta = clocksource_delta(cycle_now, tk->tkr_mono.cycle_last, tk->tkr_mono.mask);
    > @@ -682,10 +689,13 @@ static void timekeeping_forward_now(struct timekeeper *tk)
    > /* If arch requires, add in get_arch_timeoffset() */
    > tk->tkr_mono.xtime_nsec += (u64)arch_gettimeoffset() << tk->tkr_mono.shift;
    >
    > - tk_normalize_xtime(tk);
    >
    > - nsec = clocksource_cyc2ns(delta, tk->tkr_raw.mult, tk->tkr_raw.shift);
    > - timespec64_add_ns(&tk->raw_time, nsec);
    > + tk->tkr_raw.xtime_nsec += delta * tk->tkr_raw.mult;
    > +
    > + /* If arch requires, add in get_arch_timeoffset() */
    > + tk->tkr_raw.xtime_nsec += (u64)arch_gettimeoffset() << tk->tkr_raw.shift;
    > +
    > + tk_normalize_xtime(tk);
    > }
    >
    > /**
    > @@ -1371,19 +1381,18 @@ int timekeeping_notify(struct clocksource *clock)
    > void getrawmonotonic64(struct timespec64 *ts)
    > {
    > struct timekeeper *tk = &tk_core.timekeeper;
    > - struct timespec64 ts64;
    > unsigned long seq;
    > u64 nsecs;
    >
    > do {
    > seq = read_seqcount_begin(&tk_core.seq);
    > + ts->tv_sec = tk->raw_sec;
    > nsecs = timekeeping_get_ns(&tk->tkr_raw);
    > - ts64 = tk->raw_time;
    >
    > } while (read_seqcount_retry(&tk_core.seq, seq));
    >
    > - timespec64_add_ns(&ts64, nsecs);
    > - *ts = ts64;
    > + ts->tv_nsec = 0;
    > + timespec64_add_ns(ts, nsecs);
    > }
    > EXPORT_SYMBOL(getrawmonotonic64);
    >
    > @@ -1507,8 +1516,7 @@ void __init timekeeping_init(void)
    > tk_setup_internals(tk, clock);
    >
    > tk_set_xtime(tk, &now);
    > - tk->raw_time.tv_sec = 0;
    > - tk->raw_time.tv_nsec = 0;
    > + tk->raw_sec = 0;
    > if (boot.tv_sec == 0 && boot.tv_nsec == 0)
    > boot = tk_xtime(tk);
    >
    > @@ -2009,17 +2017,12 @@ static u64 logarithmic_accumulation(struct timekeeper *tk, u64 offset,
    > *clock_set |= accumulate_nsecs_to_secs(tk);
    >
    > /* Accumulate raw time */
    > - tk->tkr_raw.xtime_nsec += (u64)tk->raw_time.tv_nsec
    > - << tk->tkr_raw.shift;
    > tk->tkr_raw.xtime_nsec += tk->raw_interval << shift;
    > nsecps = (u64)NSEC_PER_SEC << tk->tkr_raw.shift;
    > while (tk->tkr_raw.xtime_nsec >= nsecps) {
    > tk->tkr_raw.xtime_nsec -= nsecps;
    > - tk->raw_time.tv_sec++;
    > + tk->raw_sec++;
    > }
    > - tk->raw_time.tv_nsec = tk->tkr_raw.xtime_nsec >> tk->tkr_raw.shift;
    > - tk->tkr_raw.xtime_nsec -= (u64)tk->raw_time.tv_nsec
    > - << tk->tkr_raw.shift;
    >
    > /* Accumulate error between NTP and clock interval */
    > tk->ntp_error += tk->ntp_tick << shift;
    > --
    > 2.7.4
    >

    \
     
     \ /
      Last update: 2017-08-25 15:41    [W:4.128 / U:0.024 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site