* [PATCH 1/5] perf tools: Make sure kptr_restrict warnings fit 80 col terms
2011-05-27 21:45 [GIT PULL 0/5] perf/urgent fixes Arnaldo Carvalho de Melo
@ 2011-05-27 21:45 ` Arnaldo Carvalho de Melo
2011-05-27 21:45 ` [PATCH 2/5] perf events: initialize fd array to -1 instead of 0 Arnaldo Carvalho de Melo
` (4 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-05-27 21:45 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Arnaldo Carvalho de Melo, Frederic Weisbecker,
Mike Galbraith, Paul Mackerras, Peter Zijlstra, Stephane Eranian,
Tom Zanussi
From: Arnaldo Carvalho de Melo <acme@redhat.com>
Suggested-by: Ingo Molnar <mingo@elte.hu>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
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>
Link: http://lkml.kernel.org/n/tip-i1p8vrhq7xveyui6t1sc914e@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-record.c | 19 ++++++++-----------
tools/perf/builtin-report.c | 17 +++++++----------
2 files changed, 15 insertions(+), 21 deletions(-)
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 2ca107f..8e2c857 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -824,17 +824,14 @@ int cmd_record(int argc, const char **argv, const char *prefix __used)
symbol__init();
if (symbol_conf.kptr_restrict)
- pr_warning("WARNING: Kernel address maps "
- "(/proc/{kallsyms,modules}) are restricted, "
- "check /proc/sys/kernel/kptr_restrict.\n\n"
- "Samples in kernel functions may not be resolved "
- "if a suitable vmlinux file is not found in the "
- "buildid cache or in the vmlinux path.\n\n"
- "Samples in kernel modules won't be resolved "
- "at all.\n\n"
- "If some relocation was applied (e.g. kexec) "
- "symbols may be misresolved even with a suitable "
- "vmlinux or kallsyms file.\n\n");
+ pr_warning(
+"WARNING: Kernel address maps (/proc/{kallsyms,modules}) are restricted,\n"
+"check /proc/sys/kernel/kptr_restrict.\n\n"
+"Samples in kernel functions may not be resolved if a suitable vmlinux\n"
+"file is not found in the buildid cache or in the vmlinux path.\n\n"
+"Samples in kernel modules won't be resolved at all.\n\n"
+"If some relocation was applied (e.g. kexec) symbols may be misresolved\n"
+"even with a suitable vmlinux or kallsyms file.\n\n");
if (no_buildid_cache || no_buildid)
disable_buildid_cache();
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 99156c3..287a173 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -281,17 +281,14 @@ static int __cmd_report(void)
kernel_kmap->ref_reloc_sym->addr == 0))) {
const struct dso *kdso = kernel_map->dso;
- ui__warning("Kernel address maps "
- "(/proc/{kallsyms,modules}) were restricted, "
- "check /proc/sys/kernel/kptr_restrict before "
- "running 'perf record'.\n\n%s\n\n"
- "Samples in kernel modules can't be resolved "
- "as well.\n\n",
+ ui__warning(
+"Kernel address maps (/proc/{kallsyms,modules}) were restricted.\n\n"
+"Check /proc/sys/kernel/kptr_restrict before running 'perf record'.\n\n%s\n\n"
+"Samples in kernel modules can't be resolved as well.\n\n",
RB_EMPTY_ROOT(&kdso->symbols[MAP__FUNCTION]) ?
- "As no suitable kallsyms nor vmlinux was found, "
- "kernel samples can't be resolved." :
- "If some relocation was applied (e.g. kexec) "
- "symbols may be misresolved.");
+"As no suitable kallsyms nor vmlinux was found, kernel samples\n"
+"can't be resolved." :
+"If some relocation was applied (e.g. kexec) symbols may be misresolved.");
}
if (dump_trace) {
--
1.6.2.5
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 2/5] perf events: initialize fd array to -1 instead of 0
2011-05-27 21:45 [GIT PULL 0/5] perf/urgent fixes Arnaldo Carvalho de Melo
2011-05-27 21:45 ` [PATCH 1/5] perf tools: Make sure kptr_restrict warnings fit 80 col terms Arnaldo Carvalho de Melo
@ 2011-05-27 21:45 ` Arnaldo Carvalho de Melo
2011-05-27 21:45 ` [PATCH 3/5] perf top: Remove unused macro Arnaldo Carvalho de Melo
` (3 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-05-27 21:45 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, David Ahern, Frederic Weisbecker, Ingo Molnar,
Paul Mackerras, Peter Zijlstra, Thomas Gleixner,
Arnaldo Carvalho de Melo
From: David Ahern <dsahern@gmail.com>
perf_evsel__alloc_fd allocates an array of file descriptors with the
memory initialized to 0. The array has dimensions for cpus and threads.
Later, __perf_evsel__open calls sys_perf_event_open for each cpu and thread
dimensions. If the open fails for any of the cpus or threads then the fd's
for this event are closed and the fd entry in the array is set to -1. Now,
if the first attempt fails for the event (e.g., the event is not supported)
the remaining dimensions (cpu > 0 and thread > 0) are not touched and left
at the initialized value of 0.
builtin-stat catches ENOENT and ENOSYS failures and allows the command to
continue. The end result is that stat attempts to read from an fd of 0 which
of course is stdin and so the command hangs until you type ctrl-D.
Resolve by initializing the array to -1 since an fd < 0 is already
handled.
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/1306511914-8016-1-git-send-email-dsahern@gmail.com
Signed-off-by: David Ahern <dsahern@gmail.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/evsel.c | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index ee0fe0d..cca29ed 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -35,7 +35,17 @@ struct perf_evsel *perf_evsel__new(struct perf_event_attr *attr, int idx)
int perf_evsel__alloc_fd(struct perf_evsel *evsel, int ncpus, int nthreads)
{
+ int cpu, thread;
evsel->fd = xyarray__new(ncpus, nthreads, sizeof(int));
+
+ if (evsel->fd) {
+ for (cpu = 0; cpu < ncpus; cpu++) {
+ for (thread = 0; thread < nthreads; thread++) {
+ FD(evsel, cpu, thread) = -1;
+ }
+ }
+ }
+
return evsel->fd != NULL ? 0 : -ENOMEM;
}
--
1.6.2.5
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 3/5] perf top: Remove unused macro
2011-05-27 21:45 [GIT PULL 0/5] perf/urgent fixes Arnaldo Carvalho de Melo
2011-05-27 21:45 ` [PATCH 1/5] perf tools: Make sure kptr_restrict warnings fit 80 col terms Arnaldo Carvalho de Melo
2011-05-27 21:45 ` [PATCH 2/5] perf events: initialize fd array to -1 instead of 0 Arnaldo Carvalho de Melo
@ 2011-05-27 21:45 ` Arnaldo Carvalho de Melo
2011-05-27 21:45 ` [PATCH 4/5] perf top: Handle kptr_restrict Arnaldo Carvalho de Melo
` (2 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-05-27 21:45 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>
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>
Link: http://lkml.kernel.org/n/tip-weqbs0tkk2u0qp1xxdxxosfg@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-top.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 2d7934e..375ed16 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -62,8 +62,6 @@
#include <linux/unistd.h>
#include <linux/types.h>
-#define FD(e, x, y) (*(int *)xyarray__entry(e->fd, x, y))
-
static struct perf_top top = {
.count_filter = 5,
.delay_secs = 2,
--
1.6.2.5
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 4/5] perf top: Handle kptr_restrict
2011-05-27 21:45 [GIT PULL 0/5] perf/urgent fixes Arnaldo Carvalho de Melo
` (2 preceding siblings ...)
2011-05-27 21:45 ` [PATCH 3/5] perf top: Remove unused macro Arnaldo Carvalho de Melo
@ 2011-05-27 21:45 ` Arnaldo Carvalho de Melo
2011-05-27 21:45 ` [PATCH 5/5] perf top: Don't stop if no kernel symtab is found Arnaldo Carvalho de Melo
2011-05-28 9:41 ` [GIT PULL 0/5] perf/urgent fixes Ingo Molnar
5 siblings, 0 replies; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-05-27 21:45 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Arnaldo Carvalho de Melo, Frederic Weisbecker,
Mike Galbraith, Paul Mackerras, Peter Zijlstra, Stephane Eranian,
Tom Zanussi
From: Arnaldo Carvalho de Melo <acme@redhat.com>
Reported-by: Ingo Molnar <mingo@elte.hu>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
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>
Link: http://lkml.kernel.org/n/tip-cyl5zmi1nu35vyu7l5im2pyv@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-top.c | 15 +++++++++++++++
1 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 375ed16..472f627 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -80,6 +80,7 @@ static bool use_tui, use_stdio;
static int default_interval = 0;
+static bool kptr_restrict_warned;
static bool inherit = false;
static int realtime_prio = 0;
static bool group = false;
@@ -738,6 +739,20 @@ static void perf_event__process_sample(const union perf_event *event,
al.filtered)
return;
+ if (!kptr_restrict_warned &&
+ symbol_conf.kptr_restrict &&
+ al.cpumode == PERF_RECORD_MISC_KERNEL) {
+ ui__warning(
+"Kernel address maps (/proc/{kallsyms,modules}) are restricted.\n\n"
+"Check /proc/sys/kernel/kptr_restrict.\n\n"
+"Kernel%s samples will not be resolved.\n",
+ !RB_EMPTY_ROOT(&al.map->dso->symbols[MAP__FUNCTION]) ?
+ " modules" : "");
+ if (use_browser <= 0)
+ sleep(5);
+ kptr_restrict_warned = true;
+ }
+
if (al.sym == NULL) {
/*
* As we do lazy loading of symtabs we only will know if the
--
1.6.2.5
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 5/5] perf top: Don't stop if no kernel symtab is found
2011-05-27 21:45 [GIT PULL 0/5] perf/urgent fixes Arnaldo Carvalho de Melo
` (3 preceding siblings ...)
2011-05-27 21:45 ` [PATCH 4/5] perf top: Handle kptr_restrict Arnaldo Carvalho de Melo
@ 2011-05-27 21:45 ` Arnaldo Carvalho de Melo
2011-05-28 9:41 ` [GIT PULL 0/5] perf/urgent fixes Ingo Molnar
5 siblings, 0 replies; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-05-27 21:45 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Arnaldo Carvalho de Melo, Frederic Weisbecker,
Mike Galbraith, Paul Mackerras, Peter Zijlstra, Stephane Eranian,
Tom Zanussi
From: Arnaldo Carvalho de Melo <acme@redhat.com>
We now just warn the user about the fact and go on providing just
userspace samples.
This fixes a problem when no vmlinux is explicetely passed by the user,
thus symbol_conf.vmlinux_name is NULL, no suitable vmlinux is found, and
then we get:
aldebaran:~> perf top -p 7557
[kernel.kallsyms] with build id 44d9a989eabbd79e486bc079d6b743d397c204e0
not found, continuing without symbols
The (null) file can't be used
Reported-by: Ingo Molnar <mingo@elte.hu>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
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>
Link: http://lkml.kernel.org/n/tip-cj2g81hn64wv2bipmqk4fy2m@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-top.c | 20 +++++++++++++++-----
1 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 472f627..f2f3f49 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -81,6 +81,7 @@ static bool use_tui, use_stdio;
static int default_interval = 0;
static bool kptr_restrict_warned;
+static bool vmlinux_warned;
static bool inherit = false;
static int realtime_prio = 0;
static bool group = false;
@@ -754,6 +755,7 @@ static void perf_event__process_sample(const union perf_event *event,
}
if (al.sym == NULL) {
+ const char *msg = "Kernel samples will not be resolved.\n";
/*
* As we do lazy loading of symtabs we only will know if the
* specified vmlinux file is invalid when we actually have a
@@ -765,12 +767,20 @@ static void perf_event__process_sample(const union perf_event *event,
* --hide-kernel-symbols, even if the user specifies an
* invalid --vmlinux ;-)
*/
- if (al.map == machine->vmlinux_maps[MAP__FUNCTION] &&
+ if (!kptr_restrict_warned && !vmlinux_warned &&
+ al.map == machine->vmlinux_maps[MAP__FUNCTION] &&
RB_EMPTY_ROOT(&al.map->dso->symbols[MAP__FUNCTION])) {
- ui__warning("The %s file can't be used\n",
- symbol_conf.vmlinux_name);
- exit_browser(0);
- exit(1);
+ if (symbol_conf.vmlinux_name) {
+ ui__warning("The %s file can't be used.\n%s",
+ symbol_conf.vmlinux_name, msg);
+ } else {
+ ui__warning("A vmlinux file was not found.\n%s",
+ msg);
+ }
+
+ if (use_browser <= 0)
+ sleep(5);
+ vmlinux_warned = true;
}
return;
--
1.6.2.5
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [GIT PULL 0/5] perf/urgent fixes
2011-05-27 21:45 [GIT PULL 0/5] perf/urgent fixes Arnaldo Carvalho de Melo
` (4 preceding siblings ...)
2011-05-27 21:45 ` [PATCH 5/5] perf top: Don't stop if no kernel symtab is found Arnaldo Carvalho de Melo
@ 2011-05-28 9:41 ` Ingo Molnar
2011-05-29 14:12 ` Arnaldo Carvalho de Melo
5 siblings, 1 reply; 9+ messages in thread
From: Ingo Molnar @ 2011-05-28 9:41 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: linux-kernel, David Ahern, Frederic Weisbecker, Mike Galbraith,
Paul Mackerras, Peter Zijlstra, Stephane Eranian,
Thomas Gleixner, Tom Zanussi, Arnaldo Carvalho de Melo
* 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 tools: Make sure kptr_restrict warnings fit 80 col terms
> perf top: Remove unused macro
> perf top: Handle kptr_restrict
> perf top: Don't stop if no kernel symtab is found
>
> David Ahern (1):
> perf events: initialize fd array to -1 instead of 0
>
> tools/perf/builtin-record.c | 19 ++++++++-----------
> tools/perf/builtin-report.c | 17 +++++++----------
> tools/perf/builtin-top.c | 37 ++++++++++++++++++++++++++++++-------
> tools/perf/util/evsel.c | 10 ++++++++++
> 4 files changed, 55 insertions(+), 28 deletions(-)
Pulled, thanks a lot Arnaldo!
The various variations of kptr_restrict now work fine AFAICS - will
send the fixes to Linus today.
One (very) small issue i noticed, if the user only samples user-space
events then i still get the warning:
aldebaran:~> perf record -e cycles:upp sleep 1
WARNING: Kernel address maps (/proc/{kallsyms,modules}) are restricted,
although perf does not record any kernel samples so kptr_restrict is
irrelevant in this case.
Ingo
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [GIT PULL 0/5] perf/urgent fixes
2011-05-28 9:41 ` [GIT PULL 0/5] perf/urgent fixes Ingo Molnar
@ 2011-05-29 14:12 ` Arnaldo Carvalho de Melo
2011-05-29 14:30 ` Ingo Molnar
0 siblings, 1 reply; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-05-29 14:12 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, David Ahern, Frederic Weisbecker, Mike Galbraith,
Paul Mackerras, Peter Zijlstra, Stephane Eranian,
Thomas Gleixner, Tom Zanussi
Em Sat, May 28, 2011 at 11:41:50AM +0200, Ingo Molnar escreveu:
> The various variations of kptr_restrict now work fine AFAICS - will
> send the fixes to Linus today.
>
> One (very) small issue i noticed, if the user only samples user-space
> events then i still get the warning:
>
> aldebaran:~> perf record -e cycles:upp sleep 1
> WARNING: Kernel address maps (/proc/{kallsyms,modules}) are restricted,
>
> although perf does not record any kernel samples so kptr_restrict is
> irrelevant in this case.
I'll have that fixed too, oversight. We really should avoid confusing
messages, and this one, as you mention, seems just noise as I still have
to look again how vsyscall symbols get resolved, IIRC they need a kernel
symtab, but then, if that is the case, the message should be changed
accordingly.
- Arnaldo
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [GIT PULL 0/5] perf/urgent fixes
2011-05-29 14:12 ` Arnaldo Carvalho de Melo
@ 2011-05-29 14:30 ` Ingo Molnar
0 siblings, 0 replies; 9+ messages in thread
From: Ingo Molnar @ 2011-05-29 14:30 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: linux-kernel, David Ahern, Frederic Weisbecker, Mike Galbraith,
Paul Mackerras, Peter Zijlstra, Stephane Eranian,
Thomas Gleixner, Tom Zanussi
* Arnaldo Carvalho de Melo <acme@infradead.org> wrote:
> Em Sat, May 28, 2011 at 11:41:50AM +0200, Ingo Molnar escreveu:
> > The various variations of kptr_restrict now work fine AFAICS - will
> > send the fixes to Linus today.
> >
> > One (very) small issue i noticed, if the user only samples user-space
> > events then i still get the warning:
> >
> > aldebaran:~> perf record -e cycles:upp sleep 1
> > WARNING: Kernel address maps (/proc/{kallsyms,modules}) are restricted,
> >
> > although perf does not record any kernel samples so kptr_restrict is
> > irrelevant in this case.
>
> I'll have that fixed too, oversight. We really should avoid
> confusing messages, and this one, as you mention, seems just noise
> as I still have to look again how vsyscall symbols get resolved,
> IIRC they need a kernel symtab, but then, if that is the case, the
> message should be changed accordingly.
Btw., if you touch that code, would you be interested in adding a
--user-events kind of option which would have the implicit effect of
adding :u to every event that is listed (or implied)? (and a
--kernel-events counterpart, for :k)
The reason is that right now there's no easy way to run say 'perf
stat -ddd' but measure userspace events only.
[ perf record --repeat would be nice as well :-) And a pony! ]
Thanks,
Ingo
^ permalink raw reply [flat|nested] 9+ messages in thread