| Message ID | 20230706170537.95959-2-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | accel/tcg: Fix race condition in tb create/invalidate | expand |
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
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>
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 --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);
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(-)