Re: KVM: x86: add module parameter to disable periodic kvmclock sync

[Marcelo Tosatti <mtosatti@redhat.com>]

On Thu, Nov 13, 2014 at 09:40:41AM +0100, Andrew Jones wrote:
> On Thu, Nov 13, 2014 at 12:44:39AM -0200, Marcelo Tosatti wrote:
> > 
> > The periodic kvmclock sync can be an undesired source of latencies.
> > 
> > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> > 
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 0033df3..be56fd3 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -98,6 +98,9 @@ module_param(ignore_msrs, bool, S_IRUGO | S_IWUSR);
> >  unsigned int min_timer_period_us = 500;
> >  module_param(min_timer_period_us, uint, S_IRUGO | S_IWUSR);
> >  
> > +static bool kvmclock_periodic_sync = 1;
> 
> Using 'true' would look nicer.
> 
> > +module_param(kvmclock_periodic_sync, bool, S_IRUGO | S_IWUSR);
> > +
> >  bool kvm_has_tsc_control;
> >  EXPORT_SYMBOL_GPL(kvm_has_tsc_control);
> >  u32  kvm_max_guest_tsc_khz;
> > @@ -1718,7 +1721,8 @@ static void kvmclock_sync_fn(struct work_struct *work)
> >  	struct kvm *kvm = container_of(ka, struct kvm, arch);
> >  
> >  	schedule_delayed_work(&kvm->arch.kvmclock_update_work, 0);
> > -	schedule_delayed_work(&kvm->arch.kvmclock_sync_work,
> > +	if (kvmclock_periodic_sync)
> > +		schedule_delayed_work(&kvm->arch.kvmclock_sync_work,
> >  					KVMCLOCK_SYNC_PERIOD);
> >  }
> 
> The above hunk shouldn't be necessary, as we'll never get there if we
> don't do the first scheduling with the below hunk.
> 
> >  
> > @@ -6971,7 +6975,8 @@ int kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu)
> >  	kvm_write_tsc(vcpu, &msr);
> >  	vcpu_put(vcpu);
> >  
> > -	schedule_delayed_work(&kvm->arch.kvmclock_sync_work,
> > +	if (kvmclock_periodic_sync)
> > +		schedule_delayed_work(&kvm->arch.kvmclock_sync_work,
> >  					KVMCLOCK_SYNC_PERIOD);
> >  
> >  	return r;
> > 
> > 
> 
> I'm not opposed to making this optional, but just curious. Were
> general use cases getting adversely affected? Or is this part of
> some RT work trying to kill as many sources of asynchronous latency
> as possible?

The latter.