From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexander Graf Date: Thu, 20 Nov 2014 17:36:40 +0000 Subject: Re: [PATCH 5/5] KVM: PPC: Book3S HV: Check wait conditions before sleeping in kvmppc_vcore_blocked Message-Id: <546E26A8.1080904@suse.de> List-Id: References: <1414990320-6378-1-git-send-email-paulus@samba.org> <1414990320-6378-6-git-send-email-paulus@samba.org> In-Reply-To: <1414990320-6378-6-git-send-email-paulus@samba.org> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Paul Mackerras , kvm-ppc@vger.kernel.org Cc: kvm@vger.kernel.org, "Suresh E. Warrier" On 03.11.14 05:52, Paul Mackerras wrote: > From: "Suresh E. Warrier" > > The kvmppc_vcore_blocked() code does not check for the wait condition > after putting the process on the wait queue. This means that it is > possible for an external interrupt to become pending, but the vcpu to > remain asleep until the next decrementer interrupt. The fix is to > make one last check for pending exceptions and ceded state before > calling schedule(). > > Signed-off-by: Suresh Warrier > Signed-off-by: Paul Mackerras I don't understand the race you're fixing here. Can you please explain it? Alex > --- > arch/powerpc/kvm/book3s_hv.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c > index cd7e030..1a7a281 100644 > --- a/arch/powerpc/kvm/book3s_hv.c > +++ b/arch/powerpc/kvm/book3s_hv.c > @@ -1828,9 +1828,29 @@ static void kvmppc_wait_for_exec(struct kvm_vcpu *vcpu, int wait_state) > */ > static void kvmppc_vcore_blocked(struct kvmppc_vcore *vc) > { > + struct kvm_vcpu *vcpu; > + int do_sleep = 1; > + > DEFINE_WAIT(wait); > > prepare_to_wait(&vc->wq, &wait, TASK_INTERRUPTIBLE); > + > + /* > + * Check one last time for pending exceptions and ceded state after > + * we put ourselves on the wait queue > + */ > + list_for_each_entry(vcpu, &vc->runnable_threads, arch.run_list) { > + if (vcpu->arch.pending_exceptions || !vcpu->arch.ceded) { > + do_sleep = 0; > + break; > + } > + } > + > + if (!do_sleep) { > + finish_wait(&vc->wq, &wait); > + return; > + } > + > vc->vcore_state = VCORE_SLEEPING; > spin_unlock(&vc->lock); > schedule(); > From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexander Graf Subject: Re: [PATCH 5/5] KVM: PPC: Book3S HV: Check wait conditions before sleeping in kvmppc_vcore_blocked Date: Thu, 20 Nov 2014 18:36:40 +0100 Message-ID: <546E26A8.1080904@suse.de> References: <1414990320-6378-1-git-send-email-paulus@samba.org> <1414990320-6378-6-git-send-email-paulus@samba.org> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, "Suresh E. Warrier" To: Paul Mackerras , kvm-ppc@vger.kernel.org Return-path: Received: from cantor2.suse.de ([195.135.220.15]:36122 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756859AbaKTRgm (ORCPT ); Thu, 20 Nov 2014 12:36:42 -0500 In-Reply-To: <1414990320-6378-6-git-send-email-paulus@samba.org> Sender: kvm-owner@vger.kernel.org List-ID: On 03.11.14 05:52, Paul Mackerras wrote: > From: "Suresh E. Warrier" > > The kvmppc_vcore_blocked() code does not check for the wait condition > after putting the process on the wait queue. This means that it is > possible for an external interrupt to become pending, but the vcpu to > remain asleep until the next decrementer interrupt. The fix is to > make one last check for pending exceptions and ceded state before > calling schedule(). > > Signed-off-by: Suresh Warrier > Signed-off-by: Paul Mackerras I don't understand the race you're fixing here. Can you please explain it? Alex > --- > arch/powerpc/kvm/book3s_hv.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c > index cd7e030..1a7a281 100644 > --- a/arch/powerpc/kvm/book3s_hv.c > +++ b/arch/powerpc/kvm/book3s_hv.c > @@ -1828,9 +1828,29 @@ static void kvmppc_wait_for_exec(struct kvm_vcpu *vcpu, int wait_state) > */ > static void kvmppc_vcore_blocked(struct kvmppc_vcore *vc) > { > + struct kvm_vcpu *vcpu; > + int do_sleep = 1; > + > DEFINE_WAIT(wait); > > prepare_to_wait(&vc->wq, &wait, TASK_INTERRUPTIBLE); > + > + /* > + * Check one last time for pending exceptions and ceded state after > + * we put ourselves on the wait queue > + */ > + list_for_each_entry(vcpu, &vc->runnable_threads, arch.run_list) { > + if (vcpu->arch.pending_exceptions || !vcpu->arch.ceded) { > + do_sleep = 0; > + break; > + } > + } > + > + if (!do_sleep) { > + finish_wait(&vc->wq, &wait); > + return; > + } > + > vc->vcore_state = VCORE_SLEEPING; > spin_unlock(&vc->lock); > schedule(); >