Re: [PATCH] correct inconsistent ntp interval/tick_length usage

Roman Zippel
2008-01-31T05:03:19Z

Hi,

On Wed, 30 Jan 2008, john stultz wrote:

> My concern is we state the accumulation interval is X ns long. Then
> current_tick_length() is to return (X + ntp_adjustment), so each
> accumulation interval we can keep track of the error and adjust our
> interval length.
>
> So if ntp_update_frequency() sets tick_length_base to be:
>
> u64 second_length = (u64)(tick_usec * NSEC_PER_USEC * USER_HZ)
> << TICK_LENGTH_SHIFT;
> second_length += (s64)CLOCK_TICK_ADJUST << TICK_LENGTH_SHIFT;
> second_length += (s64)time_freq
> << (TICK_LENGTH_SHIFT - SHIFT_NSEC);
>
> tick_length_base = second_length;
> do_div(tick_length_base, NTP_INTERVAL_FREQ);
>
>
> The above is basically (X + part of ntp_adjustment)

CLOCK_TICK_ADJUST is based on LATCH and HZ, if the update frequency isn't
based on HZ, there is no point in using it!

Let's look at what actually needs to be done:

1. initializing clock interval:

clock_cycle_interval = timer_cycle_interval * clock_frequency / timer_frequency

It's simply about converting timer cycles into clock cycles, so they're
about the same time interval.
We already make it a bit more complicated than necessary as we go via
nsec:

ntp_interval = timer_cycle_interval * 10^9nsec / timer_frequency

and in clocksource_calculate_interval() basically:

clock_cycle_interval = ntp_interval * clock_frequency / 10^9nsec

Without a fixed timer tick it's actually even easier, then we use the same
frequency for clock and timer and the cycle interval is simply:

clock_cycle_interval = timer_cycle_interval = clock_frequency / NTP_INTERVAL_FREQ

There is no need to use the adjustment here, you'll only cause a mismatch
between the clock and timer cycle interval, which had to be corrected by
NTP.

2. initializing clock adjustment:

clock_adjust = timer_cycle_interval * NTP_INTERVAL_FREQ / timer_frequency - 1sec

This adjustment is used make up for the difference that the timer
frequency isn't evenly divisible by HZ, so that the clock is advanced by
1sec after timer_frequency cycles.

Like above the clock frequency is used for the timer frequency for this
calculation for CONFIG_NO_HZ, so it would be incorrect to use
CLOCK_TICK_RATE/LATCH/HZ here and since NTP_INTERVAL_FREQ is quite small
the resulting adjustment would be rather small, it's easier not to bother
in this case.

What you're basically trying is to add an error to the clock
initialization, so that we can later compensate for it. The correct
solution is really to not add the error in first place, so that there is
no need to compensate for it.

bye. Roman