Re: [PATCH v6 2/3] perf tools: Add lexical definition of event name

On Tue, Feb 16, 2021 at 05:23:18PM +0900, Namhyung Kim wrote:

On Mon, Feb 15, 2021 at 9:34 PM Jiri Olsa [off-list ref] wrote:

quoted

On Fri, Feb 12, 2021 at 06:03:17PM +0900, Shunsuke Nakamura wrote:

quoted

Add the lexical definition of event name so that the numbers are recognizable.

A64FX defines an event name that starts with a number.
 - 0inst_commit
 - 1inst_commit
 - 2inst_commit
 - 3inst_commit
 - 4inst_commit

Signed-off-by: Shunsuke Nakamura <redacted>
---
 tools/perf/util/parse-events.l | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 0b36285a9435..33f627187415 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l

@@ -205,7 +205,7 @@ bpf_source        [^,{}]+\.c[a-zA-Z0-9._]*
 num_dec              [0-9]+
 num_hex              0x[a-fA-F0-9]+
 num_raw_hex  [a-fA-F0-9]+
-name         [a-zA-Z_*?\[\]][a-zA-Z0-9_*?.\[\]]*
+name         [a-zA-Z0-9_*?\[\]][a-zA-Z0-9_*?.\[\]]*

I can't see anything broken because of this change,
also numbers are defined few lines before so we should
be fine

But this change makes the syntax ambiguous with numbers
and names. I don't think it's a good idea.

I'll check again, but we already have hex numbers above
which could easily clash, but the syntax context makes
this work.. tests should help to clear this up

Is it possible to change the event names not having numbers
at the beginning like inst_commit0, inst_commit1 and so on?

that'd be of course much better

thanks,
jirka

Thanks,
Namhyung

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel