Re: [PATCH/RFC] KVM: track pid for VCPU only on KVM_RUN ioctl

Paolo Bonzini
2014-12-03T13:20:40Z

On 05/08/2014 16:44, Christian Borntraeger wrote:
> We currently track the pid of the task that runs the VCPU in
> vcpu_load. Since we call vcpu_load for all kind of ioctls on a
> CPU, this causes hickups due to synchronize_rcu if one CPU is
> modified by another CPU or the main thread (e.g. initialization,
> reset). We track the pid only for the purpose of yielding, so
> let's update the pid only in the KVM_RUN ioctl.
>
> In addition, don't do a synchronize_rcu on startup (pid == 0).
>
> This speeds up guest boot time on s390 noticably for some configs, e.g.
> HZ=100, no full state tracking, 64 guest cpus 32 host cpus.
>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> CC: Rik van Riel <riel@redhat.com>
> CC: Raghavendra K T <raghavendra.kt@linux.vnet.ibm.com>
> CC: Michael Mueller <mimu@linux.vnet.ibm.com>
> ---
> virt/kvm/kvm_main.c | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 9ae9135..ebc8f54 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -124,14 +124,6 @@ int vcpu_load(struct kvm_vcpu *vcpu)
>
> if (mutex_lock_killable(&vcpu->mutex))
> return -EINTR;
> - if (unlikely(vcpu->pid != current->pids[PIDTYPE_PID].pid)) {
> - /* The thread running this VCPU changed. */
> - struct pid *oldpid = vcpu->pid;
> - struct pid *newpid = get_task_pid(current, PIDTYPE_PID);
> - rcu_assign_pointer(vcpu->pid, newpid);
> - synchronize_rcu();
> - put_pid(oldpid);
> - }
> cpu = get_cpu();
> preempt_notifier_register(&vcpu->preempt_notifier);
> kvm_arch_vcpu_load(vcpu, cpu);
> @@ -1991,6 +1983,15 @@ static long kvm_vcpu_ioctl(struct file *filp,
> r = -EINVAL;
> if (arg)
> goto out;
> + if (unlikely(vcpu->pid != current->pids[PIDTYPE_PID].pid)) {
> + /* The thread running this VCPU changed. */
> + struct pid *oldpid = vcpu->pid;
> + struct pid *newpid = get_task_pid(current, PIDTYPE_PID);
> + rcu_assign_pointer(vcpu->pid, newpid);
> + if (oldpid)
> + synchronize_rcu();
> + put_pid(oldpid);
> + }
> r = kvm_arch_vcpu_ioctl_run(vcpu, vcpu->run);
> trace_kvm_userspace_exit(vcpu->run->exit_reason, r);
> break;
>

Applied with rewritten commit message:

KVM: track pid for VCPU only on KVM_RUN ioctl

We currently track the pid of the task that runs the VCPU in vcpu_load.
If a yield to that VCPU is triggered while the PID of the wrong thread
is active, the wrong thread might receive a yield, but this will most
likely not help the executing thread at all. Instead, if we only track
the pid on the KVM_RUN ioctl, there are two possibilities:

1) the thread that did a non-KVM_RUN ioctl is holding a mutex that
the VCPU thread is waiting for. In this case, the VCPU thread is not
runnable, but we also do not do a wrong yield.

2) the thread that did a non-KVM_RUN ioctl is sleeping, or doing
something that does not block the VCPU thread. In this case, the
VCPU thread can receive the directed yield correctly.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
CC: Rik van Riel <riel@redhat.com>
CC: Raghavendra K T <raghavendra.kt@linux.vnet.ibm.com>
CC: Michael Mueller <mimu@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Thanks,

Paolo