Re: [PATCH 2/4] perf tools: Add perf_event__synthesize_{start,stop}()
From: Namhyung Kim
Date: Tue Sep 20 2022 - 14:17:34 EST
On Tue, Sep 20, 2022 at 6:50 AM Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote:
>
> On 16/09/22 20:59, Namhyung Kim wrote:
> > These functions are to prepare and cleanup necessary work for synthesizing.
> > It doesn't do anything yet but later patch will add it.
> >
> > Signed-off-by: Namhyung Kim <namhyung@xxxxxxxxxx>
> > ---
> > tools/perf/builtin-inject.c | 3 +++
> > tools/perf/builtin-record.c | 3 +++
> > tools/perf/builtin-stat.c | 2 ++
> > tools/perf/builtin-top.c | 4 ++++
> > tools/perf/util/auxtrace.c | 2 ++
> > tools/perf/util/synthetic-events.c | 8 ++++++++
> > tools/perf/util/synthetic-events.h | 3 +++
> > 7 files changed, 25 insertions(+)
> >
> > diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> > index e254f18986f7..2e91a887919b 100644
> > --- a/tools/perf/builtin-inject.c
> > +++ b/tools/perf/builtin-inject.c
> > @@ -2368,9 +2368,12 @@ int cmd_inject(int argc, const char **argv)
> > if (ret < 0)
> > goto out_delete;
> >
> > + perf_event__synthesize_start();
> > +
> > ret = __cmd_inject(&inject);
> >
> > guest_session__exit(&inject.guest_session);
> > + perf_event__synthesize_stop();
>
> AFAICT perf inject synthesizes mmap events only for JIT and that is
> open-coded in jitdump.c. i.e. perf_event__synthesize_start / stop
> not needed
Right, it's not strictly necessary. While perf inject -b can synthesize
some build-id events but it also doesn't require this even with the
patch 3.
Originally I thought this might be a good place to add something
we want to do for synthesis. But I only need it for DSO build-IDs
in MMAP2 and we can remove it from other places.
>
> >
> > out_delete:
> > strlist__delete(inject.known_build_ids);
> > diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> > index 02e38f50a138..5b7b9ad2a280 100644
> > --- a/tools/perf/builtin-record.c
> > +++ b/tools/perf/builtin-record.c
> > @@ -1966,6 +1966,8 @@ static int record__synthesize(struct record *rec, bool tail)
> > if (rec->opts.tail_synthesize != tail)
> > return 0;
> >
> > + perf_event__synthesize_start();
>
> Perhaps also record__synthesize_workload() ?
Oh, right. I thought it's handled during exec but it seems
switch-output things need it additionally. Will add.
>
> > +
> > if (data->is_pipe) {
> > err = perf_event__synthesize_for_pipe(tool, session, data,
> > process_synthesized_event);
> > @@ -2072,6 +2074,7 @@ static int record__synthesize(struct record *rec, bool tail)
> > }
> >
> > out:
> > + perf_event__synthesize_stop();
> > return err;
> > }
> >
> > diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> > index e05fe72c1d87..f6f61e08f4c2 100644
> > --- a/tools/perf/builtin-stat.c
> > +++ b/tools/perf/builtin-stat.c
> > @@ -962,6 +962,7 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
> > if (err < 0)
> > return err;
> >
> > + perf_event__synthesize_start();
> > err = perf_event__synthesize_stat_events(&stat_config, NULL, evsel_list,
> > process_synthesized_event, is_pipe);
> > if (err < 0)
> > @@ -2641,6 +2642,7 @@ int cmd_stat(int argc, const char **argv)
> > perf_session__write_header(perf_stat.session, evsel_list, fd, true);
> > }
> >
> > + perf_event__synthesize_stop();
> > evlist__close(evsel_list);
> > perf_session__delete(perf_stat.session);
> > }
> > diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> > index e89208b4ad4b..1eff894e6b5f 100644
> > --- a/tools/perf/builtin-top.c
> > +++ b/tools/perf/builtin-top.c
> > @@ -1258,6 +1258,8 @@ static int __cmd_top(struct perf_top *top)
> > #endif
> > }
> >
> > + perf_event__synthesize_start();
> > +
> > ret = perf_event__synthesize_bpf_events(top->session, perf_event__process,
> > &top->session->machines.host,
> > &top->record_opts);
> > @@ -1273,6 +1275,8 @@ static int __cmd_top(struct perf_top *top)
> > top->evlist->core.threads, true, false,
> > top->nr_threads_synthesize);
> >
> > + perf_event__synthesize_stop();
> > +
> > if (top->nr_threads_synthesize > 1)
> > perf_set_singlethreaded();
> >
> > diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
> > index b59c278fe9ed..1bfe076c22fb 100644
> > --- a/tools/perf/util/auxtrace.c
> > +++ b/tools/perf/util/auxtrace.c
> > @@ -1328,6 +1328,7 @@ int perf_event__process_auxtrace_info(struct perf_session *session,
> > if (err)
> > return err;
> >
> > + perf_event__synthesize_start();
> > unleader_auxtrace(session);
> >
> > return 0;
> > @@ -2834,6 +2835,7 @@ void auxtrace__free(struct perf_session *session)
> > if (!session->auxtrace)
> > return;
> >
> > + perf_event__synthesize_stop();
>
> auxtrace does not synthesize mmap events
Yeah, I don't have a strong need for it. I just added it in case
we need something for synthesis later. But I will remove it
if you don't think it's worth it.
Thanks,
Namhyung
>
> > return session->auxtrace->free(session);
> > }
> >
> > diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
> > index 0ff57ca24577..9d4f5dacd154 100644
> > --- a/tools/perf/util/synthetic-events.c
> > +++ b/tools/perf/util/synthetic-events.c
> > @@ -47,6 +47,14 @@
> >
> > unsigned int proc_map_timeout = DEFAULT_PROC_MAP_PARSE_TIMEOUT;
> >
> > +void perf_event__synthesize_start(void)
> > +{
> > +}
> > +
> > +void perf_event__synthesize_stop(void)
> > +{
> > +}
> > +
> > int perf_tool__process_synth_event(struct perf_tool *tool,
> > union perf_event *event,
> > struct machine *machine,
> > diff --git a/tools/perf/util/synthetic-events.h b/tools/perf/util/synthetic-events.h
> > index 53737d1619a4..e4414616080c 100644
> > --- a/tools/perf/util/synthetic-events.h
> > +++ b/tools/perf/util/synthetic-events.h
> > @@ -43,6 +43,9 @@ int parse_synth_opt(char *str);
> > typedef int (*perf_event__handler_t)(struct perf_tool *tool, union perf_event *event,
> > struct perf_sample *sample, struct machine *machine);
> >
> > +void perf_event__synthesize_start(void);
> > +void perf_event__synthesize_stop(void);
> > +
> > int perf_event__synthesize_attrs(struct perf_tool *tool, struct evlist *evlist, perf_event__handler_t process);
> > int perf_event__synthesize_attr(struct perf_tool *tool, struct perf_event_attr *attr, u32 ids, u64 *id, perf_event__handler_t process);
> > int perf_event__synthesize_build_id(struct perf_tool *tool, struct dso *pos, u16 misc, perf_event__handler_t process, struct machine *machine);
>