Re: [PATCH 05/14] perf tools: Add parse_events_fake interface

Arnaldo Carvalho de Melo
2020-06-01T15:04:33Z

Em Mon, Jun 01, 2020 at 11:08:50AM +0200, Jiri Olsa escreveu:
> On Mon, Jun 01, 2020 at 12:28:31AM -0700, Ian Rogers wrote:
> > On Sun, May 24, 2020 at 3:42 PM Jiri Olsa <jolsa@kernel.org> wrote:
> > >
> > > Adding parse_events_fake interface to parse events
> > > and use fake pmu event in case pmu event is parsed.
> > >
> > > This way it's possible to parse events from PMUs
> > > which are not present in the system. It's available
> > > only for testing purposes coming in following
> > > changes.
> > >
> > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> >
> > Acked-by: Ian Rogers <irogers@google.com>
> >
> > Alternatively fake_pmu could be an argument to parse_events.
>
> yea I checked that and I was surprised how many parse_events calls
> we have in perf, so I went this way.. but I haven't really tried it,
> so it might look actually etter despite the many places we need to change,
> I'll try

Thanks! My admitedly unchecked thinking is that most places will just
pass NULL, only the test case will pass it, right?

- Arnaldo

> thanks,
> jirka
>
> >
> > Thanks,
> > Ian
> >
> > > ---
> > > tools/perf/util/parse-events.c | 48 +++++++++++++++++++++++++---------
> > > tools/perf/util/parse-events.h | 2 ++
> > > 2 files changed, 37 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> > > index 8304f9b6e6be..89239695a728 100644
> > > --- a/tools/perf/util/parse-events.c
> > > +++ b/tools/perf/util/parse-events.c
> > > @@ -2082,22 +2082,15 @@ int parse_events_terms(struct list_head *terms, const char *str)
> > > return ret;
> > > }
> > >
> > > -int parse_events(struct evlist *evlist, const char *str,
> > > - struct parse_events_error *err)
> > > +static int parse_events_state(struct parse_events_state *parse_state,
> > > + struct evlist *evlist, const char *str)
> > > {
> > > - struct parse_events_state parse_state = {
> > > - .list = LIST_HEAD_INIT(parse_state.list),
> > > - .idx = evlist->core.nr_entries,
> > > - .error = err,
> > > - .evlist = evlist,
> > > - .stoken = PE_START_EVENTS,
> > > - };
> > > int ret;
> > >
> > > - ret = parse_events__scanner(str, &parse_state);
> > > + ret = parse_events__scanner(str, parse_state);
> > > perf_pmu__parse_cleanup();
> > >
> > > - if (!ret && list_empty(&parse_state.list)) {
> > > + if (!ret && list_empty(&parse_state->list)) {
> > > WARN_ONCE(true, "WARNING: event parser found nothing\n");
> > > return -1;
> > > }
> > > @@ -2105,12 +2098,12 @@ int parse_events(struct evlist *evlist, const char *str,
> > > /*
> > > * Add list to the evlist even with errors to allow callers to clean up.
> > > */
> > > - perf_evlist__splice_list_tail(evlist, &parse_state.list);
> > > + perf_evlist__splice_list_tail(evlist, &parse_state->list);
> > >
> > > if (!ret) {
> > > struct evsel *last;
> > >
> > > - evlist->nr_groups += parse_state.nr_groups;
> > > + evlist->nr_groups += parse_state->nr_groups;
> > > last = evlist__last(evlist);
> > > last->cmdline_group_boundary = true;
> > >
> > > @@ -2125,6 +2118,35 @@ int parse_events(struct evlist *evlist, const char *str,
> > > return ret;
> > > }
> > >
> > > +int parse_events(struct evlist *evlist, const char *str,
> > > + struct parse_events_error *err)
> > > +{
> > > + struct parse_events_state parse_state = {
> > > + .list = LIST_HEAD_INIT(parse_state.list),
> > > + .idx = evlist->core.nr_entries,
> > > + .error = err,
> > > + .evlist = evlist,
> > > + .stoken = PE_START_EVENTS,
> > > + };
> > > +
> > > + return parse_events_state(&parse_state, evlist, str);
> > > +}
> > > +
> > > +int parse_events_fake(struct evlist *evlist, const char *str,
> > > + struct parse_events_error *err)
> > > +{
> > > + struct parse_events_state parse_state = {
> > > + .list = LIST_HEAD_INIT(parse_state.list),
> > > + .idx = evlist->core.nr_entries,
> > > + .error = err,
> > > + .evlist = evlist,
> > > + .stoken = PE_START_EVENTS,
> > > + .fake_pmu = true,
> > > + };
> > > +
> > > + return parse_events_state(&parse_state, evlist, str);
> > > +}
> > > +
> > > #define MAX_WIDTH 1000
> > > static int get_term_width(void)
> > > {
> > > diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
> > > index 963b0ea6c448..4a23b6cd9924 100644
> > > --- a/tools/perf/util/parse-events.h
> > > +++ b/tools/perf/util/parse-events.h
> > > @@ -34,6 +34,8 @@ int parse_events_option(const struct option *opt, const char *str, int unset);
> > > int parse_events_option_new_evlist(const struct option *opt, const char *str, int unset);
> > > int parse_events(struct evlist *evlist, const char *str,
> > > struct parse_events_error *error);
> > > +int parse_events_fake(struct evlist *evlist, const char *str,
> > > + struct parse_events_error *error);
> > > int parse_events_terms(struct list_head *terms, const char *str);
> > > int parse_filter(const struct option *opt, const char *str, int unset);
> > > int exclude_perf(const struct option *opt, const char *arg, int unset);
> > > --
> > > 2.25.4
> > >
> >
>

--

- Arnaldo