* [PATCH v2 1/2] perf tools: Return first evsel for non-sample event on old kernel @ 2012-01-31 1:15 Namhyung Kim 2012-01-31 1:15 ` [PATCH v2 2/2] perf: Add error message for EMFILE Namhyung Kim 0 siblings, 1 reply; 4+ messages in thread From: Namhyung Kim @ 2012-01-31 1:15 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, linux-kernel 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 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v2 2/2] perf: Add error message for EMFILE 2012-01-31 1:15 [PATCH v2 1/2] perf tools: Return first evsel for non-sample event on old kernel Namhyung Kim @ 2012-01-31 1:15 ` Namhyung Kim 2012-01-31 1:30 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 4+ messages in thread From: Namhyung Kim @ 2012-01-31 1:15 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, linux-kernel, David Ahern 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 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2 2/2] perf: Add error message for EMFILE 2012-01-31 1:15 ` [PATCH v2 2/2] perf: Add error message for EMFILE Namhyung Kim @ 2012-01-31 1:30 ` Arnaldo Carvalho de Melo 2012-01-31 1:40 ` Namhyung Kim 0 siblings, 1 reply; 4+ messages in thread From: Arnaldo Carvalho de Melo @ 2012-01-31 1:30 UTC (permalink / raw) To: Namhyung Kim Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, linux-kernel, David Ahern 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 2/2] perf: Add error message for EMFILE 2012-01-31 1:30 ` Arnaldo Carvalho de Melo @ 2012-01-31 1:40 ` Namhyung Kim 0 siblings, 0 replies; 4+ messages in thread From: Namhyung Kim @ 2012-01-31 1:40 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Namhyung Kim, Peter Zijlstra, Paul Mackerras, Ingo Molnar, linux-kernel, David Ahern 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-01-31 1:40 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-01-31 1:15 [PATCH v2 1/2] perf tools: Return first evsel for non-sample event on old kernel Namhyung Kim 2012-01-31 1:15 ` [PATCH v2 2/2] perf: Add error message for EMFILE Namhyung Kim 2012-01-31 1:30 ` Arnaldo Carvalho de Melo 2012-01-31 1:40 ` Namhyung Kim
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox