[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 09/22] exec-all: shrink tb->invalid to uint8_t
From: |
Richard Henderson |
Subject: |
Re: [Qemu-devel] [PATCH 09/22] exec-all: shrink tb->invalid to uint8_t |
Date: |
Wed, 12 Jul 2017 13:06:23 -1000 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 |
On 07/12/2017 10:48 AM, Emilio G. Cota wrote:
Would it be OK for this series to just start with CF_PARALLEL? I'm not
too familiar with how icount mode recompiles code, and I'm now on
patch 27 of v2 and still have quite a few patches to go through.
Certainly.
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 49c1ecf..2531b73 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -224,31 +224,27 @@ static void cpu_exec_nocache(CPUState *cpu, int
max_cycles,
static void cpu_exec_step(CPUState *cpu)
{
CPUClass *cc = CPU_GET_CLASS(cpu);
- CPUArchState *env = (CPUArchState *)cpu->env_ptr;
TranslationBlock *tb;
target_ulong cs_base, pc;
uint32_t flags;
- cpu_get_tb_cpu_state(env, &pc, &cs_base, &flags);
if (sigsetjmp(cpu->jmp_env, 0) == 0) {
- mmap_lock();
- tb_lock();
- tb = tb_gen_code(cpu, pc, cs_base, flags,
- 1 | CF_NOCACHE | CF_IGNORE_ICOUNT);
- tb->orig_tb = NULL;
- tb_unlock();
- mmap_unlock();
+ tb = tb_lookup__cpu_state(cpu, &pc, &cs_base, &flags);
+ if (tb == NULL) {
+ mmap_lock();
+ tb_lock();
+ tb = tb_gen_code(cpu, pc, cs_base, flags,
+ 1 | CF_IGNORE_ICOUNT);
You've got a problem here in that you're not including CF_COUNT_MASK in the
hash and you dropped the flush when changing to parallel_cpus = true. That
means you could find an old TB with CF_COUNT > 1.
Not required for this patch set, but what I'd like to see eventually is
(1) cpu_exec_step merged into cpu_exec_step_atomic for clarity.
(2) callers of tb_gen_code add in CF_PARALLEL as needed; do not
pick it up from parallel_cpus within tb_gen_code.
(3) target/*/translate.c uses CF_PARALLEL instead of parallel_cpus.
(4) cpu_exec_step_atomic does the tb lookup and code gen outside
of the start_exclusive/end_exclusive lock.
And to that end I think there are some slightly different choices you can make
now in order to reduce churn for that later.
@@ -320,11 +318,12 @@ static TranslationBlock *tb_htable_lookup(CPUState *cpu,
target_ulong pc,
desc.env = (CPUArchState *)cpu->env_ptr;
desc.cs_base = cs_base;
desc.flags = flags;
+ desc.cf_mask = curr_cf_mask();
desc.trace_vcpu_dstate = *cpu->trace_dstate;
desc.pc = pc;
phys_pc = get_page_addr_code(desc.env, pc);
desc.phys_page1 = phys_pc & TARGET_PAGE_MASK;
- h = tb_hash_func(phys_pc, pc, flags, *cpu->trace_dstate);
+ h = tb_hash_func(phys_pc, pc, flags, curr_cf_mask(), *cpu->trace_dstate);
return qht_lookup(&tcg_ctx.tb_ctx.htable, tb_cmp, &desc, h);
}
E.g. this fundamental lookup function should have cf_mask passed in.
@@ -1254,6 +1256,9 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
if (use_icount && !(cflags & CF_IGNORE_ICOUNT)) {
cflags |= CF_USE_ICOUNT;
}
+ if (parallel_cpus) {
+ cflags |= CF_PARALLEL;
+ }
E.g. pass this in. Callers using curr_cf_mask() should suffice where it's not
obvious.
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 925ae11..fa40f6c 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -6312,11 +6312,10 @@ static int do_fork(CPUArchState *env, unsigned int
flags, abi_ulong newsp,
sigprocmask(SIG_BLOCK, &sigmask, &info.sigmask);
/* If this is our first additional thread, we need to ensure we
- * generate code for parallel execution and flush old translations.
+ * generate code for parallel execution.
*/
if (!parallel_cpus) {
parallel_cpus = true;
- tb_flush(cpu);
As per above, I think you must retain this for now.
I strongly suspect that it will be worthwhile forever, since we're pretty much
guaranteed that none of the existing TBs will ever be used again.
r~
- Re: [Qemu-devel] [PATCH 04/22] tcg: fix corruption of code_time profiling counter upon tb_flush, (continued)
- [Qemu-devel] [PATCH 14/22] tcg: take .helpers out of TCGContext, Emilio G. Cota, 2017/07/09
- [Qemu-devel] [PATCH 09/22] exec-all: shrink tb->invalid to uint8_t, Emilio G. Cota, 2017/07/09
- Re: [Qemu-devel] [PATCH 09/22] exec-all: shrink tb->invalid to uint8_t, Richard Henderson, 2017/07/09
- Re: [Qemu-devel] [PATCH 09/22] exec-all: shrink tb->invalid to uint8_t, Emilio G. Cota, 2017/07/10
- Re: [Qemu-devel] [PATCH 09/22] exec-all: shrink tb->invalid to uint8_t, Richard Henderson, 2017/07/11
- Re: [Qemu-devel] [PATCH 09/22] exec-all: shrink tb->invalid to uint8_t, Emilio G. Cota, 2017/07/12
- Re: [Qemu-devel] [PATCH 09/22] exec-all: shrink tb->invalid to uint8_t,
Richard Henderson <=
- Re: [Qemu-devel] [PATCH 09/22] exec-all: shrink tb->invalid to uint8_t, Emilio G. Cota, 2017/07/15
- Re: [Qemu-devel] [PATCH 09/22] exec-all: shrink tb->invalid to uint8_t, Richard Henderson, 2017/07/16
[Qemu-devel] [PATCH 12/22] translate-all: report correct avg host TB size, Emilio G. Cota, 2017/07/09
[Qemu-devel] [PATCH 16/22] tcg: keep a list of TCGContext's, Emilio G. Cota, 2017/07/09
[Qemu-devel] [PATCH 11/22] translate-all: use a binary search tree to track TBs in TBContext, Emilio G. Cota, 2017/07/09