Re: [Qemu-devel] [RFC PATCH 1/3] tcg: Release tb_lock in the order acquired

[Pranith Kumar]

Hi Alex,

Alex Bennée writes:

>
> Do you have any numbers for this? The main reason being we are trying to
> avoid bouncing the lock too much and while this is cleaner it could
> cause more contention.

I did not really consider performance while cleaning this up. However, I
looked closer and I think we can remove the tb lock acquisition while adding
the jump by using atomics. I've attached the patch below. This should remove
any concern for a negative performance impact.

I will include this patch if you think it's okay.

Thanks,

diff --git a/cpu-exec.c b/cpu-exec.c
index 13cb15de0e..93debe64b6 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -365,9 +365,7 @@ static inline TranslationBlock *tb_find(CPUState *cpu,
     /* See if we can patch the calling TB. */
     if (last_tb && !qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) {
         if (!tb->invalid) {
-            tb_lock();
             tb_add_jump(last_tb, tb_exit, tb);
-            tb_unlock();
         }
     }
     return tb;
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 84a3240df6..60597cb07e 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -336,7 +336,7 @@ static inline void tb_set_jmp_target(TranslationBlock *tb,
 static inline void tb_add_jump(TranslationBlock *tb, int n,
                                TranslationBlock *tb_next)
 {
-    if (tb->jmp_list_next[n]) {
+    if (atomic_cmpxchg(&tb->jmp_list_next[n], 0, tb_next->jmp_list_first)) {
         /* Another thread has already done this while we were
          * outside of the lock; nothing to do in this case */
         return;
@@ -351,7 +351,6 @@ static inline void tb_add_jump(TranslationBlock *tb, int n,
     tb_set_jmp_target(tb, n, (uintptr_t)tb_next->tc_ptr);
 
     /* add in TB jmp circular list */
-    tb->jmp_list_next[n] = tb_next->jmp_list_first;
     tb_next->jmp_list_first = (uintptr_t)tb | n;
 }
 

-- 
Pranith