* [PATCH 1/3] perf parse-events: pass parse_state to add_tracepoint
2024-04-07 12:18 [PATCH 0/3] perf probe: Allow names to start with digits Dominique Martinet
@ 2024-04-07 12:18 ` Dominique Martinet
2024-04-07 12:18 ` [PATCH 2/3] perf parse-events: Add new 'fake_tp' parameter for tests Dominique Martinet
2024-04-07 12:18 ` [PATCH 3/3] perf parse: Allow names to start with digits Dominique Martinet
2 siblings, 0 replies; 7+ messages in thread
From: Dominique Martinet @ 2024-04-07 12:18 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, Jiri Olsa
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 <olsajiri@gmail.com>
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.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 2/3] perf parse-events: Add new 'fake_tp' parameter for tests
2024-04-07 12:18 [PATCH 0/3] perf probe: Allow names to start with digits Dominique Martinet
2024-04-07 12:18 ` [PATCH 1/3] perf parse-events: pass parse_state to add_tracepoint Dominique Martinet
@ 2024-04-07 12:18 ` Dominique Martinet
2024-04-07 12:18 ` [PATCH 3/3] perf parse: Allow names to start with digits Dominique Martinet
2 siblings, 0 replies; 7+ messages in thread
From: Dominique Martinet @ 2024-04-07 12:18 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, Jiri Olsa
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 <olsajiri@gmail.com>
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.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 3/3] perf parse: Allow names to start with digits
2024-04-07 12:18 [PATCH 0/3] perf probe: Allow names to start with digits Dominique Martinet
2024-04-07 12:18 ` [PATCH 1/3] perf parse-events: pass parse_state to add_tracepoint Dominique Martinet
2024-04-07 12:18 ` [PATCH 2/3] perf parse-events: Add new 'fake_tp' parameter for tests Dominique Martinet
@ 2024-04-07 12:18 ` Dominique Martinet
2024-04-07 12:38 ` Dominique Martinet
2 siblings, 1 reply; 7+ messages in thread
From: Dominique Martinet @ 2024-04-07 12:18 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 probe -e '9p:*'
works
Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
---
tools/perf/tests/parse-events.c | 5 +++++
tools/perf/util/parse-events.l | 2 +-
2 files changed, 6 insertions(+), 1 deletion(-)
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..41c30ff29783 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -158,7 +158,7 @@ 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 [a-zA-Z0-9_*?\[\]][a-zA-Z0-9_*?.\[\]!\-]*
name_tag [\'][a-zA-Z_*?\[\]][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.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 3/3] perf parse: Allow names to start with digits
2024-04-07 12:18 ` [PATCH 3/3] perf parse: Allow names to start with digits Dominique Martinet
@ 2024-04-07 12:38 ` Dominique Martinet
2024-04-07 18:32 ` Ian Rogers
0 siblings, 1 reply; 7+ messages in thread
From: Dominique Martinet @ 2024-04-07 12:38 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 wrote on Sun, Apr 07, 2024 at 09:18:21PM +0900:
> diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
> index e86c45675e1d..41c30ff29783 100644
> --- a/tools/perf/util/parse-events.l
> +++ b/tools/perf/util/parse-events.l
> @@ -158,7 +158,7 @@ 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 [a-zA-Z0-9_*?\[\]][a-zA-Z0-9_*?.\[\]!\-]*
grmbl sorry I guess I didn't actually test this two years ago (?!), or
used it differently (commit message is weird and also needs fixing,
perf probe -e 9p:* does not make sense) or something changed but
that's not enough:
----
$ sudo ./perf trace -e 9p:9p_fid_ref
event syntax error: '9p:9p_fid_ref'
\___ parser error
Run 'perf list' for a list of valid events
----
Adding 0-9 to name_tag as well makes perf trace works.
I'm not sure what name_minus is for but I did't need to add that one in
my test.
That also highlights that the test I added isn't sufficient, if someone
familiar with the code could hint at a better place to test please tell!
Otherwise I'll have a look next weekend, I need to get back to my 9p
bugs now I've got a working perf command..
> name_tag [\'][a-zA-Z_*?\[\]][a-zA-Z0-9_*?\-,\.\[\]:=]*[\']
> name_minus [a-zA-Z_*?][a-zA-Z0-9\-_*?.:]*
> drv_cfg_term [a-zA-Z0-9_\.]+(=[a-zA-Z0-9_*?\.:]+)?
>
--
Dominique Martinet | Asmadeus
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 3/3] perf parse: Allow names to start with digits
2024-04-07 12:38 ` Dominique Martinet
@ 2024-04-07 18:32 ` Ian Rogers
2024-04-08 12:49 ` Dominique Martinet
0 siblings, 1 reply; 7+ messages in thread
From: Ian Rogers @ 2024-04-07 18:32 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, Apr 7, 2024 at 5:38 AM Dominique Martinet
<asmadeus@codewreck.org> wrote:
>
> Dominique Martinet wrote on Sun, Apr 07, 2024 at 09:18:21PM +0900:
> > diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
> > index e86c45675e1d..41c30ff29783 100644
> > --- a/tools/perf/util/parse-events.l
> > +++ b/tools/perf/util/parse-events.l
> > @@ -158,7 +158,7 @@ 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 [a-zA-Z0-9_*?\[\]][a-zA-Z0-9_*?.\[\]!\-]*
>
> grmbl sorry I guess I didn't actually test this two years ago (?!), or
> used it differently (commit message is weird and also needs fixing,
> perf probe -e 9p:* does not make sense) or something changed but
> that's not enough:
>
> ----
> $ sudo ./perf trace -e 9p:9p_fid_ref
> event syntax error: '9p:9p_fid_ref'
> \___ parser error
> Run 'perf list' for a list of valid events
> ----
>
> Adding 0-9 to name_tag as well makes perf trace works.
>
> I'm not sure what name_minus is for but I did't need to add that one in
> my test.
>
> That also highlights that the test I added isn't sufficient, if someone
> familiar with the code could hint at a better place to test please tell!
> Otherwise I'll have a look next weekend, I need to get back to my 9p
> bugs now I've got a working perf command..
>
> > name_tag [\'][a-zA-Z_*?\[\]][a-zA-Z0-9_*?\-,\.\[\]:=]*[\']
> > name_minus [a-zA-Z_*?][a-zA-Z0-9\-_*?.:]*
> > drv_cfg_term [a-zA-Z0-9_\.]+(=[a-zA-Z0-9_*?\.:]+)?
> >
Try adding PARSER_DEBUG=1 to your command line, I need to do the following:
```
$ make EXTRA_CFLAGS="-Wno-error=redundant-decls" PARSER_DEBUG=1
```
For your example it seems to parse with the changes, but I see (which
should be expected):
```
event syntax error: '9p:9p_fid_ref'
\___ unknown tracepoint
Error: File /sys/kernel/tracing//events/9p/9p_fid_ref not found.
Hint: Perhaps this kernel misses some CONFIG_ setting to enable this feature?.
Run 'perf list' for a list of valid events
Usage: perf stat [<options>] [<command>]
-e, --event <event> event selector. use 'perf list' to list
available events
```
I think Jiri's e-mail should be jolsa@kernel.org.
Thanks,
Ian
> --
> Dominique Martinet | Asmadeus
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 3/3] perf parse: Allow names to start with digits
2024-04-07 18:32 ` Ian Rogers
@ 2024-04-08 12:49 ` Dominique Martinet
0 siblings, 0 replies; 7+ messages in thread
From: Dominique Martinet @ 2024-04-08 12:49 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
Ian Rogers wrote on Sun, Apr 07, 2024 at 11:32:31AM -0700:
> Try adding PARSER_DEBUG=1 to your command line, I need to do the following:
> ```
> $ make EXTRA_CFLAGS="-Wno-error=redundant-decls" PARSER_DEBUG=1
> ```
Thanks for this hint!
I tried with that earlier and couldn't reproduce either (e.g. it works
with this patch as you observed), looking at my shell history it turns
out I was just sleep-deprived and I had forgotten the '-e' preceding the
probe and didn't grok the error message...
So this patch is ok.
> I think Jiri's e-mail should be jolsa@kernel.org.
Ah, right -- I used the mail he actually sent the diff with a couple of
years back, but the address in maintainers is jolsa@kernel.org so you're
probably correct that it should be prefered.
I can send a v2 with just this address changed, or whoever picks the
patches up can fix the commit messages for patches 1 & 2, just tell me.
--
Dominique Martinet | Asmadeus
^ permalink raw reply [flat|nested] 7+ messages in thread