From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Ian Rogers <irogers@google.com>
Cc: Dominique Martinet <asmadeus@codewreck.org>,
Jiri Olsa <jolsa@kernel.org>,
Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
Namhyung Kim <namhyung@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Adrian Hunter <adrian.hunter@intel.com>,
linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/3] perf parse-events: pass parse_state to add_tracepoint
Date: Thu, 9 May 2024 18:24:09 -0300 [thread overview]
Message-ID: <Zj0--YbYSm-s9vRh@x1> (raw)
In-Reply-To: <CAP-5=fUmeyd3BR7njJEDQ-=qkpvLPMoQO-7De+3mqLaSOoZZxw@mail.gmail.com>
On Wed, May 08, 2024 at 02:37:16PM -0700, Ian Rogers wrote:
> On Sun, May 5, 2024 at 4:14 AM Dominique Martinet
> <asmadeus@codewreck.org> wrote:
> >
> > The next patch will add another flag to parse_state that we will want to
> > pass to evsel__nwetp_idx(), so pass the whole parse_state all the way
> > down instead of giving only the index
>
> Nit: evsel__newtp_idx typo
> Fwiw, I think the idx value is possibly something to be done away
> with. We renumber the evsels here:
> https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/parse-events.c?h=perf-tools-next#n2077
>
> Reviewed-by: Ian Rogers <irogers@google.com>
Fixed the typo.
- Arnaldo
> Thanks,
> Ian
>
> > Originally-by: Jiri Olsa <jolsa@kernel.org>
> > Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
> > ---
> > tools/perf/util/parse-events.c | 31 ++++++++++++++++++-------------
> > tools/perf/util/parse-events.h | 3 ++-
> > tools/perf/util/parse-events.y | 2 +-
> > 3 files changed, 21 insertions(+), 15 deletions(-)
> >
> > diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> > index 6f8b0fa17689..6e8cba03f0ac 100644
> > --- a/tools/perf/util/parse-events.c
> > +++ b/tools/perf/util/parse-events.c
> > @@ -519,13 +519,14 @@ static void tracepoint_error(struct parse_events_error *e, int err,
> > parse_events_error__handle(e, column, strdup(str), strdup(help));
> > }
> >
> > -static int add_tracepoint(struct list_head *list, int *idx,
> > +static int add_tracepoint(struct parse_events_state *parse_state,
> > + struct list_head *list,
> > const char *sys_name, const char *evt_name,
> > struct parse_events_error *err,
> > struct parse_events_terms *head_config, void *loc_)
> > {
> > YYLTYPE *loc = loc_;
> > - struct evsel *evsel = evsel__newtp_idx(sys_name, evt_name, (*idx)++);
> > + struct evsel *evsel = evsel__newtp_idx(sys_name, evt_name, parse_state->idx++);
> >
> > if (IS_ERR(evsel)) {
> > tracepoint_error(err, PTR_ERR(evsel), sys_name, evt_name, loc->first_column);
> > @@ -544,7 +545,8 @@ static int add_tracepoint(struct list_head *list, int *idx,
> > return 0;
> > }
> >
> > -static int add_tracepoint_multi_event(struct list_head *list, int *idx,
> > +static int add_tracepoint_multi_event(struct parse_events_state *parse_state,
> > + struct list_head *list,
> > const char *sys_name, const char *evt_name,
> > struct parse_events_error *err,
> > struct parse_events_terms *head_config, YYLTYPE *loc)
> > @@ -578,7 +580,7 @@ static int add_tracepoint_multi_event(struct list_head *list, int *idx,
> >
> > found++;
> >
> > - ret = add_tracepoint(list, idx, sys_name, evt_ent->d_name,
> > + ret = add_tracepoint(parse_state, list, sys_name, evt_ent->d_name,
> > err, head_config, loc);
> > }
> >
> > @@ -592,19 +594,21 @@ static int add_tracepoint_multi_event(struct list_head *list, int *idx,
> > return ret;
> > }
> >
> > -static int add_tracepoint_event(struct list_head *list, int *idx,
> > +static int add_tracepoint_event(struct parse_events_state *parse_state,
> > + struct list_head *list,
> > const char *sys_name, const char *evt_name,
> > struct parse_events_error *err,
> > struct parse_events_terms *head_config, YYLTYPE *loc)
> > {
> > return strpbrk(evt_name, "*?") ?
> > - add_tracepoint_multi_event(list, idx, sys_name, evt_name,
> > + add_tracepoint_multi_event(parse_state, list, sys_name, evt_name,
> > err, head_config, loc) :
> > - add_tracepoint(list, idx, sys_name, evt_name,
> > + add_tracepoint(parse_state, list, sys_name, evt_name,
> > err, head_config, loc);
> > }
> >
> > -static int add_tracepoint_multi_sys(struct list_head *list, int *idx,
> > +static int add_tracepoint_multi_sys(struct parse_events_state *parse_state,
> > + struct list_head *list,
> > const char *sys_name, const char *evt_name,
> > struct parse_events_error *err,
> > struct parse_events_terms *head_config, YYLTYPE *loc)
> > @@ -630,7 +634,7 @@ static int add_tracepoint_multi_sys(struct list_head *list, int *idx,
> > if (!strglobmatch(events_ent->d_name, sys_name))
> > continue;
> >
> > - ret = add_tracepoint_event(list, idx, events_ent->d_name,
> > + ret = add_tracepoint_event(parse_state, list, events_ent->d_name,
> > evt_name, err, head_config, loc);
> > }
> >
> > @@ -1266,7 +1270,8 @@ static int get_config_chgs(struct perf_pmu *pmu, struct parse_events_terms *head
> > return 0;
> > }
> >
> > -int parse_events_add_tracepoint(struct list_head *list, int *idx,
> > +int parse_events_add_tracepoint(struct parse_events_state *parse_state,
> > + struct list_head *list,
> > const char *sys, const char *event,
> > struct parse_events_error *err,
> > struct parse_events_terms *head_config, void *loc_)
> > @@ -1282,14 +1287,14 @@ int parse_events_add_tracepoint(struct list_head *list, int *idx,
> > }
> >
> > if (strpbrk(sys, "*?"))
> > - return add_tracepoint_multi_sys(list, idx, sys, event,
> > + return add_tracepoint_multi_sys(parse_state, list, sys, event,
> > err, head_config, loc);
> > else
> > - return add_tracepoint_event(list, idx, sys, event,
> > + return add_tracepoint_event(parse_state, list, sys, event,
> > err, head_config, loc);
> > #else
> > + (void)parse_state;
> > (void)list;
> > - (void)idx;
> > (void)sys;
> > (void)event;
> > (void)head_config;
> > diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
> > index 809359e8544e..fd55a154ceff 100644
> > --- a/tools/perf/util/parse-events.h
> > +++ b/tools/perf/util/parse-events.h
> > @@ -189,7 +189,8 @@ int parse_events_terms__to_strbuf(const struct parse_events_terms *terms, struct
> > int parse_events__modifier_event(struct list_head *list, char *str, bool add);
> > int parse_events__modifier_group(struct list_head *list, char *event_mod);
> > int parse_events_name(struct list_head *list, const char *name);
> > -int parse_events_add_tracepoint(struct list_head *list, int *idx,
> > +int parse_events_add_tracepoint(struct parse_events_state *parse_state,
> > + struct list_head *list,
> > const char *sys, const char *event,
> > struct parse_events_error *error,
> > struct parse_events_terms *head_config, void *loc);
> > diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
> > index d70f5d84af92..0bab4263f8e3 100644
> > --- a/tools/perf/util/parse-events.y
> > +++ b/tools/perf/util/parse-events.y
> > @@ -537,7 +537,7 @@ tracepoint_name opt_event_config
> > if (!list)
> > YYNOMEM;
> >
> > - err = parse_events_add_tracepoint(list, &parse_state->idx, $1.sys, $1.event,
> > + err = parse_events_add_tracepoint(parse_state, list, $1.sys, $1.event,
> > error, $2, &@1);
> >
> > parse_events_terms__delete($2);
> >
> > --
> > 2.44.0
> >
next prev parent reply other threads:[~2024-05-09 21:24 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-05 11:13 [PATCH v2 0/3] perf probe: Allow names to start with digits Dominique Martinet
2024-05-05 11:13 ` [PATCH v2 1/3] perf parse-events: pass parse_state to add_tracepoint Dominique Martinet
2024-05-08 21:37 ` Ian Rogers
2024-05-08 23:03 ` Dominique Martinet
2024-05-09 21:24 ` Arnaldo Carvalho de Melo [this message]
2024-05-09 21:46 ` Arnaldo Carvalho de Melo
2024-05-09 21:59 ` Dominique Martinet
2024-05-05 11:13 ` [PATCH v2 2/3] perf parse-events: Add new 'fake_tp' parameter for tests Dominique Martinet
2024-05-08 21:42 ` Ian Rogers
2024-05-05 11:13 ` [PATCH v2 3/3] perf parse: Allow names to start with digits Dominique Martinet
2024-05-08 21:44 ` Ian Rogers
2024-05-08 22:47 ` [PATCH v2 0/3] perf probe: " Ian Rogers
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Zj0--YbYSm-s9vRh@x1 \
--to=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=asmadeus@codewreck.org \
--cc=irogers@google.com \
--cc=jolsa@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox