[Qemu-devel] [PATCH v1 0/3] cpus: single-threaded TCG CPU creation improvements

[Qemu-devel] [PATCH v1 0/3] cpus: single-threaded TCG CPU creation improvements

[David Hildenbrand]

While looking into CPU hotplug for s390x TCG, I noticed that
single-threaded TCG does not properly inititalize CPUs > 1. Fix this
and cleanup the code a bit.

David Hildenbrand (3):
  cpus: properly inititalize CPU > 1 under single-threaded TCG
  cpus: wait for CPU creation at central place
  cpus: CPU threads are always created initially for one CPU only

 cpus.c | 33 ++++++++++-----------------------
 1 file changed, 10 insertions(+), 23 deletions(-)

-- 
2.14.3

[Qemu-devel] [PATCH v1 1/3] cpus: properly inititalize CPU > 1 under single-threaded TCG

[David Hildenbrand]

All but the first CPU are currently not fully inititalized (e.g.
cpu->created is never set).

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 cpus.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/cpus.c b/cpus.c
index f298b659f4..ade1651032 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1863,6 +1863,9 @@ static void qemu_tcg_init_vcpu(CPUState *cpu)
         /* For non-MTTCG cases we share the thread */
         cpu->thread = single_tcg_cpu_thread;
         cpu->halt_cond = single_tcg_halt_cond;
+        cpu->thread_id = first_cpu->thread_id;
+        cpu->can_do_io = 1;
+        cpu->created = true;
     }
 }
 
-- 
2.14.3

[Qemu-devel] [PATCH v1 2/3] cpus: wait for CPU creation at central place

[David Hildenbrand]

We can now also wait for the CPU creation for single-threaded TCG, so we
can move the waiting bits further out.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 cpus.c | 22 ++++------------------
 1 file changed, 4 insertions(+), 18 deletions(-)

diff --git a/cpus.c b/cpus.c
index ade1651032..22ab15840f 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1856,9 +1856,6 @@ static void qemu_tcg_init_vcpu(CPUState *cpu)
 #ifdef _WIN32
         cpu->hThread = qemu_thread_get_handle(cpu->thread);
 #endif
-        while (!cpu->created) {
-            qemu_cond_wait(&qemu_cpu_cond, &qemu_global_mutex);
-        }
     } else {
         /* For non-MTTCG cases we share the thread */
         cpu->thread = single_tcg_cpu_thread;
@@ -1884,9 +1881,6 @@ static void qemu_hax_start_vcpu(CPUState *cpu)
 #ifdef _WIN32
     cpu->hThread = qemu_thread_get_handle(cpu->thread);
 #endif
-    while (!cpu->created) {
-        qemu_cond_wait(&qemu_cpu_cond, &qemu_global_mutex);
-    }
 }
 
 static void qemu_kvm_start_vcpu(CPUState *cpu)
@@ -1900,9 +1894,6 @@ static void qemu_kvm_start_vcpu(CPUState *cpu)
              cpu->cpu_index);
     qemu_thread_create(cpu->thread, thread_name, qemu_kvm_cpu_thread_fn,
                        cpu, QEMU_THREAD_JOINABLE);
-    while (!cpu->created) {
-        qemu_cond_wait(&qemu_cpu_cond, &qemu_global_mutex);
-    }
 }
 
 static void qemu_hvf_start_vcpu(CPUState *cpu)
@@ -1921,9 +1912,6 @@ static void qemu_hvf_start_vcpu(CPUState *cpu)
              cpu->cpu_index);
     qemu_thread_create(cpu->thread, thread_name, qemu_hvf_cpu_thread_fn,
                        cpu, QEMU_THREAD_JOINABLE);
-    while (!cpu->created) {
-        qemu_cond_wait(&qemu_cpu_cond, &qemu_global_mutex);
-    }
 }
 
 static void qemu_whpx_start_vcpu(CPUState *cpu)
@@ -1940,9 +1928,6 @@ static void qemu_whpx_start_vcpu(CPUState *cpu)
 #ifdef _WIN32
     cpu->hThread = qemu_thread_get_handle(cpu->thread);
 #endif
-    while (!cpu->created) {
-        qemu_cond_wait(&qemu_cpu_cond, &qemu_global_mutex);
-    }
 }
 
 static void qemu_dummy_start_vcpu(CPUState *cpu)
@@ -1956,9 +1941,6 @@ static void qemu_dummy_start_vcpu(CPUState *cpu)
              cpu->cpu_index);
     qemu_thread_create(cpu->thread, thread_name, qemu_dummy_cpu_thread_fn, cpu,
                        QEMU_THREAD_JOINABLE);
-    while (!cpu->created) {
-        qemu_cond_wait(&qemu_cpu_cond, &qemu_global_mutex);
-    }
 }
 
 void qemu_init_vcpu(CPUState *cpu)
@@ -1988,6 +1970,10 @@ void qemu_init_vcpu(CPUState *cpu)
     } else {
         qemu_dummy_start_vcpu(cpu);
     }
+
+    while (!cpu->created) {
+        qemu_cond_wait(&qemu_cpu_cond, &qemu_global_mutex);
+    }
 }
 
 void cpu_stop_current(void)
-- 
2.14.3

[Qemu-devel] [PATCH v1 3/3] cpus: CPU threads are always created initially for one CPU only

[David Hildenbrand]

It can never happen for single-threaded TCG that we have more than one
CPU in the list, while the first one has not been marked as "created".

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 cpus.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/cpus.c b/cpus.c
index 22ab15840f..2653a89e88 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1383,11 +1383,9 @@ static void *qemu_tcg_rr_cpu_thread_fn(void *arg)
     qemu_mutex_lock_iothread();
     qemu_thread_get_self(cpu->thread);
 
-    CPU_FOREACH(cpu) {
-        cpu->thread_id = qemu_get_thread_id();
-        cpu->created = true;
-        cpu->can_do_io = 1;
-    }
+    cpu->thread_id = qemu_get_thread_id();
+    cpu->created = true;
+    cpu->can_do_io = 1;
     qemu_cond_signal(&qemu_cpu_cond);
 
     /* wait for initial kick-off after machine start */
-- 
2.14.3

Re: [Qemu-devel] [PATCH v1 0/3] cpus: single-threaded TCG CPU creation improvements

[Richard Henderson]

On 02/09/2018 11:52 AM, David Hildenbrand wrote:
> While looking into CPU hotplug for s390x TCG, I noticed that
> single-threaded TCG does not properly inititalize CPUs > 1. Fix this
> and cleanup the code a bit.
> 
> David Hildenbrand (3):
>   cpus: properly inititalize CPU > 1 under single-threaded TCG
>   cpus: wait for CPU creation at central place
>   cpus: CPU threads are always created initially for one CPU only
> 
>  cpus.c | 33 ++++++++++-----------------------
>  1 file changed, 10 insertions(+), 23 deletions(-)
> 

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [Qemu-devel] [PATCH v1 0/3] cpus: single-threaded TCG CPU creation improvements

[Paolo Bonzini]

On 09/02/2018 20:52, David Hildenbrand wrote:
> While looking into CPU hotplug for s390x TCG, I noticed that
> single-threaded TCG does not properly inititalize CPUs > 1. Fix this
> and cleanup the code a bit.
> 
> David Hildenbrand (3):
>   cpus: properly inititalize CPU > 1 under single-threaded TCG
>   cpus: wait for CPU creation at central place
>   cpus: CPU threads are always created initially for one CPU only
> 
>  cpus.c | 33 ++++++++++-----------------------
>  1 file changed, 10 insertions(+), 23 deletions(-)
> 

Queued, thanks.

Paolo