Re: [RFC] KVM: optimize the kvm_vcpu_on_spin

[Longpeng (Mike)]

On 2017/7/31 20:31, Cornelia Huck wrote:

> On Mon, 31 Jul 2017 20:08:14 +0800
> "Longpeng (Mike)" <longpeng2@xxxxxxxxxx> wrote:
> 
>> Hi David,
>>
>> On 2017/7/31 19:31, David Hildenbrand wrote:
> 
>>>> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
>>>> index 648b34c..f8f0d74 100644
>>>> --- a/include/linux/kvm_host.h
>>>> +++ b/include/linux/kvm_host.h
>>>> @@ -272,6 +272,9 @@ struct kvm_vcpu {
>>>>  	} spin_loop;
>>>>  #endif
>>>>  	bool preempted;
>>>> +	/* If vcpu is in kernel-mode when preempted */
>>>> +	bool in_kernmode;
>>>> +  
>>>
>>> Why do you have to store that ...
>>>   
>>
>>> [...]> +	me->in_kernmode = kvm_arch_vcpu_spin_kernmode(me);
>>>>  	kvm_vcpu_set_in_spin_loop(me, true);
>>>>  	/*
>>>>  	 * We boost the priority of a VCPU that is runnable but not
>>>> @@ -2351,6 +2353,8 @@ void kvm_vcpu_on_spin(struct kvm_vcpu *me)
>>>>  				continue;
>>>>  			if (swait_active(&vcpu->wq) && !kvm_arch_vcpu_runnable(vcpu))
>>>>  				continue;
>>>> +			if (me->in_kernmode && !vcpu->in_kernmode)  
>>>
>>> Wouldn't it be easier to simply have
>>>
>>> in_kernel = kvm_arch_vcpu_in_kernel(me);
>>> ...
>>> if (in_kernel && !kvm_arch_vcpu_in_kernel(vcpu))
>>> ...
>>>   
>>
>> I'm not sure whether the operation of get the vcpu's priority-level is
>> expensive on all architectures, so I record it in kvm_sched_out() for
>> minimal the extra cycles cost in kvm_vcpu_on_spin().
> 
> As it is now, this handling looks a bit inconsistent. You only update
> the field on sched-out via preemption _or_ if kvm_vcpu_on_spin is
> called for the vcpu. In most contexts, this field will have stale
> content.
> 
> Also, would checking for kernel mode be more expensive than the various
> other checks already done in this function?
> 

> [I like David's suggestion.]
> 


Hi Cornelia & David,

I'll take your suggestion, thanks :)

>>
>>>> +				continue;
>>>>  			if (!kvm_vcpu_eligible_for_directed_yield(vcpu))
>>>>  				continue;
>>>>  
> 
> .
> 


-- 
Regards,
Longpeng(Mike)