Re: [Qemu-devel] [PATCH 2/2] cpu-exec: remove tb_lock from the hot-path

[Paolo Bonzini]

On 05/07/2016 15:11, Alex Bennée wrote:
> 
> Paolo Bonzini <address@hidden> writes:
> 
>> On 05/07/2016 13:14, Alex Bennée wrote:
>>> /*
>>>  * Patch the last TB with a jump to the current TB.
>>>  *
>>>  * Modification of the TB has to be protected with tb_lock which can
>>>  * either be already held or taken here.
>>>  */
>>> static inline void maybe_patch_last_tb(CPUState *cpu,
>>>                                        TranslationBlock *tb,
>>>                                        TranslationBlock **last_tb,
>>>                                        int tb_exit,
>>>                                        bool locked)
>>> {
>>>     if (cpu->tb_flushed) {
>>>         /* Ensure that no TB jump will be modified as the
>>>          * translation buffer has been flushed.
>>>          */
>>>         *last_tb = NULL;
>>>         cpu->tb_flushed = false;
>>>     }
>>> #ifndef CONFIG_USER_ONLY
>>>     /* We don't take care of direct jumps when address mapping changes in
>>>      * system emulation. So it's not safe to make a direct jump to a TB
>>>      * spanning two pages because the mapping for the second page can 
>>> change.
>>>      */
>>>     if (tb->page_addr[1] != -1) {
>>>         *last_tb = NULL;
>>>     }
>>> #endif
>>>     /* See if we can patch the calling TB. */
>>>     if (*last_tb && !qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) {
>>>         if (!locked) {
>>>             tb_lock();
>>>         }
>>>         tb_add_jump(*last_tb, tb_exit, tb);
>>>         if (!locked) {
>>>             tb_unlock();
>>>         }
>>>     }
>>> }
>>
>> Why not add tb_lock_recursive() and tb_lock_reset()?
> 
> I thought we didn't like having recursive locking? I agree it would make
> things a little neater though.

I didn't like having recursive mutexes (because recursive mutexes
encourage you to be sloppy).  Explicitly tagging some tb_lock()s as
recursive is fine, though.  The explicit tag tells you to be careful.


Paolo