Subject: Re: xen/clock.c:get_tsc_offset_ns
To: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
From: Manuel Bouyer <bouyer@antioche.eu.org>
List: port-xen
Date: 01/13/2006 19:54:17
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>
NetBSD: 26 ans d'experience feront toujours la difference
--