From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44567) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z8Wgb-0007Ln-CG for qemu-devel@nongnu.org; Fri, 26 Jun 2015 12:41:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z8WgW-0007JE-Ay for qemu-devel@nongnu.org; Fri, 26 Jun 2015 12:41:05 -0400 Received: from greensocs.com ([193.104.36.180]:37295) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z8WgV-0007JA-QH for qemu-devel@nongnu.org; Fri, 26 Jun 2015 12:41:00 -0400 Message-ID: <558D8098.8070008@greensocs.com> Date: Fri, 26 Jun 2015 18:40:56 +0200 From: Frederic Konrad MIME-Version: 1.0 References: <1435330053-18733-1-git-send-email-fred.konrad@greensocs.com> <1435330053-18733-15-git-send-email-fred.konrad@greensocs.com> <558D7BC6.4050300@redhat.com> In-Reply-To: <558D7BC6.4050300@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH V6 14/18] add a callback when tb_invalidate is called. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , qemu-devel@nongnu.org, mttcg@greensocs.com Cc: peter.maydell@linaro.org, a.spyridakis@virtualopensystems.com, mark.burton@greensocs.com, agraf@suse.de, alistair.francis@xilinx.com, guillaume.delbergue@greensocs.com, alex.bennee@linaro.org On 26/06/2015 18:20, Paolo Bonzini wrote: > > On 26/06/2015 16:47, fred.konrad@greensocs.com wrote: >> From: KONRAD Frederic >> >> Instead of doing the jump cache invalidation directly in tb_invalidate delay it >> after the exit so we don't have an other CPU trying to execute the code being >> invalidated. >> >> Signed-off-by: KONRAD Frederic >> --- >> translate-all.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-- >> 1 file changed, 59 insertions(+), 2 deletions(-) >> >> diff --git a/translate-all.c b/translate-all.c >> index ade2269..468648d 100644 >> --- a/translate-all.c >> +++ b/translate-all.c >> @@ -61,6 +61,7 @@ >> #include "translate-all.h" >> #include "qemu/bitmap.h" >> #include "qemu/timer.h" >> +#include "sysemu/cpus.h" >> >> //#define DEBUG_TB_INVALIDATE >> //#define DEBUG_FLUSH >> @@ -966,14 +967,58 @@ static inline void tb_reset_jump(TranslationBlock *tb, int n) >> tb_set_jmp_target(tb, n, (uintptr_t)(tb->tc_ptr + tb->tb_next_offset[n])); >> } >> >> +struct CPUDiscardTBParams { >> + CPUState *cpu; >> + TranslationBlock *tb; >> +}; >> + >> +static void cpu_discard_tb_from_jmp_cache(void *opaque) >> +{ >> + unsigned int h; >> + struct CPUDiscardTBParams *params = opaque; >> + >> + h = tb_jmp_cache_hash_func(params->tb->pc); >> + if (params->cpu->tb_jmp_cache[h] == params->tb) { >> + params->cpu->tb_jmp_cache[h] = NULL; >> + } > It is a bit more tricky, but I think you can avoid async_run_on_cpu by > doing this: > > 1) introduce a QemuSeqLock in TBContext, e.g. invalidate_seqlock. > > 2) wrap this "if" with seqlock_write_lock/unlock > > 3) in cpu-exec.c do this: > > /* we add the TB in the virtual pc hash table */ > + idx = seqlock_read_begin(&tcg_ctx.tb_ctx.invalidate_seqlock); > cpu->tb_jmp_cache[tb_jmp_cache_hash_func(pc)] = tb; > + if (seqlock_read_retry(&tcg_ctx.tb_ctx.invalidate_seqlock)) { > + /* Another CPU invalidated a tb in the meanwhile. We do not > + * know if it's this one, but play it safe and avoid caching > + * it. > + */ > + cpu->tb_jmp_cache[tb_jmp_cache_hash_func(pc)] = NULL; > + } > >> + /* suppress this TB from the two jump lists */ >> + tb_jmp_remove(tb, 0); >> + tb_jmp_remove(tb, 1); > If you do the above synchronously, this part doesn't need to be deferred > either. > > Then, immediately after the two tb_jmp_remove calls you can also check > whether "(tb->jmp_first & 3) == 2": if so, the expensive expensive > async_run_safe_work_on_cpu can be skipped. > > Paolo Ok seems tricky :) I'll take a look at this. Fred >> +#endif /* MTTCG */ >> >> tcg_ctx.tb_ctx.tb_phys_invalidate_count++; >> tb_unlock(); >>