Re: [PATCH 02/12] clocksource: Simplify logic around clocksource wrapping safety margins

[Ingo Molnar]

* John Stultz <john.stultz@xxxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/