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(-)
From: Arnaldo Carvalho de Melo <[email protected]>
Suggested-by: Ingo Molnar <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Cc: Tom Zanussi <[email protected]>
Link: http://lkml.kernel.org/n/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
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
From: David Ahern <[email protected]>
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 <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: David Ahern <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
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
From: Arnaldo Carvalho de Melo <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Cc: Tom Zanussi <[email protected]>
Link: http://lkml.kernel.org/n/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
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
From: Arnaldo Carvalho de Melo <[email protected]>
Reported-by: Ingo Molnar <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Cc: Tom Zanussi <[email protected]>
Link: http://lkml.kernel.org/n/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
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
From: Arnaldo Carvalho de Melo <[email protected]>
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 <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Cc: Tom Zanussi <[email protected]>
Link: http://lkml.kernel.org/n/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
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
* Arnaldo Carvalho de Melo <[email protected]> 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
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
* Arnaldo Carvalho de Melo <[email protected]> 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