Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1651792

Re: [RFC][PATCH 2/4] time: Fix CLOCK_MONOTONIC_RAW sub-nanosecond accounting

Show all headers | View raw

* John Stultz <john.stultz@linaro.org> wrote:

> Due to how the MONOTONIC_RAW accumulation logic was handled,
> there is the potential for a 1ns discontinuity when we do
> accumulations. This small discontinuity has for the most part
> gone un-noticed, but since ARM64 enabled CLOCK_MONOTONIC_RAW
> in their vDSO clock_gettime implementation, we've seen failures
> with the inconsistency-check test in kselftest.
> 
> This patch addresses the issue by using the same sub-ns
> accumulation handling that CLOCK_MONOTONIC uses, which avoids
> the issue for in-kernel users.
> 
> Since the ARM64 vDSO implementation has its own clock_gettime
> calculation logic, this patch reduces the frequency of errors,
> but failures are still seen. The ARM64 vDSO will need to be
> updated to include the sub-nanosecond xtime_nsec values in its
> calculation for this issue to be completely fixed.
> 
> 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>
> ---
>  include/linux/timekeeper_internal.h |  4 ++--
>  kernel/time/timekeeping.c           | 21 ++++++++++++---------
>  2 files changed, 14 insertions(+), 11 deletions(-)
> 
> diff --git a/include/linux/timekeeper_internal.h b/include/linux/timekeeper_internal.h
> index 110f453..528cc86 100644
> --- a/include/linux/timekeeper_internal.h
> +++ b/include/linux/timekeeper_internal.h
> @@ -58,7 +58,7 @@ struct tk_read_base {
>   *			interval.
>   * @xtime_remainder:	Shifted nano seconds left over when rounding
>   *			@cycle_interval
> - * @raw_interval:	Raw nano seconds accumulated per NTP interval.
> + * @raw_interval:	Shifted raw nano seconds accumulated per NTP interval.
>   * @ntp_error:		Difference between accumulated time and NTP time in ntp
>   *			shifted nano seconds.
>   * @ntp_error_shift:	Shift conversion between clock shifted nano seconds and
> @@ -100,7 +100,7 @@ struct timekeeper {
>  	u64			cycle_interval;
>  	u64			xtime_interval;
>  	s64			xtime_remainder;
> -	u32			raw_interval;
> +	u64			raw_interval;
>  	/* The ntp_tick_length() value currently being used.
>  	 * This cached copy ensures we consistently apply the tick
>  	 * length for an entire tick, as ntp_tick_length may change
> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> index abc1968..35d3ba3 100644
> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
> @@ -282,7 +282,7 @@ static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock)
>  	/* Go back from cycles -> shifted ns */
>  	tk->xtime_interval = interval * clock->mult;
>  	tk->xtime_remainder = ntpinterval - tk->xtime_interval;
> -	tk->raw_interval = (interval * clock->mult) >> clock->shift;
> +	tk->raw_interval = interval * clock->mult;
>  
>  	 /* if changing clocks, convert xtime_nsec shift units */
>  	if (old_clock) {
> @@ -1994,7 +1994,7 @@ static u64 logarithmic_accumulation(struct timekeeper *tk, u64 offset,
>  				    u32 shift, unsigned int *clock_set)
>  {
>  	u64 interval = tk->cycle_interval << shift;
> -	u64 raw_nsecs;
> +	u64 nsecps;

What does the 'ps' postfix stand for? It's not obvious (to me).

> +	tk->tkr_raw.xtime_nsec += (u64)tk->raw_time.tv_nsec
> +					<< tk->tkr_raw.shift;
> +	tk->tkr_raw.xtime_nsec -= (u64)tk->raw_time.tv_nsec
> +					<< tk->tkr_raw.shift;

Please don't break the line in such an ugly, random way, just write:

	tk->tkr_raw.xtime_nsec += (u64)tk->raw_time.tv_nsec << tk->tkr_raw.shift;
	tk->tkr_raw.xtime_nsec -= (u64)tk->raw_time.tv_nsec << tk->tkr_raw.shift;

Thanks,

	Ingo

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[RFC][PATCH 0/4] Fixes for two recently found timekeeping bugs John Stultz <john.stultz@linaro.org> - 2017-05-27 05:40 +0200
  [RFC][PATCH 4/4] time: Clean up CLOCK_MONOTONIC_RAW time handling John Stultz <john.stultz@linaro.org> - 2017-05-27 05:40 +0200
  [RFC][PATCH 2/4] time: Fix CLOCK_MONOTONIC_RAW sub-nanosecond accounting John Stultz <john.stultz@linaro.org> - 2017-05-27 05:40 +0200
    Re: [RFC][PATCH 2/4] time: Fix CLOCK_MONOTONIC_RAW sub-nanosecond  accounting Ingo Molnar <mingo@kernel.org> - 2017-05-27 09:40 +0200
      Re: [RFC][PATCH 2/4] time: Fix CLOCK_MONOTONIC_RAW sub-nanosecond accounting Daniel Mentz <danielmentz@google.com> - 2017-05-30 20:50 +0200
  [RFC][PATCH 1/4] time: Fix clock->read(clock) race around clocksource changes John Stultz <john.stultz@linaro.org> - 2017-05-27 05:40 +0200
    Re: [RFC][PATCH 1/4] time: Fix clock->read(clock) race around  clocksource changes Ingo Molnar <mingo@kernel.org> - 2017-05-27 09:40 +0200
  [RFC][PATCH 3/4] arm64: vdso: Fix nsec handling for CLOCK_MONOTONIC_RAW John Stultz <john.stultz@linaro.org> - 2017-05-27 06:20 +0200
    Re: [RFC][PATCH 3/4] arm64: vdso: Fix nsec handling for  CLOCK_MONOTONIC_RAW Will Deacon <will.deacon@arm.com> - 2017-05-30 11:40 +0200
  Re: [RFC][PATCH 0/4] Fixes for two recently found timekeeping bugs Ingo Molnar <mingo@kernel.org> - 2017-05-27 09:40 +0200
    Re: [RFC][PATCH 0/4] Fixes for two recently found timekeeping bugs John Stultz <john.stultz@linaro.org> - 2017-05-27 18:20 +0200
      Re: [RFC][PATCH 0/4] Fixes for two recently found timekeeping bugs Ingo Molnar <mingo@kernel.org> - 2017-05-28 11:00 +0200
  Re: [RFC][PATCH 0/4] Fixes for two recently found timekeeping bugs Daniel Mentz <danielmentz@google.com> - 2017-05-31 02:20 +0200

csiph-web