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

[Alex Bennée]

Emilio G. Cota <address@hidden> writes:

> On Fri, Jul 01, 2016 at 17:16:10 +0100, Alex Bennée wrote:
>> Lock contention in the hot path of moving between existing patched
>> TranslationBlocks is the main drag in multithreaded performance. This
>> patch pushes the tb_lock() usage down to the two places that really need
>> it:
>>
>>   - code generation (tb_gen_code)
>>   - jump patching (tb_add_jump)
>>
>> The rest of the code doesn't really need to hold a lock as it is either
>> using per-CPU structures, atomically updated or designed to be used in
>> concurrent read situations (qht_lookup).
>>
>> To keep things simple I removed the #ifdef CONFIG_USER_ONLY stuff as the
>> locks become NOPs anyway until the MTTCG work is completed.
>
> From a scalability point of view it would be better to have a single
> critical section.

You mean merge the critical region for patching and code-generation?

>
> From a correctness point of view, we're reading tb->page_addr[1]
> without holding a lock. This field is set after qht_insert(tb),
> so we might read a yet-uninitialized value.
>
> I propose to just extend the critical section, like we used to
> do with tcg_lock_reset.
>
>               Emilio


--
Alex Bennée