Re: [Xen-devel] Re: [PATCH 3/5] x86/pvclock: add vsyscall implementation

Avi Kivity
2009-10-07T10:27:05Z

On 10/06/2009 08:46 PM, Jeremy Fitzhardinge wrote:
>
>> Instead of using vgetcpu() and rdtsc() independently, you can use
>> rdtscp to read both atomically. This removes the need for the preempt
>> notifier.
>>
> rdtscp first appeared on Intel with Nehalem, so we need to support older
> Intel chips.
>

We can support them by falling back to the kernel. I'm a bit worried
about the kernel playing with the hypervisor's version field. It's
better to introduce yet a new version for the kernel, and check both.

> You could use rdscp to get (tsc,cpu) atomically, but that's not
> sufficient to be able to get a consistent snapshot of (tsc, time_info)
> because it doesn't give you the pvclock_vcpu_time_info version number.
> If TSC_AUX contained that too, it might be possible. Alternatively you
> could compare the tsc with pvclock.tsc_timestamp, but unfortunately the
> ABI doesn't specify that tsc_timestamp is updated in any particular
> order compared to the rest of the fields, so you still can't use that to
> get a consistent snapshot (we can revise the ABI, of course).
>
> So either way it doesn't avoid the need to iterate. vgetcpu will use
> rdtscp if available, but I agree it is unfortunate we need to do a
> redundant rdtsc in that case.
>
>

def try_pvclock_vtime():
tsc, p0 = rdtscp()
v0 = pvclock[p0].version
tsc, p = rdtscp()
t = pvclock_time(pvclock[p], tsc)
if p != p0 or pvclock[p].version != v0:
raise Exception("Processor or timebased change under our feet")
return t

def pvclock_time():
while True:
try:
return try_pvlock_time()
except:
pass

So, two rdtscps and two compares.

>>> + for (cpu = 0; cpu< nr_cpu_ids; cpu++)
>>> + pvclock_vsyscall_time_info[cpu].version = ~0;
>>> +
>>> + __set_fixmap(FIX_PVCLOCK_TIME_INFO,
>>> __pa(pvclock_vsyscall_time_info),
>>> + PAGE_KERNEL_VSYSCALL);
>>> +
>>> + preempt_notifier_init(&pvclock_vsyscall_notifier,
>>> +&pvclock_vsyscall_preempt_ops);
>>> + preempt_notifier_register(&pvclock_vsyscall_notifier);
>>> +
>>>
>> preempt notifiers are per-thread, not global, and will upset the cycle
>> counters.
>>
> Ah, so I need to register it on every new thread? That's a bit awkward.
>

It's used to manage processor registers, much like the fpu. If a thread
uses a register that's not saved and restored by the normal context
switch code, it can register a preempt notifier to do that instead.

> This is intended to satisfy the cycle-counters who want to do
> gettimeofday a million times a second, where I guess the tradeoff of
> avoiding a pile of syscalls is worth a bit of context-switch overhead.
>

It's sufficient to increment a version counter on thread migration, no
need to do it on context switch.

--
Do not meddle in the internals of kernels, for they are subtle and quick to panic.