On 3/23/22 10:17, Philippe Mathieu-Daudé wrote:
From: Philippe Mathieu-Daudé <f4...@amsat.org>

All accelerators implement a very similar create_vcpu_thread()
handler. Extract the common part as common_vcpu_thread_create(),
which only requires the AccelOpsClass::vcpu_thread_fn() routine
and the accelerator AccelOpsClass::thread_name for debugging
purpose.

The single exception is TCG/RR which re-use a single vCPU. Have
it use the same logic by using the .precheck/.postcheck handlers.

Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org>

Too much all at once.
But I see now why you moved code in the direction you did in patch 5.

@@ -279,28 +279,13 @@ bool rr_create_vcpu_thread_precheck(CPUState *cpu)
      return !single_tcg_cpu_thread;
  }
-void rr_start_vcpu_thread(CPUState *cpu)
+void rr_create_vcpu_thread_postcheck(CPUState *cpu)
  {
-    char thread_name[VCPU_THREAD_NAME_SIZE];
      static QemuCond *single_tcg_halt_cond;
-    static QemuThread *single_tcg_cpu_thread;

Patch 8 is not really standalone, since you didn't move this variable.
I think it's probably a mistake to split out precheck and postcheck separately.

-
-    if (!single_tcg_cpu_thread) {
+    if (! single_tcg_cpu_thread) {

Extraneous whitespace.

@@ -30,7 +30,8 @@ struct AccelOpsClass {
bool (*cpus_are_resettable)(void); - void (*create_vcpu_thread)(CPUState *cpu); /* MANDATORY NON-NULL */
+    const char *thread_name;

Why don't we just get this name from the AccelClass?

+static void common_vcpu_thread_create(CPUState *cpu)
+{
+    char thread_name[VCPU_THREAD_NAME_SIZE];
+
+    cpu->thread = g_new0(QemuThread, 1);
+    cpu->halt_cond = g_new0(QemuCond, 1);
+    qemu_cond_init(cpu->halt_cond);
+    snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/%s",
+             cpu->cpu_index, cpus_accel->thread_name ?: "DUMMY");

Surely g_strdup_printf is better than the static size buffer.


r~

Reply via email to