Re: [Qemu-devel] [RFC PATCH V6 08/18] cpu: remove exit_request global.

[Frederic Konrad]

On 07/07/2015 15:04, Alex Bennée wrote:
fred.kon...@greensocs.com writes:
From: KONRAD Frederic <fred.kon...@greensocs.com>

This removes exit_request global and adds a variable in CPUState for this.
Only the flag for the first cpu is used for the moment as we are still with one
TCG thread.

Signed-off-by: KONRAD Frederic <fred.kon...@greensocs.com>
---
  cpu-exec.c | 15 ---------------
  cpus.c     | 17 ++++++++++++++---
  2 files changed, 14 insertions(+), 18 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index 5d9b518..0644383 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -364,8 +364,6 @@ static void cpu_handle_debug_exception(CPUArchState *env)
  
  /* main execution loop */
  
-volatile sig_atomic_t exit_request;
-
  int cpu_exec(CPUArchState *env)
  {
      CPUState *cpu = ENV_GET_CPU(env);
@@ -394,20 +392,8 @@ int cpu_exec(CPUArchState *env)
  
      current_cpu = cpu;
  
-    /* As long as current_cpu is null, up to the assignment just above,
-     * requests by other threads to exit the execution loop are expected to
-     * be issued using the exit_request global. We must make sure that our
-     * evaluation of the global value is performed past the current_cpu
-     * value transition point, which requires a memory barrier as well as
-     * an instruction scheduling constraint on modern architectures.  */
-    smp_mb();
-
      rcu_read_lock();
  
-    if (unlikely(exit_request)) {
-        cpu->exit_request = 1;
-    }
-
      cc->cpu_exec_enter(cpu);
  
      /* Calculate difference between guest clock and host clock.
@@ -496,7 +482,6 @@ int cpu_exec(CPUArchState *env)
                      }
                  }
                  if (unlikely(cpu->exit_request)) {
-                    cpu->exit_request = 0;
                      cpu->exception_index = EXCP_INTERRUPT;
                      cpu_loop_exit(cpu);
                  }
diff --git a/cpus.c b/cpus.c
index 23c316c..2541c56 100644
--- a/cpus.c
+++ b/cpus.c
@@ -137,6 +137,8 @@ typedef struct TimersState {
  } TimersState;
  
  static TimersState timers_state;
+/* CPU associated to this thread. */
+static __thread CPUState *tcg_thread_cpu;
  
  int64_t cpu_get_icount_raw(void)
  {
@@ -661,12 +663,18 @@ static void cpu_handle_guest_debug(CPUState *cpu)
      cpu->stopped = true;
  }
  
+/**
+ * cpu_signal
+ * Signal handler when using TCG.
+ */
  static void cpu_signal(int sig)
  {
      if (current_cpu) {
          cpu_exit(current_cpu);
      }
-    exit_request = 1;
+
+    /* FIXME: We might want to check if the cpu is running? */
+    tcg_thread_cpu->exit_request = true;
I guess the potential problem is race conditions here? What happens if
the cpu is signalled by two different threads for two different reasons?

Hmmm yes, I need to take a look at that and check all the reason why it 
should exit.

But maybe it's OK, the first time the cpu get the signal it will set 
exit_request..
and the second times as well?

  }
  
  #ifdef CONFIG_LINUX
@@ -1031,6 +1039,7 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
  {
      CPUState *cpu = arg;
  
+    tcg_thread_cpu = cpu;
      qemu_tcg_init_cpu_signals();
      qemu_thread_get_self(cpu->thread);
  
@@ -1393,7 +1402,8 @@ static void tcg_exec_all(void)
      if (next_cpu == NULL) {
          next_cpu = first_cpu;
      }
-    for (; next_cpu != NULL && !exit_request; next_cpu = CPU_NEXT(next_cpu)) {
+    for (; next_cpu != NULL && !first_cpu->exit_request;
+           next_cpu = CPU_NEXT(next_cpu)) {
          CPUState *cpu = next_cpu;
          CPUArchState *env = cpu->env_ptr;
  
@@ -1410,7 +1420,8 @@ static void tcg_exec_all(void)
              break;
          }
      }
-    exit_request = 0;
+
+    first_cpu->exit_request = 0;
  }
  
  void list_cpus(FILE *f, fprintf_function cpu_fprintf, const char *optarg)