On Mon, Sep 19, 2016 at 14:50:58 +0200, Paolo Bonzini wrote:
> From: Sergey Fedorov <[email protected]>
>
> Use async_safe_run_on_cpu() to make tb_flush() thread safe. This is
> possible now that code generation does not happen in the middle of
> execution.
>
> It can happen that multiple threads schedule a safe work to flush the
> translation buffer. To keep statistics and debugging output sane, always
> check if the translation buffer has already been flushed.
>
> Signed-off-by: Sergey Fedorov <[email protected]>
> Signed-off-by: Sergey Fedorov <[email protected]>
> [AJB: minor re-base fixes]
> Signed-off-by: Alex Bennée <[email protected]>
> Message-Id: <[email protected]>
> Signed-off-by: Paolo Bonzini <[email protected]>
> ---
(snip)
> @@ -38,7 +38,7 @@ struct TBContext {
> QemuMutex tb_lock;
>
> /* statistics */
> - int tb_flush_count;
> + unsigned tb_flush_count;
(snip)
> /* flush all the translation blocks */
> -/* XXX: tb_flush is currently not thread safe */
> -void tb_flush(CPUState *cpu)
> +static void do_tb_flush(CPUState *cpu, void *data)
> {
> - if (!tcg_enabled()) {
> - return;
> + unsigned tb_flush_req = (unsigned) (uintptr_t) data;
> +
> + tb_lock();
> +
> + /* If it's already been done on request of another CPU,
> + * just retry.
> + */
> + if (atomic_read(&tcg_ctx.tb_ctx.tb_flush_count) != tb_flush_req) {
> + goto done;
tb_flush_count is always accessed with tb_lock held, right? If so, I don't
see a reason to access it with atomic_read/set.
(snip)
> @@ -1773,7 +1790,8 @@ void dump_exec_info(FILE *f, fprintf_function
> cpu_fprintf)
> qht_statistics_destroy(&hst);
>
> cpu_fprintf(f, "\nStatistics:\n");
> - cpu_fprintf(f, "TB flush count %d\n",
> tcg_ctx.tb_ctx.tb_flush_count);
> + cpu_fprintf(f, "TB flush count %d\n",
> + atomic_read(&tcg_ctx.tb_ctx.tb_flush_count));
s/%d/%u/ would be more appropriate given the type change.
E.