[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAP-5=fUmeyd3BR7njJEDQ-=qkpvLPMoQO-7De+3mqLaSOoZZxw@mail.gmail.com>
Date: Wed, 8 May 2024 14:37:16 -0700
From: Ian Rogers <irogers@...gle.com>
To: Dominique Martinet <asmadeus@...ewreck.org>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>, Jiri Olsa <jolsa@...nel.org>,
Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
Namhyung Kim <namhyung@...nel.org>, Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Adrian Hunter <adrian.hunter@...el.com>, linux-perf-users@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/3] perf parse-events: pass parse_state to add_tracepoint
On Sun, May 5, 2024 at 4:14 AM Dominique Martinet
<asmadeus@...ewreck.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@...gle.com>
Thanks,
Ian
> Originally-by: Jiri Olsa <jolsa@...nel.org>
> Signed-off-by: Dominique Martinet <asmadeus@...ewreck.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, $1event,
> error, $2, &@1);
>
> parse_events_terms__delete($2);
>
> --
> 2.44.0
>
Powered by blists - more mailing lists