* [PATCH v2 1/3] perf parse-events: pass parse_state to add_tracepoint
2024-05-05 11:13 [PATCH v2 0/3] perf probe: Allow names to start with digits Dominique Martinet
@ 2024-05-05 11:13 ` Dominique Martinet
2024-05-08 21:37 ` Ian Rogers
2024-05-05 11:13 ` [PATCH v2 2/3] perf parse-events: Add new 'fake_tp' parameter for tests Dominique Martinet
` (2 subsequent siblings)
3 siblings, 1 reply; 12+ messages in thread
From: Dominique Martinet @ 2024-05-05 11:13 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa, Peter Zijlstra, Ingo Molnar,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Ian Rogers,
Adrian Hunter
Cc: linux-perf-users, linux-kernel, Dominique Martinet
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
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
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2 1/3] perf parse-events: pass parse_state to add_tracepoint
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
0 siblings, 2 replies; 12+ messages in thread
From: Ian Rogers @ 2024-05-08 21:37 UTC (permalink / raw)
To: Dominique Martinet
Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Peter Zijlstra, Ingo Molnar,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Adrian Hunter,
linux-perf-users, linux-kernel
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>
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
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v2 1/3] perf parse-events: pass parse_state to add_tracepoint
2024-05-08 21:37 ` Ian Rogers
@ 2024-05-08 23:03 ` Dominique Martinet
2024-05-09 21:24 ` Arnaldo Carvalho de Melo
1 sibling, 0 replies; 12+ messages in thread
From: Dominique Martinet @ 2024-05-08 23:03 UTC (permalink / raw)
To: Ian Rogers
Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Peter Zijlstra, Ingo Molnar,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Adrian Hunter,
linux-perf-users, linux-kernel
Thanks for the review!
Ian Rogers wrote on Wed, May 08, 2024 at 02:37:16PM -0700:
> Nit: evsel__newtp_idx typo
Fixed in v3
> 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
(I'm not sure what's immediately actionable here, so I'll consider this
a higher level comment for later cleanup)
Ian Rogers wrote on Wed, May 08, 2024 at 03:47:29PM -0700:
> Note, this isn't applying cleanly to the perf-tools-next branch on:
> https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/
Thanks for this note, I've rebased on this branch for v3
--
Dominique Martinet | Asmadeus
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/3] perf parse-events: pass parse_state to add_tracepoint
2024-05-08 21:37 ` Ian Rogers
2024-05-08 23:03 ` Dominique Martinet
@ 2024-05-09 21:24 ` Arnaldo Carvalho de Melo
2024-05-09 21:46 ` Arnaldo Carvalho de Melo
1 sibling, 1 reply; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-05-09 21:24 UTC (permalink / raw)
To: Ian Rogers
Cc: Dominique Martinet, Jiri Olsa, Peter Zijlstra, Ingo Molnar,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Adrian Hunter,
linux-perf-users, linux-kernel
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
> >
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v2 1/3] perf parse-events: pass parse_state to add_tracepoint
2024-05-09 21:24 ` Arnaldo Carvalho de Melo
@ 2024-05-09 21:46 ` Arnaldo Carvalho de Melo
2024-05-09 21:59 ` Dominique Martinet
0 siblings, 1 reply; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-05-09 21:46 UTC (permalink / raw)
To: Ian Rogers
Cc: Dominique Martinet, Jiri Olsa, Peter Zijlstra, Ingo Molnar,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Adrian Hunter,
linux-perf-users, linux-kernel
On Thu, May 09, 2024 at 06:24:12PM -0300, Arnaldo Carvalho de Melo wrote:
> 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.
But I'm removing the series due to it not passing:
⬢[acme@toolbox perf-tools-next]$ git log --oneline -1 ; time make -C tools/perf build-test
838f9d554bbe1636 (HEAD -> perf-tools-next) perf tracepoint: Don't scan all tracepoints to test if one exists
make: Entering directory '/home/acme/git/perf-tools-next/tools/perf'
- tarpkg: ./tests/perf-targz-src-pkg .
make_static: cd . && make LDFLAGS=-static NO_PERF_READ_VDSO32=1 NO_PERF_READ_VDSOX32=1 NO_JVMTI=1 NO_LIBTRACEEVENT=1 NO_LIBELF=1 -j28 DESTDIR=/tmp/tmp.JIPWT3E0yJ
cd . && make LDFLAGS=-static NO_PERF_READ_VDSO32=1 NO_PERF_READ_VDSOX32=1 NO_JVMTI=1 NO_LIBTRACEEVENT=1 NO_LIBELF=1 -j28 DESTDIR=/tmp/tmp.JIPWT3E0yJ
BUILD: Doing 'make -j28' parallel build
HOSTCC fixdep.o
HOSTLD fixdep-in.o
LINK fixdep
Makefile.config:692: Warning: Disabled BPF skeletons as libelf is required by bpftool
Makefile.config:733: Disabling post unwind, no support found.
Makefile.config:801: No libcrypto.h found, disables jitted code injection, please install openssl-devel or libssl-dev
Makefile.config:813: slang not found, disables TUI support. Please install slang-devel, libslang-dev or libslang2-dev
Makefile.config:860: Missing perl devel files. Disabling perl scripting support, please install perl-ExtUtils-Embed/libperl-dev
Makefile.config:900: No 'Python.h' was found: disables Python support - please install python-devel/python-dev
Makefile.config:1000: No liblzma found, disables xz kernel module decompression, please install xz-devel/liblzma-dev
Makefile.config:1013: No libzstd found, disables trace compression, please install libzstd-dev[el] and/or set LIBZSTD_DIR
Makefile.config:1024: No libcap found, disables capability support, please install libcap-devel/libcap-dev
Makefile.config:1037: No numa.h found, disables 'perf bench numa mem' benchmark, please install numactl-devel/libnuma-devel/libnuma-dev
Makefile.config:1096: No libbabeltrace found, disables 'perf data' CTF format support, please install libbabeltrace-dev[el]/libbabeltrace-ctf-dev
Makefile.config:1108: No libcapstone found, disables disasm engine support for 'perf script', please install libcapstone-dev/capstone-devel
Makefile.config:1173: libpfm4 not found, disables libpfm4 support. Please install libpfm4-dev
Auto-detecting system features:
... dwarf: [ OFF ]
... dwarf_getlocations: [ OFF ]
... glibc: [ on ]
... libbfd: [ OFF ]
... libbfd-buildid: [ OFF ]
... libcap: [ OFF ]
... libelf: [ OFF ]
... libnuma: [ OFF ]
... numa_num_possible_cpus: [ OFF ]
... libperl: [ OFF ]
<SNIP>
CC tests/expr.o
CC util/mmap.o
CC util/memswap.o
BISON util/parse-events-bison.c
CC tests/backward-ring-buffer.o
CC util/print-events.o
CC tests/sdt.o
CC util/tracepoint.o
CC util/perf_regs.o
CC util/perf-regs-arch/perf_regs_aarch64.o
CC util/intel-pt-decoder/intel-pt-pkt-decoder.o
GEN pmu-events/pmu-events.c
CC util/perf-regs-arch/perf_regs_arm.o
GEN util/intel-pt-decoder/inat-tables.c
CC tests/is_printable_array.o
CC util/perf-regs-arch/perf_regs_csky.o
CC util/perf-regs-arch/perf_regs_loongarch.o
CC util/arm-spe-decoder/arm-spe-pkt-decoder.o
CC util/arm-spe-decoder/arm-spe-decoder.o
CC util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.o
CC tests/bitmap.o
CC tests/perf-hooks.o
CC util/perf-regs-arch/perf_regs_mips.o
CC util/intel-pt-decoder/intel-pt-log.o
CC tests/unit_number__scnprintf.o
CC util/perf-regs-arch/perf_regs_powerpc.o
CC tests/mem2node.o
LD util/arm-spe-decoder/perf-in.o
CC util/perf-regs-arch/perf_regs_riscv.o
CC util/perf-regs-arch/perf_regs_s390.o
CC util/perf-regs-arch/perf_regs_x86.o
LD util/scripting-engines/perf-in.o
CC util/path.o
CC tests/maps.o
LD util/hisi-ptt-decoder/perf-in.o
CC tests/time-utils-test.o
CC tests/genelf.o
LD util/perf-regs-arch/perf-in.o
CC tests/api-io.o
CC util/intel-pt-decoder/intel-pt-decoder.o
CC util/intel-pt-decoder/intel-pt-insn-decoder.o
CC util/print_binary.o
CC tests/demangle-java-test.o
CC util/print_insn.o
CC tests/demangle-ocaml-test.o
CC tests/pfm.o
CC tests/parse-metric.o
LD util/intel-pt-decoder/perf-in.o
CC util/rlimit.o
CC util/argv_split.o
CC util/rbtree.o
CC util/libstring.o
CC util/bitmap.o
CC util/hweight.o
CC util/smt.o
CC util/strbuf.o
CC tests/pe-file-parsing.o
CC util/string.o
CC util/strlist.o
CC tests/expand-cgroup.o
CC util/strfilter.o
CC tests/perf-time-to-tsc.o
CC util/top.o
CC util/usage.o
CC tests/dlfilter-test.o
CC tests/sigtrap.o
CC util/dso.o
CC tests/event_groups.o
CC util/dsos.o
CC tests/symbols.o
CC util/symbol.o
CC tests/util.o
CC util/symbol_fprintf.o
tests/parse-events.c:2274:26: error: ‘test__checkevent_tracepoint’ undeclared here (not in a function); did you mean ‘test__checkevent_breakpoint’?
2274 | .check = test__checkevent_tracepoint,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
| test__checkevent_breakpoint
make[6]: *** [/home/acme/git/perf-tools-next/tools/build/Makefile.build:105: tests/parse-events.o] Error 1
make[6]: *** Waiting for unfinished jobs....
CC util/map_symbol.o
CC util/color.o
CC util/color_config.o
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v2 1/3] perf parse-events: pass parse_state to add_tracepoint
2024-05-09 21:46 ` Arnaldo Carvalho de Melo
@ 2024-05-09 21:59 ` Dominique Martinet
0 siblings, 0 replies; 12+ messages in thread
From: Dominique Martinet @ 2024-05-09 21:59 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ian Rogers, Jiri Olsa, Peter Zijlstra, Ingo Molnar, Namhyung Kim,
Mark Rutland, Alexander Shishkin, Adrian Hunter, linux-perf-users,
linux-kernel
Arnaldo Carvalho de Melo wrote on Thu, May 09, 2024 at 06:46:19PM -0300:
> ⬢[acme@toolbox perf-tools-next]$ git log --oneline -1 ; time make -C tools/perf build-test
> [...]
> tests/parse-events.c:2274:26: error: ‘test__checkevent_tracepoint’ undeclared here (not in a function); did you mean ‘test__checkevent_breakpoint’?
> 2274 | .check = test__checkevent_tracepoint,
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> | test__checkevent_breakpoint
> make[6]: *** [/home/acme/git/perf-tools-next/tools/build/Makefile.build:105: tests/parse-events.o] Error 1
> make[6]: *** Waiting for unfinished jobs....
Sorry, didn't know about build-test; I've confirmed the problem [and
will eventually want to check how to build this cleanly on nixos, it's a
pain to shuffle the patch around to rebuild perf...]
It looks like the test case just needs an extra ifdef for
LIBTRACEEVEENT?
----
diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
index 417d4782a520..edc2adcf1bae 100644
--- a/tools/perf/tests/parse-events.c
+++ b/tools/perf/tests/parse-events.c
@@ -2269,11 +2269,13 @@ static const struct evlist_test test__events[] = {
.check = test__checkevent_breakpoint_2_events,
/* 3 */
},
+#ifdef HAVE_LIBTRACEEVENT
{
.name = "9p:9p_client_req",
.check = test__checkevent_tracepoint,
/* 4 */
},
+#endif
};
static const struct evlist_test test__events_pmu[] = {
----
I'll send a v4 with that rolled in after confirming the full build-test
passes.
--
Dominique Martinet | Asmadeus
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 2/3] perf parse-events: Add new 'fake_tp' parameter for tests
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-05 11:13 ` 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 22:47 ` [PATCH v2 0/3] perf probe: " Ian Rogers
3 siblings, 1 reply; 12+ messages in thread
From: Dominique Martinet @ 2024-05-05 11:13 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa, Peter Zijlstra, Ingo Molnar,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Ian Rogers,
Adrian Hunter
Cc: linux-perf-users, linux-kernel, Dominique Martinet
The next commit will allow tracepoints starting with digits, but most
systems do not have any available by default so tests should skip the
actual "check if it exists in /sys/kernel/debug/tracing" step.
In order to do that, add a new boolean flag specifying if we should
actually "format" the probe or not.
Originally-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
---
tools/perf/tests/parse-events.c | 6 ++++--
tools/perf/tests/pmu-events.c | 2 +-
tools/perf/util/evlist.c | 3 ++-
tools/perf/util/evsel.c | 20 +++++++++++++-------
tools/perf/util/evsel.h | 4 ++--
tools/perf/util/metricgroup.c | 3 ++-
tools/perf/util/parse-events.c | 9 ++++++---
tools/perf/util/parse-events.h | 6 ++++--
8 files changed, 34 insertions(+), 19 deletions(-)
diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
index feb5727584d1..ef056e8740fe 100644
--- a/tools/perf/tests/parse-events.c
+++ b/tools/perf/tests/parse-events.c
@@ -2504,7 +2504,8 @@ static int test_event(const struct evlist_test *e)
return TEST_FAIL;
}
parse_events_error__init(&err);
- ret = parse_events(evlist, e->name, &err);
+ ret = __parse_events(evlist, e->name, /*pmu_filter=*/NULL, &err, /*fake_pmu=*/NULL,
+ /*warn_if_reordered=*/true, /*fake_tp=*/true);
if (ret) {
pr_debug("failed to parse event '%s', err %d\n", e->name, ret);
parse_events_error__print(&err, e->name);
@@ -2532,7 +2533,8 @@ static int test_event_fake_pmu(const char *str)
parse_events_error__init(&err);
ret = __parse_events(evlist, str, /*pmu_filter=*/NULL, &err,
- &perf_pmu__fake, /*warn_if_reordered=*/true);
+ &perf_pmu__fake, /*warn_if_reordered=*/true,
+ /*fake_tp=*/true);
if (ret) {
pr_debug("failed to parse event '%s', err %d\n",
str, ret);
diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c
index 47a7c3277540..0a1308d84e9e 100644
--- a/tools/perf/tests/pmu-events.c
+++ b/tools/perf/tests/pmu-events.c
@@ -842,7 +842,7 @@ static int check_parse_id(const char *id, struct parse_events_error *error,
*cur = '/';
ret = __parse_events(evlist, dup, /*pmu_filter=*/NULL, error, fake_pmu,
- /*warn_if_reordered=*/true);
+ /*warn_if_reordered=*/true, /*fake_tp=*/false);
free(dup);
evlist__delete(evlist);
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 55a300a0977b..3a719edafc7a 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -298,7 +298,8 @@ struct evsel *evlist__add_aux_dummy(struct evlist *evlist, bool system_wide)
#ifdef HAVE_LIBTRACEEVENT
struct evsel *evlist__add_sched_switch(struct evlist *evlist, bool system_wide)
{
- struct evsel *evsel = evsel__newtp_idx("sched", "sched_switch", 0);
+ struct evsel *evsel = evsel__newtp_idx("sched", "sched_switch", 0,
+ /*format=*/true);
if (IS_ERR(evsel))
return evsel;
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 3536404e9447..4f818ab6b662 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -452,7 +452,7 @@ struct evsel *evsel__clone(struct evsel *orig)
* Returns pointer with encoded error via <linux/err.h> interface.
*/
#ifdef HAVE_LIBTRACEEVENT
-struct evsel *evsel__newtp_idx(const char *sys, const char *name, int idx)
+struct evsel *evsel__newtp_idx(const char *sys, const char *name, int idx, bool format)
{
struct evsel *evsel = zalloc(perf_evsel__object.size);
int err = -ENOMEM;
@@ -469,14 +469,20 @@ struct evsel *evsel__newtp_idx(const char *sys, const char *name, int idx)
if (asprintf(&evsel->name, "%s:%s", sys, name) < 0)
goto out_free;
- evsel->tp_format = trace_event__tp_format(sys, name);
- if (IS_ERR(evsel->tp_format)) {
- err = PTR_ERR(evsel->tp_format);
- goto out_free;
+ event_attr_init(&attr);
+
+ if (format) {
+ evsel->tp_format = trace_event__tp_format(sys, name);
+ if (IS_ERR(evsel->tp_format)) {
+ err = PTR_ERR(evsel->tp_format);
+ goto out_free;
+ }
+ attr.config = evsel->tp_format->id;
+ } else {
+ attr.config = (__u64) -1;
}
- event_attr_init(&attr);
- attr.config = evsel->tp_format->id;
+
attr.sample_period = 1;
evsel__init(evsel, &attr, idx);
}
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 517cff431de2..375a38e15cd9 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -234,14 +234,14 @@ void free_config_terms(struct list_head *config_terms);
#ifdef HAVE_LIBTRACEEVENT
-struct evsel *evsel__newtp_idx(const char *sys, const char *name, int idx);
+struct evsel *evsel__newtp_idx(const char *sys, const char *name, int idx, bool format);
/*
* Returns pointer with encoded error via <linux/err.h> interface.
*/
static inline struct evsel *evsel__newtp(const char *sys, const char *name)
{
- return evsel__newtp_idx(sys, name, 0);
+ return evsel__newtp_idx(sys, name, 0, true);
}
#endif
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 79ef6095ab28..c21f05f8f255 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -1502,7 +1502,8 @@ static int parse_ids(bool metric_no_merge, struct perf_pmu *fake_pmu,
pr_debug("Parsing metric events '%s'\n", events.buf);
parse_events_error__init(&parse_error);
ret = __parse_events(parsed_evlist, events.buf, /*pmu_filter=*/NULL,
- &parse_error, fake_pmu, /*warn_if_reordered=*/false);
+ &parse_error, fake_pmu, /*warn_if_reordered=*/false,
+ /*fake_tp=*/false);
if (ret) {
parse_events_error__print(&parse_error, events.buf);
goto err_out;
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 6e8cba03f0ac..04508e07569d 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -526,7 +526,8 @@ static int add_tracepoint(struct parse_events_state *parse_state,
struct parse_events_terms *head_config, void *loc_)
{
YYLTYPE *loc = loc_;
- struct evsel *evsel = evsel__newtp_idx(sys_name, evt_name, parse_state->idx++);
+ struct evsel *evsel = evsel__newtp_idx(sys_name, evt_name, parse_state->idx++,
+ !parse_state->fake_tp);
if (IS_ERR(evsel)) {
tracepoint_error(err, PTR_ERR(evsel), sys_name, evt_name, loc->first_column);
@@ -2126,7 +2127,7 @@ static int parse_events__sort_events_and_fix_groups(struct list_head *list)
int __parse_events(struct evlist *evlist, const char *str, const char *pmu_filter,
struct parse_events_error *err, struct perf_pmu *fake_pmu,
- bool warn_if_reordered)
+ bool warn_if_reordered, bool fake_tp)
{
struct parse_events_state parse_state = {
.list = LIST_HEAD_INIT(parse_state.list),
@@ -2134,6 +2135,7 @@ int __parse_events(struct evlist *evlist, const char *str, const char *pmu_filte
.error = err,
.stoken = PE_START_EVENTS,
.fake_pmu = fake_pmu,
+ .fake_tp = fake_tp,
.pmu_filter = pmu_filter,
.match_legacy_cache_terms = true,
};
@@ -2343,7 +2345,8 @@ int parse_events_option(const struct option *opt, const char *str,
parse_events_error__init(&err);
ret = __parse_events(*args->evlistp, str, args->pmu_filter, &err,
- /*fake_pmu=*/NULL, /*warn_if_reordered=*/true);
+ /*fake_pmu=*/NULL, /*warn_if_reordered=*/true,
+ /*fake_tp=*/false);
if (ret) {
parse_events_error__print(&err, str);
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index fd55a154ceff..b985a821546b 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -32,14 +32,14 @@ int parse_events_option_new_evlist(const struct option *opt, const char *str, in
__attribute__((nonnull(1, 2, 4)))
int __parse_events(struct evlist *evlist, const char *str, const char *pmu_filter,
struct parse_events_error *error, struct perf_pmu *fake_pmu,
- bool warn_if_reordered);
+ bool warn_if_reordered, bool fake_tp);
__attribute__((nonnull(1, 2, 3)))
static inline int parse_events(struct evlist *evlist, const char *str,
struct parse_events_error *err)
{
return __parse_events(evlist, str, /*pmu_filter=*/NULL, err, /*fake_pmu=*/NULL,
- /*warn_if_reordered=*/true);
+ /*warn_if_reordered=*/true, /*fake_tp=*/false);
}
int parse_event(struct evlist *evlist, const char *str);
@@ -152,6 +152,8 @@ struct parse_events_state {
int stoken;
/* Special fake PMU marker for testing. */
struct perf_pmu *fake_pmu;
+ /* Skip actual tracepoint processing for testing. */
+ bool fake_tp;
/* If non-null, when wildcard matching only match the given PMU. */
const char *pmu_filter;
/* Should PE_LEGACY_NAME tokens be generated for config terms? */
--
2.44.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2 2/3] perf parse-events: Add new 'fake_tp' parameter for tests
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
0 siblings, 0 replies; 12+ messages in thread
From: Ian Rogers @ 2024-05-08 21:42 UTC (permalink / raw)
To: Dominique Martinet
Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Peter Zijlstra, Ingo Molnar,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Adrian Hunter,
linux-perf-users, linux-kernel
On Sun, May 5, 2024 at 4:14 AM Dominique Martinet
<asmadeus@codewreck.org> wrote:
>
> The next commit will allow tracepoints starting with digits, but most
> systems do not have any available by default so tests should skip the
> actual "check if it exists in /sys/kernel/debug/tracing" step.
>
> In order to do that, add a new boolean flag specifying if we should
> actually "format" the probe or not.
>
> Originally-by: Jiri Olsa <jolsa@kernel.org>
> Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
Reviewed-by: Ian Rogers <irogers@google.com>
Thanks,
Ian
> ---
> tools/perf/tests/parse-events.c | 6 ++++--
> tools/perf/tests/pmu-events.c | 2 +-
> tools/perf/util/evlist.c | 3 ++-
> tools/perf/util/evsel.c | 20 +++++++++++++-------
> tools/perf/util/evsel.h | 4 ++--
> tools/perf/util/metricgroup.c | 3 ++-
> tools/perf/util/parse-events.c | 9 ++++++---
> tools/perf/util/parse-events.h | 6 ++++--
> 8 files changed, 34 insertions(+), 19 deletions(-)
>
> diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
> index feb5727584d1..ef056e8740fe 100644
> --- a/tools/perf/tests/parse-events.c
> +++ b/tools/perf/tests/parse-events.c
> @@ -2504,7 +2504,8 @@ static int test_event(const struct evlist_test *e)
> return TEST_FAIL;
> }
> parse_events_error__init(&err);
> - ret = parse_events(evlist, e->name, &err);
> + ret = __parse_events(evlist, e->name, /*pmu_filter=*/NULL, &err, /*fake_pmu=*/NULL,
> + /*warn_if_reordered=*/true, /*fake_tp=*/true);
> if (ret) {
> pr_debug("failed to parse event '%s', err %d\n", e->name, ret);
> parse_events_error__print(&err, e->name);
> @@ -2532,7 +2533,8 @@ static int test_event_fake_pmu(const char *str)
>
> parse_events_error__init(&err);
> ret = __parse_events(evlist, str, /*pmu_filter=*/NULL, &err,
> - &perf_pmu__fake, /*warn_if_reordered=*/true);
> + &perf_pmu__fake, /*warn_if_reordered=*/true,
> + /*fake_tp=*/true);
> if (ret) {
> pr_debug("failed to parse event '%s', err %d\n",
> str, ret);
> diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c
> index 47a7c3277540..0a1308d84e9e 100644
> --- a/tools/perf/tests/pmu-events.c
> +++ b/tools/perf/tests/pmu-events.c
> @@ -842,7 +842,7 @@ static int check_parse_id(const char *id, struct parse_events_error *error,
> *cur = '/';
>
> ret = __parse_events(evlist, dup, /*pmu_filter=*/NULL, error, fake_pmu,
> - /*warn_if_reordered=*/true);
> + /*warn_if_reordered=*/true, /*fake_tp=*/false);
> free(dup);
>
> evlist__delete(evlist);
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index 55a300a0977b..3a719edafc7a 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -298,7 +298,8 @@ struct evsel *evlist__add_aux_dummy(struct evlist *evlist, bool system_wide)
> #ifdef HAVE_LIBTRACEEVENT
> struct evsel *evlist__add_sched_switch(struct evlist *evlist, bool system_wide)
> {
> - struct evsel *evsel = evsel__newtp_idx("sched", "sched_switch", 0);
> + struct evsel *evsel = evsel__newtp_idx("sched", "sched_switch", 0,
> + /*format=*/true);
>
> if (IS_ERR(evsel))
> return evsel;
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index 3536404e9447..4f818ab6b662 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -452,7 +452,7 @@ struct evsel *evsel__clone(struct evsel *orig)
> * Returns pointer with encoded error via <linux/err.h> interface.
> */
> #ifdef HAVE_LIBTRACEEVENT
> -struct evsel *evsel__newtp_idx(const char *sys, const char *name, int idx)
> +struct evsel *evsel__newtp_idx(const char *sys, const char *name, int idx, bool format)
> {
> struct evsel *evsel = zalloc(perf_evsel__object.size);
> int err = -ENOMEM;
> @@ -469,14 +469,20 @@ struct evsel *evsel__newtp_idx(const char *sys, const char *name, int idx)
> if (asprintf(&evsel->name, "%s:%s", sys, name) < 0)
> goto out_free;
>
> - evsel->tp_format = trace_event__tp_format(sys, name);
> - if (IS_ERR(evsel->tp_format)) {
> - err = PTR_ERR(evsel->tp_format);
> - goto out_free;
> + event_attr_init(&attr);
> +
> + if (format) {
> + evsel->tp_format = trace_event__tp_format(sys, name);
> + if (IS_ERR(evsel->tp_format)) {
> + err = PTR_ERR(evsel->tp_format);
> + goto out_free;
> + }
> + attr.config = evsel->tp_format->id;
> + } else {
> + attr.config = (__u64) -1;
> }
>
> - event_attr_init(&attr);
> - attr.config = evsel->tp_format->id;
> +
> attr.sample_period = 1;
> evsel__init(evsel, &attr, idx);
> }
> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> index 517cff431de2..375a38e15cd9 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -234,14 +234,14 @@ void free_config_terms(struct list_head *config_terms);
>
>
> #ifdef HAVE_LIBTRACEEVENT
> -struct evsel *evsel__newtp_idx(const char *sys, const char *name, int idx);
> +struct evsel *evsel__newtp_idx(const char *sys, const char *name, int idx, bool format);
>
> /*
> * Returns pointer with encoded error via <linux/err.h> interface.
> */
> static inline struct evsel *evsel__newtp(const char *sys, const char *name)
> {
> - return evsel__newtp_idx(sys, name, 0);
> + return evsel__newtp_idx(sys, name, 0, true);
> }
> #endif
>
> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
> index 79ef6095ab28..c21f05f8f255 100644
> --- a/tools/perf/util/metricgroup.c
> +++ b/tools/perf/util/metricgroup.c
> @@ -1502,7 +1502,8 @@ static int parse_ids(bool metric_no_merge, struct perf_pmu *fake_pmu,
> pr_debug("Parsing metric events '%s'\n", events.buf);
> parse_events_error__init(&parse_error);
> ret = __parse_events(parsed_evlist, events.buf, /*pmu_filter=*/NULL,
> - &parse_error, fake_pmu, /*warn_if_reordered=*/false);
> + &parse_error, fake_pmu, /*warn_if_reordered=*/false,
> + /*fake_tp=*/false);
> if (ret) {
> parse_events_error__print(&parse_error, events.buf);
> goto err_out;
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 6e8cba03f0ac..04508e07569d 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -526,7 +526,8 @@ static int add_tracepoint(struct parse_events_state *parse_state,
> struct parse_events_terms *head_config, void *loc_)
> {
> YYLTYPE *loc = loc_;
> - struct evsel *evsel = evsel__newtp_idx(sys_name, evt_name, parse_state->idx++);
> + struct evsel *evsel = evsel__newtp_idx(sys_name, evt_name, parse_state->idx++,
> + !parse_state->fake_tp);
>
> if (IS_ERR(evsel)) {
> tracepoint_error(err, PTR_ERR(evsel), sys_name, evt_name, loc->first_column);
> @@ -2126,7 +2127,7 @@ static int parse_events__sort_events_and_fix_groups(struct list_head *list)
>
> int __parse_events(struct evlist *evlist, const char *str, const char *pmu_filter,
> struct parse_events_error *err, struct perf_pmu *fake_pmu,
> - bool warn_if_reordered)
> + bool warn_if_reordered, bool fake_tp)
> {
> struct parse_events_state parse_state = {
> .list = LIST_HEAD_INIT(parse_state.list),
> @@ -2134,6 +2135,7 @@ int __parse_events(struct evlist *evlist, const char *str, const char *pmu_filte
> .error = err,
> .stoken = PE_START_EVENTS,
> .fake_pmu = fake_pmu,
> + .fake_tp = fake_tp,
> .pmu_filter = pmu_filter,
> .match_legacy_cache_terms = true,
> };
> @@ -2343,7 +2345,8 @@ int parse_events_option(const struct option *opt, const char *str,
>
> parse_events_error__init(&err);
> ret = __parse_events(*args->evlistp, str, args->pmu_filter, &err,
> - /*fake_pmu=*/NULL, /*warn_if_reordered=*/true);
> + /*fake_pmu=*/NULL, /*warn_if_reordered=*/true,
> + /*fake_tp=*/false);
>
> if (ret) {
> parse_events_error__print(&err, str);
> diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
> index fd55a154ceff..b985a821546b 100644
> --- a/tools/perf/util/parse-events.h
> +++ b/tools/perf/util/parse-events.h
> @@ -32,14 +32,14 @@ int parse_events_option_new_evlist(const struct option *opt, const char *str, in
> __attribute__((nonnull(1, 2, 4)))
> int __parse_events(struct evlist *evlist, const char *str, const char *pmu_filter,
> struct parse_events_error *error, struct perf_pmu *fake_pmu,
> - bool warn_if_reordered);
> + bool warn_if_reordered, bool fake_tp);
>
> __attribute__((nonnull(1, 2, 3)))
> static inline int parse_events(struct evlist *evlist, const char *str,
> struct parse_events_error *err)
> {
> return __parse_events(evlist, str, /*pmu_filter=*/NULL, err, /*fake_pmu=*/NULL,
> - /*warn_if_reordered=*/true);
> + /*warn_if_reordered=*/true, /*fake_tp=*/false);
> }
>
> int parse_event(struct evlist *evlist, const char *str);
> @@ -152,6 +152,8 @@ struct parse_events_state {
> int stoken;
> /* Special fake PMU marker for testing. */
> struct perf_pmu *fake_pmu;
> + /* Skip actual tracepoint processing for testing. */
> + bool fake_tp;
> /* If non-null, when wildcard matching only match the given PMU. */
> const char *pmu_filter;
> /* Should PE_LEGACY_NAME tokens be generated for config terms? */
>
> --
> 2.44.0
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 3/3] perf parse: Allow names to start with digits
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-05 11:13 ` [PATCH v2 2/3] perf parse-events: Add new 'fake_tp' parameter for tests Dominique Martinet
@ 2024-05-05 11:13 ` Dominique Martinet
2024-05-08 21:44 ` Ian Rogers
2024-05-08 22:47 ` [PATCH v2 0/3] perf probe: " Ian Rogers
3 siblings, 1 reply; 12+ messages in thread
From: Dominique Martinet @ 2024-05-05 11:13 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa, Peter Zijlstra, Ingo Molnar,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Ian Rogers,
Adrian Hunter
Cc: linux-perf-users, linux-kernel, Dominique Martinet
Tracepoints can start with digits, although we don't have many of these:
$ rg -g '*.h' '\bTRACE_EVENT\([0-9]'
net/mac802154/trace.h
53:TRACE_EVENT(802154_drv_return_int,
...
net/ieee802154/trace.h
66:TRACE_EVENT(802154_rdev_add_virtual_intf,
...
include/trace/events/9p.h
124:TRACE_EVENT(9p_client_req,
...
Just allow names to start with digits too so e.g. perf trace -e '9p:*'
works
Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
---
tools/perf/tests/parse-events.c | 5 +++++
tools/perf/util/parse-events.l | 4 ++--
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
index ef056e8740fe..6cf055dd5c09 100644
--- a/tools/perf/tests/parse-events.c
+++ b/tools/perf/tests/parse-events.c
@@ -2280,6 +2280,11 @@ static const struct evlist_test test__events[] = {
.check = test__checkevent_breakpoint_2_events,
/* 3 */
},
+ {
+ .name = "9p:9p_client_req",
+ .check = test__checkevent_tracepoint,
+ /* 4 */
+ },
};
static const struct evlist_test test__events_pmu[] = {
diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index e86c45675e1d..c36f8dbf593a 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -158,8 +158,8 @@ event [^,{}/]+
num_dec [0-9]+
num_hex 0x[a-fA-F0-9]{1,16}
num_raw_hex [a-fA-F0-9]{1,16}
-name [a-zA-Z_*?\[\]][a-zA-Z0-9_*?.\[\]!\-]*
-name_tag [\'][a-zA-Z_*?\[\]][a-zA-Z0-9_*?\-,\.\[\]:=]*[\']
+name [a-zA-Z0-9_*?\[\]][a-zA-Z0-9_*?.\[\]!\-]*
+name_tag [\'][a-zA-Z0-9_*?\[\]][a-zA-Z0-9_*?\-,\.\[\]:=]*[\']
name_minus [a-zA-Z_*?][a-zA-Z0-9\-_*?.:]*
drv_cfg_term [a-zA-Z0-9_\.]+(=[a-zA-Z0-9_*?\.:]+)?
/*
--
2.44.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2 3/3] perf parse: Allow names to start with digits
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
0 siblings, 0 replies; 12+ messages in thread
From: Ian Rogers @ 2024-05-08 21:44 UTC (permalink / raw)
To: Dominique Martinet
Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Peter Zijlstra, Ingo Molnar,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Adrian Hunter,
linux-perf-users, linux-kernel
On Sun, May 5, 2024 at 4:14 AM Dominique Martinet
<asmadeus@codewreck.org> wrote:
>
> Tracepoints can start with digits, although we don't have many of these:
>
> $ rg -g '*.h' '\bTRACE_EVENT\([0-9]'
> net/mac802154/trace.h
> 53:TRACE_EVENT(802154_drv_return_int,
> ...
>
> net/ieee802154/trace.h
> 66:TRACE_EVENT(802154_rdev_add_virtual_intf,
> ...
>
> include/trace/events/9p.h
> 124:TRACE_EVENT(9p_client_req,
> ...
>
> Just allow names to start with digits too so e.g. perf trace -e '9p:*'
> works
>
> Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
Reviewed-by: Ian Rogers <irogers@google.com>
Tested-by: Ian Rogers <irogers@google.com>
Thanks,
Ian
> ---
> tools/perf/tests/parse-events.c | 5 +++++
> tools/perf/util/parse-events.l | 4 ++--
> 2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
> index ef056e8740fe..6cf055dd5c09 100644
> --- a/tools/perf/tests/parse-events.c
> +++ b/tools/perf/tests/parse-events.c
> @@ -2280,6 +2280,11 @@ static const struct evlist_test test__events[] = {
> .check = test__checkevent_breakpoint_2_events,
> /* 3 */
> },
> + {
> + .name = "9p:9p_client_req",
> + .check = test__checkevent_tracepoint,
> + /* 4 */
> + },
> };
>
> static const struct evlist_test test__events_pmu[] = {
> diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
> index e86c45675e1d..c36f8dbf593a 100644
> --- a/tools/perf/util/parse-events.l
> +++ b/tools/perf/util/parse-events.l
> @@ -158,8 +158,8 @@ event [^,{}/]+
> num_dec [0-9]+
> num_hex 0x[a-fA-F0-9]{1,16}
> num_raw_hex [a-fA-F0-9]{1,16}
> -name [a-zA-Z_*?\[\]][a-zA-Z0-9_*?.\[\]!\-]*
> -name_tag [\'][a-zA-Z_*?\[\]][a-zA-Z0-9_*?\-,\.\[\]:=]*[\']
> +name [a-zA-Z0-9_*?\[\]][a-zA-Z0-9_*?.\[\]!\-]*
> +name_tag [\'][a-zA-Z0-9_*?\[\]][a-zA-Z0-9_*?\-,\.\[\]:=]*[\']
> name_minus [a-zA-Z_*?][a-zA-Z0-9\-_*?.:]*
> drv_cfg_term [a-zA-Z0-9_\.]+(=[a-zA-Z0-9_*?\.:]+)?
> /*
>
> --
> 2.44.0
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 0/3] perf probe: Allow names to start with digits
2024-05-05 11:13 [PATCH v2 0/3] perf probe: Allow names to start with digits Dominique Martinet
` (2 preceding siblings ...)
2024-05-05 11:13 ` [PATCH v2 3/3] perf parse: Allow names to start with digits Dominique Martinet
@ 2024-05-08 22:47 ` Ian Rogers
3 siblings, 0 replies; 12+ messages in thread
From: Ian Rogers @ 2024-05-08 22:47 UTC (permalink / raw)
To: Dominique Martinet
Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Peter Zijlstra, Ingo Molnar,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Adrian Hunter,
linux-perf-users, linux-kernel
On Sun, May 5, 2024 at 4:14 AM Dominique Martinet
<asmadeus@codewreck.org> wrote:
>
> This is a rebase of the patch orginally sent almost two years ago here:
> https://lkml.kernel.org/r/20220612061508.1449636-1-asmadeus@codewreck.org
>
> At the time I was asked to add tests, and Jiri whipped up something to
> make the test pass even for probes that don't exist on most systems but
> that ended up never being formatted or sent... I asked what happened of
> it and got asked to send it myself, but obviously also totally forget
> about it myself until I needed it again now.
>
> I've taken the diff from that thread, adapted it a little bit to the
> current master branch and checked things still fall in place -- I didn't
> see any obvious problem.
>
> Thanks!
>
> Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
Reviewed-by: Ian Rogers <irogers@google.com>
Note, this isn't applying cleanly to the perf-tools-next branch on:
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/
Thanks,
Ian
> ---
> Changes in v2:
> - update Jiri's email in commit tags
> - (not a change, but after being brain-dead and Ian helpful
> reply I'm confirming patch 3/3 works as expected)
> - Link to v1: https://lore.kernel.org/r/20240407-perf_digit-v1-0-57ec37c63394@codewreck.org
>
> ---
> Dominique Martinet (3):
> perf parse-events: pass parse_state to add_tracepoint
> perf parse-events: Add new 'fake_tp' parameter for tests
> perf parse: Allow names to start with digits
>
> tools/perf/tests/parse-events.c | 11 +++++++++--
> tools/perf/tests/pmu-events.c | 2 +-
> tools/perf/util/evlist.c | 3 ++-
> tools/perf/util/evsel.c | 20 +++++++++++++-------
> tools/perf/util/evsel.h | 4 ++--
> tools/perf/util/metricgroup.c | 3 ++-
> tools/perf/util/parse-events.c | 38 +++++++++++++++++++++++---------------
> tools/perf/util/parse-events.h | 9 ++++++---
> tools/perf/util/parse-events.l | 4 ++--
> tools/perf/util/parse-events.y | 2 +-
> 10 files changed, 61 insertions(+), 35 deletions(-)
> ---
> base-commit: 7367539ad4b0f8f9b396baf02110962333719a48
> change-id: 20240407-perf_digit-72445b5edb62
>
> Best regards,
> --
> Dominique Martinet | Asmadeus
>
^ permalink raw reply [flat|nested] 12+ messages in thread