[PATCH v2 1/2] perf tools: Return first evsel for non-sample event on old kernel

[PATCH v2 1/2] perf tools: Return first evsel for non-sample event on old kernel

[Namhyung Kim]

On old kernels that don't support sample_id_all feature,
perf_evlist__id2evsel() returns NULL for non-sampling events.
This breaks perf top when multiple events are given on command
line. Fix it by using first evsel in the evlist. This will also
prevent getting the same (potential) problem in such new tool/
old kernel combo.

Suggested-by: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
---
 tools/perf/util/evlist.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index a6d50e376257..3ffb3203f335 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -349,6 +349,10 @@ struct perf_evsel *perf_evlist__id2evsel(struct perf_evlist *evlist, u64 id)
 	hlist_for_each_entry(sid, pos, head, node)
 		if (sid->id == id)
 			return sid->evsel;
+
+	if (!perf_evlist__sample_id_all(evlist))
+		return list_entry(evlist->entries.next, struct perf_evsel, node);
+
 	return NULL;
 }
 
-- 
1.7.9.rc1.dirty

[PATCH v2 2/2] perf: Add error message for EMFILE

[Namhyung Kim]

When a user tries to open so many event on multiple tasks/cpus,
perf_event_open syscall may fail with EMFILE. Provide an advice
for that case.

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
Cc: David Ahern <dsahern@gmail.com>
---
 tools/perf/builtin-record.c |    5 +++++
 tools/perf/builtin-stat.c   |    4 ++++
 tools/perf/builtin-top.c    |    3 ++-
 3 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 32870eef952f..a334008a1853 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -249,6 +249,11 @@ try_again:
 				ui__warning("The %s event is not supported.\n",
 					    event_name(pos));
 				exit(EXIT_FAILURE);
+			} else if (err == EMFILE) {
+				ui__warning("Too many events are opened.\n"
+					    "Try again after reducing the number of events,\n"
+					    "tasks and/or cpus.\n");
+				exit(EXIT_FAILURE);
 			}
 
 			printf("\n");
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 459b8620a5d9..7c5421c7af06 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -477,6 +477,10 @@ static int run_perf_stat(int argc __used, const char **argv)
 				      "\t Consider tweaking"
 				      " /proc/sys/kernel/perf_event_paranoid or running as root.",
 				      system_wide ? "system-wide " : "");
+			} else if (errno == EMFILE) {
+				error("Too many events are opened.\n"
+				      "\t Try again after reducing the number of events,\n"
+				      "\t tasks and/or cpus.");
 			} else {
 				error("open_counter returned with %d (%s). "
 				      "/bin/dmesg may provide additional information.\n",
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index e8b033c074f9..987af3695218 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -899,7 +899,8 @@ try_again:
 				goto out_err;
 			} else if (err == EMFILE) {
 				ui__warning("Too many events are opened.\n"
-					    "Try again after reducing the number of events\n");
+					    "Try again after reducing the number of events,\n"
+					    "tasks and/or cpus.\n");
 				goto out_err;
 			}
 
-- 
1.7.9.rc1.dirty

Re: [PATCH v2 2/2] perf: Add error message for EMFILE

[Arnaldo Carvalho de Melo]

Em Tue, Jan 31, 2012 at 10:15:16AM +0900, Namhyung Kim escreveu:
> When a user tries to open so many event on multiple tasks/cpus,
> perf_event_open syscall may fail with EMFILE. Provide an advice
> for that case.

I'll apply this one, but can you investigate a way to share this error
handling accross the tools?

A helper routine that doesn't call exit, but returns failure that will
then cause the tool to decide if it exits or what (TUI ones could
continue and the user could then select some other operation that would
work, etc).

It should standardize on ui__warning(), that already takes into account
if the UI is --stdio or --tui (in the future a GUI too), etc.

Thanks a lot!

- Arnaldo
 
> Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
> Cc: David Ahern <dsahern@gmail.com>
> ---
>  tools/perf/builtin-record.c |    5 +++++
>  tools/perf/builtin-stat.c   |    4 ++++
>  tools/perf/builtin-top.c    |    3 ++-
>  3 files changed, 11 insertions(+), 1 deletions(-)
> 
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index 32870eef952f..a334008a1853 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -249,6 +249,11 @@ try_again:
>  				ui__warning("The %s event is not supported.\n",
>  					    event_name(pos));
>  				exit(EXIT_FAILURE);
> +			} else if (err == EMFILE) {
> +				ui__warning("Too many events are opened.\n"
> +					    "Try again after reducing the number of events,\n"
> +					    "tasks and/or cpus.\n");
> +				exit(EXIT_FAILURE);
>  			}
>  
>  			printf("\n");
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index 459b8620a5d9..7c5421c7af06 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -477,6 +477,10 @@ static int run_perf_stat(int argc __used, const char **argv)
>  				      "\t Consider tweaking"
>  				      " /proc/sys/kernel/perf_event_paranoid or running as root.",
>  				      system_wide ? "system-wide " : "");
> +			} else if (errno == EMFILE) {
> +				error("Too many events are opened.\n"
> +				      "\t Try again after reducing the number of events,\n"
> +				      "\t tasks and/or cpus.");
>  			} else {
>  				error("open_counter returned with %d (%s). "
>  				      "/bin/dmesg may provide additional information.\n",
> diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> index e8b033c074f9..987af3695218 100644
> --- a/tools/perf/builtin-top.c
> +++ b/tools/perf/builtin-top.c
> @@ -899,7 +899,8 @@ try_again:
>  				goto out_err;
>  			} else if (err == EMFILE) {
>  				ui__warning("Too many events are opened.\n"
> -					    "Try again after reducing the number of events\n");
> +					    "Try again after reducing the number of events,\n"
> +					    "tasks and/or cpus.\n");
>  				goto out_err;
>  			}
>  
> -- 
> 1.7.9.rc1.dirty

Re: [PATCH v2 2/2] perf: Add error message for EMFILE

[Namhyung Kim]

2012-01-31 10:30 AM, Arnaldo Carvalho de Melo wrote:
> Em Tue, Jan 31, 2012 at 10:15:16AM +0900, Namhyung Kim escreveu:
>> When a user tries to open so many event on multiple tasks/cpus,
>> perf_event_open syscall may fail with EMFILE. Provide an advice
>> for that case.
>
> I'll apply this one, but can you investigate a way to share this error
> handling accross the tools?
>
> A helper routine that doesn't call exit, but returns failure that will
> then cause the tool to decide if it exits or what (TUI ones could
> continue and the user could then select some other operation that would
> work, etc).
>
> It should standardize on ui__warning(), that already takes into account
> if the UI is --stdio or --tui (in the future a GUI too), etc.
>
> Thanks a lot!
>
> - Arnaldo
>

Yep, I'll take a look at it.

Thanks,
Namhyung