* Peter Xu (pet...@redhat.com) wrote:
> On Wed, Dec 08, 2021 at 07:46:20PM +0000, Dr. David Alan Gilbert wrote:
> > * Peter Xu (pet...@redhat.com) wrote:
> > > The enablement of postcopy listening has a few steps, add a few
> > > tracepoints to
> > > be there ready for some basic measurements for them.
> > >
> > > Signed-off-by: Peter Xu <pet...@redhat.com>
> > > ---
> > > migration/savevm.c | 5 ++++-
> > > migration/trace-events | 2 +-
> > > 2 files changed, 5 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/migration/savevm.c b/migration/savevm.c
> > > index 17b8e25e00..5b3f31eab2 100644
> > > --- a/migration/savevm.c
> > > +++ b/migration/savevm.c
> > > @@ -1946,7 +1946,7 @@ static void *postcopy_ram_listen_thread(void
> > > *opaque)
> > > static int loadvm_postcopy_handle_listen(MigrationIncomingState *mis)
> > > {
> > > PostcopyState ps = postcopy_state_set(POSTCOPY_INCOMING_LISTENING);
> > > - trace_loadvm_postcopy_handle_listen();
> > > + trace_loadvm_postcopy_handle_listen(1);
> >
> > I think we tend just to split this into separate traces in many places;
> > or if we're using the same one then we should use a string
> >
> > I'd make this:
> > trace_loadvm_postcopy_handle_listen_entry();
> >
> > for example.
> >
> > > Error *local_err = NULL;
> > >
> > > if (ps != POSTCOPY_INCOMING_ADVISE && ps !=
> > > POSTCOPY_INCOMING_DISCARD) {
> > > @@ -1962,6 +1962,7 @@ static int
> > > loadvm_postcopy_handle_listen(MigrationIncomingState *mis)
> > > postcopy_ram_prepare_discard(mis);
> > > }
> > > }
> > > + trace_loadvm_postcopy_handle_listen(2);
> > >
> > > /*
> > > * Sensitise RAM - can now generate requests for blocks that don't
> > > exist
> > > @@ -1974,6 +1975,7 @@ static int
> > > loadvm_postcopy_handle_listen(MigrationIncomingState *mis)
> > > return -1;
> > > }
> > > }
> > > + trace_loadvm_postcopy_handle_listen(3);
> > >
> > > if (postcopy_notify(POSTCOPY_NOTIFY_INBOUND_LISTEN, &local_err)) {
> > > error_report_err(local_err);
> > > @@ -1988,6 +1990,7 @@ static int
> > > loadvm_postcopy_handle_listen(MigrationIncomingState *mis)
> > > QEMU_THREAD_DETACHED);
> > > qemu_sem_wait(&mis->listen_thread_sem);
> > > qemu_sem_destroy(&mis->listen_thread_sem);
> > > + trace_loadvm_postcopy_handle_listen(4);
> >
> > trace_loadvm_postcopy_handle_listen_entry_end();
>
> I see, I can use it. It's just that I want to trap more than entry/exit.
>
> For the "4 steps" here I split it into four procedures, the 2 steps inside are
> majorly to trap either uffd registration time or external uffd handling of
> other processes.
>
> One example:
>
> We may want to know how slow is
> postcopy_notify(POSTCOPY_NOTIFY_INBOUND_LISTEN)
> when there're some external process attached. I wanted to be prepared for
> that
> so when there's need to evaluate slowness of this procedure with vhost-user
> enabled we have some tracepoints without replacing the binaries.
>
> It's easy to use strings too if that's better looking than numbers. How
> about:
>
> trace_loadvm_postcopy_handle_listen("entry")
> trace_loadvm_postcopy_handle_listen("uffd-reg")
> trace_loadvm_postcopy_handle_listen("external")
> trace_loadvm_postcopy_handle_listen("exit")
>
Yes, that's fine; but it would also be fine to create 4 separate traces.
Dave
> ?
>
> Thanks,
>
> --
> Peter Xu
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
