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

Groups > linux.kernel > #1410342 > unrolled thread

[PATCH] timekeeping: Fix 1ns/tick drift with GENERIC_TIME_VSYSCALL_OLD

Back to article view | Back to linux.kernel

Contents

  [PATCH] timekeeping: Fix 1ns/tick drift with GENERIC_TIME_VSYSCALL_OLD Thomas Graziadei <thomas.graziadei@omicronenergy.com> - 2016-05-31 16:10 +0200
    Re: [PATCH] timekeeping: Fix 1ns/tick drift with GENERIC_TIME_VSYSCALL_OLD John Stultz <john.stultz@linaro.org> - 2016-06-01 01:20 +0200
      Re: [PATCH] timekeeping: Fix 1ns/tick drift with  GENERIC_TIME_VSYSCALL_OLD Thomas Graziadei <thomas.graziadei@omicronenergy.com> - 2016-06-01 14:10 +0200

#1410342 — [PATCH] timekeeping: Fix 1ns/tick drift with GENERIC_TIME_VSYSCALL_OLD

From: Thomas Graziadei <thomas.graziadei@omicronenergy.com>

The user notices the problem in a raw and real time drift, calling
clock_gettime with CLOCK_REALTIME / CLOCK_MONOTONIC_RAW on a system
with no ntp correction taking place (no ntpd or ptp stuff running).

The problem is, that old_vsyscall_fixup adds an extra 1ns even though
xtime_nsec is already held in full nsecs and the remainder in this
case is 0. Do the rounding up buisness only if needed.

Signed-off-by: Thomas Graziadei <thomas.graziadei@omicronenergy.com>
---
 kernel/time/timekeeping.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 479d25c..a196e08 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -480,10 +480,12 @@ static inline void old_vsyscall_fixup(struct timekeeper *tk)
 	* users are removed, this can be killed.
 	*/
 	remainder = tk->tkr_mono.xtime_nsec & ((1ULL << tk->tkr_mono.shift) - 1);
-	tk->tkr_mono.xtime_nsec -= remainder;
-	tk->tkr_mono.xtime_nsec += 1ULL << tk->tkr_mono.shift;
-	tk->ntp_error += remainder << tk->ntp_error_shift;
-	tk->ntp_error -= (1ULL << tk->tkr_mono.shift) << tk->ntp_error_shift;
+	if (remainder != 0) {
+		tk->tkr_mono.xtime_nsec -= remainder;
+		tk->tkr_mono.xtime_nsec += 1ULL << tk->tkr_mono.shift;
+		tk->ntp_error += remainder << tk->ntp_error_shift;
+		tk->ntp_error -= (1ULL << tk->tkr_mono.shift) << tk->ntp_error_shift;
+	}
 }
 #else
 #define old_vsyscall_fixup(tk)
-- 
1.9.1

[toc] | [next] | [standalone]

#1410676

On Tue, May 31, 2016 at 6:06 AM, Thomas Graziadei
<thomas.graziadei@omicronenergy.com> wrote:
> From: Thomas Graziadei <thomas.graziadei@omicronenergy.com>
>
> The user notices the problem in a raw and real time drift, calling
> clock_gettime with CLOCK_REALTIME / CLOCK_MONOTONIC_RAW on a system
> with no ntp correction taking place (no ntpd or ptp stuff running).

Hmm.. Curious. Was it actually drifting, or was it just
oscillating/ringing near the RAW clock's value?

> The problem is, that old_vsyscall_fixup adds an extra 1ns even though
> xtime_nsec is already held in full nsecs and the remainder in this
> case is 0. Do the rounding up buisness only if needed.

The patch looks ok. But I'm curious what architecture you were seeing
this on (ia64, powerpc?), as it would be much nicer to have those
architectures migrate off of the old low-res vsyscall calculation and
use the newer method with sub-ns precision, instead of trying to
further fix up the deprecated method.

I had submitted a patch to convert ia64 awhile back, but I don't
recall getting much feedback.

thanks
-john

[toc] | [prev] | [next] | [standalone]

#1411136 — Re: [PATCH] timekeeping: Fix 1ns/tick drift with GENERIC_TIME_VSYSCALL_OLD

On 06/01/2016 01:11 AM, John Stultz wrote:
> On Tue, May 31, 2016 at 6:06 AM, Thomas Graziadei
> <thomas.graziadei@omicronenergy.com> wrote:
>> From: Thomas Graziadei <thomas.graziadei@omicronenergy.com>
>>
>> The user notices the problem in a raw and real time drift, calling
>> clock_gettime with CLOCK_REALTIME / CLOCK_MONOTONIC_RAW on a system
>> with no ntp correction taking place (no ntpd or ptp stuff running).
>
> Hmm.. Curious. Was it actually drifting, or was it just
> oscillating/ringing near the RAW clock's value?

It is actually drifting.

This is the output from a little test program:

realtime  : 1464775074:846282133
raw time  : 1054:851963700
drift_real: 999402ns

total duration: 1000s 158517540ns

>
>> The problem is, that old_vsyscall_fixup adds an extra 1ns even though
>> xtime_nsec is already held in full nsecs and the remainder in this
>> case is 0. Do the rounding up buisness only if needed.
>
> The patch looks ok. But I'm curious what architecture you were seeing
> this on (ia64, powerpc?), as it would be much nicer to have those
> architectures migrate off of the old low-res vsyscall calculation and
> use the newer method with sub-ns precision, instead of trying to
> further fix up the deprecated method.
>
> I had submitted a patch to convert ia64 awhile back, but I don't
> recall getting much feedback.
>

We are using a powerpc architecture.

I guess you are right, it would be nicer to use the new method but then 
on the other hand, this timing topic is rather new to me.

> thanks
> -john
>

thanks,
Thomas

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web