diff mbox

Re: [PATCH] fix smp with tcg mode and --enable-io-thread

Message ID 4C1FE40C.3070506@web.de
State New
Headers show

Commit Message

Jan Kiszka June 21, 2010, 10:13 p.m. UTC 
Jan Kiszka wrote:
> And there is some race that cause a lock up in qemu_mutex_lock_iothread
> after a while (the cpu_unlink_tb seems to race with the linking - just a
> guess so far).

This seems to fix a long-standing race between cpu_exec and
signal-driven cpu_unlink_tb:


Still testing, though.

Jan

Comments

Alexander Graf June 21, 2010, 10:25 p.m. UTC | #1 
On 22.06.2010, at 00:13, Jan Kiszka wrote:

> Jan Kiszka wrote:
>> And there is some race that cause a lock up in qemu_mutex_lock_iothread
>> after a while (the cpu_unlink_tb seems to race with the linking - just a
>> guess so far).
> 
> This seems to fix a long-standing race between cpu_exec and
> signal-driven cpu_unlink_tb:
> 
> diff --git a/cpu-exec.c b/cpu-exec.c
> index 026980a..bfc34e4 100644
> --- a/cpu-exec.c
> +++ b/cpu-exec.c
> @@ -599,8 +598,9 @@ int cpu_exec(CPUState *env1)
>                    TB, but before it is linked into a potentially
>                    infinite loop and becomes env->current_tb. Avoid
>                    starting execution if there is a pending interrupt. */
> -                if (!unlikely (env->exit_request)) {
> -                    env->current_tb = tb;
> +                env->current_tb = tb;
> +                asm("");

This is just barrier(), no?


Alex
Jan Kiszka June 22, 2010, 7:59 a.m. UTC | #2 
Alexander Graf wrote:
> On 22.06.2010, at 00:13, Jan Kiszka wrote:
> 
>> Jan Kiszka wrote:
>>> And there is some race that cause a lock up in qemu_mutex_lock_iothread
>>> after a while (the cpu_unlink_tb seems to race with the linking - just a
>>> guess so far).
>> This seems to fix a long-standing race between cpu_exec and
>> signal-driven cpu_unlink_tb:
>>
>> diff --git a/cpu-exec.c b/cpu-exec.c
>> index 026980a..bfc34e4 100644
>> --- a/cpu-exec.c
>> +++ b/cpu-exec.c
>> @@ -599,8 +598,9 @@ int cpu_exec(CPUState *env1)
>>                    TB, but before it is linked into a potentially
>>                    infinite loop and becomes env->current_tb. Avoid
>>                    starting execution if there is a pending interrupt. */
>> -                if (!unlikely (env->exit_request)) {
>> -                    env->current_tb = tb;
>> +                env->current_tb = tb;
>> +                asm("");
> 
> This is just barrier(), no?
> 

Yes, thoughtlessly copied from other places in cpu-exec.c. Guess it's
time to define this properly in qemu-barriers.h, also clobbering "memory".

Jan
diff mbox

Patch

diff --git a/cpu-exec.c b/cpu-exec.c
index 026980a..bfc34e4 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -599,8 +598,9 @@  int cpu_exec(CPUState *env1)
                    TB, but before it is linked into a potentially
                    infinite loop and becomes env->current_tb. Avoid
                    starting execution if there is a pending interrupt. */
-                if (!unlikely (env->exit_request)) {
-                    env->current_tb = tb;
+                env->current_tb = tb;
+                asm("");
+                if (likely(!env->exit_request)) {
                     tc_ptr = tb->tc_ptr;
                 /* execute the generated code */
 #if defined(__sparc__) && !defined(CONFIG_SOLARIS)
@@ -609,7 +609,6 @@  int cpu_exec(CPUState *env1)
 #define env cpu_single_env
 #endif
                     next_tb = tcg_qemu_tb_exec(tc_ptr);
-                    env->current_tb = NULL;
                     if ((next_tb & 3) == 2) {
                         /* Instruction counter expired.  */
                         int insns_left;
@@ -638,6 +637,7 @@  int cpu_exec(CPUState *env1)
                         }
                     }
                 }
+                env->current_tb = NULL;
                 /* reset soft MMU for next block (it can currently
                    only be set by a memory fault) */
             } /* for(;;) */