| Message ID | 20130318165432.GA12342@amt.cnet (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Mar 18, 2013 at 01:54:32PM -0300, Marcelo Tosatti wrote: > > There is a deadlock in pvclock handling: > > cpu0: cpu1: > kvm_gen_update_masterclock() > kvm_guest_time_update() > spin_lock(pvclock_gtod_sync_lock) > local_irq_save(flags) > > spin_lock(pvclock_gtod_sync_lock) > > kvm_make_mclock_inprogress_request(kvm) > make_all_cpus_request() > smp_call_function_many() > > Now if smp_call_function_many() called by cpu0 tries to call function on > cpu1 there will be a deadlock. > > Fix by moving pvclock_gtod_sync_lock protected section outside irq > disabled section. > > Analyzed by Gleb Natapov <gleb@redhat.com> > Reported-and-Tested-by: Yongjie Ren <yongjie.ren@intel.com> > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> > Acked-by: Gleb Natapov <gleb@redhat.com> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index f71500a..f7c850b 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -1416,15 +1416,6 @@ static int kvm_guest_time_update(struct kvm_vcpu *v) > kernel_ns = 0; > host_tsc = 0; > > - /* Keep irq disabled to prevent changes to the clock */ > - local_irq_save(flags); > - this_tsc_khz = __get_cpu_var(cpu_tsc_khz); > - if (unlikely(this_tsc_khz == 0)) { > - local_irq_restore(flags); > - kvm_make_request(KVM_REQ_CLOCK_UPDATE, v); > - return 1; > - } > - > /* > * If the host uses TSC clock, then passthrough TSC as stable > * to the guest. > @@ -1436,6 +1427,15 @@ static int kvm_guest_time_update(struct kvm_vcpu *v) > kernel_ns = ka->master_kernel_ns; > } > spin_unlock(&ka->pvclock_gtod_sync_lock); > + > + /* Keep irq disabled to prevent changes to the clock */ > + local_irq_save(flags); > + this_tsc_khz = __get_cpu_var(cpu_tsc_khz); > + if (unlikely(this_tsc_khz == 0)) { > + local_irq_restore(flags); > + kvm_make_request(KVM_REQ_CLOCK_UPDATE, v); > + return 1; > + } > if (!use_master_clock) { > host_tsc = native_read_tsc(); > kernel_ns = get_kernel_ns(); -- Gleb. -- 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 f71500a..f7c850b 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -1416,15 +1416,6 @@ static int kvm_guest_time_update(struct kvm_vcpu *v) kernel_ns = 0; host_tsc = 0; - /* Keep irq disabled to prevent changes to the clock */ - local_irq_save(flags); - this_tsc_khz = __get_cpu_var(cpu_tsc_khz); - if (unlikely(this_tsc_khz == 0)) { - local_irq_restore(flags); - kvm_make_request(KVM_REQ_CLOCK_UPDATE, v); - return 1; - } - /* * If the host uses TSC clock, then passthrough TSC as stable * to the guest. @@ -1436,6 +1427,15 @@ static int kvm_guest_time_update(struct kvm_vcpu *v) kernel_ns = ka->master_kernel_ns; } spin_unlock(&ka->pvclock_gtod_sync_lock); + + /* Keep irq disabled to prevent changes to the clock */ + local_irq_save(flags); + this_tsc_khz = __get_cpu_var(cpu_tsc_khz); + if (unlikely(this_tsc_khz == 0)) { + local_irq_restore(flags); + kvm_make_request(KVM_REQ_CLOCK_UPDATE, v); + return 1; + } if (!use_master_clock) { host_tsc = native_read_tsc(); kernel_ns = get_kernel_ns();
There is a deadlock in pvclock handling: cpu0: cpu1: kvm_gen_update_masterclock() kvm_guest_time_update() spin_lock(pvclock_gtod_sync_lock) local_irq_save(flags) spin_lock(pvclock_gtod_sync_lock) kvm_make_mclock_inprogress_request(kvm) make_all_cpus_request() smp_call_function_many() Now if smp_call_function_many() called by cpu0 tries to call function on cpu1 there will be a deadlock. Fix by moving pvclock_gtod_sync_lock protected section outside irq disabled section. Analyzed by Gleb Natapov <gleb@redhat.com> Reported-and-Tested-by: Yongjie Ren <yongjie.ren@intel.com> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> -- 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