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)
