From patchwork Wed Dec 16 05:48:06 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Stultz X-Patchwork-Id: 58484 Delivered-To: patches@linaro.org Received: by 10.112.89.199 with SMTP id bq7csp491489lbb; Tue, 15 Dec 2015 21:48:12 -0800 (PST) X-Received: by 10.98.8.73 with SMTP id c70mr2802389pfd.41.1450244892650; Tue, 15 Dec 2015 21:48:12 -0800 (PST) Return-Path: Received: from mail-pf0-x231.google.com (mail-pf0-x231.google.com. [2607:f8b0:400e:c00::231]) by mx.google.com with ESMTPS id yd1si6929678pab.39.2015.12.15.21.48.12 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 15 Dec 2015 21:48:12 -0800 (PST) Received-SPF: pass (google.com: domain of john.stultz@linaro.org designates 2607:f8b0:400e:c00::231 as permitted sender) client-ip=2607:f8b0:400e:c00::231; Authentication-Results: mx.google.com; spf=pass (google.com: domain of john.stultz@linaro.org designates 2607:f8b0:400e:c00::231 as permitted sender) smtp.mailfrom=john.stultz@linaro.org; dkim=pass header.i=@linaro.org Received: by mail-pf0-x231.google.com with SMTP id v86so6467155pfa.2 for ; Tue, 15 Dec 2015 21:48:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id; bh=gLuPGog+MwmL4Gu/ysDOZkxrRfVgN/W0nuYYKdALsHo=; b=SMIGklowsXXSJpgv1vRgwA3UASlSeOS04K77Nj2bI8QOpzEyfkyCTZJZEY5UgtvZhU 7FgM+zY98DFGwbh8FS7C2I4UOg7lkckS/hhy4zjT5Svg7ZzKx13FrO4fPA2wd0RN/1xI XqBOMHjnOLTcIJHWf5+La1eTgbGPK+kR3Pepo= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=gLuPGog+MwmL4Gu/ysDOZkxrRfVgN/W0nuYYKdALsHo=; b=U6G3TkAVKXaACwG/X5xtU3nR6cWsCtf8ykrvn51yHYKNgQSvR4QT5Fv5gTBjVIMQP2 88tOkoDC5SlpH4CDKU3Hxt41Ba67Zz31yN3vp5ADnskm+GOIz8cI9qWhJgBnwVFB3hGc rqXQW41+X6rUR/c8lQOEkgCarBG05f3na8vOuHESWEwrISTl9Pn00og7p+iG5jvUdCBy 0jIMtlWHjHfCk2pDhJSIqD4OrtFMAD1rT9n6DKmKcahFPfWlUuyp7Gs0ObAkcBin5vCz cUzmS619X7RayCGrF66tjPZsPY7MQ3sxCQWLnwPEhkL290h7HS32Ct2Gkzipzg2h3sCs SiRQ== X-Gm-Message-State: ALoCoQmIQLICdtaFwMVvQvMqTkYZ7QLk3AhKqmceaExOM7iOSFp8lbaHOfkVqQDSc36Cj+k+4Xony4e1GKYX4VsekSfQOMsOSA== X-Received: by 10.98.2.2 with SMTP id 2mr2762072pfc.13.1450244892271; Tue, 15 Dec 2015 21:48:12 -0800 (PST) Return-Path: Received: from localhost.localdomain (c-76-115-103-22.hsd1.or.comcast.net. [76.115.103.22]) by smtp.gmail.com with ESMTPSA id yn8sm6106772pac.32.2015.12.15.21.48.11 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Tue, 15 Dec 2015 21:48:11 -0800 (PST) From: John Stultz To: lkml Cc: John Stultz , Miroslav Lichvar , Thomas Gleixner , Richard Cochran , Prarit Bhargava , Andy Lutomirski Subject: [RFC][PATCH v3] timekeeping: Cap adjustments so they don't exceed the maxadj value Date: Tue, 15 Dec 2015 21:48:06 -0800 Message-Id: <1450244886-9931-1-git-send-email-john.stultz@linaro.org> X-Mailer: git-send-email 1.9.1 Thus its been occasionally noted that users have seen confusing warnings like: Adjusting tsc more than 11% (5941981 vs 7759439) We try to limit the maximum total adjustment to 11% (10% tick adjustment + 0.5% frequency adjustment). But this is done by bounding the requested adjustment values, and the internal steering that is done by tracking the error from what was requested and what was applied, does not have any such limits. This is usually not problematic, but in some cases has a risk that an adjustment could cause the clocksource mult value to overflow, so its an indication things are outside of what is expected. It ends up most of the reports of this 11% warning are on systems using chrony, which utilizes the adjtimex() ADJ_TICK interface (which allows a +-10% adjustment). The original rational for ADJ_TICK unclear to me but my assumption it was originally added to allow broken systems to get a big constant correction at boot (see adjtimex userspace package for an example) which would allow the system to work w/ ntpd's 0.5% adjustment limit. Chrony uses ADJ_TICK to make very aggressive short term corrections (usually right at startup). Which push us close enough to the max bound that a few late ticks can cause the internal steering to push past the max adjust value (tripping the warning). Thus this patch adds some extra logic to enforce the max adjustment cap in the internal steering. Note: This has the potential to slow corrections when the ADJ_TICK value is furthest away from the default value. So it would be good to get some testing from folks using chrony, to make sure we don't cause any troubles there. Cc: Miroslav Lichvar Cc: Thomas Gleixner Cc: Richard Cochran Cc: Prarit Bhargava Cc: Andy Lutomirski Reported-by: Andy Lutomirski Signed-off-by: John Stultz --- v2: Catch single unit adjustment that was being made repeatedly to push us past the limit, as pointed out by Miroslav. v3: Fix off-by-one case that could result in warning still triggering, pointed out by Miroslav. kernel/time/timekeeping.c | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) -- 1.9.1 diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c index d9249da..a17cdc3 100644 --- a/kernel/time/timekeeping.c +++ b/kernel/time/timekeeping.c @@ -1591,9 +1591,12 @@ static __always_inline void timekeeping_freqadjust(struct timekeeper *tk, { s64 interval = tk->cycle_interval; s64 xinterval = tk->xtime_interval; + u32 base = tk->tkr_mono.clock->mult; + u32 max = tk->tkr_mono.clock->maxadj; + u32 cur_adj = tk->tkr_mono.mult; s64 tick_error; bool negative; - u32 adj; + u32 adj_scale; /* Remove any current error adj from freq calculation */ if (tk->ntp_err_mult) @@ -1612,13 +1615,33 @@ static __always_inline void timekeeping_freqadjust(struct timekeeper *tk, /* preserve the direction of correction */ negative = (tick_error < 0); - /* Sort out the magnitude of the correction */ + /* If any adjustment would pass the max, just return */ + if (negative && (cur_adj - 1) <= (base - max)) + return; + if (!negative && (cur_adj + 1) >= (base + max)) + return; + /* + * Sort out the magnitude of the correction, but + * avoid making so large a correction that we go + * over the max adjustment. + */ + adj_scale = 0; tick_error = abs(tick_error); - for (adj = 0; tick_error > interval; adj++) + while (tick_error > interval) { + u32 adj = 1 << (adj_scale + 1); + + /* Check if adjustment gets us within 1 unit from the max */ + if (negative && (cur_adj - adj) <= (base - max)) + break; + if (!negative && (cur_adj + adj) >= (base + max)) + break; + + adj_scale++; tick_error >>= 1; + } /* scale the corrections */ - timekeeping_apply_adjustment(tk, offset, negative, adj); + timekeeping_apply_adjustment(tk, offset, negative, adj_scale); } /*