Re: [PATCH/RFC 2/3] s390/kvm: Platform specific kvm_arch_vcpu_dont_yield

[Paolo Bonzini <pbonzini@redhat.com>]

Il 11/02/2014 12:45, Christian Borntraeger ha scritto:
> From: Michael Mueller <mimu@linux.vnet.ibm.com>
>
> Commit "s390/kvm: Use common waitqueue" caused a performance regression
> on s390. It turned out that a yield candidate was missed by just a simple
> test on its non-empty waitqueue. If an interrupt is outstanding, the candidate
> might be suitable. kvm_arch_vcpu_dont_yield is extended by a test that
> additionally tests for not yet delivered interrupts.
>
> Significant performance measurement work and code analysis to solve
> this issue was provided by Mao Chuan Li and his team in Beijing.
>
> Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  arch/s390/kvm/Kconfig    | 1 +
>  arch/s390/kvm/kvm-s390.c | 7 +++++++
>  2 files changed, 8 insertions(+)
>
> diff --git a/arch/s390/kvm/Kconfig b/arch/s390/kvm/Kconfig
> index c8bacbc..e44adef 100644
> --- a/arch/s390/kvm/Kconfig
> +++ b/arch/s390/kvm/Kconfig
> @@ -25,6 +25,7 @@ config KVM
>  	select HAVE_KVM_EVENTFD
>  	select KVM_ASYNC_PF
>  	select KVM_ASYNC_PF_SYNC
> +	select HAVE_KVM_ARCH_VCPU_DONT_YIELD
>  	---help---
>  	  Support hosting paravirtualized guest machines using the SIE
>  	  virtualization capability on the mainframe. This should work
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index a5da2cc..1a33e1e 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -1231,6 +1231,13 @@ int kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf)
>  	return VM_FAULT_SIGBUS;
>  }
>
> +#ifdef CONFIG_HAVE_KVM_ARCH_VCPU_DONT_YIELD
> +bool kvm_arch_vcpu_dont_yield(struct kvm_vcpu *vcpu)
> +{
> +	return waitqueue_active(&vcpu->wq) && !kvm_cpu_has_interrupt(vcpu);
> +}
> +#endif

I wonder if just using "&& !kvm_arch_vcpu_runnable(vcpu)" in 
kvm_vcpu_on_spin would be better.

Right now, you do not need it in s390 because kvm_vcpu_block is not used 
either.  But you could simply define it to kvm_cpu_has_interrupt(vcpu) 
instead.

Paolo

> +
>  void kvm_arch_free_memslot(struct kvm *kvm, struct kvm_memory_slot *free,
>  			   struct kvm_memory_slot *dont)
>  {
>