qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Paolo Bonzini
Subject: Re: [Qemu-devel] [RFC PATCH 1/3] tcg: Release tb_lock in the order acquired
Date: Thu, 8 Dec 2016 18:52:27 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0


On 07/12/2016 16:38, Pranith Kumar wrote:
> 
> 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.

I don't like this, it's too tricky.  You'd have to prove that it's
correct and that it keeps the list consistent.  There's nothing wrong in
locking like

        lock(L1)
        lock(L2)
        unlock(L1)
        unlock(L2)

so I'm not sure I see the point of this patch.

Paolo

> 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;
>  }
>  
> 



reply via email to

[Prev in Thread] Current Thread [Next in Thread]