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.]
>
> >> + continue;
> >> if (!kvm_vcpu_eligible_for_directed_yield(vcpu))
> >> continue;
> >>
