[patch] perf tools: perf list broken on ARM

[patch] perf tools: perf list broken on ARM

[Vince Weaver]

"perf list" listing of hardware events doesn't work on older ARM devices.
The change enabling event detection:

 commit b41f1cec91c37eeea6fdb15effbfa24ea0a5536b
 Author: Namhyung Kim <namhyung.kim@lge.com>
 Date:   Tue Aug 27 11:41:53 2013 +0900

     perf list: Skip unsupported events


uses the following code in tools/perf/util/parse-events.c:

        struct perf_event_attr attr = {
                .type = type,
                .config = config,
                .disabled = 1,
                .exclude_kernel = 1,
        };

On ARM machines pre-dating the Cortex-A15 this doesn't work, as
these machines don't support .exclude_kernel.  So starting with 3.12
"perf list" does not report any hardware events at all on older
machines (seen on Rasp-Pi, Pandaboard, Beagleboard, etc).

This version of the patch makes changes suggested by Namhyung Kim
to check for EACCESS and retry (instead of just dropping
the exclude_kernel) so we can properly handle machines where 
/proc/sys/kernel/perf_event_paranoid is set to 2.

Reported-by: Chad Paradis <chad.paradis@umit.maine.edu>
Signed-off-by: Vince Weaver <vincent.weaver@maine.edu>
Acked-by: Namhyung Kim <namhyung@kernel.org>

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 6de6f89..1fa98b9 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1082,12 +1082,12 @@ int is_valid_tracepoint(const char *event_string)
 static bool is_event_supported(u8 type, unsigned config)
 {
 	bool ret = true;
+	int open_return;
 	struct perf_evsel *evsel;
 	struct perf_event_attr attr = {
 		.type = type,
 		.config = config,
 		.disabled = 1,
-		.exclude_kernel = 1,
 	};
 	struct {
 		struct thread_map map;
@@ -1099,7 +1099,20 @@ static bool is_event_supported(u8 type, unsigned config)
 
 	evsel = perf_evsel__new(&attr);
 	if (evsel) {
-		ret = perf_evsel__open(evsel, NULL, &tmap.map) >= 0;
+		open_return = perf_evsel__open(evsel, NULL, &tmap.map);
+		ret = open_return >= 0;
+
+		if (open_return == -EACCES) {
+			/*
+			 * This happens if the paranoid value
+			 * /proc/sys/kernel/perf_event_paranoid is set to 2
+			 * Re-run with exclude_kernel set; we don't do that
+			 * by default as some ARM machines do not support it.
+			 *
+			 */
+			evsel->attr.exclude_kernel = 1;
+			ret = perf_evsel__open(evsel, NULL, &tmap.map) >= 0;
+		}
 		perf_evsel__delete(evsel);
 	}
 

Re: [patch] perf tools: perf list broken on ARM

[Vince Weaver]

just checking on the status of this patch.  Am I sending it to the wrong 
place?  Did it get lost in the post-holiday e-mail purges?  

The bug is annoying if you're trying to use perf on ARM systems.

Vince

On Mon, 30 Dec 2013, Vince Weaver wrote:

> 
> "perf list" listing of hardware events doesn't work on older ARM devices.
> The change enabling event detection:
> 
>  commit b41f1cec91c37eeea6fdb15effbfa24ea0a5536b
>  Author: Namhyung Kim <namhyung.kim@lge.com>
>  Date:   Tue Aug 27 11:41:53 2013 +0900
> 
>      perf list: Skip unsupported events
> 
> 
> uses the following code in tools/perf/util/parse-events.c:
> 
>         struct perf_event_attr attr = {
>                 .type = type,
>                 .config = config,
>                 .disabled = 1,
>                 .exclude_kernel = 1,
>         };
> 
> On ARM machines pre-dating the Cortex-A15 this doesn't work, as
> these machines don't support .exclude_kernel.  So starting with 3.12
> "perf list" does not report any hardware events at all on older
> machines (seen on Rasp-Pi, Pandaboard, Beagleboard, etc).
> 
> This version of the patch makes changes suggested by Namhyung Kim
> to check for EACCESS and retry (instead of just dropping
> the exclude_kernel) so we can properly handle machines where 
> /proc/sys/kernel/perf_event_paranoid is set to 2.
> 
> Reported-by: Chad Paradis <chad.paradis@umit.maine.edu>
> Signed-off-by: Vince Weaver <vincent.weaver@maine.edu>
> Acked-by: Namhyung Kim <namhyung@kernel.org>
> 
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 6de6f89..1fa98b9 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -1082,12 +1082,12 @@ int is_valid_tracepoint(const char *event_string)
>  static bool is_event_supported(u8 type, unsigned config)
>  {
>  	bool ret = true;
> +	int open_return;
>  	struct perf_evsel *evsel;
>  	struct perf_event_attr attr = {
>  		.type = type,
>  		.config = config,
>  		.disabled = 1,
> -		.exclude_kernel = 1,
>  	};
>  	struct {
>  		struct thread_map map;
> @@ -1099,7 +1099,20 @@ static bool is_event_supported(u8 type, unsigned config)
>  
>  	evsel = perf_evsel__new(&attr);
>  	if (evsel) {
> -		ret = perf_evsel__open(evsel, NULL, &tmap.map) >= 0;
> +		open_return = perf_evsel__open(evsel, NULL, &tmap.map);
> +		ret = open_return >= 0;
> +
> +		if (open_return == -EACCES) {
> +			/*
> +			 * This happens if the paranoid value
> +			 * /proc/sys/kernel/perf_event_paranoid is set to 2
> +			 * Re-run with exclude_kernel set; we don't do that
> +			 * by default as some ARM machines do not support it.
> +			 *
> +			 */
> +			evsel->attr.exclude_kernel = 1;
> +			ret = perf_evsel__open(evsel, NULL, &tmap.map) >= 0;
> +		}
>  		perf_evsel__delete(evsel);
>  	}
>  
> 

Vince Weaver
vincent.weaver@maine.edu
http://www.eece.maine.edu/~vweaver/

Re: [patch] perf tools: perf list broken on ARM

[Namhyung Kim]

Hi Vince,

On Wed, 8 Jan 2014 16:30:17 -0500 (EST), Vince Weaver wrote:
> just checking on the status of this patch.  Am I sending it to the wrong 
> place?  Did it get lost in the post-holiday e-mail purges?  
>
> The bug is annoying if you're trying to use perf on ARM systems.

Right, acme, would you please merge this?

Thanks,
Namhyung

>
> On Mon, 30 Dec 2013, Vince Weaver wrote:
>
>> 
>> "perf list" listing of hardware events doesn't work on older ARM devices.
>> The change enabling event detection:
>> 
>>  commit b41f1cec91c37eeea6fdb15effbfa24ea0a5536b
>>  Author: Namhyung Kim <namhyung.kim@lge.com>
>>  Date:   Tue Aug 27 11:41:53 2013 +0900
>> 
>>      perf list: Skip unsupported events
>> 
>> 
>> uses the following code in tools/perf/util/parse-events.c:
>> 
>>         struct perf_event_attr attr = {
>>                 .type = type,
>>                 .config = config,
>>                 .disabled = 1,
>>                 .exclude_kernel = 1,
>>         };
>> 
>> On ARM machines pre-dating the Cortex-A15 this doesn't work, as
>> these machines don't support .exclude_kernel.  So starting with 3.12
>> "perf list" does not report any hardware events at all on older
>> machines (seen on Rasp-Pi, Pandaboard, Beagleboard, etc).
>> 
>> This version of the patch makes changes suggested by Namhyung Kim
>> to check for EACCESS and retry (instead of just dropping
>> the exclude_kernel) so we can properly handle machines where 
>> /proc/sys/kernel/perf_event_paranoid is set to 2.
>> 
>> Reported-by: Chad Paradis <chad.paradis@umit.maine.edu>
>> Signed-off-by: Vince Weaver <vincent.weaver@maine.edu>
>> Acked-by: Namhyung Kim <namhyung@kernel.org>
>> 
>> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
>> index 6de6f89..1fa98b9 100644
>> --- a/tools/perf/util/parse-events.c
>> +++ b/tools/perf/util/parse-events.c
>> @@ -1082,12 +1082,12 @@ int is_valid_tracepoint(const char *event_string)
>>  static bool is_event_supported(u8 type, unsigned config)
>>  {
>>  	bool ret = true;
>> +	int open_return;
>>  	struct perf_evsel *evsel;
>>  	struct perf_event_attr attr = {
>>  		.type = type,
>>  		.config = config,
>>  		.disabled = 1,
>> -		.exclude_kernel = 1,
>>  	};
>>  	struct {
>>  		struct thread_map map;
>> @@ -1099,7 +1099,20 @@ static bool is_event_supported(u8 type, unsigned config)
>>  
>>  	evsel = perf_evsel__new(&attr);
>>  	if (evsel) {
>> -		ret = perf_evsel__open(evsel, NULL, &tmap.map) >= 0;
>> +		open_return = perf_evsel__open(evsel, NULL, &tmap.map);
>> +		ret = open_return >= 0;
>> +
>> +		if (open_return == -EACCES) {
>> +			/*
>> +			 * This happens if the paranoid value
>> +			 * /proc/sys/kernel/perf_event_paranoid is set to 2
>> +			 * Re-run with exclude_kernel set; we don't do that
>> +			 * by default as some ARM machines do not support it.
>> +			 *
>> +			 */
>> +			evsel->attr.exclude_kernel = 1;
>> +			ret = perf_evsel__open(evsel, NULL, &tmap.map) >= 0;
>> +		}
>>  		perf_evsel__delete(evsel);
>>  	}
>>  
>> 
>
> Vince Weaver
> vincent.weaver@maine.edu
> http://www.eece.maine.edu/~vweaver/

Re: [patch] perf tools: perf list broken on ARM

[Arnaldo Carvalho de Melo]

Em Thu, Jan 09, 2014 at 05:17:27PM +0900, Namhyung Kim escreveu:
> Hi Vince,
> 
> On Wed, 8 Jan 2014 16:30:17 -0500 (EST), Vince Weaver wrote:
> > just checking on the status of this patch.  Am I sending it to the wrong 
> > place?  Did it get lost in the post-holiday e-mail purges?  
> >
> > The bug is annoying if you're trying to use perf on ARM systems.
> 
> Right, acme, would you please merge this?

I will, but having a patch authored by someone and Acked-by that same
person looks weird, so, to clarify, this was written by Vince, acked by
you, right?

- Arnaldo
 
> Thanks,
> Namhyung
> 
> >
> > On Mon, 30 Dec 2013, Vince Weaver wrote:
> >
> >> 
> >> "perf list" listing of hardware events doesn't work on older ARM devices.
> >> The change enabling event detection:
> >> 
> >>  commit b41f1cec91c37eeea6fdb15effbfa24ea0a5536b
> >>  Author: Namhyung Kim <namhyung.kim@lge.com>
> >>  Date:   Tue Aug 27 11:41:53 2013 +0900
> >> 
> >>      perf list: Skip unsupported events
> >> 
> >> 
> >> uses the following code in tools/perf/util/parse-events.c:
> >> 
> >>         struct perf_event_attr attr = {
> >>                 .type = type,
> >>                 .config = config,
> >>                 .disabled = 1,
> >>                 .exclude_kernel = 1,
> >>         };
> >> 
> >> On ARM machines pre-dating the Cortex-A15 this doesn't work, as
> >> these machines don't support .exclude_kernel.  So starting with 3.12
> >> "perf list" does not report any hardware events at all on older
> >> machines (seen on Rasp-Pi, Pandaboard, Beagleboard, etc).
> >> 
> >> This version of the patch makes changes suggested by Namhyung Kim
> >> to check for EACCESS and retry (instead of just dropping
> >> the exclude_kernel) so we can properly handle machines where 
> >> /proc/sys/kernel/perf_event_paranoid is set to 2.
> >> 
> >> Reported-by: Chad Paradis <chad.paradis@umit.maine.edu>
> >> Signed-off-by: Vince Weaver <vincent.weaver@maine.edu>
> >> Acked-by: Namhyung Kim <namhyung@kernel.org>
> >> 
> >> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> >> index 6de6f89..1fa98b9 100644
> >> --- a/tools/perf/util/parse-events.c
> >> +++ b/tools/perf/util/parse-events.c
> >> @@ -1082,12 +1082,12 @@ int is_valid_tracepoint(const char *event_string)
> >>  static bool is_event_supported(u8 type, unsigned config)
> >>  {
> >>  	bool ret = true;
> >> +	int open_return;
> >>  	struct perf_evsel *evsel;
> >>  	struct perf_event_attr attr = {
> >>  		.type = type,
> >>  		.config = config,
> >>  		.disabled = 1,
> >> -		.exclude_kernel = 1,
> >>  	};
> >>  	struct {
> >>  		struct thread_map map;
> >> @@ -1099,7 +1099,20 @@ static bool is_event_supported(u8 type, unsigned config)
> >>  
> >>  	evsel = perf_evsel__new(&attr);
> >>  	if (evsel) {
> >> -		ret = perf_evsel__open(evsel, NULL, &tmap.map) >= 0;
> >> +		open_return = perf_evsel__open(evsel, NULL, &tmap.map);
> >> +		ret = open_return >= 0;
> >> +
> >> +		if (open_return == -EACCES) {
> >> +			/*
> >> +			 * This happens if the paranoid value
> >> +			 * /proc/sys/kernel/perf_event_paranoid is set to 2
> >> +			 * Re-run with exclude_kernel set; we don't do that
> >> +			 * by default as some ARM machines do not support it.
> >> +			 *
> >> +			 */
> >> +			evsel->attr.exclude_kernel = 1;
> >> +			ret = perf_evsel__open(evsel, NULL, &tmap.map) >= 0;
> >> +		}
> >>  		perf_evsel__delete(evsel);
> >>  	}
> >>  
> >> 
> >
> > Vince Weaver
> > vincent.weaver@maine.edu
> > http://www.eece.maine.edu/~vweaver/

Re: [patch] perf tools: perf list broken on ARM

[Namhyung Kim]

2014-01-09 (목), 09:12 -0300, Arnaldo Carvalho de Melo:
> Em Thu, Jan 09, 2014 at 05:17:27PM +0900, Namhyung Kim escreveu:
> > Hi Vince,
> > 
> > On Wed, 8 Jan 2014 16:30:17 -0500 (EST), Vince Weaver wrote:
> > > just checking on the status of this patch.  Am I sending it to the wrong 
> > > place?  Did it get lost in the post-holiday e-mail purges?  
> > >
> > > The bug is annoying if you're trying to use perf on ARM systems.
> > 
> > Right, acme, would you please merge this?
> 
> I will, but having a patch authored by someone and Acked-by that same
> person looks weird, so, to clarify, this was written by Vince, acked by
> you, right?

You seemed to be confused - it's not authored by me.  He was just
referring a problematic commit b41f1cec91c3 which was written by me.  So
yeah, this commit was written by Vince.

Thanks,
Namhyung

Re: [patch] perf tools: perf list broken on ARM

[Arnaldo Carvalho de Melo]

Em Thu, Jan 09, 2014 at 09:37:03PM +0900, Namhyung Kim escreveu:
> 2014-01-09 (목), 09:12 -0300, Arnaldo Carvalho de Melo:
> > Em Thu, Jan 09, 2014 at 05:17:27PM +0900, Namhyung Kim escreveu:
> > > Hi Vince,
> > > 
> > > On Wed, 8 Jan 2014 16:30:17 -0500 (EST), Vince Weaver wrote:
> > > > just checking on the status of this patch.  Am I sending it to the wrong 
> > > > place?  Did it get lost in the post-holiday e-mail purges?  
> > > >
> > > > The bug is annoying if you're trying to use perf on ARM systems.
> > > 
> > > Right, acme, would you please merge this?
> > 
> > I will, but having a patch authored by someone and Acked-by that same
> > person looks weird, so, to clarify, this was written by Vince, acked by
> > you, right?
> 
> You seemed to be confused - it's not authored by me.  He was just
> referring a problematic commit b41f1cec91c3 which was written by me.  So
> yeah, this commit was written by Vince.

You are right, I saw that Author: line in the changeset comment, close
to the start of the message and got confused, thanks for clarifying.

- Arnaldo

Re: [patch] perf tools: perf list broken on ARM

[Vince Weaver]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1248 bytes --]

On Thu, 9 Jan 2014, Arnaldo Carvalho de Melo wrote:

> Em Thu, Jan 09, 2014 at 09:37:03PM +0900, Namhyung Kim escreveu:
> > 2014-01-09 (목), 09:12 -0300, Arnaldo Carvalho de Melo:
> > > Em Thu, Jan 09, 2014 at 05:17:27PM +0900, Namhyung Kim escreveu:
> > > > Hi Vince,
> > > > 
> > > > On Wed, 8 Jan 2014 16:30:17 -0500 (EST), Vince Weaver wrote:
> > > > > just checking on the status of this patch.  Am I sending it to the wrong 
> > > > > place?  Did it get lost in the post-holiday e-mail purges?  
> > > > >
> > > > > The bug is annoying if you're trying to use perf on ARM systems.
> > > > 
> > > > Right, acme, would you please merge this?
> > > 
> > > I will, but having a patch authored by someone and Acked-by that same
> > > person looks weird, so, to clarify, this was written by Vince, acked by
> > > you, right?
> > 
> > You seemed to be confused - it's not authored by me.  He was just
> > referring a problematic commit b41f1cec91c3 which was written by me.  So
> > yeah, this commit was written by Vince.
> 
> You are right, I saw that Author: line in the changeset comment, close
> to the start of the message and got confused, thanks for clarifying.

Do I need to re-send this patch?  It still applies cleanly to 3.14-rc1.

Vince

Re: [patch] perf tools: perf list broken on ARM

[Arnaldo Carvalho de Melo]

Em Wed, Feb 05, 2014 at 03:34:42PM -0500, Vince Weaver escreveu:
> > You are right, I saw that Author: line in the changeset comment, close
> > to the start of the message and got confused, thanks for clarifying.
> 
> Do I need to re-send this patch?  It still applies cleanly to 3.14-rc1.

Sorry, picked it up now, will send it to Ingo today.

- Arnaldo

Re: [patch] perf tools: perf list broken on ARM

[Peter Zijlstra]

On Wed, Jan 08, 2014 at 04:30:17PM -0500, Vince Weaver wrote:
> 
> just checking on the status of this patch.  Am I sending it to the wrong 
> place?  Did it get lost in the post-holiday e-mail purges?  
> 
> The bug is annoying if you're trying to use perf on ARM systems.

Its for acme and he seems on the Cc, I suppose you got stuck in the
holiday backlog.

[tip:perf/core] perf list: Fix checking for supported events on older kernels

[tip-bot for Vince Weaver]

Commit-ID:  88fee52e58ca14d8465b614774ed0bf08e1a7790
Gitweb:     http://git.kernel.org/tip/88fee52e58ca14d8465b614774ed0bf08e1a7790
Author:     Vince Weaver <vincent.weaver@maine.edu>
AuthorDate: Mon, 30 Dec 2013 15:39:45 -0500
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 10 Feb 2014 11:34:31 -0300

perf list: Fix checking for supported events on older kernels

"perf list" listing of hardware events doesn't work on older ARM devices.
The change enabling event detection:

 commit b41f1cec91c37eeea6fdb15effbfa24ea0a5536b
 Author: Namhyung Kim <namhyung.kim@lge.com>
 Date:   Tue Aug 27 11:41:53 2013 +0900

     perf list: Skip unsupported events

uses the following code in tools/perf/util/parse-events.c:

        struct perf_event_attr attr = {
                .type = type,
                .config = config,
                .disabled = 1,
                .exclude_kernel = 1,
        };

On ARM machines pre-dating the Cortex-A15 this doesn't work, as these
machines don't support .exclude_kernel.  So starting with 3.12 "perf
list" does not report any hardware events at all on older machines (seen
on Rasp-Pi, Pandaboard, Beagleboard, etc).

This version of the patch makes changes suggested by Namhyung Kim to
check for EACCESS and retry (instead of just dropping the
exclude_kernel) so we can properly handle machines where
/proc/sys/kernel/perf_event_paranoid is set to 2.

Reported-by: Chad Paradis <chad.paradis@umit.maine.edu>
Signed-off-by: Vince Weaver <vincent.weaver@maine.edu>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Chad Paradis <chad.paradis@umit.maine.edu>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/alpine.DEB.2.10.1312301536150.28814@vincent-weaver-1.um.maine.edu
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/parse-events.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index d248fca..1e15df1 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1091,12 +1091,12 @@ int is_valid_tracepoint(const char *event_string)
 static bool is_event_supported(u8 type, unsigned config)
 {
 	bool ret = true;
+	int open_return;
 	struct perf_evsel *evsel;
 	struct perf_event_attr attr = {
 		.type = type,
 		.config = config,
 		.disabled = 1,
-		.exclude_kernel = 1,
 	};
 	struct {
 		struct thread_map map;
@@ -1108,7 +1108,20 @@ static bool is_event_supported(u8 type, unsigned config)
 
 	evsel = perf_evsel__new(&attr);
 	if (evsel) {
-		ret = perf_evsel__open(evsel, NULL, &tmap.map) >= 0;
+		open_return = perf_evsel__open(evsel, NULL, &tmap.map);
+		ret = open_return >= 0;
+
+		if (open_return == -EACCES) {
+			/*
+			 * This happens if the paranoid value
+			 * /proc/sys/kernel/perf_event_paranoid is set to 2
+			 * Re-run with exclude_kernel set; we don't do that
+			 * by default as some ARM machines do not support it.
+			 *
+			 */
+			evsel->attr.exclude_kernel = 1;
+			ret = perf_evsel__open(evsel, NULL, &tmap.map) >= 0;
+		}
 		perf_evsel__delete(evsel);
 	}