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

[Avi Kivity <avi@xxxxxxxxxx>]

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.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel