[kvm-devel] [PATCH] Don't race while creating a VCPU

[Anthony L. <ali...@us...>]

We hold qemu_mutex while machine->init() executes, which issues a VCPU create.
We need to make sure to not return from the VCPU creation until the VCPU
file descriptor is valid to ensure that APIC creation succeeds.

However, we also need to make sure that the VCPU thread doesn't start running
until the machine->init() is complete.  This is addressed today because the
VCPU thread tries to grab the qemu_mutex before doing anything interesting.
If we release qemu_mutex to wait for VCPU creation, then we open a window for
a race to occur.

This patch introduces two wait conditions.  The first lets the VCPU create
code that runs in the IO thread to wait for a VCPU to initialize.  The second
condition lets the VCPU thread wait for the machine to fully initialize before
running.

An added benefit of this patch is it makes the dependencies now explicit.

Signed-off-by: Anthony Liguori <ali...@us...>

diff --git a/qemu/qemu-kvm.c b/qemu/qemu-kvm.c
index 78127de..9a9bf59 100644
--- a/qemu/qemu-kvm.c
+++ b/qemu/qemu-kvm.c
@@ -32,8 +32,12 @@ static int qemu_kvm_reset_requested;
 
 pthread_mutex_t qemu_mutex = PTHREAD_MUTEX_INITIALIZER;
 pthread_cond_t qemu_aio_cond = PTHREAD_COND_INITIALIZER;
+pthread_cond_t qemu_vcpu_cond = PTHREAD_COND_INITIALIZER;
+pthread_cond_t qemu_system_cond = PTHREAD_COND_INITIALIZER;
 __thread struct vcpu_info *vcpu;
 
+static int qemu_system_ready;
+
 struct qemu_kvm_signal_table {
     sigset_t sigset;
     sigset_t negsigset;
@@ -53,6 +57,7 @@ struct vcpu_info {
     int stop;
     int stopped;
     int reload_regs;
+    int created;
 } vcpu_info[256];
 
 pthread_t io_thread;
@@ -324,6 +329,7 @@ static int kvm_main_loop_cpu(CPUState *env)
     struct vcpu_info *info = &vcpu_info[env->cpu_index];
 
     setup_kernel_sigmask(env);
+
     pthread_mutex_lock(&qemu_mutex);
     if (kvm_irqchip_in_kernel(kvm_context))
 	env->hflags &= ~HF_HALTED_MASK;
@@ -370,6 +376,17 @@ static void *ap_main_loop(void *_env)
     sigprocmask(SIG_BLOCK, &signals, NULL);
     kvm_create_vcpu(kvm_context, env->cpu_index);
     kvm_qemu_init_env(env);
+
+    /* signal VCPU creation */
+    pthread_mutex_lock(&qemu_mutex);
+    vcpu->created = 1;
+    pthread_cond_signal(&qemu_vcpu_cond);
+
+    /* and wait for machine initialization */
+    while (!qemu_system_ready)
+	pthread_cond_wait(&qemu_system_cond, &qemu_mutex);
+    pthread_mutex_unlock(&qemu_mutex);
+
     kvm_main_loop_cpu(env);
     return NULL;
 }
@@ -389,8 +406,9 @@ static void kvm_add_signal(struct qemu_kvm_signal_table *sigtab, int signum)
 void kvm_init_new_ap(int cpu, CPUState *env)
 {
     pthread_create(&vcpu_info[cpu].thread, NULL, ap_main_loop, env);
-    /* FIXME: wait for thread to spin up */
-    usleep(200);
+
+    while (vcpu_info[cpu].created == 0)
+	pthread_cond_wait(&qemu_vcpu_cond, &qemu_mutex);
 }
 
 static void qemu_kvm_init_signal_tables(void)
@@ -435,7 +453,11 @@ void qemu_kvm_notify_work(void)
 int kvm_main_loop(void)
 {
     io_thread = pthread_self();
+    qemu_system_ready = 1;
     pthread_mutex_unlock(&qemu_mutex);
+
+    pthread_cond_broadcast(&qemu_system_cond);
+
     while (1) {
         kvm_eat_signal(&io_signal_table, NULL, 1000);
         pthread_mutex_lock(&qemu_mutex);