diff mbox series

[1/2] accel/tcg: Split out cpu_exec_longjmp_cleanup

Message ID 20230706170537.95959-2-richard.henderson@linaro.org
State Superseded
Headers show
Series accel/tcg: Fix race condition in tb create/invalidate | expand

Commit Message

Richard Henderson July 6, 2023, 5:05 p.m. UTC
Share the setjmp cleanup between cpu_exec_step_atomic
and cpu_exec_setjmp.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 accel/tcg/cpu-exec.c | 43 +++++++++++++++++++------------------------
 1 file changed, 19 insertions(+), 24 deletions(-)

Comments

Richard W.M. Jones July 6, 2023, 5:46 p.m. UTC | #1
On Thu, Jul 06, 2023 at 06:05:36PM +0100, Richard Henderson wrote:
> Share the setjmp cleanup between cpu_exec_step_atomic
> and cpu_exec_setjmp.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Richard W.M. Jones <rjones@redhat.com>

(I'm still testing the other one, but already up to 600 iterations)

Rich.

> ---
>  accel/tcg/cpu-exec.c | 43 +++++++++++++++++++------------------------
>  1 file changed, 19 insertions(+), 24 deletions(-)
> 
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index ba1890a373..31aa320513 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -526,6 +526,23 @@ static void cpu_exec_exit(CPUState *cpu)
>      }
>  }
>  
> +static void cpu_exec_longjmp_cleanup(CPUState *cpu)
> +{
> +    /* Non-buggy compilers preserve this; assert the correct value. */
> +    g_assert(cpu == current_cpu);
> +
> +#ifdef CONFIG_USER_ONLY
> +    clear_helper_retaddr();
> +    if (have_mmap_lock()) {
> +        mmap_unlock();
> +    }
> +#endif
> +    if (qemu_mutex_iothread_locked()) {
> +        qemu_mutex_unlock_iothread();
> +    }
> +    assert_no_pages_locked();
> +}
> +
>  void cpu_exec_step_atomic(CPUState *cpu)
>  {
>      CPUArchState *env = cpu->env_ptr;
> @@ -568,16 +585,7 @@ void cpu_exec_step_atomic(CPUState *cpu)
>          cpu_tb_exec(cpu, tb, &tb_exit);
>          cpu_exec_exit(cpu);
>      } else {
> -#ifdef CONFIG_USER_ONLY
> -        clear_helper_retaddr();
> -        if (have_mmap_lock()) {
> -            mmap_unlock();
> -        }
> -#endif
> -        if (qemu_mutex_iothread_locked()) {
> -            qemu_mutex_unlock_iothread();
> -        }
> -        assert_no_pages_locked();
> +        cpu_exec_longjmp_cleanup(cpu);
>      }
>  
>      /*
> @@ -1023,20 +1031,7 @@ static int cpu_exec_setjmp(CPUState *cpu, SyncClocks *sc)
>  {
>      /* Prepare setjmp context for exception handling. */
>      if (unlikely(sigsetjmp(cpu->jmp_env, 0) != 0)) {
> -        /* Non-buggy compilers preserve this; assert the correct value. */
> -        g_assert(cpu == current_cpu);
> -
> -#ifdef CONFIG_USER_ONLY
> -        clear_helper_retaddr();
> -        if (have_mmap_lock()) {
> -            mmap_unlock();
> -        }
> -#endif
> -        if (qemu_mutex_iothread_locked()) {
> -            qemu_mutex_unlock_iothread();
> -        }
> -
> -        assert_no_pages_locked();
> +        cpu_exec_longjmp_cleanup(cpu);
>      }
>  
>      return cpu_exec_loop(cpu, sc);
> -- 
> 2.34.1
Philippe Mathieu-Daudé July 7, 2023, 8:09 a.m. UTC | #2
On 6/7/23 19:05, Richard Henderson wrote:
> Share the setjmp cleanup between cpu_exec_step_atomic
> and cpu_exec_setjmp.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   accel/tcg/cpu-exec.c | 43 +++++++++++++++++++------------------------
>   1 file changed, 19 insertions(+), 24 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Richard Henderson July 7, 2023, 10:37 a.m. UTC | #3
On 7/7/23 09:09, Philippe Mathieu-Daudé wrote:
> On 6/7/23 19:05, Richard Henderson wrote:
>> Share the setjmp cleanup between cpu_exec_step_atomic
>> and cpu_exec_setjmp.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   accel/tcg/cpu-exec.c | 43 +++++++++++++++++++------------------------
>>   1 file changed, 19 insertions(+), 24 deletions(-)
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 

Sorry, Phil, missed applying this this morning before sending v2.
I'll update the branch now.


r~
diff mbox series

Patch

diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index ba1890a373..31aa320513 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -526,6 +526,23 @@  static void cpu_exec_exit(CPUState *cpu)
     }
 }
 
+static void cpu_exec_longjmp_cleanup(CPUState *cpu)
+{
+    /* Non-buggy compilers preserve this; assert the correct value. */
+    g_assert(cpu == current_cpu);
+
+#ifdef CONFIG_USER_ONLY
+    clear_helper_retaddr();
+    if (have_mmap_lock()) {
+        mmap_unlock();
+    }
+#endif
+    if (qemu_mutex_iothread_locked()) {
+        qemu_mutex_unlock_iothread();
+    }
+    assert_no_pages_locked();
+}
+
 void cpu_exec_step_atomic(CPUState *cpu)
 {
     CPUArchState *env = cpu->env_ptr;
@@ -568,16 +585,7 @@  void cpu_exec_step_atomic(CPUState *cpu)
         cpu_tb_exec(cpu, tb, &tb_exit);
         cpu_exec_exit(cpu);
     } else {
-#ifdef CONFIG_USER_ONLY
-        clear_helper_retaddr();
-        if (have_mmap_lock()) {
-            mmap_unlock();
-        }
-#endif
-        if (qemu_mutex_iothread_locked()) {
-            qemu_mutex_unlock_iothread();
-        }
-        assert_no_pages_locked();
+        cpu_exec_longjmp_cleanup(cpu);
     }
 
     /*
@@ -1023,20 +1031,7 @@  static int cpu_exec_setjmp(CPUState *cpu, SyncClocks *sc)
 {
     /* Prepare setjmp context for exception handling. */
     if (unlikely(sigsetjmp(cpu->jmp_env, 0) != 0)) {
-        /* Non-buggy compilers preserve this; assert the correct value. */
-        g_assert(cpu == current_cpu);
-
-#ifdef CONFIG_USER_ONLY
-        clear_helper_retaddr();
-        if (have_mmap_lock()) {
-            mmap_unlock();
-        }
-#endif
-        if (qemu_mutex_iothread_locked()) {
-            qemu_mutex_unlock_iothread();
-        }
-
-        assert_no_pages_locked();
+        cpu_exec_longjmp_cleanup(cpu);
     }
 
     return cpu_exec_loop(cpu, sc);