Re: [Qemu-devel] [PATCH v2 45/45] tcg: enable multiple TCG contexts in softmmu

[Emilio G. Cota]

On Mon, Jul 17, 2017 at 19:25:14 -1000, Richard Henderson wrote:
> On 07/16/2017 10:04 AM, Emilio G. Cota wrote:
> >+
> >+    /* claim the first free pointer in tcg_ctxs and increment n_tcg_ctxs */
> >+    for (i = 0; i < smp_cpus; i++) {
> >+        if (atomic_cmpxchg(&tcg_ctxs[i], NULL, s) == NULL) {
> >+            unsigned int n;
> >+
> >+            n = atomic_fetch_inc(&n_tcg_ctxs);
> 
> Surely this is too much effort.  The increment on n_tcg_ctxs is sufficient
> to produce an index for assignment.  We never free the contexts...
> 
> Which also suggests that it might be better to avoid an indirection in
> tcg_ctxs and allocate all of the structures in one big block?  I.e.
> 
> TCGContext *tcg_ctxs;
> 
> // At the end of tcg_context_init.
> #ifdef CONFIG_USER_ONLY
>     tcg_ctxs = s;
> #else
>     // No need to zero; we'll completely overwrite each structure
>     // during tcg_register_thread.
>     tcg_ctxs = g_new(TCGContext, smp_cpus);
> #endif

The primary reason is that at this point we don't know yet whether
we'll need smp_cpus contexts or just one context (!mttcg), since
qemu_tcg_mttcg_enabled() is set up after this function is called
(the path is: vl.c -> configure_accelerator() -> tcg_init_context ->
this function; quite a bit later, qemu_tcg_configure() is
called, setting up the bool behind qemu_tcg_mttcg_enabled().)

A secondary reason is to avoid false sharing of cachelines. But
it seems quite unlikely that the last cacheline of TCGContext
will ever be used, so this isn't really an issue.

                E.