Re: [PATCH] New x86_64 time code for 2.5.70

[Vojtech Pavlik]

On Thu, Jun 12, 2003 at 12:39:55PM -0700, john stultz wrote:
> On Thu, 2003-06-12 at 10:47, Bryan O'Sullivan wrote:
> > On Wed, 2003-06-11 at 17:33, john stultz wrote:
> > 
> > > Let me know if you have any issues with this patch. 
> > 
> > Thanks, John.  Your updated patch has survived some beating on my test
> > systems.  I've also applied Vojtech's fix to hpet_tick.
> 
> One little tweak, you're still subtracting tick_usec when calculating
> offset. Doesn't cause any major problems since you're catching error
> with the offset<0 switch. However we won't catch lost interrupts
> properly if we're always negative. 
> 
> thanks
> -john
> 
> 
> Patch for that should be:
> 
> --- 1.22/arch/x86_64/kernel/time.c	Thu Jun 12 11:40:33 2003
> +++ edited/arch/x86_64/kernel/time.c	Thu Jun 12 11:42:43 2003
> @@ -254,7 +254,7 @@
>  		vxtime.last = offset;
>  	} else {
>  		offset = (((tsc - vxtime.last_tsc) *
> -			   vxtime.tsc_quot) >> 32) - tick_usec;
> +			   vxtime.tsc_quot) >> 32) - (USEC_PER_SEC / HZ);
>  
>  		if (offset < 0)
>  			offset = 0;

In my opinion the "if (offset < 0)" check above is not needed once the
problem you describe is fixed, since the code below actually expects
that offset can be negative and doesn't do anything in that case.

Thinking more about it, the

	if ((((tsc - vxtime.last_tsc) * vxtime.tsc_quot) >> 32) < offset)

statement is comparing unsigned and signed values, and should probably
be fixed to do a signed comparison.

-- 
Vojtech Pavlik
SuSE Labs, SuSE CR