test10's version of do_gettimeofday is using tick_usec which is
defined in terms of USER_HZ not HZ.
Against 2.6.0-test10-bk1. Compiled, not tested, for comment only.
Joe
--- base/arch/i386/kernel/time.c 2003-11-23 20:31:55.000000000 -0500
+++ new/arch/i386/kernel/time.c 2003-11-25 11:22:38.000000000 -0500
@@ -94,7 +94,7 @@
{
unsigned long seq;
unsigned long usec, sec;
- unsigned long max_ntp_tick = tick_usec - tickadj;
+ unsigned long max_ntp_tick;
do {
unsigned long lost;
@@ -110,13 +110,14 @@
* Better to lose some accuracy than have time go backwards..
*/
if (unlikely(time_adjust < 0)) {
+ max_ntp_tick = (USEC_PER_SEC / HZ) - tickadj;
usec = min(usec, max_ntp_tick);
if (lost)
usec += lost * max_ntp_tick;
}
else if (unlikely(lost))
- usec += lost * tick_usec;
+ usec += lost * (USEC_PER_SEC / HZ);
sec = xtime.tv_sec;
usec += (xtime.tv_nsec / 1000);
On Tue, 25 Nov 2003 11:42:38 -0500
Joe Korty <[email protected]> wrote:
> test10's version of do_gettimeofday is using tick_usec which is
> defined in terms of USER_HZ not HZ.
>
> Against 2.6.0-test10-bk1. Compiled, not tested, for comment only.
Your right. tick_usec is in user hz so the value of max_ntp_tick would be
too large.
Joe Korty wrote:
> test10's version of do_gettimeofday is using tick_usec which is
> defined in terms of USER_HZ not HZ.
>
> Against 2.6.0-test10-bk1. Compiled, not tested, for comment only.
We still have the problem that we are doing this calculation in usecs while the
wall clock uses nsecs. This would be fine if there were an even number of usecs
in tick_nsec, but in fact it is somewhat less than (USEC_PER_SEC / HZ). This
means that this correction (if we are behind by 7 or more ticks) will push the
clock past current time. Here are the numbers:
tick_nsec =999849 or 1ms less 151 ns. So if we are behind 7 or more ticks we
will report the time out 1 us too high. (7 * 151 = 1057 or 1.057 usec).
Question is, do we care? Will we ever be 7ms late in updating the wall clock?
As I recall, the wall clock is updated in the interrupt handler for the tick so,
to be this late, we would need to suffer a long interrupt hold off AND the tick
recovery code would need to have done its thing. But this whole time is covered
by a write_seqlock on xtime_lock, so how can this even happen? Seems like it is
only possible when we are locked and we then throw the whole thing away.
A test I would like to see is to put this in the code AFTER the read unlock:
if (lost )
printk("Lost is %d\n", lost);
(need to pull " unsigned long lost;" out of the do{}while loop to do this)
In short, I think we are beating a dead issue.
-g
>
> Joe
>
> --- base/arch/i386/kernel/time.c 2003-11-23 20:31:55.000000000 -0500
> +++ new/arch/i386/kernel/time.c 2003-11-25 11:22:38.000000000 -0500
> @@ -94,7 +94,7 @@
> {
> unsigned long seq;
> unsigned long usec, sec;
> - unsigned long max_ntp_tick = tick_usec - tickadj;
> + unsigned long max_ntp_tick;
>
> do {
> unsigned long lost;
> @@ -110,13 +110,14 @@
> * Better to lose some accuracy than have time go backwards..
> */
> if (unlikely(time_adjust < 0)) {
> + max_ntp_tick = (USEC_PER_SEC / HZ) - tickadj;
> usec = min(usec, max_ntp_tick);
>
> if (lost)
> usec += lost * max_ntp_tick;
> }
> else if (unlikely(lost))
> - usec += lost * tick_usec;
> + usec += lost * (USEC_PER_SEC / HZ);
>
> sec = xtime.tv_sec;
> usec += (xtime.tv_nsec / 1000);
>
>
--
George Anzinger [email protected]
High-res-timers: http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml
On Tue, Nov 25, 2003 at 11:57:55AM -0800, George Anzinger wrote:
> Joe Korty wrote:
> >test10's version of do_gettimeofday is using tick_usec which is
> >defined in terms of USER_HZ not HZ.
>
> We still have the problem that we are doing this calculation in usecs while
> the wall clock uses nsecs. This would be fine if there were an even number
> of usecs in tick_nsec, but in fact it is somewhat less than (USEC_PER_SEC /
> HZ). This means that this correction (if we are behind by 7 or more ticks)
> will push the clock past current time. Here are the numbers:
>
> tick_nsec =999849 or 1ms less 151 ns. So if we are behind 7 or more ticks
> we will report the time out 1 us too high. (7 * 151 = 1057 or 1.057 usec).
>
> Question is, do we care? Will we ever be 7ms late in updating the wall
> clock? As I recall, the wall clock is updated in the interrupt handler for
> the tick so, to be this late, we would need to suffer a long interrupt hold
> off AND the tick recovery code would need to have done its thing. But this
> whole time is covered by a write_seqlock on xtime_lock, so how can this
> even happen? Seems like it is only possible when we are locked and we then
> throw the whole thing away.
>
> A test I would like to see is to put this in the code AFTER the read unlock:
>
> if (lost )
> printk("Lost is %d\n", lost);
>
> (need to pull " unsigned long lost;" out of the do{}while loop to do
> this)
>
> In short, I think we are beating a dead issue.
There are other issues too: the 'lost' calculation is a prediction
over the next 'lost' number of ticks. That prediction will be wrong
if 1) adjtime goes to zero within that interval or, 2) adjtime was
zero but went nonzero in that interval due to a adjtimex(2) call.
Despite these flaws the patch replaces truly broken code with code
that is good but slightly inaccurate, which is good enough for now.
Joe
Joe Korty wrote:
> On Tue, Nov 25, 2003 at 11:57:55AM -0800, George Anzinger wrote:
>
>>Joe Korty wrote:
>>
>>>test10's version of do_gettimeofday is using tick_usec which is
>>>defined in terms of USER_HZ not HZ.
>>
>>We still have the problem that we are doing this calculation in usecs while
>>the wall clock uses nsecs. This would be fine if there were an even number
>>of usecs in tick_nsec, but in fact it is somewhat less than (USEC_PER_SEC /
>>HZ). This means that this correction (if we are behind by 7 or more ticks)
>>will push the clock past current time. Here are the numbers:
>>
>>tick_nsec =999849 or 1ms less 151 ns. So if we are behind 7 or more ticks
>>we will report the time out 1 us too high. (7 * 151 = 1057 or 1.057 usec).
>>
>>Question is, do we care? Will we ever be 7ms late in updating the wall
>>clock? As I recall, the wall clock is updated in the interrupt handler for
>>the tick so, to be this late, we would need to suffer a long interrupt hold
>>off AND the tick recovery code would need to have done its thing. But this
>>whole time is covered by a write_seqlock on xtime_lock, so how can this
>>even happen? Seems like it is only possible when we are locked and we then
>>throw the whole thing away.
>>
>>A test I would like to see is to put this in the code AFTER the read unlock:
>>
>>if (lost )
>> printk("Lost is %d\n", lost);
>>
>>(need to pull " unsigned long lost;" out of the do{}while loop to do
>>this)
>>
>>In short, I think we are beating a dead issue.
>
>
> There are other issues too: the 'lost' calculation is a prediction
> over the next 'lost' number of ticks. That prediction will be wrong
> if 1) adjtime goes to zero within that interval or, 2) adjtime was
> zero but went nonzero in that interval due to a adjtimex(2) call.
>
> Despite these flaws the patch replaces truly broken code with code
> that is good but slightly inaccurate, which is good enough for now.
Can you prove that "lost" is EVER non-zero in a case we care about? I.e. a case
where the read_seq will exit the loop?
I could be wrong here, but I don't think it can happen. That is why I suggested
the if(lost) test.
--
George Anzinger [email protected]
High-res-timers: http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml
Joe Korty <[email protected]> writes:
> test10's version of do_gettimeofday is using tick_usec which is
> defined in terms of USER_HZ not HZ.
>
> Against 2.6.0-test10-bk1. Compiled, not tested, for comment only.
>
I added the changes to x86-64, but at least ping still complains
that the time is going backwards. The machine is running ntpd
and has a high drift (AMD 8111 chipset, doesn't have the most stable
timer in the world)
-Andi