Adding support to specify raw event with 'r0<HEX>' syntax within
pmu term syntax like:
-e cpu/r0xdead/
It will be used to specify raw events in cases where they conflict
with real pmu terms, like 'read', which is valid raw event syntax,
but also a possible pmu term name as reported by Jin Yao.
Reported-by: Jin Yao <[email protected]>
Acked-by: Ian Rogers <[email protected]>
Signed-off-by: Jiri Olsa <[email protected]>
---
tools/perf/Documentation/perf-list.txt | 1 +
tools/perf/tests/parse-events.c | 5 +++++
tools/perf/util/parse-events.l | 1 +
3 files changed, 7 insertions(+)
diff --git a/tools/perf/Documentation/perf-list.txt b/tools/perf/Documentation/perf-list.txt
index 376a50b3452d..10ed539a8859 100644
--- a/tools/perf/Documentation/perf-list.txt
+++ b/tools/perf/Documentation/perf-list.txt
@@ -119,6 +119,7 @@ It's also possible to use pmu syntax:
perf record -e r1a8 -a sleep 1
perf record -e cpu/r1a8/ ...
+ perf record -e cpu/r0x1a8/ ...
You should refer to the processor specific documentation for getting these
details. Some of them are referenced in the SEE ALSO section below.
diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
index 895188b63f96..5aaddcb0058a 100644
--- a/tools/perf/tests/parse-events.c
+++ b/tools/perf/tests/parse-events.c
@@ -1766,6 +1766,11 @@ static struct evlist_test test__events_pmu[] = {
.check = test__checkevent_raw_pmu,
.id = 4,
},
+ {
+ .name = "software/r0x1a/",
+ .check = test__checkevent_raw_pmu,
+ .id = 4,
+ },
};
struct terms_test {
diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 56912c9641f5..44c85fea5d00 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -293,6 +293,7 @@ percore { return term(yyscanner, PARSE_EVENTS__TERM_TYPE_PERCORE); }
aux-output { return term(yyscanner, PARSE_EVENTS__TERM_TYPE_AUX_OUTPUT); }
aux-sample-size { return term(yyscanner, PARSE_EVENTS__TERM_TYPE_AUX_SAMPLE_SIZE); }
r{num_raw_hex} { return raw(yyscanner); }
+r0x{num_raw_hex} { return raw(yyscanner); }
, { return ','; }
"/" { BEGIN(INITIAL); return '/'; }
{name_minus} { return str(yyscanner, PE_NAME); }
--
2.25.4
Jin Yao reported issue with possible conflict between raw
events and term values in pmu event syntax.
Currently following syntax is resolved as raw event with
0xead value:
uncore_imc_free_running/read/
instead of using 'read' term from uncore_imc_free_running pmu,
because 'read' is correct raw event syntax with 0xead value.
To solve this issue we do following:
- check existing terms during rXXXX syntax processing
and make them priority in case of conflict
- allow pmu/r0x1234/ syntax to be able to specify conflicting
raw event (implemented in previous patch)
Also adding automated tests for this and perf_pmu__parse_cleanup
call to parse_events_terms, so the test gets properly cleaned up.
Reported-by: Jin Yao <[email protected]>
Signed-off-by: Jiri Olsa <[email protected]>
---
v2 changes:
- added comment to perf_pmu__test_parse_init
tools/perf/tests/parse-events.c | 37 ++++++++++++++++++++++++++++++++-
tools/perf/util/parse-events.c | 28 +++++++++++++++++++++++++
tools/perf/util/parse-events.h | 2 ++
tools/perf/util/parse-events.l | 19 ++++++++++-------
4 files changed, 77 insertions(+), 9 deletions(-)
diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
index 5aaddcb0058a..7f9f87a470c3 100644
--- a/tools/perf/tests/parse-events.c
+++ b/tools/perf/tests/parse-events.c
@@ -631,6 +631,34 @@ static int test__checkterms_simple(struct list_head *terms)
TEST_ASSERT_VAL("wrong val", term->val.num == 1);
TEST_ASSERT_VAL("wrong config", !strcmp(term->config, "umask"));
+ /*
+ * read
+ *
+ * The perf_pmu__test_parse_init injects 'read' term into
+ * perf_pmu_events_list, so 'read' is evaluated as read term
+ * and not as raw event with 'ead' hex value.
+ */
+ term = list_entry(term->list.next, struct parse_events_term, list);
+ TEST_ASSERT_VAL("wrong type term",
+ term->type_term == PARSE_EVENTS__TERM_TYPE_USER);
+ TEST_ASSERT_VAL("wrong type val",
+ term->type_val == PARSE_EVENTS__TERM_TYPE_NUM);
+ TEST_ASSERT_VAL("wrong val", term->val.num == 1);
+ TEST_ASSERT_VAL("wrong config", !strcmp(term->config, "read"));
+
+ /*
+ * r0xead
+ *
+ * To be still able to pass 'ead' value with 'r' syntax,
+ * we added support to parse 'r0xHEX' event.
+ */
+ term = list_entry(term->list.next, struct parse_events_term, list);
+ TEST_ASSERT_VAL("wrong type term",
+ term->type_term == PARSE_EVENTS__TERM_TYPE_CONFIG);
+ TEST_ASSERT_VAL("wrong type val",
+ term->type_val == PARSE_EVENTS__TERM_TYPE_NUM);
+ TEST_ASSERT_VAL("wrong val", term->val.num == 0xead);
+ TEST_ASSERT_VAL("wrong config", !term->config);
return 0;
}
@@ -1781,7 +1809,7 @@ struct terms_test {
static struct terms_test test__terms[] = {
[0] = {
- .str = "config=10,config1,config2=3,umask=1",
+ .str = "config=10,config1,config2=3,umask=1,read,r0xead",
.check = test__checkterms_simple,
},
};
@@ -1841,6 +1869,13 @@ static int test_term(struct terms_test *t)
INIT_LIST_HEAD(&terms);
+ /*
+ * The perf_pmu__test_parse_init prepares perf_pmu_events_list
+ * which gets freed in parse_events_terms.
+ */
+ if (perf_pmu__test_parse_init())
+ return -1;
+
ret = parse_events_terms(&terms, t->str);
if (ret) {
pr_debug("failed to parse terms '%s', err %d\n",
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index e88e4c7a2a9a..9f7260e69113 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -2019,6 +2019,32 @@ static void perf_pmu__parse_init(void)
perf_pmu__parse_cleanup();
}
+/*
+ * This function injects special term in
+ * perf_pmu_events_list so the test code
+ * can check on this functionality.
+ */
+int perf_pmu__test_parse_init(void)
+{
+ struct perf_pmu_event_symbol *list;
+
+ list = malloc(sizeof(*list) * 1);
+ if (!list)
+ return -ENOMEM;
+
+ list->type = PMU_EVENT_SYMBOL;
+ list->symbol = strdup("read");
+
+ if (!list->symbol) {
+ free(list);
+ return -ENOMEM;
+ }
+
+ perf_pmu_events_list = list;
+ perf_pmu_events_list_num = 1;
+ return 0;
+}
+
enum perf_pmu_event_symbol_type
perf_pmu__parse_check(const char *name)
{
@@ -2080,6 +2106,8 @@ int parse_events_terms(struct list_head *terms, const char *str)
int ret;
ret = parse_events__scanner(str, &parse_state);
+ perf_pmu__parse_cleanup();
+
if (!ret) {
list_splice(parse_state.terms, terms);
zfree(&parse_state.terms);
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index f0010095fc8c..00cde7d2e30c 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -261,4 +261,6 @@ static inline bool is_sdt_event(char *str __maybe_unused)
}
#endif /* HAVE_LIBELF_SUPPORT */
+int perf_pmu__test_parse_init(void);
+
#endif /* __PERF_PARSE_EVENTS_H */
diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 44c85fea5d00..3ca5fd2829ca 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -41,14 +41,6 @@ static int value(yyscan_t scanner, int base)
return __value(yylval, text, base, PE_VALUE);
}
-static int raw(yyscan_t scanner)
-{
- YYSTYPE *yylval = parse_events_get_lval(scanner);
- char *text = parse_events_get_text(scanner);
-
- return __value(yylval, text + 1, 16, PE_RAW);
-}
-
static int str(yyscan_t scanner, int token)
{
YYSTYPE *yylval = parse_events_get_lval(scanner);
@@ -72,6 +64,17 @@ static int str(yyscan_t scanner, int token)
return token;
}
+static int raw(yyscan_t scanner)
+{
+ YYSTYPE *yylval = parse_events_get_lval(scanner);
+ char *text = parse_events_get_text(scanner);
+
+ if (perf_pmu__parse_check(text) == PMU_EVENT_SYMBOL)
+ return str(scanner, PE_NAME);
+
+ return __value(yylval, text + 1, 16, PE_RAW);
+}
+
static bool isbpf_suffix(char *text)
{
int len = strlen(text);
--
2.25.4
On Sun, Jul 26, 2020 at 12:53 AM Jiri Olsa <[email protected]> wrote:
>
> Jin Yao reported issue with possible conflict between raw
> events and term values in pmu event syntax.
>
> Currently following syntax is resolved as raw event with
> 0xead value:
> uncore_imc_free_running/read/
>
> instead of using 'read' term from uncore_imc_free_running pmu,
> because 'read' is correct raw event syntax with 0xead value.
>
> To solve this issue we do following:
> - check existing terms during rXXXX syntax processing
> and make them priority in case of conflict
> - allow pmu/r0x1234/ syntax to be able to specify conflicting
> raw event (implemented in previous patch)
>
> Also adding automated tests for this and perf_pmu__parse_cleanup
> call to parse_events_terms, so the test gets properly cleaned up.
>
> Reported-by: Jin Yao <[email protected]>
> Signed-off-by: Jiri Olsa <[email protected]>
> ---
> v2 changes:
> - added comment to perf_pmu__test_parse_init
Acked-by: Ian Rogers <[email protected]>
Thanks,
Ian
> tools/perf/tests/parse-events.c | 37 ++++++++++++++++++++++++++++++++-
> tools/perf/util/parse-events.c | 28 +++++++++++++++++++++++++
> tools/perf/util/parse-events.h | 2 ++
> tools/perf/util/parse-events.l | 19 ++++++++++-------
> 4 files changed, 77 insertions(+), 9 deletions(-)
>
> diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
> index 5aaddcb0058a..7f9f87a470c3 100644
> --- a/tools/perf/tests/parse-events.c
> +++ b/tools/perf/tests/parse-events.c
> @@ -631,6 +631,34 @@ static int test__checkterms_simple(struct list_head *terms)
> TEST_ASSERT_VAL("wrong val", term->val.num == 1);
> TEST_ASSERT_VAL("wrong config", !strcmp(term->config, "umask"));
>
> + /*
> + * read
> + *
> + * The perf_pmu__test_parse_init injects 'read' term into
> + * perf_pmu_events_list, so 'read' is evaluated as read term
> + * and not as raw event with 'ead' hex value.
> + */
> + term = list_entry(term->list.next, struct parse_events_term, list);
> + TEST_ASSERT_VAL("wrong type term",
> + term->type_term == PARSE_EVENTS__TERM_TYPE_USER);
> + TEST_ASSERT_VAL("wrong type val",
> + term->type_val == PARSE_EVENTS__TERM_TYPE_NUM);
> + TEST_ASSERT_VAL("wrong val", term->val.num == 1);
> + TEST_ASSERT_VAL("wrong config", !strcmp(term->config, "read"));
> +
> + /*
> + * r0xead
> + *
> + * To be still able to pass 'ead' value with 'r' syntax,
> + * we added support to parse 'r0xHEX' event.
> + */
> + term = list_entry(term->list.next, struct parse_events_term, list);
> + TEST_ASSERT_VAL("wrong type term",
> + term->type_term == PARSE_EVENTS__TERM_TYPE_CONFIG);
> + TEST_ASSERT_VAL("wrong type val",
> + term->type_val == PARSE_EVENTS__TERM_TYPE_NUM);
> + TEST_ASSERT_VAL("wrong val", term->val.num == 0xead);
> + TEST_ASSERT_VAL("wrong config", !term->config);
> return 0;
> }
>
> @@ -1781,7 +1809,7 @@ struct terms_test {
>
> static struct terms_test test__terms[] = {
> [0] = {
> - .str = "config=10,config1,config2=3,umask=1",
> + .str = "config=10,config1,config2=3,umask=1,read,r0xead",
> .check = test__checkterms_simple,
> },
> };
> @@ -1841,6 +1869,13 @@ static int test_term(struct terms_test *t)
>
> INIT_LIST_HEAD(&terms);
>
> + /*
> + * The perf_pmu__test_parse_init prepares perf_pmu_events_list
> + * which gets freed in parse_events_terms.
> + */
> + if (perf_pmu__test_parse_init())
> + return -1;
> +
> ret = parse_events_terms(&terms, t->str);
> if (ret) {
> pr_debug("failed to parse terms '%s', err %d\n",
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index e88e4c7a2a9a..9f7260e69113 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -2019,6 +2019,32 @@ static void perf_pmu__parse_init(void)
> perf_pmu__parse_cleanup();
> }
>
> +/*
> + * This function injects special term in
> + * perf_pmu_events_list so the test code
> + * can check on this functionality.
> + */
> +int perf_pmu__test_parse_init(void)
> +{
> + struct perf_pmu_event_symbol *list;
> +
> + list = malloc(sizeof(*list) * 1);
> + if (!list)
> + return -ENOMEM;
> +
> + list->type = PMU_EVENT_SYMBOL;
> + list->symbol = strdup("read");
> +
> + if (!list->symbol) {
> + free(list);
> + return -ENOMEM;
> + }
> +
> + perf_pmu_events_list = list;
> + perf_pmu_events_list_num = 1;
> + return 0;
> +}
> +
> enum perf_pmu_event_symbol_type
> perf_pmu__parse_check(const char *name)
> {
> @@ -2080,6 +2106,8 @@ int parse_events_terms(struct list_head *terms, const char *str)
> int ret;
>
> ret = parse_events__scanner(str, &parse_state);
> + perf_pmu__parse_cleanup();
> +
> if (!ret) {
> list_splice(parse_state.terms, terms);
> zfree(&parse_state.terms);
> diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
> index f0010095fc8c..00cde7d2e30c 100644
> --- a/tools/perf/util/parse-events.h
> +++ b/tools/perf/util/parse-events.h
> @@ -261,4 +261,6 @@ static inline bool is_sdt_event(char *str __maybe_unused)
> }
> #endif /* HAVE_LIBELF_SUPPORT */
>
> +int perf_pmu__test_parse_init(void);
> +
> #endif /* __PERF_PARSE_EVENTS_H */
> diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
> index 44c85fea5d00..3ca5fd2829ca 100644
> --- a/tools/perf/util/parse-events.l
> +++ b/tools/perf/util/parse-events.l
> @@ -41,14 +41,6 @@ static int value(yyscan_t scanner, int base)
> return __value(yylval, text, base, PE_VALUE);
> }
>
> -static int raw(yyscan_t scanner)
> -{
> - YYSTYPE *yylval = parse_events_get_lval(scanner);
> - char *text = parse_events_get_text(scanner);
> -
> - return __value(yylval, text + 1, 16, PE_RAW);
> -}
> -
> static int str(yyscan_t scanner, int token)
> {
> YYSTYPE *yylval = parse_events_get_lval(scanner);
> @@ -72,6 +64,17 @@ static int str(yyscan_t scanner, int token)
> return token;
> }
>
> +static int raw(yyscan_t scanner)
> +{
> + YYSTYPE *yylval = parse_events_get_lval(scanner);
> + char *text = parse_events_get_text(scanner);
> +
> + if (perf_pmu__parse_check(text) == PMU_EVENT_SYMBOL)
> + return str(scanner, PE_NAME);
> +
> + return __value(yylval, text + 1, 16, PE_RAW);
> +}
> +
> static bool isbpf_suffix(char *text)
> {
> int len = strlen(text);
> --
> 2.25.4
>
On 7/26/2020 3:52 PM, Jiri Olsa wrote:
> Jin Yao reported issue with possible conflict between raw
> events and term values in pmu event syntax.
>
> Currently following syntax is resolved as raw event with
> 0xead value:
> uncore_imc_free_running/read/
>
> instead of using 'read' term from uncore_imc_free_running pmu,
> because 'read' is correct raw event syntax with 0xead value.
>
> To solve this issue we do following:
> - check existing terms during rXXXX syntax processing
> and make them priority in case of conflict
> - allow pmu/r0x1234/ syntax to be able to specify conflicting
> raw event (implemented in previous patch)
>
> Also adding automated tests for this and perf_pmu__parse_cleanup
> call to parse_events_terms, so the test gets properly cleaned up.
>
> Reported-by: Jin Yao<[email protected]>
> Signed-off-by: Jiri Olsa<[email protected]>
> ---
> v2 changes:
> - added comment to perf_pmu__test_parse_init
Acked-by: Jin Yao <[email protected]>
Thanks
Jin Yao
On 7/27/2020 8:21 AM, Jin, Yao wrote:
>
>
> On 7/26/2020 3:52 PM, Jiri Olsa wrote:
>> Jin Yao reported issue with possible conflict between raw
>> events and term values in pmu event syntax.
>>
>> Currently following syntax is resolved as raw event with
>> 0xead value:
>> ?? uncore_imc_free_running/read/
>>
>> instead of using 'read' term from uncore_imc_free_running pmu,
>> because 'read' is correct raw event syntax with 0xead value.
>>
>> To solve this issue we do following:
>> ?? - check existing terms during rXXXX syntax processing
>> ???? and make them priority in case of conflict
>> ?? - allow pmu/r0x1234/ syntax to be able to specify conflicting
>> ???? raw event (implemented in previous patch)
>>
>> Also adding automated tests for this and perf_pmu__parse_cleanup
>> call to parse_events_terms, so the test gets properly cleaned up.
>>
>> Reported-by: Jin Yao<[email protected]>
>> Signed-off-by: Jiri Olsa<[email protected]>
>> ---
>> v2 changes:
>> ? - added comment to perf_pmu__test_parse_init
>
> Acked-by: Jin Yao <[email protected]>
>
> Thanks
> Jin Yao
Also added with:
Fixes: 3a6c51e4d66c ("perf parser: Add support to specify rXXX event with pmu")?
Thanks
Jin Yao
Em Mon, Jul 27, 2020 at 08:26:27AM +0800, Jin, Yao escreveu:
>
>
> On 7/27/2020 8:21 AM, Jin, Yao wrote:
> >
> >
> > On 7/26/2020 3:52 PM, Jiri Olsa wrote:
> > > Jin Yao reported issue with possible conflict between raw
> > > events and term values in pmu event syntax.
> > >
> > > Currently following syntax is resolved as raw event with
> > > 0xead value:
> > > ?? uncore_imc_free_running/read/
> > >
> > > instead of using 'read' term from uncore_imc_free_running pmu,
> > > because 'read' is correct raw event syntax with 0xead value.
> > >
> > > To solve this issue we do following:
> > > ?? - check existing terms during rXXXX syntax processing
> > > ???? and make them priority in case of conflict
> > > ?? - allow pmu/r0x1234/ syntax to be able to specify conflicting
> > > ???? raw event (implemented in previous patch)
> > >
> > > Also adding automated tests for this and perf_pmu__parse_cleanup
> > > call to parse_events_terms, so the test gets properly cleaned up.
> > >
> > > Reported-by: Jin Yao<[email protected]>
> > > Signed-off-by: Jiri Olsa<[email protected]>
> > > ---
> > > v2 changes:
> > > ? - added comment to perf_pmu__test_parse_init
> >
> > Acked-by: Jin Yao <[email protected]>
> >
> > Thanks
> > Jin Yao
>
> Also added with:
> Fixes: 3a6c51e4d66c ("perf parser: Add support to specify rXXX event with pmu")?
Thanks, applied.
- Arnaldo