From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752159AbbCLFzc (ORCPT ); Thu, 12 Mar 2015 01:55:32 -0400 Received: from mail-wi0-f182.google.com ([209.85.212.182]:45776 "EHLO mail-wi0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751977AbbCLFza (ORCPT ); Thu, 12 Mar 2015 01:55:30 -0400 Date: Thu, 12 Mar 2015 06:55:24 +0100 From: Ingo Molnar To: John Stultz Cc: lkml , Dave Jones , Linus Torvalds , Thomas Gleixner , Richard Cochran , Prarit Bhargava , Stephen Boyd , Peter Zijlstra Subject: Re: [PATCH 02/12] clocksource: Simplify logic around clocksource wrapping safety margins Message-ID: <20150312055524.GA13221@gmail.com> References: <1426133800-29329-1-git-send-email-john.stultz@linaro.org> <1426133800-29329-3-git-send-email-john.stultz@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1426133800-29329-3-git-send-email-john.stultz@linaro.org> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * John Stultz wrote: > The clocksource logic has a number of places where we try to > include a safety margin. Most of these are 12% safety margins, > but they are inconsistently applied and sometimes are applied > on top of each other. > > Additionally, in the previous patch, we corrected an issue > where we unintentionally in effect created a 50% safety margin, > which these 12.5% margins where then added to. > > So to simplify the logic here, this patch removes the various > 12.5% margins, and consolidates adding the margin in one place: > clocks_calc_max_nsecs(). > > Additionally, Linus prefers a 50% safety margin, as it allows > bad clock values to be more easily caught. This should really > have no net effect, due to the corrected issue earlier which > caused greater then 50% margins to be used w/o issue. > +++ b/kernel/time/clocksource.c > @@ -469,6 +469,9 @@ static u32 clocksource_max_adjustment(struct clocksource *cs) > * @shift: cycle to nanosecond divisor (power of two) > * @maxadj: maximum adjustment value to mult (~11%) > * @mask: bitmask for two's complement subtraction of non 64 bit counters > + * > + * NOTE: This function includes a safety margin of 50%, so that bad clock values > + * can be detected. > */ > u64 clocks_calc_max_nsecs(u32 mult, u32 shift, u32 maxadj, u64 mask) > { So it would be nice if there was also a comment here explaining the 'safety margin': what values are checked, what stream of values is expected from clocksources, why clocksources can be off, what the core clocksource code does with that, what symptoms it can cause, what is considered 'normal', what is considered 'abnormal', what happens if a 'safety margin' is exceeded, etc. I.e. all the code and all the changelogs talk about 'safety margins' in a somewhat circular, self-defining fashion - without there being any easily visible place where it's explained from first principles. Thanks, Ingo