|
From: | Richard Henderson |
Subject: | Re: [Qemu-devel] [PATCH 20/22] tcg: dynamically allocate from code_gen_buffer using equally-sized regions |
Date: | Sun, 9 Jul 2017 11:03:45 -1000 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 |
On 07/08/2017 09:50 PM, Emilio G. Cota wrote:
+static void code_gen_set_region_size(TCGContext *s) +{ + size_t per_cpu = s->code_gen_buffer_size / smp_cpus; + size_t div; + + assert(per_cpu); + /* + * Use a single region if all we have is one vCPU. + * We could also use a single region with !mttcg, but at this time we have + * not yet processed the thread=single|multi flag. + */ + if (smp_cpus == 1) { + tcg_region_set_size(0); + return; + } + + for (div = 8; div > 0; div--) { + size_t region_size = per_cpu / div; + + if (region_size >= 2 * MIN_CODE_GEN_BUFFER_SIZE) { + tcg_region_set_size(region_size); + return; + } + } + tcg_region_set_size(per_cpu); +}
Is it worth setting a guard page after each of these regions? The guard page on the main buffer has caught bugs before (although not in a while now). If not, then we might drop the guard page on the main buffer too.
+static void tcg_region_set_size__locked(size_t size) +{ + if (!size) { + region.size = tcg_init_ctx->code_gen_buffer_size; + region.n = 1; + } else { + region.size = size; + region.n = tcg_init_ctx->code_gen_buffer_size / size; + } + if (unlikely(region.size < TCG_HIGHWATER)) { + tcg_abort(); + } +} + +/* + * Call this function at init time (i.e. only once). Calling this function is + * optional: if no region size is set, a single region will be used. + * + * Note: calling this function *after* calling tcg_region_init() is a bug. + */ +void tcg_region_set_size(size_t size) +{ + tcg_debug_assert(!region.size); + + qemu_mutex_lock(&tcg_lock); + tcg_region_set_size__locked(size); + qemu_mutex_unlock(&tcg_lock); +}
If this is called during init, then why does it need a lock? Surely this is before we spawn threads.
if (unlikely(next > s->code_gen_highwater)) { - return NULL; + if (!tcg_region_alloc(s)) { + return NULL; + } + goto retry; }
Nit: positive logic is almost always clearer: if (region_alloc) goto retry; r~
[Prev in Thread] | Current Thread | [Next in Thread] |