Re: [Qemu-devel] [RFC 26/38] cpu: protect tb_jmp_cache with seqlock

[Emilio G. Cota]

On Sun, Aug 23, 2015 at 18:14:58 -0700, Paolo Bonzini wrote:
> On 23/08/2015 17:23, Emilio G. Cota wrote:
> > This paves the way for a lockless tb_find_fast.
> > 
> > Signed-off-by: Emilio G. Cota <address@hidden>
> > ---
(snip)
> > @@ -1707,12 +1735,14 @@ void tb_flush_jmp_cache(CPUState *cpu, target_ulong 
> > addr)
> >      /* Discard jump cache entries for any tb which might potentially
> >         overlap the flushed page.  */
> >      i = tb_jmp_cache_hash_page(addr - TARGET_PAGE_SIZE);
> > +    seqlock_write_lock(&cpu->tb_jmp_cache_sequence);
> >      memset(&cpu->tb_jmp_cache[i], 0,
> >             TB_JMP_PAGE_SIZE * sizeof(TranslationBlock *));
> >  
> >      i = tb_jmp_cache_hash_page(addr);
> >      memset(&cpu->tb_jmp_cache[i], 0,
> >             TB_JMP_PAGE_SIZE * sizeof(TranslationBlock *));
> > +    seqlock_write_unlock(&cpu->tb_jmp_cache_sequence);
> >  }
> >  
> >  void dump_exec_info(FILE *f, fprintf_function cpu_fprintf)
> > 
> 
> I'm not sure how the last three patches compare with the existing "tcg:
> move tb_find_fast outside the tb_lock critical section"?

The seqlock for tb_jmp_cache is necessary the moment that the
array can be wiped out with a memset(), as shown above. That
function (tb_flush_jmp_cache) is called by tlb_flush_page,
which has many callers.

One could argue that we could enforce calling tlb_flush_page to be
a) always done by the owner thread or b) done while all others CPUs
are paused.

I argue that worrying about that is not worth it; let's protect
the array with a seqlock, which on TSO is essentially free, and
worry about more important things.

Wrt the next two patches:

Patch 27 is an improvement in that each TB has its own valid flag,
which makes sense because this should only affect TB's that are
trying to chain to/from it, not all TBs.

Patch 28 uses the RCU QLIST which to me seems cleaner and less
error-prone than open-coding an RCU LIST.

Thanks,

                Emilio