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

Zachary Amsden
2010-04-23T01:44:50Z

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