* [GIT PULL 0/6] perf/urgent fixes and improvements @ 2011-01-11 0:28 Arnaldo Carvalho de Melo 2011-01-11 0:28 ` [PATCH 1/6] perf sched: Fix allocation result check Arnaldo Carvalho de Melo ` (6 more replies) 0 siblings, 7 replies; 10+ messages in thread From: Arnaldo Carvalho de Melo @ 2011-01-11 0:28 UTC (permalink / raw) To: Ingo Molnar Cc: linux-kernel, Arnaldo Carvalho de Melo, Chris Samuel, David Ahern, Frederic Weisbecker, Ingo Molnar, Jiri Pirko, Linus Torvalds, Mike Galbraith, Paul Mackerras, Peter Zijlstra, Stephane Eranian, Thomas Gleixner, Tom Zanussi Hi Ingo, Please consider pulling from: git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux-2.6 perf/urgent Regards, - Arnaldo Arnaldo Carvalho de Melo (4): perf sched: Fix allocation result check perf tools: Emit clearer message for sys_perf_event_open ENOENT return perf evsel: Support perf_evsel__open(cpus > 1 && threads > 1) perf session: Fix infinite loop in __perf_session__process_events David Ahern (1): perf stat: better error message for unsupported events Jiri Pirko (1): perf sched: Use PTHREAD_STACK_MIN to avoid pthread_attr_setstacksize() fail tools/perf/builtin-record.c | 3 + tools/perf/builtin-sched.c | 5 +- tools/perf/builtin-stat.c | 2 + tools/perf/builtin-test.c | 110 +++++++++++++++++++++++++++++++++++++++++++ tools/perf/builtin-top.c | 2 + tools/perf/util/evsel.c | 82 +++++++++++++++++++------------- tools/perf/util/session.c | 2 +- 7 files changed, 170 insertions(+), 36 deletions(-) ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/6] perf sched: Fix allocation result check 2011-01-11 0:28 [GIT PULL 0/6] perf/urgent fixes and improvements Arnaldo Carvalho de Melo @ 2011-01-11 0:28 ` Arnaldo Carvalho de Melo 2011-01-11 0:28 ` [PATCH 2/6] perf stat: better error message for unsupported events Arnaldo Carvalho de Melo ` (5 subsequent siblings) 6 siblings, 0 replies; 10+ messages in thread From: Arnaldo Carvalho de Melo @ 2011-01-11 0:28 UTC (permalink / raw) To: Ingo Molnar Cc: linux-kernel, Arnaldo Carvalho de Melo, Chris Samuel, Frederic Weisbecker, Ingo Molnar, Mike Galbraith, Paul Mackerras, Peter Zijlstra, Stephane Eranian, Tom Zanussi From: Arnaldo Carvalho de Melo <acme@redhat.com> Bug introduced in ce47dc56. Reported-by: Mike Galbraith <efault@gmx.de> Cc: Chris Samuel <chris@csamuel.org> Cc: Frederic Weisbecker <fweisbec@gmail.com> Cc: Ingo Molnar <mingo@elte.hu> Cc: Mike Galbraith <efault@gmx.de> Cc: Paul Mackerras <paulus@samba.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Stephane Eranian <eranian@google.com> Cc: Tom Zanussi <tzanussi@gmail.com> LKML-Reference: <new-submission> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- tools/perf/builtin-sched.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c index 7a4ebeb..54024d2 100644 --- a/tools/perf/builtin-sched.c +++ b/tools/perf/builtin-sched.c @@ -1861,7 +1861,7 @@ static int __cmd_record(int argc, const char **argv) rec_argc = ARRAY_SIZE(record_args) + argc - 1; rec_argv = calloc(rec_argc + 1, sizeof(char *)); - if (rec_argv) + if (rec_argv == NULL) return -ENOMEM; for (i = 0; i < ARRAY_SIZE(record_args); i++) -- 1.6.2.5 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/6] perf stat: better error message for unsupported events 2011-01-11 0:28 [GIT PULL 0/6] perf/urgent fixes and improvements Arnaldo Carvalho de Melo 2011-01-11 0:28 ` [PATCH 1/6] perf sched: Fix allocation result check Arnaldo Carvalho de Melo @ 2011-01-11 0:28 ` Arnaldo Carvalho de Melo 2011-01-11 0:28 ` [PATCH 3/6] perf tools: Emit clearer message for sys_perf_event_open ENOENT return Arnaldo Carvalho de Melo ` (4 subsequent siblings) 6 siblings, 0 replies; 10+ messages in thread From: Arnaldo Carvalho de Melo @ 2011-01-11 0:28 UTC (permalink / raw) To: Ingo Molnar Cc: linux-kernel, David Ahern, Ingo Molnar, Paul Mackerras, a.p.zijlstra, Arnaldo Carvalho de Melo From: David Ahern <daahern@cisco.com> For unsupported events (e.g., H/W events when running in a VM) perf stat currently fails with the error message: Error: open_counter returned with 2 (No such file or directory). /bin/dmesg may provide additional information. Fatal: Not all events could be opened. dmesg is of no help and it is not clear as to why it fails to open the counter. This patch changes the error message to Error: cache-misses event is not supported. Fatal: Not all events could be opened. Cc: Ingo Molnar <mingo@elte.hu> Cc: Paul Mackerras <paulus@samba.org> Cc: a.p.zijlstra@chello.nl LPU-Reference: <1294597272-17335-1-git-send-email-daahern@cisco.com> Signed-off-by: David Ahern <daahern@cisco.com> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- tools/perf/builtin-stat.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c index 2dfcb61..c385a63 100644 --- a/tools/perf/builtin-stat.c +++ b/tools/perf/builtin-stat.c @@ -316,6 +316,8 @@ 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 == ENOENT) { + error("%s event is not supported. ", event_name(counter)); } else { error("open_counter returned with %d (%s). " "/bin/dmesg may provide additional information.\n", -- 1.6.2.5 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/6] perf tools: Emit clearer message for sys_perf_event_open ENOENT return 2011-01-11 0:28 [GIT PULL 0/6] perf/urgent fixes and improvements Arnaldo Carvalho de Melo 2011-01-11 0:28 ` [PATCH 1/6] perf sched: Fix allocation result check Arnaldo Carvalho de Melo 2011-01-11 0:28 ` [PATCH 2/6] perf stat: better error message for unsupported events Arnaldo Carvalho de Melo @ 2011-01-11 0:28 ` Arnaldo Carvalho de Melo 2011-01-11 18:40 ` David Ahern 2011-01-11 0:28 ` [PATCH 4/6] perf sched: Use PTHREAD_STACK_MIN to avoid pthread_attr_setstacksize() fail Arnaldo Carvalho de Melo ` (3 subsequent siblings) 6 siblings, 1 reply; 10+ messages in thread From: Arnaldo Carvalho de Melo @ 2011-01-11 0:28 UTC (permalink / raw) To: Ingo Molnar Cc: linux-kernel, Arnaldo Carvalho de Melo, David Ahern, Frederic Weisbecker, Ingo Molnar, Mike Galbraith, Paul Mackerras, Peter Zijlstra, Stephane Eranian, Tom Zanussi From: Arnaldo Carvalho de Melo <acme@redhat.com> Improve sys_perf_event_open ENOENT return handling in top and record, just like 5a3446b does for stat. Cc: David Ahern <daahern@cisco.com> Cc: Frederic Weisbecker <fweisbec@gmail.com> Cc: Ingo Molnar <mingo@elte.hu> Cc: Mike Galbraith <efault@gmx.de> Cc: Paul Mackerras <paulus@samba.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Stephane Eranian <eranian@google.com> Cc: Tom Zanussi <tzanussi@gmail.com> LKML-Reference: <new-submission> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- tools/perf/builtin-record.c | 3 +++ tools/perf/builtin-top.c | 2 ++ 2 files changed, 5 insertions(+), 0 deletions(-) diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c index 7bc0490..7069bd3 100644 --- a/tools/perf/builtin-record.c +++ b/tools/perf/builtin-record.c @@ -331,6 +331,9 @@ try_again: else if (err == ENODEV && cpu_list) { die("No such device - did you specify" " an out-of-range profile CPU?\n"); + } else if (err == ENOENT) { + die("%s event is not supported. ", + event_name(evsel)); } else if (err == EINVAL && sample_id_all_avail) { /* * Old kernel, no attr->sample_id_type_all field diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c index 1e67ab9..6ce4042 100644 --- a/tools/perf/builtin-top.c +++ b/tools/perf/builtin-top.c @@ -1247,6 +1247,8 @@ try_again: die("Permission error - are you root?\n" "\t Consider tweaking" " /proc/sys/kernel/perf_event_paranoid.\n"); + if (err == ENOENT) + die("%s event is not supported. ", event_name(evsel)); /* * If it's cycles then fall back to hrtimer * based cpu-clock-tick sw counter, which -- 1.6.2.5 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 3/6] perf tools: Emit clearer message for sys_perf_event_open ENOENT return 2011-01-11 0:28 ` [PATCH 3/6] perf tools: Emit clearer message for sys_perf_event_open ENOENT return Arnaldo Carvalho de Melo @ 2011-01-11 18:40 ` David Ahern 2011-01-11 18:58 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 10+ messages in thread From: David Ahern @ 2011-01-11 18:40 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Ingo Molnar, linux-kernel, Arnaldo Carvalho de Melo, Frederic Weisbecker, Mike Galbraith, Paul Mackerras, Peter Zijlstra, Stephane Eranian, Tom Zanussi On 01/10/11 17:28, Arnaldo Carvalho de Melo wrote: > From: Arnaldo Carvalho de Melo <acme@redhat.com> > > Improve sys_perf_event_open ENOENT return handling in top and record, just > like 5a3446b does for stat. > > Cc: David Ahern <daahern@cisco.com> > Cc: Frederic Weisbecker <fweisbec@gmail.com> > Cc: Ingo Molnar <mingo@elte.hu> > Cc: Mike Galbraith <efault@gmx.de> > Cc: Paul Mackerras <paulus@samba.org> > Cc: Peter Zijlstra <peterz@infradead.org> > Cc: Stephane Eranian <eranian@google.com> > Cc: Tom Zanussi <tzanussi@gmail.com> > LKML-Reference: <new-submission> > Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> > --- > tools/perf/builtin-record.c | 3 +++ > tools/perf/builtin-top.c | 2 ++ > 2 files changed, 5 insertions(+), 0 deletions(-) > > diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c > index 7bc0490..7069bd3 100644 > --- a/tools/perf/builtin-record.c > +++ b/tools/perf/builtin-record.c > @@ -331,6 +331,9 @@ try_again: > else if (err == ENODEV && cpu_list) { > die("No such device - did you specify" > " an out-of-range profile CPU?\n"); > + } else if (err == ENOENT) { > + die("%s event is not supported. ", > + event_name(evsel)); I think this interferes with the fallback from hardware profiling to software profiling. .e.g., in a VM with no PMU. David > } else if (err == EINVAL && sample_id_all_avail) { > /* > * Old kernel, no attr->sample_id_type_all field > diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c > index 1e67ab9..6ce4042 100644 > --- a/tools/perf/builtin-top.c > +++ b/tools/perf/builtin-top.c > @@ -1247,6 +1247,8 @@ try_again: > die("Permission error - are you root?\n" > "\t Consider tweaking" > " /proc/sys/kernel/perf_event_paranoid.\n"); > + if (err == ENOENT) > + die("%s event is not supported. ", event_name(evsel)); > /* > * If it's cycles then fall back to hrtimer > * based cpu-clock-tick sw counter, which ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/6] perf tools: Emit clearer message for sys_perf_event_open ENOENT return 2011-01-11 18:40 ` David Ahern @ 2011-01-11 18:58 ` Arnaldo Carvalho de Melo 0 siblings, 0 replies; 10+ messages in thread From: Arnaldo Carvalho de Melo @ 2011-01-11 18:58 UTC (permalink / raw) To: David Ahern Cc: Ingo Molnar, linux-kernel, Frederic Weisbecker, Mike Galbraith, Paul Mackerras, Peter Zijlstra, Stephane Eranian, Tom Zanussi Em Tue, Jan 11, 2011 at 11:40:23AM -0700, David Ahern escreveu: > > > On 01/10/11 17:28, Arnaldo Carvalho de Melo wrote: > > From: Arnaldo Carvalho de Melo <acme@redhat.com> > > > > Improve sys_perf_event_open ENOENT return handling in top and record, just > > like 5a3446b does for stat. > > > > Cc: David Ahern <daahern@cisco.com> > > Cc: Frederic Weisbecker <fweisbec@gmail.com> > > Cc: Ingo Molnar <mingo@elte.hu> > > Cc: Mike Galbraith <efault@gmx.de> > > Cc: Paul Mackerras <paulus@samba.org> > > Cc: Peter Zijlstra <peterz@infradead.org> > > Cc: Stephane Eranian <eranian@google.com> > > Cc: Tom Zanussi <tzanussi@gmail.com> > > LKML-Reference: <new-submission> > > Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> > > --- > > tools/perf/builtin-record.c | 3 +++ > > tools/perf/builtin-top.c | 2 ++ > > 2 files changed, 5 insertions(+), 0 deletions(-) > > > > diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c > > index 7bc0490..7069bd3 100644 > > --- a/tools/perf/builtin-record.c > > +++ b/tools/perf/builtin-record.c > > @@ -331,6 +331,9 @@ try_again: > > else if (err == ENODEV && cpu_list) { > > die("No such device - did you specify" > > " an out-of-range profile CPU?\n"); > > + } else if (err == ENOENT) { > > + die("%s event is not supported. ", > > + event_name(evsel)); > > I think this interferes with the fallback from hardware profiling to > software profiling. .e.g., in a VM with no PMU. Argh, you're right, in both cases (top and record) :-\ Reverting. > David > > > > } else if (err == EINVAL && sample_id_all_avail) { > > /* > > * Old kernel, no attr->sample_id_type_all field > > diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c > > index 1e67ab9..6ce4042 100644 > > --- a/tools/perf/builtin-top.c > > +++ b/tools/perf/builtin-top.c > > @@ -1247,6 +1247,8 @@ try_again: > > die("Permission error - are you root?\n" > > "\t Consider tweaking" > > " /proc/sys/kernel/perf_event_paranoid.\n"); > > + if (err == ENOENT) > > + die("%s event is not supported. ", event_name(evsel)); > > /* > > * If it's cycles then fall back to hrtimer > > * based cpu-clock-tick sw counter, which ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 4/6] perf sched: Use PTHREAD_STACK_MIN to avoid pthread_attr_setstacksize() fail 2011-01-11 0:28 [GIT PULL 0/6] perf/urgent fixes and improvements Arnaldo Carvalho de Melo ` (2 preceding siblings ...) 2011-01-11 0:28 ` [PATCH 3/6] perf tools: Emit clearer message for sys_perf_event_open ENOENT return Arnaldo Carvalho de Melo @ 2011-01-11 0:28 ` Arnaldo Carvalho de Melo 2011-01-11 0:28 ` [PATCH 5/6] perf evsel: Support perf_evsel__open(cpus > 1 && threads > 1) Arnaldo Carvalho de Melo ` (2 subsequent siblings) 6 siblings, 0 replies; 10+ messages in thread From: Arnaldo Carvalho de Melo @ 2011-01-11 0:28 UTC (permalink / raw) To: Ingo Molnar Cc: linux-kernel, Jiri Pirko, Ingo Molnar, Paul Mackerras, Peter Zijlstra, Arnaldo Carvalho de Melo From: Jiri Pirko <jpirko@redhat.com> on ppc64: /usr/include/bits/local_lim.h:#define PTHREAD_STACK_MIN 131072 therefore following set of commands: gives: perf.2.6.37test: builtin-sched.c:493: create_tasks: Assertion `!(err)' failed. So make sure we do not set stack size lower than PTHREAD_STACK_MIN. Cc: Ingo Molnar <mingo@elte.hu> Cc: Paul Mackerras <paulus@samba.org> Cc: Peter Zijlstra <peterz@infradead.org> LKML-Reference: <20110110160417.GB2685@psychotron.brq.redhat.com> Signed-off-by: Jiri Pirko <jpirko@redhat.com> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- tools/perf/builtin-sched.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c index 54024d2..abd4b84 100644 --- a/tools/perf/builtin-sched.c +++ b/tools/perf/builtin-sched.c @@ -489,7 +489,8 @@ static void create_tasks(void) err = pthread_attr_init(&attr); BUG_ON(err); - err = pthread_attr_setstacksize(&attr, (size_t)(16*1024)); + err = pthread_attr_setstacksize(&attr, + (size_t) max(16 * 1024, PTHREAD_STACK_MIN)); BUG_ON(err); err = pthread_mutex_lock(&start_work_mutex); BUG_ON(err); -- 1.6.2.5 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 5/6] perf evsel: Support perf_evsel__open(cpus > 1 && threads > 1) 2011-01-11 0:28 [GIT PULL 0/6] perf/urgent fixes and improvements Arnaldo Carvalho de Melo ` (3 preceding siblings ...) 2011-01-11 0:28 ` [PATCH 4/6] perf sched: Use PTHREAD_STACK_MIN to avoid pthread_attr_setstacksize() fail Arnaldo Carvalho de Melo @ 2011-01-11 0:28 ` Arnaldo Carvalho de Melo 2011-01-11 0:28 ` [PATCH 6/6] perf session: Fix infinite loop in __perf_session__process_events Arnaldo Carvalho de Melo 2011-01-11 10:57 ` [GIT PULL 0/6] perf/urgent fixes and improvements Ingo Molnar 6 siblings, 0 replies; 10+ messages in thread From: Arnaldo Carvalho de Melo @ 2011-01-11 0:28 UTC (permalink / raw) To: Ingo Molnar Cc: linux-kernel, Arnaldo Carvalho de Melo, Frederic Weisbecker, Ingo Molnar, Mike Galbraith, Paul Mackerras, Peter Zijlstra, Stephane Eranian, Tom Zanussi From: Arnaldo Carvalho de Melo <acme@redhat.com> And a test for it: [acme@felicio linux]$ perf test 1: vmlinux symtab matches kallsyms: Ok 2: detect open syscall event: Ok 3: detect open syscall event on all cpus: Ok [acme@felicio linux]$ Translating C the test does: 1. generates different number of open syscalls on each CPU by using sched_setaffinity 2. Verifies that the expected number of events is generated on each CPU It works as expected. LKML-Reference: <new-submission> Cc: Frederic Weisbecker <fweisbec@gmail.com> Cc: Ingo Molnar <mingo@elte.hu> Cc: Mike Galbraith <efault@gmx.de> Cc: Paul Mackerras <paulus@samba.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Stephane Eranian <eranian@google.com> Cc: Tom Zanussi <tzanussi@gmail.com> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- tools/perf/builtin-test.c | 110 +++++++++++++++++++++++++++++++++++++++++++++ tools/perf/util/evsel.c | 82 ++++++++++++++++++++------------- 2 files changed, 159 insertions(+), 33 deletions(-) diff --git a/tools/perf/builtin-test.c b/tools/perf/builtin-test.c index e12753f..ed56961 100644 --- a/tools/perf/builtin-test.c +++ b/tools/perf/builtin-test.c @@ -234,6 +234,7 @@ out: return err; } +#include "util/cpumap.h" #include "util/evsel.h" #include <sys/types.h> @@ -321,6 +322,111 @@ out_thread_map_delete: return err; } +#include <sched.h> + +static int test__open_syscall_event_on_all_cpus(void) +{ + int err = -1, fd, cpu; + struct thread_map *threads; + struct cpu_map *cpus; + struct perf_evsel *evsel; + struct perf_event_attr attr; + unsigned int nr_open_calls = 111, i; + cpu_set_t *cpu_set; + size_t cpu_set_size; + int id = trace_event__id("sys_enter_open"); + + if (id < 0) { + pr_debug("is debugfs mounted on /sys/kernel/debug?\n"); + return -1; + } + + threads = thread_map__new(-1, getpid()); + if (threads == NULL) { + pr_debug("thread_map__new\n"); + return -1; + } + + cpus = cpu_map__new(NULL); + if (threads == NULL) { + pr_debug("thread_map__new\n"); + return -1; + } + + cpu_set = CPU_ALLOC(cpus->nr); + + if (cpu_set == NULL) + goto out_thread_map_delete; + + cpu_set_size = CPU_ALLOC_SIZE(cpus->nr); + CPU_ZERO_S(cpu_set_size, cpu_set); + + memset(&attr, 0, sizeof(attr)); + attr.type = PERF_TYPE_TRACEPOINT; + attr.config = id; + evsel = perf_evsel__new(&attr, 0); + if (evsel == NULL) { + pr_debug("perf_evsel__new\n"); + goto out_cpu_free; + } + + if (perf_evsel__open(evsel, cpus, threads) < 0) { + pr_debug("failed to open counter: %s, " + "tweak /proc/sys/kernel/perf_event_paranoid?\n", + strerror(errno)); + goto out_evsel_delete; + } + + for (cpu = 0; cpu < cpus->nr; ++cpu) { + unsigned int ncalls = nr_open_calls + cpu; + + CPU_SET(cpu, cpu_set); + sched_setaffinity(0, cpu_set_size, cpu_set); + for (i = 0; i < ncalls; ++i) { + fd = open("/etc/passwd", O_RDONLY); + close(fd); + } + CPU_CLR(cpu, cpu_set); + } + + /* + * Here we need to explicitely preallocate the counts, as if + * we use the auto allocation it will allocate just for 1 cpu, + * as we start by cpu 0. + */ + if (perf_evsel__alloc_counts(evsel, cpus->nr) < 0) { + pr_debug("perf_evsel__alloc_counts(ncpus=%d)\n", cpus->nr); + goto out_close_fd; + } + + for (cpu = 0; cpu < cpus->nr; ++cpu) { + unsigned int expected; + + if (perf_evsel__read_on_cpu(evsel, cpu, 0) < 0) { + pr_debug("perf_evsel__open_read_on_cpu\n"); + goto out_close_fd; + } + + expected = nr_open_calls + cpu; + if (evsel->counts->cpu[cpu].val != expected) { + pr_debug("perf_evsel__read_on_cpu: expected to intercept %d calls on cpu %d, got %Ld\n", + expected, cpu, evsel->counts->cpu[cpu].val); + goto out_close_fd; + } + } + + err = 0; +out_close_fd: + perf_evsel__close_fd(evsel, 1, threads->nr); +out_evsel_delete: + perf_evsel__delete(evsel); +out_cpu_free: + CPU_FREE(cpu_set); +out_thread_map_delete: + thread_map__delete(threads); + return err; +} + static struct test { const char *desc; int (*func)(void); @@ -334,6 +440,10 @@ static struct test { .func = test__open_syscall_event, }, { + .desc = "detect open syscall event on all cpus", + .func = test__open_syscall_event_on_all_cpus, + }, + { .func = NULL, }, }; diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c index 1a5591d..f5cfed6 100644 --- a/tools/perf/util/evsel.c +++ b/tools/perf/util/evsel.c @@ -127,59 +127,75 @@ int __perf_evsel__read(struct perf_evsel *evsel, return 0; } -int perf_evsel__open_per_cpu(struct perf_evsel *evsel, struct cpu_map *cpus) +static int __perf_evsel__open(struct perf_evsel *evsel, struct cpu_map *cpus, + struct thread_map *threads) { - int cpu; + int cpu, thread; - if (evsel->fd == NULL && perf_evsel__alloc_fd(evsel, cpus->nr, 1) < 0) + if (evsel->fd == NULL && + perf_evsel__alloc_fd(evsel, cpus->nr, threads->nr) < 0) return -1; for (cpu = 0; cpu < cpus->nr; cpu++) { - FD(evsel, cpu, 0) = sys_perf_event_open(&evsel->attr, -1, - cpus->map[cpu], -1, 0); - if (FD(evsel, cpu, 0) < 0) - goto out_close; + for (thread = 0; thread < threads->nr; thread++) { + FD(evsel, cpu, thread) = sys_perf_event_open(&evsel->attr, + threads->map[thread], + cpus->map[cpu], -1, 0); + if (FD(evsel, cpu, thread) < 0) + goto out_close; + } } return 0; out_close: - while (--cpu >= 0) { - close(FD(evsel, cpu, 0)); - FD(evsel, cpu, 0) = -1; - } + do { + while (--thread >= 0) { + close(FD(evsel, cpu, thread)); + FD(evsel, cpu, thread) = -1; + } + thread = threads->nr; + } while (--cpu >= 0); return -1; } -int perf_evsel__open_per_thread(struct perf_evsel *evsel, struct thread_map *threads) +static struct { + struct cpu_map map; + int cpus[1]; +} empty_cpu_map = { + .map.nr = 1, + .cpus = { -1, }, +}; + +static struct { + struct thread_map map; + int threads[1]; +} empty_thread_map = { + .map.nr = 1, + .threads = { -1, }, +}; + +int perf_evsel__open(struct perf_evsel *evsel, + struct cpu_map *cpus, struct thread_map *threads) { - int thread; - - if (evsel->fd == NULL && perf_evsel__alloc_fd(evsel, 1, threads->nr)) - return -1; - for (thread = 0; thread < threads->nr; thread++) { - FD(evsel, 0, thread) = sys_perf_event_open(&evsel->attr, - threads->map[thread], -1, -1, 0); - if (FD(evsel, 0, thread) < 0) - goto out_close; + if (cpus == NULL) { + /* Work around old compiler warnings about strict aliasing */ + cpus = &empty_cpu_map.map; } - return 0; + if (threads == NULL) + threads = &empty_thread_map.map; -out_close: - while (--thread >= 0) { - close(FD(evsel, 0, thread)); - FD(evsel, 0, thread) = -1; - } - return -1; + return __perf_evsel__open(evsel, cpus, threads); } -int perf_evsel__open(struct perf_evsel *evsel, - struct cpu_map *cpus, struct thread_map *threads) +int perf_evsel__open_per_cpu(struct perf_evsel *evsel, struct cpu_map *cpus) { - if (threads == NULL) - return perf_evsel__open_per_cpu(evsel, cpus); + return __perf_evsel__open(evsel, cpus, &empty_thread_map.map); +} - return perf_evsel__open_per_thread(evsel, threads); +int perf_evsel__open_per_thread(struct perf_evsel *evsel, struct thread_map *threads) +{ + return __perf_evsel__open(evsel, &empty_cpu_map.map, threads); } -- 1.6.2.5 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 6/6] perf session: Fix infinite loop in __perf_session__process_events 2011-01-11 0:28 [GIT PULL 0/6] perf/urgent fixes and improvements Arnaldo Carvalho de Melo ` (4 preceding siblings ...) 2011-01-11 0:28 ` [PATCH 5/6] perf evsel: Support perf_evsel__open(cpus > 1 && threads > 1) Arnaldo Carvalho de Melo @ 2011-01-11 0:28 ` Arnaldo Carvalho de Melo 2011-01-11 10:57 ` [GIT PULL 0/6] perf/urgent fixes and improvements Ingo Molnar 6 siblings, 0 replies; 10+ messages in thread From: Arnaldo Carvalho de Melo @ 2011-01-11 0:28 UTC (permalink / raw) To: Ingo Molnar Cc: linux-kernel, Arnaldo Carvalho de Melo, David Ahern, Frederic Weisbecker, Ingo Molnar, Mike Galbraith, Paul Mackerras, Peter Zijlstra, Stephane Eranian, Thomas Gleixner, Tom Zanussi From: Arnaldo Carvalho de Melo <acme@redhat.com> In this if statement: if (head + event->header.size >= mmap_size) { if (mmaps[map_idx]) { munmap(mmaps[map_idx], mmap_size); mmaps[map_idx] = NULL; } page_offset = page_size * (head / page_size); file_offset += page_offset; head -= page_offset; goto remap; } With, for instance, these values: head=2992 event->header.size=48 mmap_size=3040 We end up endlessly looping back to remap. Off by one. Problem introduced in 55b4462. Reported-by: Linus Torvalds <torvalds@linux-foundation.org> Reported-by: Ingo Molnar <mingo@elte.hu> Reported-by: David Ahern <daahern@cisco.com> Bisected-by: David Ahern <daahern@cisco.com> Tested-by: David Ahern <daahern@cisco.com> Cc: David Ahern <daahern@cisco.com> Cc: Frederic Weisbecker <fweisbec@gmail.com> Cc: Ingo Molnar <mingo@elte.hu> Cc: Mike Galbraith <efault@gmx.de> Cc: Paul Mackerras <paulus@samba.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Stephane Eranian <eranian@google.com> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Tom Zanussi <tzanussi@gmail.com> LKML-Reference: <new-submission> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- tools/perf/util/session.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c index 6fb4694..313dac2 100644 --- a/tools/perf/util/session.c +++ b/tools/perf/util/session.c @@ -1007,7 +1007,7 @@ more: if (size == 0) size = 8; - if (head + event->header.size >= mmap_size) { + if (head + event->header.size > mmap_size) { if (mmaps[map_idx]) { munmap(mmaps[map_idx], mmap_size); mmaps[map_idx] = NULL; -- 1.6.2.5 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [GIT PULL 0/6] perf/urgent fixes and improvements 2011-01-11 0:28 [GIT PULL 0/6] perf/urgent fixes and improvements Arnaldo Carvalho de Melo ` (5 preceding siblings ...) 2011-01-11 0:28 ` [PATCH 6/6] perf session: Fix infinite loop in __perf_session__process_events Arnaldo Carvalho de Melo @ 2011-01-11 10:57 ` Ingo Molnar 6 siblings, 0 replies; 10+ messages in thread From: Ingo Molnar @ 2011-01-11 10:57 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: linux-kernel, Chris Samuel, David Ahern, Frederic Weisbecker, Jiri Pirko, Linus Torvalds, Mike Galbraith, Paul Mackerras, Peter Zijlstra, Stephane Eranian, Thomas Gleixner, Tom Zanussi * Arnaldo Carvalho de Melo <acme@infradead.org> wrote: > Hi Ingo, > > Please consider pulling from: > > git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux-2.6 perf/urgent > > Regards, > > - Arnaldo > > Arnaldo Carvalho de Melo (4): > perf sched: Fix allocation result check > perf tools: Emit clearer message for sys_perf_event_open ENOENT return > perf evsel: Support perf_evsel__open(cpus > 1 && threads > 1) > perf session: Fix infinite loop in __perf_session__process_events > > David Ahern (1): > perf stat: better error message for unsupported events > > Jiri Pirko (1): > perf sched: Use PTHREAD_STACK_MIN to avoid pthread_attr_setstacksize() fail > > tools/perf/builtin-record.c | 3 + > tools/perf/builtin-sched.c | 5 +- > tools/perf/builtin-stat.c | 2 + > tools/perf/builtin-test.c | 110 +++++++++++++++++++++++++++++++++++++++++++ > tools/perf/builtin-top.c | 2 + > tools/perf/util/evsel.c | 82 +++++++++++++++++++------------- > tools/perf/util/session.c | 2 +- > 7 files changed, 170 insertions(+), 36 deletions(-) Pulled, thanks a lot Arnaldo! Ingo ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2011-01-11 18:58 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-01-11 0:28 [GIT PULL 0/6] perf/urgent fixes and improvements Arnaldo Carvalho de Melo 2011-01-11 0:28 ` [PATCH 1/6] perf sched: Fix allocation result check Arnaldo Carvalho de Melo 2011-01-11 0:28 ` [PATCH 2/6] perf stat: better error message for unsupported events Arnaldo Carvalho de Melo 2011-01-11 0:28 ` [PATCH 3/6] perf tools: Emit clearer message for sys_perf_event_open ENOENT return Arnaldo Carvalho de Melo 2011-01-11 18:40 ` David Ahern 2011-01-11 18:58 ` Arnaldo Carvalho de Melo 2011-01-11 0:28 ` [PATCH 4/6] perf sched: Use PTHREAD_STACK_MIN to avoid pthread_attr_setstacksize() fail Arnaldo Carvalho de Melo 2011-01-11 0:28 ` [PATCH 5/6] perf evsel: Support perf_evsel__open(cpus > 1 && threads > 1) Arnaldo Carvalho de Melo 2011-01-11 0:28 ` [PATCH 6/6] perf session: Fix infinite loop in __perf_session__process_events Arnaldo Carvalho de Melo 2011-01-11 10:57 ` [GIT PULL 0/6] perf/urgent fixes and improvements Ingo Molnar
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).