Re: [Qemu-devel] [PATCH 15/16] tcg: Make tb_flush() thread safe

[Emilio G. Cota]

On Mon, Sep 19, 2016 at 14:50:58 +0200, Paolo Bonzini wrote:
> From: Sergey Fedorov <address@hidden>
> 
> 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 <address@hidden>
> Signed-off-by: Sergey Fedorov <address@hidden>
> [AJB: minor re-base fixes]
> Signed-off-by: Alex Bennée <address@hidden>
> Message-Id: <address@hidden>
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
(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.