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

[Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>]

On 01/05/2011 05:48 PM, Avi Kivity wrote:

>>       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)

......

>> -        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().

Yeah, you're right, i'll fix these in the next version.

> 
>>       }
>>       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.
> 

OK, will do it in the separate patch in the future, thanks!