Re: [PULL 15/20] include/hw/core: Create struct CPUJumpCache

[Richard Henderson <richard.henderson@linaro.org>]

On 10/28/22 00:44, Ilya Leoshkevich wrote:
> Putting CPUJumpCache inside CPUState made problem go away:
> 
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index 18ca701b443..3ea528566c3 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -32,6 +32,7 @@
>   #include "qemu/thread.h"
>   #include "qemu/plugin.h"
>   #include "qom/object.h"
> +#include "accel/tcg/tb-jmp-cache.h"
>   
>   typedef int (*WriteCoreDumpFunction)(const void *buf, size_t size,
>                                        void *opaque);
> @@ -366,7 +367,7 @@ struct CPUState {
>       CPUArchState *env_ptr;
>       IcountDecr *icount_decr_ptr;
>   
> -    CPUJumpCache *tb_jmp_cache;
> +    CPUJumpCache tb_jmp_cache;

Yes, well.  That structure is quite large (128kB?) and I had been hoping to (1) save that 
extra memory for e.g. KVM and (2) hide the tcg-specific stuff from core.

But clearly something went wrong during some threadedness with your test case.


>   void tcg_flush_jmp_cache(CPUState *cpu)
>   {
> -    CPUJumpCache *jc = cpu->tb_jmp_cache;
>   
> -    if (likely(jc)) {
> -        for (int i = 0; i < TB_JMP_CACHE_SIZE; i++) {
> -            qatomic_set(&jc->array[i].tb, NULL);
> -        }
> -    } else {
> -        /* This should happen once during realize, and thus never race. */
> -        jc = g_new0(CPUJumpCache, 1);
> -        jc = qatomic_xchg(&cpu->tb_jmp_cache, jc);
> -        assert(jc == NULL);
>       }
>   }
>   
> So there must be a race in tcg_flush_jmp_cache() after all?

If there had been a race here, we would abort with the assert.
It must be something else...


r~