Re: [Qemu-devel] [RFC 19/48] translate-all: notify plugin code of tb_flush

[Emilio G. Cota]

On Fri, Nov 23, 2018 at 17:00:59 +0000, Alex Bennée wrote:
> 
> Emilio G. Cota <address@hidden> writes:
> 
> > Signed-off-by: Emilio G. Cota <address@hidden>
> > ---
> >  accel/tcg/translate-all.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
> > index 3423cf74db..1690e3fd5b 100644
> > --- a/accel/tcg/translate-all.c
> > +++ b/accel/tcg/translate-all.c
> > @@ -1233,6 +1233,8 @@ static gboolean tb_host_size_iter(gpointer key, 
> > gpointer value, gpointer data)
> >  /* flush all the translation blocks */
> >  void do_tb_flush(CPUState *cpu, run_on_cpu_data tb_flush_count)
> >  {
> > +    bool did_flush = false;
> > +
> >      mmap_lock();
> >      /* If it is already been done on request of another CPU,
> >       * just retry.
> > @@ -1240,6 +1242,7 @@ void do_tb_flush(CPUState *cpu, run_on_cpu_data 
> > tb_flush_count)
> >      if (tb_ctx.tb_flush_count != tb_flush_count.host_int) {
> >          goto done;
> >      }
> > +    did_flush = true;
> >
> >      if (DEBUG_TB_FLUSH_GATE) {
> >          size_t nb_tbs = tcg_nb_tbs();
> > @@ -1265,6 +1268,9 @@ void do_tb_flush(CPUState *cpu, run_on_cpu_data 
> > tb_flush_count)
> >
> >  done:
> >      mmap_unlock();
> > +    if (did_flush) {
> > +        qemu_plugin_flush_cb();
> > +    }
> 
> Are we introducing a race here?

A race, how? We're in an async safe environment here, i.e. no other
vCPU is running.

> What is the purpose of letting the plugin know a flush has occurred?

Plugins might allocate per-TB data that then they get passed each
time the TB is executed (via the *userdata pointer). For example,
in a simulator we'd allocate a per-TB struct that describes the
guest instructions, after having disassembled them at translate time.

It is therefore useful for plugins to know when all TB's have been
flushed, so that they can then free that per-TB data.

> It shouldn't have any knowledge of the details of liveliness of the
> translated code and if it still exits or not. If all it wants to do is
> look at the counts then I think we can provide a simpler less abuse-able
> way to do this.

I'm confused. What does "look at the counts" mean here?

To reiterate, plugins should have a way to know when a TB doesn't
exist any longer, so that they can reclaim memory.

Thanks,

                Emilio