From patchwork Wed May 6 05:21:16 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Stultz X-Patchwork-Id: 48048 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-la0-f71.google.com (mail-la0-f71.google.com [209.85.215.71]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 4636D2121F for ; Wed, 6 May 2015 05:21:23 +0000 (UTC) Received: by layy10 with SMTP id y10sf62890115lay.0 for ; Tue, 05 May 2015 22:21:22 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:delivered-to:from:to:subject:date :message-id:x-original-sender:x-original-authentication-results :precedence:mailing-list:list-id:list-post:list-help:list-archive :list-unsubscribe; bh=ehVPOeH185yZg0xv2U3BlgVv6n9B8TLd1NJoOFTmC7A=; b=J3o7Ej7Bo/nUGYmEMhsx/qhPywIWc1EaLCpq5Nj32cs28oNn0DAdu3srpyBVgwM1wF gHYsilJdvrKkALbR8j6J7FLUT0M5mtWauMhpg0XDcxE+03fWiSEZ/SxkXe0XK+5/pBCB QLIa29/k3LOSo7C9C6fhgT+oalWi5tKfVyttkMtO2g0NoT+qIdzreSGqM7J4LxChncWZ uGKlWfdWj9gy8PeZMTXYqrvnELN8yylMeR56SXf1JLyUh94E7fJaJ0/DM/RqN6exXzsG S9CFMfeLfbvLsl0/0AoaHpK+G4rqv136Kcq+EQp0PDDPCXusz9Q72Rbczth3kzb8WQPO ai8A== X-Gm-Message-State: ALoCoQnPmQVYQsokgrsTiIvTtKZzWJ4stEWOm/nolmX6M6NXNkWiboqcSmSfnNQdphAuZcMTeOwA X-Received: by 10.180.211.168 with SMTP id nd8mr591640wic.4.1430889682209; Tue, 05 May 2015 22:21:22 -0700 (PDT) MIME-Version: 1.0 X-BeenThere: patchwork-forward@linaro.org Received: by 10.152.179.232 with SMTP id dj8ls984901lac.74.gmail; Tue, 05 May 2015 22:21:22 -0700 (PDT) X-Received: by 10.112.17.8 with SMTP id k8mr27035917lbd.28.1430889682043; Tue, 05 May 2015 22:21:22 -0700 (PDT) Received: from mail-lb0-f178.google.com (mail-lb0-f178.google.com. [209.85.217.178]) by mx.google.com with ESMTPS id wk8si13999491lbb.96.2015.05.05.22.21.21 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 05 May 2015 22:21:21 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.217.178 as permitted sender) client-ip=209.85.217.178; Received: by lbbqq2 with SMTP id qq2so144433882lbb.3 for ; Tue, 05 May 2015 22:21:21 -0700 (PDT) X-Received: by 10.112.204.6 with SMTP id ku6mr26694304lbc.73.1430889681835; Tue, 05 May 2015 22:21:21 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patches@linaro.org Received: by 10.112.67.65 with SMTP id l1csp2647795lbt; Tue, 5 May 2015 22:21:20 -0700 (PDT) X-Received: by 10.68.161.4 with SMTP id xo4mr58498665pbb.65.1430889679807; Tue, 05 May 2015 22:21:19 -0700 (PDT) Received: from mail-pd0-f178.google.com (mail-pd0-f178.google.com. [209.85.192.178]) by mx.google.com with ESMTPS id fe2si27569397pab.161.2015.05.05.22.21.18 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 05 May 2015 22:21:19 -0700 (PDT) Received-SPF: pass (google.com: domain of john.stultz@linaro.org designates 209.85.192.178 as permitted sender) client-ip=209.85.192.178; Received: by pdea3 with SMTP id a3so220083702pde.3 for ; Tue, 05 May 2015 22:21:18 -0700 (PDT) X-Received: by 10.70.103.45 with SMTP id ft13mr6149913pdb.149.1430889678802; Tue, 05 May 2015 22:21:18 -0700 (PDT) Received: from localhost.localdomain (c-67-170-153-23.hsd1.or.comcast.net. [67.170.153.23]) by mx.google.com with ESMTPSA id ms7sm583202pdb.11.2015.05.05.22.21.17 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Tue, 05 May 2015 22:21:17 -0700 (PDT) From: John Stultz To: vmauery@gmail.com Subject: [PATCH] [RFC] timekeeping: Calculate freq adjustment directly Date: Tue, 5 May 2015 22:21:16 -0700 Message-Id: <1430889676-4789-1-git-send-email-john.stultz@linaro.org> X-Mailer: git-send-email 1.9.1 X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: john.stultz@linaro.org X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.217.178 as permitted sender) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Precedence: list Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org List-ID: X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , After working with Mirolsav's simulator and his initial patch, I've grown more comfortable with his approach of calculating the freq adjustment directly using a division, rather then trying to aproximate it over a number of ticks. Part of the rational here is that now the error adjustment is very small (only a 0 or 1 unit adjustment to the multiplier) it can take quite some time to reduce any accumulated error. The approximation method strains this, since it takes log(adjustment) number of updates to approximate, and we can accumulate quite a bit of error in that period. The downside with this approach is it requires doing a 64bit divide and a few multiplies to properly make and apply the calculation, which will occur about every second or whenever the ntp_tick_length() value is adjusted. The positive side of this approach is that we see very small error accumulation. I do have some concern that compared with the simulator, the scale of the error without this patch in the real world will be hard to ever observe, the extra overhead of the computation won't actually provide a benefit. Cc: Miroslav Lichvar Cc: Richard Cochran Cc: Prarit Bhargava Signed-off-by: John Stultz --- include/linux/timekeeper_internal.h | 4 ++ kernel/time/timekeeping.c | 73 ++++++++++++------------------------- 2 files changed, 27 insertions(+), 50 deletions(-) diff --git a/include/linux/timekeeper_internal.h b/include/linux/timekeeper_internal.h index 05af9a3..97464e9 100644 --- a/include/linux/timekeeper_internal.h +++ b/include/linux/timekeeper_internal.h @@ -103,6 +103,10 @@ struct timekeeper { * shifted nano seconds. */ s64 ntp_error; u32 ntp_error_shift; + + /* Mult adjustment being applied to correct freq error */ + u32 ntp_freq_mult; + /* Mult adjustment being applied to correct ntp_error */ u32 ntp_err_mult; }; diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c index 91db941..315bcb8 100644 --- a/kernel/time/timekeeping.c +++ b/kernel/time/timekeeping.c @@ -179,6 +179,7 @@ static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock) * to counteract clock drifting. */ tk->tkr.mult = clock->mult; + tk->ntp_freq_mult = clock->mult; tk->ntp_err_mult = 0; } @@ -1352,20 +1353,20 @@ device_initcall(timekeeping_init_ops); */ static __always_inline void timekeeping_apply_adjustment(struct timekeeper *tk, s64 offset, - bool negative, - int adj_scale) + int adj) { s64 interval = tk->cycle_interval; - s32 mult_adj = 1; - if (negative) { - mult_adj = -mult_adj; + if (!adj) + return; + + if (unlikely(abs(adj) > 1)) { + interval *= adj; + offset *= adj; + } else if (adj < 0) { interval = -interval; offset = -offset; } - mult_adj <<= adj_scale; - interval <<= adj_scale; - offset <<= adj_scale; /* * So the following can be confusing. @@ -1373,7 +1374,7 @@ static __always_inline void timekeeping_apply_adjustment(struct timekeeper *tk, * To keep things simple, lets assume mult_adj == 1 for now. * * When mult_adj != 1, remember that the interval and offset values - * have been appropriately scaled so the math is the same. + * have been appropriately multiplied so the math is the same. * * The basic idea here is that we're increasing the multiplier * by one, this causes the xtime_interval to be incremented by @@ -1416,13 +1417,7 @@ static __always_inline void timekeeping_apply_adjustment(struct timekeeper *tk, * * XXX - TODO: Doc ntp_error calculation. */ - if ((mult_adj > 0) && (tk->tkr.mult + mult_adj < mult_adj)) { - /* NTP adjustment caused clocksource mult overflow */ - WARN_ON_ONCE(1); - return; - } - - tk->tkr.mult += mult_adj; + tk->tkr.mult += adj; tk->xtime_interval += interval; tk->tkr.xtime_nsec -= offset; tk->ntp_error -= (interval - offset) << tk->ntp_error_shift; @@ -1435,36 +1430,13 @@ static __always_inline void timekeeping_apply_adjustment(struct timekeeper *tk, static __always_inline void timekeeping_freqadjust(struct timekeeper *tk, s64 offset) { - s64 interval = tk->cycle_interval; - s64 xinterval = tk->xtime_interval; - s64 tick_error; - bool negative; - u32 adj; - - /* Remove any current error adj from freq calculation */ - if (tk->ntp_err_mult) - xinterval -= tk->cycle_interval; - - tk->ntp_tick = ntp_tick_length(); + s64 tick_ns; - /* Calculate current error per tick */ - tick_error = ntp_tick_length() >> tk->ntp_error_shift; - tick_error -= (xinterval + tk->xtime_remainder); - - /* Don't worry about correcting it if its small */ - if (likely((tick_error >= 0) && (tick_error <= interval))) + if (likely(tk->ntp_tick == ntp_tick_length())) return; - - /* preserve the direction of correction */ - negative = (tick_error < 0); - - /* Sort out the magnitude of the correction */ - tick_error = abs(tick_error); - for (adj = 0; tick_error > interval; adj++) - tick_error >>= 1; - - /* scale the corrections */ - timekeeping_apply_adjustment(tk, offset, negative, adj); + tk->ntp_tick = ntp_tick_length(); + tick_ns = (tk->ntp_tick >> tk->ntp_error_shift) - tk->xtime_remainder; + tk->ntp_freq_mult = div64_u64(tick_ns, tk->cycle_interval); } /* @@ -1473,18 +1445,19 @@ static __always_inline void timekeeping_freqadjust(struct timekeeper *tk, */ static void timekeeping_adjust(struct timekeeper *tk, s64 offset) { + u32 new_mult; + /* Correct for the current frequency error */ timekeeping_freqadjust(tk, offset); /* Next make a small adjustment to fix any cumulative error */ - if (!tk->ntp_err_mult && (tk->ntp_error > 0)) { + if (tk->ntp_error > 0) tk->ntp_err_mult = 1; - timekeeping_apply_adjustment(tk, offset, 0, 0); - } else if (tk->ntp_err_mult && (tk->ntp_error <= 0)) { - /* Undo any existing error adjustment */ - timekeeping_apply_adjustment(tk, offset, 1, 0); + else tk->ntp_err_mult = 0; - } + + new_mult = tk->ntp_freq_mult + tk->ntp_err_mult; + timekeeping_apply_adjustment(tk, offset, new_mult - tk->mult); if (unlikely(tk->tkr.clock->maxadj && (abs(tk->tkr.mult - tk->tkr.clock->mult)