[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC 19/48] translate-all: notify plugin code of tb_flu
From: |
Alex Bennée |
Subject: |
Re: [Qemu-devel] [RFC 19/48] translate-all: notify plugin code of tb_flush |
Date: |
Mon, 26 Nov 2018 11:02:24 +0000 |
User-agent: |
mu4e 1.1.0; emacs 26.1.90 |
Emilio G. Cota <address@hidden> writes:
> 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.
You are right, I was thrown by the mmap_lock/unlocks - but I guess even
they aren't needed now if tb flush is always in an exclusive context.
>
>> 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.
Would they need a generation count propagated here or would it just be
flush anything translation related at this point?
>> 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.
OK that makes sense. Could you expand the commit message just to explain
the use case?
Reviewed-by: Alex Bennée <address@hidden>
--
Alex Bennée