Messages in this thread | | | Date | Sun, 13 Sep 2015 10:32:44 +0200 | From | Ingo Molnar <> | Subject | Re: [PATCH 1/2] time: Fix timekeeping_freqadjust()'s incorrect use of abs() instead of abs64() |
| |
* John Stultz <john.stultz@linaro.org> wrote:
> The internal clocksteering done for fine-grained error correction > uses a logarithmic approximation, so any time adjtimex() adjusts > the clock steering, timekeeping_freqadjust() quickly approximates > the correct clock frequency over a series of ticks. > > Unfortunately, the logic in timekeeping_freqadjust(), introduced > in commit dc491596f639438 (Rework frequency adjustments to work > better w/ nohz), used the abs() function with a s64 error value > to calculate the size of the approximated adjustment to be made. > > Per include/linux/kernel.h: "abs() should not be used for 64-bit > types (s64, u64, long long) - use abs64()". > > Thus on 32-bit platforms, this resulted in the clocksteering to > take a quite dampended random walk trying to converge on the > proper frequency, which caused the adjustments to be made much > slower then intended (most easily observed when large adjustments > are made). > > This patch fixes the issue by using abs64() instead.
> /* Sort out the magnitude of the correction */ > - tick_error = abs(tick_error); > + tick_error = abs64(tick_error);
Ugh, and we had this bug for almost two years!
Would it be possible to make abs() warn about 64-bit types during build time, to prevent such mishaps?
Thanks,
Ingo
| |