lkml.org 
[lkml]   [2015]   [Sep]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Date
SubjectRe: Regression: can't apply frequency offsets above 1000ppm.
Considering your last message I just tried to use abs64 instead. Fixes
the problem for me.

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 946acb7..414d9df 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -1615,7 +1615,7 @@ static __always_inline void
timekeeping_freqadjust(struct timekeeper *tk,
negative = (tick_error < 0);

/* Sort out the magnitude of the correction */
- tick_error = abs(tick_error);
+ tick_error = abs64(tick_error);
for (adj = 0; tick_error > interval; adj++)
tick_error >>= 1;

Thanks!
Nuno
On Sat, Sep 5, 2015 at 2:00 AM, John Stultz <john.stultz@linaro.org> wrote:
> On Fri, Sep 4, 2015 at 5:57 PM, John Stultz <john.stultz@linaro.org> wrote:
>> On Thu, Sep 3, 2015 at 4:26 AM, Miroslav Lichvar <mlichvar@redhat.com> wrote:
>>> On Wed, Sep 02, 2015 at 04:16:00PM -0700, John Stultz wrote:
>>>> On Tue, Sep 1, 2015 at 6:14 PM, Nuno Gonçalves <nunojpg@gmail.com> wrote:
>>>> > And just installing chrony from the feeds. With any kernel from 3.17
>>>> > you'll have wrong estimates at chronyc sourcestats.
>>>>
>>>> Wrong estimates? Could you be more specific about what the failure
>>>> you're seeing is here? The
>>>>
>>>> I installed the image above, which comes with a 4.1.6 kernel, and
>>>> chrony seems to have gotten my BBB into ~1ms sync w/ servers over the
>>>> internet fairly quickly (at least according to chronyc tracking).
>>>
>>> To see the bug with chronyd the initial offset shouldn't be very close
>>> to zero, so it's forced to correct the offset by adjusting the
>>> frequency in a larger step.
>>>
>>> I'm attaching a simple C program that prints the frequency offset
>>> as measured between the REALTIME and MONOTONIC_RAW clocks when the
>>> adjtimex tick is set to 9000. It should show values close to -100000
>>> ppm and I suspect on the BBB it will be much smaller.
>>
>> So I spent some time on this late last night and this afternoon.
>>
>> It was a little odd because things don't seem totally broken, but
>> something isn't quite right.
>>
>> Digging around it seems the iterative logrithmic approximation done in
>> timekeeping_freqadjust() wasn't working right. Instead of making
>> smaller order alternating positive and negative adjustments, it was
>> doing strange growing adjustments for the same value that wern't large
>> enough to actually correct things very quickly. This made it much
>> slower to adapt to specified frequency values.
>>
>> The odd bit, is it seems to come down to:
>> tick_error = abs(tick_error);
>>
>> Haven't chased down why yet, but apparently abs() isn't doing what one
>> would think when passed a s64 value.
>
> Well.. chasing it down wasn't hard.. from include/linux/kernel.h:
> /*
> * abs() handles unsigned and signed longs, ints, shorts and chars. For all
> * input types abs() returns a signed long.
> * abs() should not be used for 64-bit types (s64, u64, long long) - use abs64()
> * for those.
> */
>
> Ouch.
> -john


\
 
 \ /
  Last update: 2015-09-05 16:01    [W:0.985 / U:0.088 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site