Re: [PATCH 1/4] cpu: free cpu->tb_jmp_cache with RCU

[Emilio Cota]

On Sun, Jan 08, 2023 at 11:19:53 -0800, Richard Henderson wrote:
> On 1/8/23 08:39, Emilio Cota wrote:
(snip)
> > diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> > index 356fe348de..ca95d21528 100644
> > --- a/accel/tcg/cpu-exec.c
> > +++ b/accel/tcg/cpu-exec.c
> > @@ -1070,7 +1070,6 @@ void tcg_exec_unrealizefn(CPUState *cpu)
> >   #endif /* !CONFIG_USER_ONLY */
> >       tlb_destroy(cpu);
> > -    g_free(cpu->tb_jmp_cache);
> 
> Can you simply use g_free_rcu here?

Yes, although we must have removed the CPU from the RCU list
before doing so.

> > diff --git a/cpu.c b/cpu.c
> > index 4a7d865427..564200559f 100644
> > --- a/cpu.c
> > +++ b/cpu.c
> > @@ -164,6 +164,12 @@ void cpu_exec_realizefn(CPUState *cpu, Error **errp)
> >   #endif /* CONFIG_USER_ONLY */
> >   }
> > +static void cpu_free_rcu(CPUState *cpu)
> > +{
> > +    /* .tb_jmp_cache is NULL except under TCG */
> > +    g_free(cpu->tb_jmp_cache);
> > +}
> > +
> >   void cpu_exec_unrealizefn(CPUState *cpu)
> >   {
> >   #ifndef CONFIG_USER_ONLY
> > @@ -181,6 +187,7 @@ void cpu_exec_unrealizefn(CPUState *cpu)
> >       }
> >       cpu_list_remove(cpu);
> > +    call_rcu(cpu, cpu_free_rcu, rcu);
> 
> Certainly this seems wrong, exposing tb_jmp_cache beyond tcg.

I've changed this in v2 to call tcg_exec_unrealizefn after
cpu_list_remove.

An alternative would be to call the whole cpu_exec_unrealizefn
after an RCU grace period, but I think that might be more trouble
than it's worth.

Thanks,
                Emilio