Replace qemu_system_cond with VCPU stop mechanism

PATCH RESEND Superseded Jan Kiszka 3 participants 4 messages Aug 22, 2011

Share & tools

Jan Kiszka

author

2011-08-22 15:47 UTC #0 lore raw

PATCH RESEND 1/1 Replace qemu_system_cond with VCPU stop mechanism

We can express the VCPU thread wakeup with the stop mechanism, saving both qemu_system_ready and the qemu_system_cond. For KVM threads, we can just enter the main loop as long as the thread is stopped. The central TCG thread is better held back before the loop as there can be side effects of the services called even when all CPUs are stopped.

Creating VCPUs in stopped state will also be required for proper CPU hotplugging support.

Signed-off-by Jan Kiszka <[email protected]>

---
 cpus.c |   20 ++++++--------------
 1 files changed, 6 insertions(+), 14 deletions(-)

cpus.c +6 ~~-14~~

index b60410c..1df00b5 100644
@@ -644,11 +644,9 @@ static QemuThread io_thread;
 static QemuThread *tcg_cpu_thread;
 static QemuCond *tcg_halt_cond;
 
-static int qemu_system_ready;
 /* cpu creation */
 static QemuCond qemu_cpu_cond;
 /* system init */
-static QemuCond qemu_system_cond;
 static QemuCond qemu_pause_cond;
 static QemuCond qemu_work_cond;

@@ -670,7 +668,6 @@ int qemu_init_main_loop(void)
     }
 
     qemu_cond_init(&qemu_cpu_cond);
-    qemu_cond_init(&qemu_system_cond);
     qemu_cond_init(&qemu_pause_cond);
     qemu_cond_init(&qemu_work_cond);
     qemu_cond_init(&qemu_io_proceeded_cond);

@@ -684,8 +681,7 @@ int qemu_init_main_loop(void)
 
 void qemu_main_loop_start(void)
 {
-    qemu_system_ready = 1;
-    qemu_cond_broadcast(&qemu_system_cond);
+    resume_all_vcpus();
 }
 
 void run_on_cpu(CPUState *env, void (*func)(void *data), void *data)

@@ -796,11 +792,6 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
     env->created = 1;
     qemu_cond_signal(&qemu_cpu_cond);
 
-    /* and wait for machine initialization */
-    while (!qemu_system_ready) {
-        qemu_cond_wait(&qemu_system_cond, &qemu_global_mutex);
-    }
-
     while (1) {
         if (cpu_can_run(env)) {
             r = kvm_cpu_exec(env);

@@ -829,9 +820,9 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
     }
     qemu_cond_signal(&qemu_cpu_cond);
 
-    /* and wait for machine initialization */
-    while (!qemu_system_ready) {
-        qemu_cond_wait(&qemu_system_cond, &qemu_global_mutex);
+    /* wait for initial kick-off after machine start */
+    while (!first_cpu->stopped) {
+        qemu_cond_wait(tcg_halt_cond, &qemu_global_mutex);
     }
 
     while (1) {

@@ -971,12 +962,12 @@ static void qemu_tcg_init_vcpu(void *_env)
         env->thread = g_malloc0(sizeof(QemuThread));
         env->halt_cond = g_malloc0(sizeof(QemuCond));
         qemu_cond_init(env->halt_cond);
+        tcg_halt_cond = env->halt_cond;
         qemu_thread_create(env->thread, qemu_tcg_cpu_thread_fn, env);
         while (env->created == 0) {
             qemu_cond_wait(&qemu_cpu_cond, &qemu_global_mutex);
         }
         tcg_cpu_thread = env->thread;
-        tcg_halt_cond = env->halt_cond;
     } else {
         env->thread = tcg_cpu_thread;
         env->halt_cond = tcg_halt_cond;

@@ -1000,6 +991,7 @@ void qemu_init_vcpu(void *_env)
 
     env->nr_cores = smp_cores;
     env->nr_threads = smp_threads;
+    env->stopped = 1;
     if (kvm_enabled()) {
         qemu_kvm_start_vcpu(env);
     } else {

Signature · 1 line

1.7.3.4

Avi Kivity

2011-08-22 16:18 UTC #1 lore raw

Quoted 10 lines · Jan Kiszka

We can express the VCPU thread wakeup with the stop mechanism, saving both qemu_system_ready and the qemu_system_cond. For KVM threads, we can just enter the main loop as long as the thread is stopped. The central TCG thread is better held back before the loop as there can be side effects of the services called even when all CPUs are stopped.
Creating VCPUs in stopped state will also be required for proper CPU hotplugging support.

cpus.c static void *qemu_tcg_cpu_thread_fn(void *arg) · line 826

Quoted 7 lines

cpus.c +3 -3

-    /* and wait for machine initialization */
-    while (!qemu_system_ready) {
-        qemu_cond_wait(&qemu_system_cond,&qemu_global_mutex);
+    /* wait for initial kick-off after machine start */
+    while (!first_cpu->stopped) {
+        qemu_cond_wait(tcg_halt_cond,&qemu_global_mutex);
      }

Seems inverted - do we want to wait until first_cpu is stopped, or while first_cpu is stopped?

Do we want to process run_on_cpu() events after the machine is created but before a cpu is started (if starting with -S)?

Signature · 1 line

error compiling committee.c: too many arguments to function

Jan Kiszka

author

2011-08-22 16:36 UTC #2 lore raw

PATCH v3 1/1 Replace qemu_system_cond with VCPU stop mechanism

Quoted 19 lines · Avi Kivity

Quoted 10 lines · Jan Kiszka
We can express the VCPU thread wakeup with the stop mechanism, saving both qemu_system_ready and the qemu_system_cond. For KVM threads, we can just enter the main loop as long as the thread is stopped. The central TCG thread is better held back before the loop as there can be side effects of the services called even when all CPUs are stopped.
Creating VCPUs in stopped state will also be required for proper CPU hotplugging support.
Review
Quoted 7 lines
(quoted diff) +3 -3
-    /* and wait for machine initialization */
-    while (!qemu_system_ready) {
-        qemu_cond_wait(&qemu_system_cond,&qemu_global_mutex);
+    /* wait for initial kick-off after machine start */
+    while (!first_cpu->stopped) {
+        qemu_cond_wait(tcg_halt_cond,&qemu_global_mutex);
      }

Quoted 2 lines

Seems inverted - do we want to wait until first_cpu is stopped, or while first_cpu is stopped?

Oops, fixed below.

Quoted 3 lines

Do we want to process run_on_cpu() events after the machine is created but before a cpu is started (if starting with -S)?

-S affects vm_running, not stopped states of individual VCPUs. So they will get kicked off on main_loop entry, but then stop again due to vm_running == 0.

Jan

-------8<--------

Creating VCPUs in stopped state will also be required for proper CPU hotplugging support.

Signed-off-by Jan Kiszka <[email protected]>

---
 cpus.c |   20 ++++++--------------
 1 files changed, 6 insertions(+), 14 deletions(-)

cpus.c +6 ~~-14~~

index b60410c..b163efe 100644
@@ -644,11 +644,9 @@ static QemuThread io_thread;
 static QemuThread *tcg_cpu_thread;
 static QemuCond *tcg_halt_cond;
 
-static int qemu_system_ready;
 /* cpu creation */
 static QemuCond qemu_cpu_cond;
 /* system init */
-static QemuCond qemu_system_cond;
 static QemuCond qemu_pause_cond;
 static QemuCond qemu_work_cond;

@@ -670,7 +668,6 @@ int qemu_init_main_loop(void)
     }
 
     qemu_cond_init(&qemu_cpu_cond);
-    qemu_cond_init(&qemu_system_cond);
     qemu_cond_init(&qemu_pause_cond);
     qemu_cond_init(&qemu_work_cond);
     qemu_cond_init(&qemu_io_proceeded_cond);

@@ -684,8 +681,7 @@ int qemu_init_main_loop(void)
 
 void qemu_main_loop_start(void)
 {
-    qemu_system_ready = 1;
-    qemu_cond_broadcast(&qemu_system_cond);
+    resume_all_vcpus();
 }
 
 void run_on_cpu(CPUState *env, void (*func)(void *data), void *data)

@@ -796,11 +792,6 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
     env->created = 1;
     qemu_cond_signal(&qemu_cpu_cond);
 
-    /* and wait for machine initialization */
-    while (!qemu_system_ready) {
-        qemu_cond_wait(&qemu_system_cond, &qemu_global_mutex);
-    }
-
     while (1) {
         if (cpu_can_run(env)) {
             r = kvm_cpu_exec(env);

@@ -829,9 +820,9 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
     }
     qemu_cond_signal(&qemu_cpu_cond);
 
-    /* and wait for machine initialization */
-    while (!qemu_system_ready) {
-        qemu_cond_wait(&qemu_system_cond, &qemu_global_mutex);
+    /* wait for initial kick-off after machine start */
+    while (first_cpu->stopped) {
+        qemu_cond_wait(tcg_halt_cond, &qemu_global_mutex);
     }
 
     while (1) {

@@ -971,12 +962,12 @@ static void qemu_tcg_init_vcpu(void *_env)
         env->thread = g_malloc0(sizeof(QemuThread));
         env->halt_cond = g_malloc0(sizeof(QemuCond));
         qemu_cond_init(env->halt_cond);
+        tcg_halt_cond = env->halt_cond;
         qemu_thread_create(env->thread, qemu_tcg_cpu_thread_fn, env);
         while (env->created == 0) {
             qemu_cond_wait(&qemu_cpu_cond, &qemu_global_mutex);
         }
         tcg_cpu_thread = env->thread;
-        tcg_halt_cond = env->halt_cond;
     } else {
         env->thread = tcg_cpu_thread;
         env->halt_cond = tcg_halt_cond;

@@ -1000,6 +991,7 @@ void qemu_init_vcpu(void *_env)
 
     env->nr_cores = smp_cores;
     env->nr_threads = smp_threads;
+    env->stopped = 1;
     if (kvm_enabled()) {
         qemu_kvm_start_vcpu(env);
     } else {

Signature · 1 line

1.7.3.4

Anthony Liguori

Applied

2011-08-22 20:19 UTC #3 lore raw

Quoted 43 lines · Jan Kiszka

Quoted 19 lines · Avi Kivity
Quoted 10 lines · Jan Kiszka
We can express the VCPU thread wakeup with the stop mechanism, saving both qemu_system_ready and the qemu_system_cond. For KVM threads, we can just enter the main loop as long as the thread is stopped. The central TCG thread is better held back before the loop as there can be side effects of the services called even when all CPUs are stopped.
Creating VCPUs in stopped state will also be required for proper CPU hotplugging support.
cpus.c static void *qemu_tcg_cpu_thread_fn(void *arg) · line 826
Quoted 7 lines
cpus.c +3 -3
-    /* and wait for machine initialization */
-    while (!qemu_system_ready) {
-        qemu_cond_wait(&qemu_system_cond,&qemu_global_mutex);
+    /* wait for initial kick-off after machine start */
+    while (!first_cpu->stopped) {
+        qemu_cond_wait(tcg_halt_cond,&qemu_global_mutex);
       }
Quoted 2 lines
Seems inverted - do we want to wait until first_cpu is stopped, or while first_cpu is stopped?
Oops, fixed below.
Quoted 3 lines
Do we want to process run_on_cpu() events after the machine is created but before a cpu is started (if starting with -S)?
-S affects vm_running, not stopped states of individual VCPUs. So they will get kicked off on main_loop entry, but then stop again due to vm_running == 0.
Jan
-------8<--------
We can express the VCPU thread wakeup with the stop mechanism, saving both qemu_system_ready and the qemu_system_cond. For KVM threads, we can just enter the main loop as long as the thread is stopped. The central TCG thread is better held back before the loop as there can be side effects of the services called even when all CPUs are stopped.

Quoted 4 lines

Creating VCPUs in stopped state will also be required for proper CPU hotplugging support.
Signed-off-by Jan Kiszka<[email protected]>

Applied. Thanks.

Regards,

Anthony Liguori