| Message ID | 1393256549-7743-1-git-send-email-drjones@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Feb 24, 2014 at 04:42:29PM +0100, Andrew Jones wrote: > When the tsc is marked unstable on the host it causes global clock > updates to be requested each time a vcpu is loaded, nearly halting > all progress on guests with a large number of vcpus. > > Fix this by only requesting a local clock update unless the vcpu > is migrating to another cpu. > > Signed-off-by: Andrew Jones <drjones@redhat.com> > --- > arch/x86/kvm/x86.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 6530019116b0d..ea716a162b4a3 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -2781,15 +2781,18 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu) > vcpu->arch.last_guest_tsc); > kvm_x86_ops->write_tsc_offset(vcpu, offset); > vcpu->arch.tsc_catchup = 1; > + set_bit(KVM_REQ_CLOCK_UPDATE, &vcpu->requests); > } > + } > + > + if (unlikely(vcpu->cpu != cpu)) { > /* > * On a host with synchronized TSC, there is no need to update > * kvmclock on vcpu->cpu migration > */ > if (!vcpu->kvm->arch.use_master_clock || vcpu->cpu == -1) > kvm_make_request(KVM_REQ_GLOBAL_CLOCK_UPDATE, vcpu); > - if (vcpu->cpu != cpu) > - kvm_migrate_timers(vcpu); > + kvm_migrate_timers(vcpu); > vcpu->cpu = cpu; > } > > -- > 1.8.1.4 Consider VCPU1 not doing kvm_arch_vcpu_load (guest not executing HLT, not switching VCPUs, no exits to QEMU). VCPU0 doing kvm_arch_vcpu_load (guest executing HLT, say). The updates on VCPU0 must generate updates on VCPU1 as well, otherwise NTP correction applies to VCPU0 but not VCPU1. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Feb 24, 2014 at 06:15:24PM -0300, Marcelo Tosatti wrote: > On Mon, Feb 24, 2014 at 04:42:29PM +0100, Andrew Jones wrote: > > When the tsc is marked unstable on the host it causes global clock > > updates to be requested each time a vcpu is loaded, nearly halting > > all progress on guests with a large number of vcpus. > > > > Fix this by only requesting a local clock update unless the vcpu > > is migrating to another cpu. > > > > Signed-off-by: Andrew Jones <drjones@redhat.com> > > --- > > arch/x86/kvm/x86.c | 7 +++++-- > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > > index 6530019116b0d..ea716a162b4a3 100644 > > --- a/arch/x86/kvm/x86.c > > +++ b/arch/x86/kvm/x86.c > > @@ -2781,15 +2781,18 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu) > > vcpu->arch.last_guest_tsc); > > kvm_x86_ops->write_tsc_offset(vcpu, offset); > > vcpu->arch.tsc_catchup = 1; > > + set_bit(KVM_REQ_CLOCK_UPDATE, &vcpu->requests); > > } > > + } > > + > > + if (unlikely(vcpu->cpu != cpu)) { > > /* > > * On a host with synchronized TSC, there is no need to update > > * kvmclock on vcpu->cpu migration > > */ > > if (!vcpu->kvm->arch.use_master_clock || vcpu->cpu == -1) > > kvm_make_request(KVM_REQ_GLOBAL_CLOCK_UPDATE, vcpu); > > - if (vcpu->cpu != cpu) > > - kvm_migrate_timers(vcpu); > > + kvm_migrate_timers(vcpu); > > vcpu->cpu = cpu; > > } > > > > -- > > 1.8.1.4 > > Consider VCPU1 not doing kvm_arch_vcpu_load (guest not executing HLT, > not switching VCPUs, no exits to QEMU). > > VCPU0 doing kvm_arch_vcpu_load (guest executing HLT, say). > > The updates on VCPU0 must generate updates on VCPU1 as well, otherwise > NTP correction applies to VCPU0 but not VCPU1. > OK. So, as we discussed off-list, we need to bind the time that vcpu clocks are out of synch. When vcpu0 does its local clock update it may pick up an NTP correction. We can't wait an indeterminate amount of time for vcpu1 to pick up that correction, as the clocks will further diverge. However, we can't request a global clock update on every vcpu load either. The solution is to rate-limit the global clock updates. Marcelo calculates that we should delay the global clock updates no more than 0.1s as follows: Assume an NTP correction c is applied to one vcpu, but not the other, then in n seconds the delta of the vcpu system_timestamps will be c * n. If we assume a correction of 500ppm (worst-case), then the two vcpus will diverge 100us in 0.1s, which is a considerable amount. I have a patch prepared that will rate-limit global clock updates to a max frequency of 1/0.1s. This patch should be dropped, and I'll send the new one soon. drew -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 6530019116b0d..ea716a162b4a3 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -2781,15 +2781,18 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu) vcpu->arch.last_guest_tsc); kvm_x86_ops->write_tsc_offset(vcpu, offset); vcpu->arch.tsc_catchup = 1; + set_bit(KVM_REQ_CLOCK_UPDATE, &vcpu->requests); } + } + + if (unlikely(vcpu->cpu != cpu)) { /* * On a host with synchronized TSC, there is no need to update * kvmclock on vcpu->cpu migration */ if (!vcpu->kvm->arch.use_master_clock || vcpu->cpu == -1) kvm_make_request(KVM_REQ_GLOBAL_CLOCK_UPDATE, vcpu); - if (vcpu->cpu != cpu) - kvm_migrate_timers(vcpu); + kvm_migrate_timers(vcpu); vcpu->cpu = cpu; }
When the tsc is marked unstable on the host it causes global clock updates to be requested each time a vcpu is loaded, nearly halting all progress on guests with a large number of vcpus. Fix this by only requesting a local clock update unless the vcpu is migrating to another cpu. Signed-off-by: Andrew Jones <drjones@redhat.com> --- arch/x86/kvm/x86.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)