sys: drop frequency scaling in Linux driver

Since the kernel USER_HZ constant was introduced and the internal HZ
can't be reliably detected in user-space, the frequency scaling constant
used with older kernels is just a random guess.

Remove the scaling completely and let the closed loop compensate for the
error. To prevent thrashing between two states when the system's
frequency error is close to a multiple of USER_HZ, stick to the current
tick value if it's next to the new required tick. This is used only on
archs where USER_HZ is 100 as the frequency adjustment is limited to 500
ppm.

The linux_hz and linux_freq_scale directives are no longer supported,
but allowed by the config parser.
This commit is contained in:
Miroslav Lichvar 2014-05-22 16:28:20 +02:00
parent 14687d003d
commit e147f2f11e
4 changed files with 45 additions and 164 deletions

View file

@ -1141,8 +1141,6 @@ the configuration file is ignored.
* initstepslew directive:: Trim the system clock on boot-up
* keyfile directive:: Specify location of file containing keys
* leapsectz directive:: Read leap second data from tz database
* linux_freq_scale directive:: Define a non-standard value to compensate the kernel frequency bias
* linux_hz directive:: Define a non-standard value of the kernel USER_HZ constant
* local directive:: Allow unsynchronised machine to act as server
* lock_all directive:: Require that chronyd be locked into RAM
* log directive:: Make daemon log certain sets of information
@ -1818,38 +1816,6 @@ $ TZ=right/UTC date -d 'Dec 31 2008 23:59:60'
Wed Dec 31 23:59:60 UTC 2008
@end example
@c }}}
@c {{{ linux_freq_scale
@node linux_freq_scale directive
@subsection linux_freq_scale
(This option only applies to Linux).
This option sets a scale factor needed to control the frequency of the clock by
the @code{adjtimex()} system call exactly. By default, the value is determined
by the version of the running kernel. In recent kernels it is always 1.0 (i.e.
no scaling is needed).
An example of the command is
@example
linux_freq_scale 0.99902439
@end example
@c }}}
@c {{{ linux_hz
@node linux_hz directive
@subsection linux_hz
(This option only applies to Linux).
This option defines the value of the kernel @code{USER_HZ} constant, which is
needed to use the @code{adjtimex()} system call correctly. By default, its
value is determined from the running kernel automatically and there should
rarely be a need to use this option.
An example of the command is
@example
linux_hz 100
@end example
@c }}}
@c {{{ local
@node local directive

32
conf.c
View file

@ -187,16 +187,6 @@ static char *tempcomp_file = NULL;
static double tempcomp_interval;
static double tempcomp_T0, tempcomp_k0, tempcomp_k1, tempcomp_k2;
/* Boolean for whether the Linux HZ value has been overridden, and the
* new value. */
static int set_linux_hz = 0;
static int linux_hz;
/* Boolean for whether the Linux frequency scaling value (i.e. the one that's
* approx (1<<SHIFT_HZ)/HZ) has been overridden, and the new value. */
static int set_linux_freq_scale = 0;
static double linux_freq_scale;
static int sched_priority = 0;
static int lock_memory = 0;
@ -396,9 +386,9 @@ CNF_ParseLine(const char *filename, int number, char *line)
} else if (!strcasecmp(command, "leapsectz")) {
parse_string(p, &leapsec_tz);
} else if (!strcasecmp(command, "linux_freq_scale")) {
set_linux_freq_scale = parse_double(p, &linux_freq_scale);
LOG(LOGS_WARN, LOGF_Configure, "%s directive is no longer supported", command);
} else if (!strcasecmp(command, "linux_hz")) {
set_linux_hz = parse_int(p, &linux_hz);
LOG(LOGS_WARN, LOGF_Configure, "%s directive is no longer supported", command);
} else if (!strcasecmp(command, "local")) {
parse_local(p);
} else if (!strcasecmp(command, "lock_all")) {
@ -1616,24 +1606,6 @@ CNF_GetLeapSecTimezone(void)
/* ================================================== */
void
CNF_GetLinuxHz(int *set, int *hz)
{
*set = set_linux_hz;
*hz = linux_hz;
}
/* ================================================== */
void
CNF_GetLinuxFreqScale(int *set, double *freq_scale)
{
*set = set_linux_freq_scale;
*freq_scale = linux_freq_scale ;
}
/* ================================================== */
int
CNF_GetSchedPriority(void)
{

2
conf.h
View file

@ -74,8 +74,6 @@ extern void CNF_GetBindAcquisitionAddress(int family, IPAddr *addr);
extern void CNF_GetBindCommandAddress(int family, IPAddr *addr);
extern char *CNF_GetPidFile(void);
extern char *CNF_GetLeapSecTimezone(void);
extern void CNF_GetLinuxHz(int *set, int *hz);
extern void CNF_GetLinuxFreqScale(int *set, double *freq_scale);
/* Value returned in ppm, as read from file */
extern double CNF_GetMaxUpdateSkew(void);

View file

@ -61,25 +61,14 @@ int LockAll = 0;
/* This is the uncompensated system tick value */
static int nominal_tick;
/* Current tick value */
static int current_delta_tick;
/* The maximum amount by which 'tick' can be biased away from 'nominal_tick'
(sys_adjtimex() in the kernel bounds this to 10%) */
static int max_tick_bias;
/* This is the scaling required to go between absolute ppm and the
scaled ppm used as an argument to adjtimex. Because chronyd is to an extent
'closed loop' maybe it doesn't matter if this is wrongly determined, UNLESS
the system's ppm error is close to a multiple of HZ, in which case the
relationship between changing the frequency and changing the value of 'tick'
will be wrong. This would (I imagine) cause the system to thrash between
two states.
However..., if this effect was not corrected, and the system is left offline
for a long period, a substantial error would build up. e.g. with HZ==100,
the correction required is 128/128.125, giving a drift of about 84 seconds
per day). */
static double freq_scale;
/* The kernel HZ constant (USER_HZ in recent kernels). */
/* The kernel USER_HZ constant */
static int hz;
static double dhz; /* And dbl prec version of same for arithmetic */
@ -117,29 +106,44 @@ apply_step_offset(double offset)
/* ================================================== */
/* This call sets the Linux kernel frequency to a given value in parts
per million relative to the nominal running frequency. Nominal is taken to
be tick=10000, freq=0 (for a HZ==100 system, other values otherwise). The
convention is that this is called with a positive argument if the local
be tick=10000, freq=0 (for a USER_HZ==100 system, other values otherwise).
The convention is that this is called with a positive argument if the local
clock runs fast when uncompensated. */
static double
set_frequency(double freq_ppm)
{
long required_tick;
double required_freq; /* what we use */
double scaled_freq; /* what adjtimex & the kernel use */
double required_freq;
int required_delta_tick;
required_delta_tick = our_round(freq_ppm / dhz);
required_freq = -(freq_ppm - dhz * required_delta_tick);
required_tick = nominal_tick - required_delta_tick;
scaled_freq = freq_scale * required_freq;
if (TMX_SetFrequency(&scaled_freq, required_tick) < 0) {
LOG_FATAL(LOGF_SysLinux, "adjtimex failed for set_frequency, freq_ppm=%10.4e scaled_freq=%10.4e required_tick=%ld",
freq_ppm, scaled_freq, required_tick);
/* Older kernels (pre-2.6.18) don't apply the frequency offset exactly as
set by adjtimex() and a scaling constant (that depends on the internal
kernel HZ constant) would be needed to compensate for the error. Because
chronyd is closed loop it doesn't matter much if we don't scale the
required frequency, but we want to prevent thrashing between two states
when the system's frequency error is close to a multiple of USER_HZ. With
USER_HZ <= 250, the maximum frequency adjustment of 500 ppm overlaps at
least two ticks and we can stick to the current tick if it's next to the
required tick. */
if (hz <= 250 && (required_delta_tick + 1 == current_delta_tick ||
required_delta_tick - 1 == current_delta_tick)) {
required_delta_tick = current_delta_tick;
}
return dhz * (nominal_tick - required_tick) - scaled_freq / freq_scale;
required_freq = -(freq_ppm - dhz * required_delta_tick);
required_tick = nominal_tick - required_delta_tick;
if (TMX_SetFrequency(&required_freq, required_tick) < 0) {
LOG_FATAL(LOGF_SysLinux, "adjtimex failed for set_frequency, freq_ppm=%10.4e required_freq=%10.4e required_tick=%ld",
freq_ppm, required_freq, required_tick);
}
current_delta_tick = required_delta_tick;
return dhz * current_delta_tick - required_freq;
}
/* ================================================== */
@ -148,24 +152,16 @@ set_frequency(double freq_ppm)
static double
read_frequency(void)
{
double tick_term;
double unscaled_freq;
double freq_term;
long tick;
double freq;
if (TMX_GetFrequency(&unscaled_freq, &tick) < 0) {
if (TMX_GetFrequency(&freq, &tick) < 0) {
LOG_FATAL(LOGF_SysLinux, "adjtimex() failed");
}
tick_term = dhz * (double)(nominal_tick - tick);
freq_term = unscaled_freq / freq_scale;
current_delta_tick = nominal_tick - tick;
#if 0
LOG(LOGS_INFO, LOGF_SysLinux, "txc.tick=%ld txc.freq=%ld tick_term=%f freq_term=%f",
txc.tick, txc.freq, tick_term, freq_term);
#endif
return tick_term - freq_term;
return dhz * current_delta_tick - freq;
}
/* ================================================== */
@ -183,10 +179,10 @@ set_leap(int leap)
/* ================================================== */
/* Estimate the value of HZ given the value of txc.tick that chronyd finds when
/* Estimate the value of USER_HZ given the value of txc.tick that chronyd finds when
* it starts. The only credible values are 100 (Linux/x86) or powers of 2.
* Also, the bounds checking inside the kernel's adjtimex system call enforces
* a +/- 10% movement of tick away from the nominal value 1e6/HZ. */
* a +/- 10% movement of tick away from the nominal value 1e6/USER_HZ. */
static void
guess_hz_and_shift_hz(int tick, int *hz, int *shift_hz)
@ -264,11 +260,6 @@ get_version_specific_details(void)
{
int major, minor, patch;
int shift_hz;
double dshift_hz;
double basic_freq_scale; /* what to use if HZ!=100 */
int config_hz, set_config_hz; /* values of HZ from conf file */
int set_config_freq_scale;
double config_freq_scale;
struct tmx_params tmx_params;
struct utsname uts;
@ -288,14 +279,7 @@ get_version_specific_details(void)
}
}
CNF_GetLinuxHz(&set_config_hz, &config_hz);
if (set_config_hz) hz = config_hz;
/* (If true, presumably freq_scale will be overridden anyway, making shift_hz
redundant too.) */
dhz = (double) hz;
dshift_hz = (double)(1UL << shift_hz);
basic_freq_scale = dshift_hz / dhz;
nominal_tick = (1000000L + (hz/2))/hz; /* Mirror declaration in kernel */
max_tick_bias = nominal_tick / 10;
@ -303,33 +287,6 @@ get_version_specific_details(void)
(CONFIG_NO_HZ aka tickless), assume the lowest commonly used fixed rate */
tick_update_hz = 100;
/* The basic_freq_scale comes from:
* the kernel increments the usec counter HZ times per second (if the timer
interrupt period were perfect)
* the following code in the kernel
time_adj (+/-)= ltemp >>
(SHIFT_USEC + SHIFT_HZ - SHIFT_SCALE);
causes the adjtimex 'freq' value to be divided down by 1<<SHIFT_HZ.
The net effect is that we have to scale up the value we want by the
reciprocal of all this, i.e. multiply by (1<<SHIFT_HZ)/HZ.
If HZ==100, this code in the kernel comes into play too:
#if HZ == 100
* Compensate for (HZ==100) != (1 << SHIFT_HZ).
* Add 25% and 3.125% to get 128.125; => only 0.125% error (p. 14)
*
if (time_adj < 0)
time_adj -= (-time_adj >> 2) + (-time_adj >> 5);
else
time_adj += (time_adj >> 2) + (time_adj >> 5);
#endif
Special case that later.
*/
if (uname(&uts) < 0) {
LOG_FATAL(LOGF_SysLinux, "Cannot uname(2) to get kernel version, sorry.");
}
@ -345,17 +302,11 @@ get_version_specific_details(void)
LOG_FATAL(LOGF_SysLinux, "Kernel version not supported, sorry.");
}
if (kernelvercmp(major, minor, patch, 2, 6, 27) < 0) {
freq_scale = (hz == 100) ? (128.0 / 128.125) : basic_freq_scale;
} else {
/* These don't seem to need scaling */
freq_scale = 1.0;
if (kernelvercmp(major, minor, patch, 2, 6, 33) < 0) {
/* Tickless kernels before 2.6.33 accumulated ticks only in
half-second intervals */
tick_update_hz = 2;
}
if (kernelvercmp(major, minor, patch, 2, 6, 27) >= 0 &&
kernelvercmp(major, minor, patch, 2, 6, 33) < 0) {
/* Tickless kernels before 2.6.33 accumulated ticks only in
half-second intervals */
tick_update_hz = 2;
}
/* ADJ_SETOFFSET support */
@ -365,14 +316,8 @@ get_version_specific_details(void)
have_setoffset = 1;
}
/* Override freq_scale if it appears in conf file */
CNF_GetLinuxFreqScale(&set_config_freq_scale, &config_freq_scale);
if (set_config_freq_scale) {
freq_scale = config_freq_scale;
}
DEBUG_LOG(LOGF_SysLinux, "hz=%d shift_hz=%d freq_scale=%.8f nominal_tick=%d max_tick_bias=%d",
hz, shift_hz, freq_scale, nominal_tick, max_tick_bias);
DEBUG_LOG(LOGF_SysLinux, "hz=%d nominal_tick=%d max_tick_bias=%d",
hz, nominal_tick, max_tick_bias);
}
/* ================================================== */