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

[Wanpeng Li <wanpeng.li@linux.intel.com>]

Hi Christian,
On Tue, Aug 05, 2014 at 04:44:14PM +0200, 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;

One question: 

>-	if (unlikely(vcpu->pid != current->pids[PIDTYPE_PID].pid)) {

When vcpu->pid and current->pids[PIDTYPE_PID].pid will be different?

Regards,
Wanpeng Li 

>-		/* 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;
>-- 
>1.8.4.2
>
>--
>To unsubscribe from this list: send the line "unsubscribe kvm" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html