[PATCH] perf, tools: Handle events including .c and .o

[PATCH] perf, tools: Handle events including .c and .o

[Andi Kleen]

From: Andi Kleen <ak@linux.intel.com>

This is a generic bug fix, but it helps with Sukadev's JSON event tree
where such events can happen.

Any event inclduing a .c/.o/.bpf currently triggers BPF compilation or loading
and then an error.  This can happen for some Intel JSON events, which cannot
be used.

Fix the scanner to only match for .o or .c or .bpf at the end.
This will prevent loading multiple BPF scripts separated with comma,
but I assume this is acceptable.

Cc: wangnan0@huawei.com
Cc: sukadev@linux.vnet.ibm.com
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/util/parse-events.l | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 7a2519435da0..64ca26e4ed2d 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -162,8 +162,8 @@ modifier_bp	[rwx]{1,3}
 		}
 
 {event_pmu}	|
-{bpf_object}	|
-{bpf_source}	|
+({bpf_object}$)	|
+({bpf_source}$)	|
 {event}		{
 			BEGIN(INITIAL);
 			REWIND(1);
-- 
2.5.5

Re: [PATCH] perf, tools: Handle events including .c and .o

[Jiri Olsa]

On Sat, Sep 17, 2016 at 06:02:46PM -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> This is a generic bug fix, but it helps with Sukadev's JSON event tree
> where such events can happen.
> 
> Any event inclduing a .c/.o/.bpf currently triggers BPF compilation or loading
> and then an error.  This can happen for some Intel JSON events, which cannot
> be used.
> 
> Fix the scanner to only match for .o or .c or .bpf at the end.
> This will prevent loading multiple BPF scripts separated with comma,
> but I assume this is acceptable.
> 
> Cc: wangnan0@huawei.com
> Cc: sukadev@linux.vnet.ibm.com
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> ---
>  tools/perf/util/parse-events.l | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
> index 7a2519435da0..64ca26e4ed2d 100644
> --- a/tools/perf/util/parse-events.l
> +++ b/tools/perf/util/parse-events.l
> @@ -162,8 +162,8 @@ modifier_bp	[rwx]{1,3}
>  		}
>  
>  {event_pmu}	|
> -{bpf_object}	|
> -{bpf_source}	|
> +({bpf_object}$)	|
> +({bpf_source}$)	|

why are the () braces necessary? anyway:

Acked-by: Jiri Olsa <jolsa@kernel.org>

thanks,
jirka

>  {event}		{
>  			BEGIN(INITIAL);
>  			REWIND(1);
> -- 
> 2.5.5
> 

Re: [PATCH] perf, tools: Handle events including .c and .o

[Andi Kleen]

On Sun, Sep 18, 2016 at 11:03:27AM +0200, Jiri Olsa wrote:
> On Sat, Sep 17, 2016 at 06:02:46PM -0700, Andi Kleen wrote:
> > From: Andi Kleen <ak@linux.intel.com>
> > 
> > This is a generic bug fix, but it helps with Sukadev's JSON event tree
> > where such events can happen.
> > 
> > Any event inclduing a .c/.o/.bpf currently triggers BPF compilation or loading
> > and then an error.  This can happen for some Intel JSON events, which cannot
> > be used.
> > 
> > Fix the scanner to only match for .o or .c or .bpf at the end.
> > This will prevent loading multiple BPF scripts separated with comma,
> > but I assume this is acceptable.
> > 
> > Cc: wangnan0@huawei.com
> > Cc: sukadev@linux.vnet.ibm.com
> > Signed-off-by: Andi Kleen <ak@linux.intel.com>
> > ---
> >  tools/perf/util/parse-events.l | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
> > index 7a2519435da0..64ca26e4ed2d 100644
> > --- a/tools/perf/util/parse-events.l
> > +++ b/tools/perf/util/parse-events.l
> > @@ -162,8 +162,8 @@ modifier_bp	[rwx]{1,3}
> >  		}
> >  
> >  {event_pmu}	|
> > -{bpf_object}	|
> > -{bpf_source}	|
> > +({bpf_object}$)	|
> > +({bpf_source}$)	|
> 
> why are the () braces necessary? anyway:

Flex complains otherwise, I think because it's used with OR (|)

-Andi

Re: [PATCH] perf, tools: Handle events including .c and .o

[Wangnan (F)]

On 2016/9/18 9:02, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
>
> This is a generic bug fix, but it helps with Sukadev's JSON event tree
> where such events can happen.
>
> Any event inclduing a .c/.o/.bpf currently triggers BPF compilation or loading
> and then an error.  This can happen for some Intel JSON events, which cannot
> be used.
>
> Fix the scanner to only match for .o or .c or .bpf at the end.
> This will prevent loading multiple BPF scripts separated with comma,
> but I assume this is acceptable.
>
> Cc: wangnan0@huawei.com
> Cc: sukadev@linux.vnet.ibm.com
> Signed-off-by: Andi Kleen <ak@linux.intel.com>

I tested '.c' in middle of an event:

  # perf trace --event 'aaa.ccc'
  invalid or unsupported event: 'aaa.ccc'
  Run 'perf list' for a list of valid events
  ...

It is not recongnized as a BPF source.

So could you please provide an example to show how
this potential bug breaks the parsing of new events?

> ---
>   tools/perf/util/parse-events.l | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
> index 7a2519435da0..64ca26e4ed2d 100644
> --- a/tools/perf/util/parse-events.l
> +++ b/tools/perf/util/parse-events.l
> @@ -162,8 +162,8 @@ modifier_bp	[rwx]{1,3}
>   		}
>   
>   {event_pmu}	|
> -{bpf_object}	|
> -{bpf_source}	|
> +({bpf_object}$)	|
> +({bpf_source}$)	|

What about putting '$' at the definition of bpf_xxx like this?

diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 9f43fda..d9ff690 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -136,8 +136,8 @@ do 
{                                                        \
  group          [^,{}/]*[{][^}]*[}][^,{}/]*
  event_pmu      [^,{}/]+[/][^/]*[/][^,{}/]*
  event          [^,{}/]+
-bpf_object     .*\.(o|bpf)
-bpf_source     .*\.c
+bpf_object     .*\.(o|bpf)$
+bpf_source     .*\.c$

  num_dec                [0-9]+
  num_hex                0x[a-fA-F0-9]+

Thank you.

>   {event}		{
>   			BEGIN(INITIAL);
>   			REWIND(1);

Re: [PATCH] perf, tools: Handle events including .c and .o

[Andi Kleen]

On Sun, Sep 18, 2016 at 06:20:04PM +0800, Wangnan (F) wrote:
> 
> 
> On 2016/9/18 9:02, Andi Kleen wrote:
> > From: Andi Kleen <ak@linux.intel.com>
> > 
> > This is a generic bug fix, but it helps with Sukadev's JSON event tree
> > where such events can happen.
> > 
> > Any event inclduing a .c/.o/.bpf currently triggers BPF compilation or loading
> > and then an error.  This can happen for some Intel JSON events, which cannot
> > be used.
> > 
> > Fix the scanner to only match for .o or .c or .bpf at the end.
> > This will prevent loading multiple BPF scripts separated with comma,
> > but I assume this is acceptable.
> > 
> > Cc: wangnan0@huawei.com
> > Cc: sukadev@linux.vnet.ibm.com
> > Signed-off-by: Andi Kleen <ak@linux.intel.com>
> 
> I tested '.c' in middle of an event:
> 
>  # perf trace --event 'aaa.ccc'
>  invalid or unsupported event: 'aaa.ccc'
>  Run 'perf list' for a list of valid events
>  ...
> 
> It is not recongnized as a BPF source.
> 
> So could you please provide an example to show how
> this potential bug breaks the parsing of new events?

This is with the upcoming JSON uncore events:

$ perf stat -e '{unc_p_clockticks,unc_p_power_state_occupancy.cores_c0}' -a -I 1000
ERROR: problems with path {unc_p_clockticks,unc_p_power_state_occupancy.c: No such file or directory
event syntax error: '{unc_p_clockticks,unc_p_power_state_occupancy.cores_c0}'
                     \___ Failed to load {unc_p_clockticks,unc_p_power_state_occupancy.c from source: Error when compiling BPF scriptlet

(add -v to see detail)
Run 'perf list' for a list of valid events

-Andi

Re: [PATCH] perf, tools: Handle events including .c and .o

[Wangnan (F)]

On 2016/9/18 22:56, Andi Kleen wrote:
> On Sun, Sep 18, 2016 at 06:20:04PM +0800, Wangnan (F) wrote:
>>
>> On 2016/9/18 9:02, Andi Kleen wrote:
>>> From: Andi Kleen <ak@linux.intel.com>
>>>
>>> This is a generic bug fix, but it helps with Sukadev's JSON event tree
>>> where such events can happen.
>>>
>>> Any event inclduing a .c/.o/.bpf currently triggers BPF compilation or loading
>>> and then an error.  This can happen for some Intel JSON events, which cannot
>>> be used.
>>>
>>> Fix the scanner to only match for .o or .c or .bpf at the end.
>>> This will prevent loading multiple BPF scripts separated with comma,
>>> but I assume this is acceptable.
>>>
>>> Cc: wangnan0@huawei.com
>>> Cc: sukadev@linux.vnet.ibm.com
>>> Signed-off-by: Andi Kleen <ak@linux.intel.com>
>> I tested '.c' in middle of an event:
>>
>>   # perf trace --event 'aaa.ccc'
>>   invalid or unsupported event: 'aaa.ccc'
>>   Run 'perf list' for a list of valid events
>>   ...
>>
>> It is not recongnized as a BPF source.
>>
>> So could you please provide an example to show how
>> this potential bug breaks the parsing of new events?
> This is with the upcoming JSON uncore events:
>
> $ perf stat -e '{unc_p_clockticks,unc_p_power_state_occupancy.cores_c0}' -a -I 1000
> ERROR: problems with path {unc_p_clockticks,unc_p_power_state_occupancy.c: No such file or directory
> event syntax error: '{unc_p_clockticks,unc_p_power_state_occupancy.cores_c0}'
>                       \___ Failed to load {unc_p_clockticks,unc_p_power_state_occupancy.c from source: Error when compiling BPF scriptlet
>
> (add -v to see detail)
> Run 'perf list' for a list of valid events
>
> -Andi

I see, and your patch solve problem like this.

Tested-by: Wang Nan <wangnan0@huawei.com>

Re: [PATCH 1/3] perf, tools: Handle events including .c and .o

[Arnaldo Carvalho de Melo]

Em Wed, Oct 05, 2016 at 12:47:10PM -0700, Andi Kleen escreveu:
> From: Andi Kleen <ak@linux.intel.com>
> 
> This is a generic bug fix, but it helps with Sukadev's JSON event tree
> where such events can happen.
> 
> Any event inclduing a .c/.o/.bpf currently triggers BPF compilation or loading
> and then an error.  This can happen for some Intel JSON events, which cannot
> be used.
> 
> Fix the scanner to only match for .o or .c or .bpf at the end.
> This will prevent loading multiple BPF scripts separated with comma,
> but I assume this is acceptable.

So, I tried it with the example provided in the thread for a previous
version of this patch (IIRC) and it still fails:


[acme@jouet linux]$ perf stat -e '{unc_p_clockticks,unc_p_power_state_occupancy.cores_c0}' -a -I 1000
ERROR: problems with path {unc_p_clockticks,unc_p_power_state_occupancy.c: No such file or directory
event syntax error: '{unc_p_clockticks,unc_p_power_state_occupancy.cores_c0}'
                     \___ Failed to load {unc_p_clockticks,unc_p_power_state_occupancy.c from source: Error when compiling BPF scriptlet

(add -v to see detail)
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
[acme@jouet linux]$

And with another event that for sure is available on this machine:



[acme@jouet linux]$ perf stat -e '{uops_executed.core_cycles_ge_2}' -I 1000 usleep 10
ERROR: problems with path {uops_executed.c: No such file or directory
event syntax error: '{uops_executed.core_cycles_ge_2}'
                     \___ Failed to load {uops_executed.c from source: Error when compiling BPF scriptlet

(add -v to see detail)
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
[acme@jouet linux]$


I thought this was due to the Makefile not noticing the change in the .l files, but I made
sure I deleted the build dir and rebuilt from scratch, same problem.

- Arnaldo
 
> Cc: wangnan0@huawei.com
> Cc: sukadev@linux.vnet.ibm.com
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> ---
>  tools/perf/util/parse-events.l | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
> index 9f43fda2570f..377147088a46 100644
> --- a/tools/perf/util/parse-events.l
> +++ b/tools/perf/util/parse-events.l
> @@ -183,8 +183,8 @@ modifier_bp	[rwx]{1,3}
>  		}
>  
>  {event_pmu}	|
> -{bpf_object}	|
> -{bpf_source}	|
> +({bpf_object}$)	|
> +({bpf_source}$)	|
>  {event}		{
>  			BEGIN(INITIAL);
>  			REWIND(1);
> -- 
> 2.5.5

[PATCH] perf, tools: Handle events including .c and .o

[Wang Nan]

This patch helps with Sukadev's JSON event tree where such events can happen.

>From Andi Kleen:
 Any event inclduing a .c/.o/.bpf currently triggers BPF compilation or loading
 and then an error. This can happen for some Intel JSON events, which cannot
 be used.

This patch fixes this problem by forbidding BPF file patch containing '{', '}'
and ',', make sure flex consumes the leading '{', instead of matcing it using
a BPF file path.

Tested result:

  $ perf stat -e '{unc_p_clockticks,unc_p_power_state_occupancy.cores_c0}' -a -I 1000
  invalid or unsupported event: '{unc_p_clockticks,unc_p_power_state_occupancy.cores_c0}'
  Run 'perf list' for a list of valid events
  (as expected, interperted as event)

  $ perf stat -e 'aaa.c' -a -I 1000
  ERROR: problems with path aaa.c: No such file or directory
  (as expected, interperted as BPF source)

  $ perf stat -e 'aaa.ccc' -a -I 1000
  invalid or unsupported event: 'aaa.ccc'
  (as expected, interperted as event)

  $ perf stat -e '{aaa.c}' -a -I 1000
  ERROR: problems with path aaa.c: No such file or directory
  event syntax error: '{aaa.c}'
  <SKIP>
  (as expected, interperted as BPF source)

  $ perf stat -e '{cycles,aaa.c}' -a -I 1000
  ERROR: problems with path aaa.c: No such file or directory
  event syntax error: '{cycles,aaa.c}'
  (as expected, interperted as BPF source)

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/parse-events.l | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 9f43fda..660fca0 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -136,8 +136,8 @@ do {							\
 group		[^,{}/]*[{][^}]*[}][^,{}/]*
 event_pmu	[^,{}/]+[/][^/]*[/][^,{}/]*
 event		[^,{}/]+
-bpf_object	.*\.(o|bpf)
-bpf_source	.*\.c
+bpf_object	[^,{}]+\.(o|bpf)
+bpf_source	[^,{}]+\.c
 
 num_dec		[0-9]+
 num_hex		0x[a-fA-F0-9]+
-- 
1.8.3.4