From mboxrd@z Thu Jan 1 00:00:00 1970 From: Raghavendra K T Subject: Re: [PATCH RFC V4 4/5] kvm : pv-ticketlocks support for linux guests running on KVM hypervisor Date: Wed, 18 Jan 2012 00:27:19 +0530 Message-ID: <4F15C48F.1000000@linux.vnet.ibm.com> References: <20120114182501.8604.68416.sendpatchset@oc5400248562.ibm.com> <20120114182645.8604.68884.sendpatchset@oc5400248562.ibm.com> <20120117110210.GA17420@amt.cnet> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Cc: Jeremy Fitzhardinge , Raghavendra , linux-doc@vger.kernel.org, Peter Zijlstra , Jan Kiszka , Virtualization , Paul Mackerras , "H. Peter Anvin" , Stefano Stabellini , Xen , Dave Jiang , KVM , Rob Landley , X86 , Ingo Molnar , Rik van Riel , Konrad Rzeszutek Wilk , Srivatsa Vaddagiri , Sasha Levin , Sedat Dilek , Thomas Gleixner , Greg Kroah-Hartman , LKML , To: Marcelo Tosatti , Avi Kivity Return-path: In-Reply-To: <20120117110210.GA17420@amt.cnet> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: virtualization-bounces@lists.linux-foundation.org Errors-To: virtualization-bounces@lists.linux-foundation.org List-Id: kvm.vger.kernel.org On 01/17/2012 04:32 PM, Marcelo Tosatti wrote: > On Sat, Jan 14, 2012 at 11:56:46PM +0530, Raghavendra K T wrote: >> Extends Linux guest running on KVM hypervisor to support pv-ticketlocks. >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >> index c7b05fc..4d7a950 100644 >> --- a/arch/x86/kvm/x86.c >> +++ b/arch/x86/kvm/x86.c >> @@ -5754,8 +5754,9 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) >> >> local_irq_disable(); >> >> - if (vcpu->mode == EXITING_GUEST_MODE || vcpu->requests >> - || need_resched() || signal_pending(current)) { >> + if (vcpu->mode == EXITING_GUEST_MODE >> + || (vcpu->requests& ~(1UL<> + || need_resched() || signal_pending(current)) { >> vcpu->mode = OUTSIDE_GUEST_MODE; >> smp_wmb(); >> local_irq_enable(); >> @@ -6711,6 +6712,7 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu) >> !vcpu->arch.apf.halted) >> || !list_empty_careful(&vcpu->async_pf.done) >> || vcpu->arch.mp_state == KVM_MP_STATE_SIPI_RECEIVED >> + || kvm_check_request(KVM_REQ_PVLOCK_KICK, vcpu) > > The bit should only be read here (kvm_arch_vcpu_runnable), but cleared > on vcpu entry (along with the other kvm_check_request processing). > > Then the first hunk becomes unnecessary. true. [ patch below ] > > Please do not mix host/guest patches. yes, will be taken care in next version.. > > I had tried alternative approach earlier, I think that is closer to your expectation. where - flag is read in kvm_arch_vcpu_runnable - flag cleared in vcpu entry along with others. But it needs per vcpu flag to remember that pv_unhalted while clearing the flag in vcpu enter [ patch below ]. Could not find third alternative though. Simply clearing the request bit in vcpu entry had made guest hang in *rare* scenario. [as kick will be lost]. [ I had observed guest hang after 4 iteration of kernbench with 1:3 overcommit. with 2/3 guest running while 1 hogs ] Avi, do you think having pv_unhalt flag in below patch cause problem to live migration still? (vcpu->request bit is retained as is) OR do we have to have KVM_GET_MP_STATE changes also with below patch you mentioned earlier. ---8<--- diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index c38efd7..1bf8fa8 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -5684,6 +5717,11 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) r = 1; goto out; } + if (kvm_check_request(KVM_REQ_PVKICK, vcpu)) { + vcpu->pv_unhalted = 1; + r = 1; + goto out; + } if (kvm_check_request(KVM_REQ_STEAL_UPDATE, vcpu)) record_steal_time(vcpu); if (kvm_check_request(KVM_REQ_NMI, vcpu)) @@ -6683,6 +6720,7 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu) !vcpu->arch.apf.halted) || !list_empty_careful(&vcpu->async_pf.done) || vcpu->arch.mp_state == KVM_MP_STATE_SIPI_RECEIVED + || (kvm_test_request(KVM_REQ_PVKICK, vcpu) || vcpu->pv_unhalted) || atomic_read(&vcpu->arch.nmi_queued) || (kvm_arch_interrupt_allowed(vcpu) && kvm_cpu_has_interrupt(vcpu)); diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index d526231..a48e0f2 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -154,6 +155,8 @@ struct kvm_vcpu { #endif struct kvm_vcpu_arch arch; + + int pv_unhalted; }; static inline int kvm_vcpu_exiting_guest_mode(struct kvm_vcpu *vcpu) @@ -770,5 +773,12 @@ static inline bool kvm_check_request(int req, struct kvm_vcpu *vcpu) } } +static inline bool kvm_test_request(int req, struct kvm_vcpu *vcpu) +{ + if (test_bit(req, &vcpu->requests)) + return true; + else + return false; +} #endif diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index d9cfb78..55c44a2 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -226,6 +226,7 @@ int kvm_vcpu_init(struct kvm_vcpu *vcpu, struct kvm *kvm, unsigned id) vcpu->kvm = kvm; vcpu->vcpu_id = id; vcpu->pid = NULL; + vcpu->pv_unhalted = 0; init_waitqueue_head(&vcpu->wq); kvm_async_pf_vcpu_init(vcpu); @@ -1509,11 +1510,12 @@ void mark_page_dirty(struct kvm *kvm, gfn_t gfn) void kvm_vcpu_block(struct kvm_vcpu *vcpu) { DEFINE_WAIT(wait); for (;;) { prepare_to_wait(&vcpu->wq, &wait, TASK_INTERRUPTIBLE); if (kvm_arch_vcpu_runnable(vcpu)) { + vcpu->pv_unhalted = 0; kvm_make_request(KVM_REQ_UNHALT, vcpu); break; } From mboxrd@z Thu Jan 1 00:00:00 1970 From: Raghavendra K T Subject: Re: [PATCH RFC V4 4/5] kvm : pv-ticketlocks support for linux guests running on KVM hypervisor Date: Wed, 18 Jan 2012 00:27:19 +0530 Message-ID: <4F15C48F.1000000@linux.vnet.ibm.com> References: <20120114182501.8604.68416.sendpatchset@oc5400248562.ibm.com> <20120114182645.8604.68884.sendpatchset@oc5400248562.ibm.com> <20120117110210.GA17420@amt.cnet> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20120117110210.GA17420@amt.cnet> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: virtualization-bounces@lists.linux-foundation.org Errors-To: virtualization-bounces@lists.linux-foundation.org To: Marcelo Tosatti , Avi Kivity Cc: Jeremy Fitzhardinge , Raghavendra , linux-doc@vger.kernel.org, Peter Zijlstra , Jan Kiszka , Virtualization , Paul Mackerras , "H. Peter Anvin" , Stefano Stabellini , Xen , Dave Jiang , KVM , Rob Landley , X86 , Ingo Molnar , Rik van Riel , Konrad Rzeszutek Wilk , Srivatsa Vaddagiri , Sasha Levin , Sedat Dilek , Thomas Gleixner , Greg Kroah-Hartman , LKML List-Id: virtualization@lists.linuxfoundation.org On 01/17/2012 04:32 PM, Marcelo Tosatti wrote: > On Sat, Jan 14, 2012 at 11:56:46PM +0530, Raghavendra K T wrote: >> Extends Linux guest running on KVM hypervisor to support pv-ticketlocks. >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >> index c7b05fc..4d7a950 100644 >> --- a/arch/x86/kvm/x86.c >> +++ b/arch/x86/kvm/x86.c >> @@ -5754,8 +5754,9 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) >> >> local_irq_disable(); >> >> - if (vcpu->mode == EXITING_GUEST_MODE || vcpu->requests >> - || need_resched() || signal_pending(current)) { >> + if (vcpu->mode == EXITING_GUEST_MODE >> + || (vcpu->requests& ~(1UL<> + || need_resched() || signal_pending(current)) { >> vcpu->mode = OUTSIDE_GUEST_MODE; >> smp_wmb(); >> local_irq_enable(); >> @@ -6711,6 +6712,7 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu) >> !vcpu->arch.apf.halted) >> || !list_empty_careful(&vcpu->async_pf.done) >> || vcpu->arch.mp_state == KVM_MP_STATE_SIPI_RECEIVED >> + || kvm_check_request(KVM_REQ_PVLOCK_KICK, vcpu) > > The bit should only be read here (kvm_arch_vcpu_runnable), but cleared > on vcpu entry (along with the other kvm_check_request processing). > > Then the first hunk becomes unnecessary. true. [ patch below ] > > Please do not mix host/guest patches. yes, will be taken care in next version.. > > I had tried alternative approach earlier, I think that is closer to your expectation. where - flag is read in kvm_arch_vcpu_runnable - flag cleared in vcpu entry along with others. But it needs per vcpu flag to remember that pv_unhalted while clearing the flag in vcpu enter [ patch below ]. Could not find third alternative though. Simply clearing the request bit in vcpu entry had made guest hang in *rare* scenario. [as kick will be lost]. [ I had observed guest hang after 4 iteration of kernbench with 1:3 overcommit. with 2/3 guest running while 1 hogs ] Avi, do you think having pv_unhalt flag in below patch cause problem to live migration still? (vcpu->request bit is retained as is) OR do we have to have KVM_GET_MP_STATE changes also with below patch you mentioned earlier. ---8<--- diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index c38efd7..1bf8fa8 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -5684,6 +5717,11 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) r = 1; goto out; } + if (kvm_check_request(KVM_REQ_PVKICK, vcpu)) { + vcpu->pv_unhalted = 1; + r = 1; + goto out; + } if (kvm_check_request(KVM_REQ_STEAL_UPDATE, vcpu)) record_steal_time(vcpu); if (kvm_check_request(KVM_REQ_NMI, vcpu)) @@ -6683,6 +6720,7 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu) !vcpu->arch.apf.halted) || !list_empty_careful(&vcpu->async_pf.done) || vcpu->arch.mp_state == KVM_MP_STATE_SIPI_RECEIVED + || (kvm_test_request(KVM_REQ_PVKICK, vcpu) || vcpu->pv_unhalted) || atomic_read(&vcpu->arch.nmi_queued) || (kvm_arch_interrupt_allowed(vcpu) && kvm_cpu_has_interrupt(vcpu)); diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index d526231..a48e0f2 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -154,6 +155,8 @@ struct kvm_vcpu { #endif struct kvm_vcpu_arch arch; + + int pv_unhalted; }; static inline int kvm_vcpu_exiting_guest_mode(struct kvm_vcpu *vcpu) @@ -770,5 +773,12 @@ static inline bool kvm_check_request(int req, struct kvm_vcpu *vcpu) } } +static inline bool kvm_test_request(int req, struct kvm_vcpu *vcpu) +{ + if (test_bit(req, &vcpu->requests)) + return true; + else + return false; +} #endif diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index d9cfb78..55c44a2 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -226,6 +226,7 @@ int kvm_vcpu_init(struct kvm_vcpu *vcpu, struct kvm *kvm, unsigned id) vcpu->kvm = kvm; vcpu->vcpu_id = id; vcpu->pid = NULL; + vcpu->pv_unhalted = 0; init_waitqueue_head(&vcpu->wq); kvm_async_pf_vcpu_init(vcpu); @@ -1509,11 +1510,12 @@ void mark_page_dirty(struct kvm *kvm, gfn_t gfn) void kvm_vcpu_block(struct kvm_vcpu *vcpu) { DEFINE_WAIT(wait); for (;;) { prepare_to_wait(&vcpu->wq, &wait, TASK_INTERRUPTIBLE); if (kvm_arch_vcpu_runnable(vcpu)) { + vcpu->pv_unhalted = 0; kvm_make_request(KVM_REQ_UNHALT, vcpu); break; }