Re: [PATCH RFC V4 2/3] kvm: Note down when cpu relax intercepted or pause loop exited

[Raghavendra K T <raghavendra.kt@linux.vnet.ibm.com>]

On 07/16/2012 03:31 PM, Avi Kivity wrote:
> On 07/16/2012 11:25 AM, Raghavendra K T wrote:
>> From: Raghavendra K T<raghavendra.kt@linux.vnet.ibm.com>
>>
>> Noting pause loop exited vcpu or cpu relax intercepted helps in
>> filtering right candidate to yield. Wrong selection of vcpu;
>> i.e., a vcpu that just did a pl-exit or cpu relax intercepted may
>> contribute to performance degradation.
>>
>> Signed-off-by: Raghavendra K T<raghavendra.kt@linux.vnet.ibm.com>
>> ---
>> V2 was:
>> Reviewed-by: Rik van Riel<riel@redhat.com>
>>
>>   include/linux/kvm_host.h |   37 +++++++++++++++++++++++++++++++++++++
>>   virt/kvm/kvm_main.c      |    5 +++++
>>   2 files changed, 42 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
>> index c446435..fa353bd 100644
>> --- a/include/linux/kvm_host.h
>> +++ b/include/linux/kvm_host.h
>> @@ -183,6 +183,18 @@ struct kvm_vcpu {
>>   	} async_pf;
>>   #endif
>>
>> +#ifdef CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT
>> +	/*
>> +	 * Cpu relax intercept or pause loop exit optimization
>> +	 * cpu_relax_intercepted: set when a vcpu does a pause loop exit
>> +	 *  or cpu relax intercepted.
>> +	 * dy_eligible: indicates whether vcpu is eligible for directed yield.
>> +	 */
>> +	struct {
>> +		bool cpu_relax_intercepted;
>> +		bool dy_eligible;
>> +	} ple;
>
> Naming:
>
> - ple is the x86 name, but this is common code.  I'd call it spin_loop.

agree.

> - cpu_relax_intercepted: can call it in_spin_loop,

Yes

the original name can
> me "are we intercepting cpu_relax or not" which may be useful at one
> point (when we know we aren't overcommitted)

Okay.
So are you saying allow vcpu to spin in non over-commit scenarios? So 
that we avoid all yield_to etc...

( Or even in some other place where it is useful).

> - dy_eligible: I'd like to understand the algorithm more and would want
> the name to reflect it, will come to this later if I have a better idea.

Yes, welcome any good name.

I had thought eligible initially, but felt it is too generic.

The intention of variable is to decide whether the vcpu is eligible for 
directed yield, so made it short, and added eligible.

>> +#endif
>
> blank line.

oops! 'll take care of this.

>
>>   	struct kvm_vcpu_arch arch;
>>   };
>>
>
>