On Sun, Sep 13, 2015 at 1:32 AM, Ingo Molnar wrote: > > * John Stultz 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! Well. I sat on the patch for quite awhile, so the author date isn't really representative. It was only included in mainline in 3.17, so its been in use for a little over a year. But yea, still. > Would it be possible to make abs() warn about 64-bit types during build time, > to prevent such mishaps? Yea. I was surprised this wasn't something the compiler would catch previously. So is BUILD_BUG_ON() the best option for this? Its catching a whole bunch of other related issues (frighteningly, more then Joe's cocci script). The down-side is BUILD_BUG_ON causes build errors, not warnings, and its output isn't super easy to parse on first view. Potential BUILD_BUG_ON patch attached. I'll also try to spin some patches to fix the issues this one catches. thanks -john