Re: xen/clock.c:get_tsc_offset_ns

[Manuel Bouyer <bouyer%antioche.eu.org@localhost>]

On Fri, Jan 13, 2006 at 11:52:19AM +0900, YAMAMOTO Takashi wrote:
> > > I notice that the static function get_tsc_offset_ns in xen/clock.c
> > > multiplies a uint32_t by 1000000000, which looks to me as though it'll
> > > pretty much always overflow (if more than 4 cycles have passed since
> > > the last call to get_time_values_from_xen, it seems).  So, two questions:
> > >
> > > 1) Should that be 1000000000ULL instead? 
> > 
> > Now that I've looked at this a little more closely, change that to:
> > Why is that a uint32, and why is cpu_counter32() being called?
> > shadow_tsc_stamp is 64-bit.
> 
> i think you're right.
> it should use cpu_counter() and tsc_delta should be of uint64_t.

Yes.

> 
> > > 2) If so, how much is this breaking?
> > 
> > The answer may be "quite a lot", if the shared_info time stuff is
> > updated as infrequently as it sounds like.  Also:
> 
> given the overflow and the frequency of today's cpus,
> this function always return a quite small value, compared to NS_PER_TICK.
> so it's effectively just ignored, i think.

Not ignored, but it's probably a small error that can be corrected by
ntpd. I've never seen ntpd bailing out because the error is too large.

But this needs to be fixed.

-- 
Manuel Bouyer <bouyer%antioche.eu.org@localhost>
     NetBSD: 26 ans d'experience feront toujours la difference
--