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

[Zachary Amsden <zamsden@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1776 bytes --]

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.

[-- Attachment #2: time-warp.patch --]
[-- Type: text/plain, Size: 445 bytes --]

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);
 }