linux-kernel - Re: [RFC] KVM: optimize the kvm_vcpu_on

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <597F1DAE.4020809@huawei.com>
Date:   Mon, 31 Jul 2017 20:08:14 +0800
From:   "Longpeng (Mike)" <longpeng2@...wei.com>
To:     David Hildenbrand <david@...hat.com>
CC:     <pbonzini@...hat.com>, <rkrcmar@...hat.com>, <agraf@...e.com>,
        <borntraeger@...ibm.com>, <cohuck@...hat.com>,
        <christoffer.dall@...aro.org>, <marc.zyngier@....com>,
        <james.hogan@...tec.com>, <kvm@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <weidong.huang@...wei.com>,
        <arei.gonglei@...wei.com>, <wangxinxin.wang@...wei.com>,
        <longpeng.mike@...il.com>
Subject: Re: [RFC] KVM: optimize the kvm_vcpu_on_spin

Hi David,

On 2017/7/31 19:31, David Hildenbrand wrote:

> [no idea if this change makes sense (and especially if it has any bad
> side effects), do you have performance numbers? I'll just have a look at
> the general structure of the patch in the meanwhile]
> 

I haven't any test results yet, could you give me some suggestion about what
benchmarks are suitable ?

>> +bool kvm_arch_vcpu_spin_kernmode(struct kvm_vcpu *vcpu)
> 
> kvm_arch_vcpu_in_kernel() ?
> 

Um...yes, I'll correct this.

>> +{
>> +	return kvm_x86_ops->get_cpl(vcpu) == 0;
>> +}
>> +
>>  int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu)
>>  {
>>  	return kvm_vcpu_exiting_guest_mode(vcpu) == IN_GUEST_MODE;
>> 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().

>> +				continue;
>>  			if (!kvm_vcpu_eligible_for_directed_yield(vcpu))
>>  				continue;
>>  
>> @@ -4009,8 +4013,11 @@ static void kvm_sched_out(struct preempt_notifier *pn,
>>  {
>>  	struct kvm_vcpu *vcpu = preempt_notifier_to_vcpu(pn);
>>  
>> -	if (current->state == TASK_RUNNING)
>> +	if (current->state == TASK_RUNNING) {
>>  		vcpu->preempted = true;
>> +		vcpu->in_kernmode = kvm_arch_vcpu_spin_kernmode(vcpu);
>> +	}
>> +
> 
> so you don't have to do this change, too.
> 
>>  	kvm_arch_vcpu_put(vcpu);
>>  }
>>  
>>
> 
> 


-- 
Regards,
Longpeng(Mike)