[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC v1 01/11] tcg: move tb_find_fast outside the tb_lo
From: |
Alex Bennée |
Subject: |
Re: [Qemu-devel] [RFC v1 01/11] tcg: move tb_find_fast outside the tb_lock critical section |
Date: |
Tue, 22 Mar 2016 11:55:36 +0000 |
User-agent: |
mu4e 0.9.17; emacs 25.0.92.7 |
Emilio G. Cota <address@hidden> writes:
> On Fri, Mar 18, 2016 at 16:18:42 +0000, Alex Bennée wrote:
>> From: KONRAD Frederic <address@hidden>
>>
>> Signed-off-by: KONRAD Frederic <address@hidden>
>> Signed-off-by: Paolo Bonzini <address@hidden>
>> [AJB: minor checkpatch fixes]
>> Signed-off-by: Alex Bennée <address@hidden>
>>
>> ---
>> v1(ajb)
>> - checkpatch fixes
>> ---
>> diff --git a/cpu-exec.c b/cpu-exec.c
>> index 07545aa..52f25de 100644
>> --- a/cpu-exec.c
>> +++ b/cpu-exec.c
>> @@ -225,8 +225,9 @@ static TranslationBlock *tb_find_physical(CPUState *cpu,
>> phys_page1 = phys_pc & TARGET_PAGE_MASK;
>> h = tb_phys_hash_func(phys_pc);
>> for (ptb1 = &tcg_ctx.tb_ctx.tb_phys_hash[h];
>> - (tb = *ptb1) != NULL;
>> + (tb = atomic_read(ptb1)) != NULL;
>> ptb1 = &tb->phys_hash_next) {
>> + smp_read_barrier_depends();
>> if (tb->pc != pc ||
>> tb->page_addr[0] != phys_page1 ||
>> tb->cs_base != cs_base ||
>> @@ -254,7 +255,18 @@ static TranslationBlock *tb_find_physical(CPUState *cpu,
> [ Adding this missing line to the diff for clarity ]
I have to admit that clarity is one thing the code in this area could do
with. I find it hard to follow on the best of days.
> /* Move the TB to the head of the list */
>> *ptb1 = tb->phys_hash_next;
>> tb->phys_hash_next = tcg_ctx.tb_ctx.tb_phys_hash[h];
>> tcg_ctx.tb_ctx.tb_phys_hash[h] = tb;
>
> This function, as is, doesn't really just "find"; two concurrent "finders"
> could race here by *writing* to the head of the list at the same time.
>
> The fix is to get rid of this write entirely; moving the just-found TB to
> the head of the list is not really that necessary thanks to the CPU's
> tb_jmp_cache table. This fix would make the function read-only, which
> is what the function's name implies.
>
> Further, I'd like to see tb_phys_hash to use the RCU queue primitives; it
> makes everything easier to understand (and we avoid sprinkling the code
> base with smp_barrier_depends).
>
> I have these two changes queued up as part of my upcoming series, which I'm
> basing on your patchset.
Cool, I look forward to it.
>
> Thanks for putting these changes together!
This was exactly my aim, getting the common base stuff reviewed so the
competing plurality of approaches can build on it as we shake out the
design ;-)
>
> Emilio
--
Alex Bennée
- Re: [Qemu-devel] [RFC v1 03/11] tcg: comment on which functions have to be called with tb_lock held, (continued)
[Qemu-devel] [RFC v1 04/11] tcg: protect TBContext with tb_lock., Alex Bennée, 2016/03/18
[Qemu-devel] [RFC v1 08/11] tcg: add kick timer for single-threaded vCPU emulation, Alex Bennée, 2016/03/18
[Qemu-devel] [RFC v1 07/11] tcg: add options for enabling MTTCG, Alex Bennée, 2016/03/18
[Qemu-devel] [RFC v1 10/11] tcg: grab iothread lock in cpu-exec interrupt handling, Alex Bennée, 2016/03/18
[Qemu-devel] [RFC v1 11/11] tcg: enable thread-per-vCPU, Alex Bennée, 2016/03/18
[Qemu-devel] [RFC v1 09/11] tcg: drop global lock during TCG code execution, Alex Bennée, 2016/03/18