Re: [PATCH 1/5] Add a global synchronization point for pvclock

[Zachary Amsden]

On 04/22/2010 03:11 AM, Glauber Costa wrote:
> On Tue, Apr 20, 2010 at 12:42:17PM -0700, Jeremy Fitzhardinge wrote:
>    
> > On 04/20/2010 11:54 AM, Avi Kivity wrote:
> >      
> > > On 04/20/2010 09:23 PM, Jeremy Fitzhardinge wrote:
> > >        
> > > > On 04/20/2010 02:31 AM, Avi Kivity wrote:
> > > >
> > > >          
> > > > > btw, do you want this code in pvclock.c, or shall we keep it kvmclock
> > > > > specific?
> > > > >
> > > > >            
> > > > I think its a pvclock-level fix.  I'd been hoping to avoid having
> > > > something like this, but I think its ultimately necessary.
> > > >
> > > >          
> > > Did you observe drift on Xen, or is this "ultimately" pointing at the
> > > future?
> > >        
> > People are reporting weirdnesses that "clocksource=jiffies" apparently
> > resolves.  Xen and KVM are faced with the same hardware constraints, and
> > it wouldn't surprise me if there were small measurable
> > non-monotonicities in the PV clock under Xen.  May as well be safe.
> >
> > Of course, it kills any possibility of being able to usefully export
> > this interface down to usermode.
> >
> > My main concern about this kind of simple fix is that if there's a long
> > term systematic drift between different CPU's tscs, then this will
> > somewhat mask the problem while giving really awful time measurement on
> > the "slow" CPU(s).  In that case it really needs to adjust the scaling
> > factor to correct for the drift (*not* update the offset).  But if we're
> > definitely only talking about fixed, relatively small time offsets then
> > it is fine.
> >      
> Can you by any chance run ingo's time warp test on those machines?
>
> You need to define TOD to 1, and leave out the TSC test.
>
> For me, warps exists on every machine out there, but the nehalems, so far
>    
Or apply this patch.
diff -rup a/time-warp-test.c b/time-warp-test.c
--- a/time-warp-test.c	2010-04-15 16:30:13.955981607 -1000
+++ b/time-warp-test.c	2010-04-15 16:35:37.777982377 -1000
@@ -91,7 +91,7 @@ static inline unsigned long long __rdtsc
 {
 	DECLARE_ARGS(val, low, high);
 
-	asm volatile("cpuid; rdtsc" : EAX_EDX_RET(val, low, high));
+	asm volatile("cpuid; rdtsc" : EAX_EDX_RET(val, low, high) :: "ebx", "ecx");
 
 	return EAX_EDX_VAL(val, low, high);
 }