[PATCH RFC 2/2] KVM: thread creating a vcpu is the owner of that vcpu

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Currently, we allow changing the PID of a VCPU. This PID is used to
identify the thread to yield to if we want to yield to this specific
VCPU.

In practice (e.g. QEMU), the thread creating and executing the VCPU remains
always the same. Temporarily exchanging the PID (e.g. because an ioctl is
triggered from a wrong thread) doesn't really make sense.

The PID is exchanged and a synchronize_rcu() is called. When the executing
thread tries to run the VCPU again, another synchronize_rcu() happens.

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. The executing thread won't have a higher
priority after the wrong thread has finished with the ioctl. The wrong thread
will even receive yields afterwards that were targeted to the executing vcpu,
until the executing VCPU has replaced the PID on the next ioctl - doesn't feel
correct to me.

This patch makes the creating thread the owning thread, and therefore the only
valid yield candidate (especially because VCPU ioctls are - in theory - only
valid when triggered from the owning thread - old user space versions may not
stick to this rule). This should also speed up the initial start of all VCPUs,
when the PID is assigned for the first time.

Should be backwards compatible - if there is any old user space version out
there that doesn't stick to the creating == executing thread rule, yields will
not work as intended.

Signed-off-by: David Hildenbrand <dahi@xxxxxxxxxxxxxxxxxx>
---
 include/linux/kvm_host.h |  1 +
 virt/kvm/kvm_main.c      | 18 ++----------------
 2 files changed, 3 insertions(+), 16 deletions(-)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index aa56894..f1fe655 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -245,6 +245,7 @@ struct kvm_vcpu {
 	int fpu_active;
 	int guest_fpu_loaded, guest_xcr0_loaded;
 	wait_queue_head_t wq;
+	/* the pid owning this vcpu - target for vcpu yields */
 	struct pid *pid;
 	int sigset_active;
 	sigset_t sigset;
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 184f52e..4ba7810 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -124,15 +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);
-		if (oldpid)
-			synchronize_rcu();
-		put_pid(oldpid);
-	}
 	cpu = get_cpu();
 	preempt_notifier_register(&vcpu->preempt_notifier);
 	kvm_arch_vcpu_load(vcpu, cpu);
@@ -220,7 +211,7 @@ int kvm_vcpu_init(struct kvm_vcpu *vcpu, struct kvm *kvm, unsigned id)
 	vcpu->cpu = -1;
 	vcpu->kvm = kvm;
 	vcpu->vcpu_id = id;
-	vcpu->pid = NULL;
+	vcpu->pid = get_task_pid(current, PIDTYPE_PID);
 	init_waitqueue_head(&vcpu->wq);
 	kvm_async_pf_vcpu_init(vcpu);
 
@@ -1771,15 +1762,10 @@ EXPORT_SYMBOL_GPL(kvm_vcpu_kick);
 
 int kvm_vcpu_yield_to(struct kvm_vcpu *target)
 {
-	struct pid *pid;
 	struct task_struct *task = NULL;
 	int ret = 0;
 
-	rcu_read_lock();
-	pid = rcu_dereference(target->pid);
-	if (pid)
-		task = get_pid_task(pid, PIDTYPE_PID);
-	rcu_read_unlock();
+	task = get_pid_task(target->pid, PIDTYPE_PID);
 	if (!task)
 		return ret;
 	ret = yield_to(task, 1);
-- 
1.8.5.5

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux