svn commit: r209900 - head/sys/kern

[Bruce Evans]

On Mon, 12 Jul 2010, Alexander Motin wrote:

> John Baldwin wrote:
>> On Sunday, July 11, 2010 12:47:46 pm Alexander Motin wrote:
>>> ...
>>> Log:
>>>   Remove interval validation from cpu_tick_calibrate(). As I found, check
>>>   was needed at preliminary version of the patch, where number of CPU ticks
>>>   was divided strictly on 16 seconds. Final code instead uses real interval
>>>   duration, so precise interval should not be important. Same time aliasing
>>>   issues around second boundary causes false positives, periodically logging
>>>   useless "t_delta ... too long/short" messages when HZ set below 256.
>>
>> Hmm, did you ask phk@ about this?
>
> Yes. He agreed that code should be reconsidered.
>
>> I notice that the printfs only trigger if
>> you have bootverbose enabled, so they were not affecting normal users as most
>> people do not run their systems with bootverbose enabled.
>
> Yes, but they produce major part of kernel messages on my own systems.
> I've got tired seeing them for years. It would be fine if they were
> usable, but, as I have told here and on current@, with preset
> implementation they were absolutely meaningless.

Perhaps they are needed for some time warps, e.g., ones from the clock
being stopped while in ddb: while in ddb, time_uptime isn't updated, so
considerably more than 16 seconds have usually elapsed between calls to
cpu_tick_calibrate().  However, the timecounter is hardware so it is
updated while in ddb, so the delta-time can easily be a few seconds
different from 16 (I think it is always more if the timecounter works
right, and it can be less if the timecounter wraps or otherwise overflows
(overflow in scaling)).

However:
- entering ddb breaks timecounting (except in my version, which restores
   the time about 3 seconds after exiting ddb, and loses only about
   $(cat /var/db/ntp.drift) microseconds of accuracy per second spent
   in ddb; my timecounter frequencies are calibrated so that
   $(cat /var/db/ntp.drift) is 0+-5, so the loss is small -- ntp can't
   tell the difference between this loss and the loss from a large
   temperature change)
- entering ddb breaks cputick calibration even more than timecounting,
   except in my version.  The timecounter should just lose a few seconds
   while in ddb.  Taking deltas of it can result in a loss of almost
   100% in accuracy of the calibrated frequency, especially in the
   overflowing cases which always happen after staying in ddb long
   enough for the timecounter to wrap (~2 seconds with a TSC timecounter
   on amd64 and i386)
- the validation seemed to be of no use for detecting time warps caused
   by ddb.  I didn't investigate this.

Fix for cputick calibration:

% diff -c2 ./kern/kern_tc.c~ ./kern/kern_tc.c
% *** ./kern/kern_tc.c~	Thu Mar 20 01:05:27 2008
% --- ./kern/kern_tc.c	Thu Mar 20 01:05:29 2008
% ***************
% *** 884,888 ****
%   		return;
% 
% ! 	getbinuptime(&t_this);
%   	c_this = cpu_ticks();
%   	if (t_last.sec != 0) {
% --- 884,888 ----
%   		return;
% 
% ! 	binuptime(&t_this);
%   	c_this = cpu_ticks();
%   	if (t_last.sec != 0) {

Minor fix for accuracy.

% ***************
% *** 922,931 ****
%   			c_delta <<= 20;
%   			c_delta /= divi;
% ! 			if (c_delta  > cpu_tick_frequency) {
% ! 				if (0 && bootverbose)
% ! 					printf("cpu_tick increased to %ju Hz\n",
% ! 					    c_delta);
% ! 				cpu_tick_frequency = c_delta;
% ! 			}
%   		}
%   	}
% --- 922,930 ----
%   			c_delta <<= 20;
%   			c_delta /= divi;
% ! 			if (0 && bootverbose)
% ! 				printf(
% ! 			    "cpu_tick_frequency changed from %ju to %ju Hz\n",
% ! 				    cpu_tick_frequency, c_delta);
% ! 			cpu_tick_frequency = c_delta;
%   		}
%   	}

Variable frequencies have the interesting capability of varying in
both positive and negative directions.  This fixes the case of
negative-going changes.  Without this, the overflowing timecounter
cases caused cpu_tick_frequency to become huge (100's of times the
correct value), and it was clamped to the hugest value ever seen.  The
way this mostly fixes the problem is that cpu_tick_frequency can be
off by a factor of 100, but only for 16 seconds or so until the next
calibration fixes it.

I don't use this in the same kernels that have the fix for the
timecounters.  An integrated fix would prevent recalibration until 16
seconds after the timecounter has been restored.

Setting cpu_tick_variable to 0 is an even simpler fix if the cputicker
is not very variable.

Without this, normal frequency drift due to temperature changes caused
cpu_tick frequency to become larger than the correct value by a few
ppm.  Again it was clamped to the largest value ever seen, but errors
of a few ppm are hard to see.

There seem to be similar problems after suspend/resume, but only
transient ones -- there seems to be nothing to prevent the recalibration
running in the middle of a resume and seeing garbage, but then resume
hopefully fixes things up by restoring the frequency, and hopefully
recalibration run after that never sees garbage.  If not, then the above
recovers from lost races after about 16 seconds in the same way as for ddb.

The fix also fixes the terminology in the diagnostic message.  There is
no `cpu_tick' here, and if there were then it should be a period (like
kern.clockrate tick).  There is only a frequency here.

Bruce