Re: [PATCH 2/3] KVM: send IPI to vcpu only when it's in guest mode

[Avi Kivity <avi@redhat.com>]

On 01/05/2011 11:38 AM, Xiao Guangrong wrote:
> We can interrupt the vcpu only when it's running in guest mode
> to reduce IPI.
>
> It looks like only ia64 and x86 need to send IPI to other vcpus, so
> i only add the implementation of 'vcpu->guest_mode' in ia64, but i
> don't know ia64 well, please point out the right way for me if the
> implementation is incorrect
>
> And ia64 is not tested since i don't have ia64 box
>
> Signed-off-by: Xiao Guangrong<xiaoguangrong@cn.fujitsu.com>
> ---
>   arch/ia64/kvm/kvm-ia64.c |    2 ++
>   virt/kvm/kvm_main.c      |    3 ++-
>   2 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c
> index 70d224d..15c11b2 100644
> --- a/arch/ia64/kvm/kvm-ia64.c
> +++ b/arch/ia64/kvm/kvm-ia64.c
> @@ -662,6 +662,7 @@ again:
>   		goto vcpu_run_fail;
>
>   	srcu_read_unlock(&vcpu->kvm->srcu, idx);
> +	atomic_set(&vcpu->guest_mode, 1);
>   	kvm_guest_enter();

I think it needs an smp_wmb() (technically x86 needs it too, but x86 is 
strongly ordered)

>
>   	/*
> @@ -683,6 +684,7 @@ again:
>   	 */
>   	barrier();
>   	kvm_guest_exit();
> +	atomic_set(&vcpu->guest_mode, 0);
>   	preempt_enable();
>
>   	idx = srcu_read_lock(&vcpu->kvm->srcu);
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index b1b6cbb..6648c6e 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -153,7 +153,8 @@ static bool make_all_cpus_request(struct kvm *kvm, unsigned int req)
>   		if (kvm_make_check_request(req, vcpu))
>   			continue;
>   		cpu = vcpu->cpu;
> -		if (cpus != NULL&&  cpu != -1&&  cpu != me)
> +		if (cpus != NULL&&  cpu != -1&&  cpu != me&&
> +		      atomic_read(&vcpu->guest_mode))
>   			cpumask_set_cpu(cpu, cpus);

and smp_rmb() before the atomic_read().

>   	}
>   	if (unlikely(cpus == NULL))

Not sure if this is an optimization.  On one hand it removes an 
expensive IPI for the fraction of time the cpu is out of guest mode.  On 
the other hand it adds an unconditional cacheline bounce (and bounce back).

Hm. I see that ->guest_mode and ->requests are in fact in the same cache 
line.  So this is likely really an optimization.  We should probably 
reorganize kvm_vcpu so that this is made explicit.

-- 
error compiling committee.c: too many arguments to function