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

[Roman Zippel]

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
--
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/