[RFC][PATCH] linux-2.4.21-pre4_tsc-lost-tick_A0

[RFC][PATCH] linux-2.4.21-pre4_tsc-lost-tick_A0

[john stultz]

All,
	Occasionally due to hardware or software, its possible to miss multiple
timer interrupts. This can cause small inconsistencies in time as well
as time drifting behind other systems. 

This patch checks each timer interrupt, using the TSC, if we have missed
any ticks. Then if so, compensates jiffies for them. I have already
submitted a patch for 2.4 which does this for the cyclone-timer based
code, and I've been testing a version for 2.5 for all time sources (in
-mm6, I believe). 

Since this code affects more users then the cyclone-based version, I
want to be more careful and get more testing in the 2.5 tree before I
submit this. However, just so people wanting it can play with it and
test it themselves, I wanted to send this out for comments. 

I'm already somewhat cautious that loops_per_jiffy isn't going to cut it
with this patch (I'm thinking fast_gettimeoffset_quotient would probably
be better). So please let me know if you find any issues with this
patch.

thanks
-john

diff -Nru a/arch/i386/kernel/time.c b/arch/i386/kernel/time.c
--- a/arch/i386/kernel/time.c	Thu Jan 30 16:03:19 2003
+++ b/arch/i386/kernel/time.c	Thu Jan 30 16:03:19 2003
@@ -657,6 +657,7 @@
 	if(use_cyclone)
 		mark_timeoffset_cyclone();
 	else if (use_tsc) {
+		unsigned long delta = last_tsc_low;
 		/*
 		 * It is important that these two operations happen almost at
 		 * the same time. We do the RDTSC stuff first, since it's
@@ -700,6 +701,13 @@
 		   momentarily as they flip back to zero */
 		if (count == LATCH) {
 			count--;
+		}
+
+		/* lost tick compensation */
+		delta = last_tsc_low - delta;
+		if(delta >= 2*loops_per_jiffy){
+			delta = (delta/loops_per_jiffy)-1;
+			jiffies += delta;
 		}
 
 		count = ((LATCH-1) - count) * TICK_SIZE;

Re: [RFC][PATCH] linux-2.4.21-pre4_tsc-lost-tick_A0

[george anzinger]

john stultz wrote:
> 
> All,
>         Occasionally due to hardware or software, its possible to miss multiple
> timer interrupts. This can cause small inconsistencies in time as well
> as time drifting behind other systems.
> 
> This patch checks each timer interrupt, using the TSC, if we have missed
> any ticks. Then if so, compensates jiffies for them. I have already
> submitted a patch for 2.4 which does this for the cyclone-timer based
> code, and I've been testing a version for 2.5 for all time sources (in
> -mm6, I believe).
> 
> Since this code affects more users then the cyclone-based version, I
> want to be more careful and get more testing in the 2.5 tree before I
> submit this. However, just so people wanting it can play with it and
> test it themselves, I wanted to send this out for comments.
> 
> I'm already somewhat cautious that loops_per_jiffy isn't going to cut it
> with this patch (I'm thinking fast_gettimeoffset_quotient would probably
> be better). So please let me know if you find any issues with this
> patch.

I think you are wondering about the "/", as am I.  Possibly
a while loop, or, something like
fast_gettimeoffset_quotient, but scaled to do jiffies
instead of micro seconds.  Still you SHOULD be doing this so
seldom that one wonders if the "/" is all that bad.

Another thing, possibly not so easily fixed given the
division between "arch" code and common code, but I would
like to see jiffies updated in only ONE place.  With this
patch it is updated in .../kernel/timer.c AND in
.../arch/kernel/time.c.  In the high-res-timers patch I made
the jiffies update an inline in an "arch" header file so I
could have the best of both worlds, i.e. update from common
code using arch resources (TSC, etc).

-g
> 
> diff -Nru a/arch/i386/kernel/time.c b/arch/i386/kernel/time.c
> --- a/arch/i386/kernel/time.c   Thu Jan 30 16:03:19 2003
> +++ b/arch/i386/kernel/time.c   Thu Jan 30 16:03:19 2003
> @@ -657,6 +657,7 @@
>         if(use_cyclone)
>                 mark_timeoffset_cyclone();
>         else if (use_tsc) {
> +               unsigned long delta = last_tsc_low;
>                 /*
>                  * It is important that these two operations happen almost at
>                  * the same time. We do the RDTSC stuff first, since it's
> @@ -700,6 +701,13 @@
>                    momentarily as they flip back to zero */
>                 if (count == LATCH) {
>                         count--;
> +               }
> +
> +               /* lost tick compensation */
> +               delta = last_tsc_low - delta;
> +               if(delta >= 2*loops_per_jiffy){
> +                       delta = (delta/loops_per_jiffy)-1;
> +                       jiffies += delta;
>                 }
> 
>                 count = ((LATCH-1) - count) * TICK_SIZE;
> 

-- 
George Anzinger   george@mvista.com
High-res-timers: 
http://sourceforge.net/projects/high-res-timers/
Preemption patch:
http://www.kernel.org/pub/linux/kernel/people/rml

Re: [RFC][PATCH] linux-2.4.21-pre4_tsc-lost-tick_A0

[john stultz]

On Thu, 2003-01-30 at 17:08, george anzinger wrote:
> john stultz wrote:
> > I'm already somewhat cautious that loops_per_jiffy isn't going to cut it
> > with this patch (I'm thinking fast_gettimeoffset_quotient would probably
> > be better). So please let me know if you find any issues with this
> > patch.
> 
> I think you are wondering about the "/", as am I.  Possibly
> a while loop, or, something like
> fast_gettimeoffset_quotient, but scaled to do jiffies
> instead of micro seconds.  Still you SHOULD be doing this so
> seldom that one wonders if the "/" is all that bad.

Yea, I'm assuming it would be rare enough that the div won't be too much
of a performance drop and would make the code more clear. Although if it
is a concern I'm not opposed to speeding it up. 

> Another thing, possibly not so easily fixed given the
> division between "arch" code and common code, but I would
> like to see jiffies updated in only ONE place.  With this
> patch it is updated in .../kernel/timer.c AND in
> .../arch/kernel/time.c.  In the high-res-timers patch I made
> the jiffies update an inline in an "arch" header file so I
> could have the best of both worlds, i.e. update from common
> code using arch resources (TSC, etc).

Yea, I'm not psyched about that as well (not only is it updated twice,
but three times: arch independent, tsc and cyclone). The inline bit
sounds interesting, are you planning on pushing that in?

thanks
-john

Re: [RFC][PATCH] linux-2.4.21-pre4_tsc-lost-tick_A0

[george anzinger]

john stultz wrote:
> 
> On Thu, 2003-01-30 at 17:08, george anzinger wrote:
> > john stultz wrote:
> > > I'm already somewhat cautious that loops_per_jiffy isn't going to cut it
> > > with this patch (I'm thinking fast_gettimeoffset_quotient would probably
> > > be better). So please let me know if you find any issues with this
> > > patch.
> >
> > I think you are wondering about the "/", as am I.  Possibly
> > a while loop, or, something like
> > fast_gettimeoffset_quotient, but scaled to do jiffies
> > instead of micro seconds.  Still you SHOULD be doing this so
> > seldom that one wonders if the "/" is all that bad.
> 
> Yea, I'm assuming it would be rare enough that the div won't be too much
> of a performance drop and would make the code more clear. Although if it
> is a concern I'm not opposed to speeding it up.
> 
> > Another thing, possibly not so easily fixed given the
> > division between "arch" code and common code, but I would
> > like to see jiffies updated in only ONE place.  With this
> > patch it is updated in .../kernel/timer.c AND in
> > .../arch/kernel/time.c.  In the high-res-timers patch I made
> > the jiffies update an inline in an "arch" header file so I
> > could have the best of both worlds, i.e. update from common
> > code using arch resources (TSC, etc).
> 
> Yea, I'm not psyched about that as well (not only is it updated twice,
> but three times: arch independent, tsc and cyclone). The inline bit
> sounds interesting, are you planning on pushing that in?

I am a bit discouraged on that front.  I was hoping to get
the POSIX clocks & timers patch in, but it is long past
Halloween and, while he said he might, he didn't.  I don't
know if he is finished as yet, but...  The jiffies update
was part of the core patch to do the high res timers which
he said he would not take.  I would like to try parting it
out and pushing in some stuff, such as scmath.h which makes
things like fast_gettimeoffset_quotient not only
understandable but easy.  Folks in the cpu frequency area
would like that.  Still, my boss has other plans...

-- 
George Anzinger   george@mvista.com
High-res-timers: 
http://sourceforge.net/projects/high-res-timers/
Preemption patch:
http://www.kernel.org/pub/linux/kernel/people/rml

Re: [RFC][PATCH] linux-2.4.21-pre4_tsc-lost-tick_A0

[john stultz]

On Thu, 2003-01-30 at 17:41, george anzinger wrote:
> know if he is finished as yet, but...  The jiffies update
> was part of the core patch to do the high res timers which
> he said he would not take.  I would like to try parting it
> out and pushing in some stuff, such as scmath.h which makes
> things like fast_gettimeoffset_quotient not only
> understandable but easy.  Folks in the cpu frequency area
> would like that.  Still, my boss has other plans...

Yea, I believe I commented a while back that there are some really good
generic cleanups in the high-res code. Add me to the list of folks who
would still like to see those broken out and submitted. :)

thanks
-john