Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756048AbdHYNky convert rfc822-to-8bit (ORCPT ); Fri, 25 Aug 2017 09:40:54 -0400 Received: from mail.fireflyinternet.com ([109.228.58.192]:49589 "EHLO fireflyinternet.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1755890AbdHYNkw (ORCPT ); Fri, 25 Aug 2017 09:40:52 -0400 X-Default-Received-SPF: pass (skip=forwardok (res=PASS)) x-ip-name=78.156.65.138; Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8BIT To: John Stultz , "lkml" From: Chris Wilson In-Reply-To: <1495856035-6622-5-git-send-email-john.stultz@linaro.org> Cc: "John Stultz" , "Thomas Gleixner" , "Ingo Molnar" , "Miroslav Lichvar" , "Richard Cochran" , "Prarit Bhargava" , "Stephen Boyd" , "Kevin Brodsky" , "Will Deacon" , "Daniel Mentz" References: <1495856035-6622-1-git-send-email-john.stultz@linaro.org> <1495856035-6622-5-git-send-email-john.stultz@linaro.org> Message-ID: <150366841319.27971.6041120504203143444@mail.alporthouse.com> User-Agent: alot/0.3.6 Subject: Re: [RFC][PATCH 4/4] time: Clean up CLOCK_MONOTONIC_RAW time handling Date: Fri, 25 Aug 2017 14:40:13 +0100 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 9630 Lines: 220 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 > Cc: Ingo Molnar > Cc: Miroslav Lichvar > Cc: Richard Cochran > Cc: Prarit Bhargava > Cc: Stephen Boyd > Cc: Kevin Brodsky > Cc: Will Deacon > Cc: Daniel Mentz > Signed-off-by: John Stultz > --- > 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 >