From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: "平松雅巳 / HIRAMATU,MASAMI" <masami.hiramatsu.pt@hitachi.com>
Cc: "'Namhyung Kim'" <namhyung@kernel.org>,
Ingo Molnar <mingo@kernel.org>,
Peter Zijlstra <a.p.zijlstra@chello.nl>,
Jiri Olsa <jolsa@redhat.com>, LKML <linux-kernel@vger.kernel.org>,
David Ahern <dsahern@gmail.com>,
Frederic Weisbecker <fweisbec@gmail.com>
Subject: Re: [RFC/PATCH] perf tools: Introduce perf_thread for backtrace
Date: Fri, 20 Nov 2015 09:10:44 -0300 [thread overview]
Message-ID: <20151120121044.GJ29361@kernel.org> (raw)
In-Reply-To: <50399556C9727B4D88A595C8584AAB375262792C@GSjpTKYDCembx32.service.hitachi.net>
Em Fri, Nov 20, 2015 at 09:05:23AM +0000, 平松雅巳 / HIRAMATU,MASAMI escreveu:
> >From: Namhyung Kim [mailto:namhyung@kernel.org]
> >
> >Backtrace is a crucial info for debugging. And upcoming refcnt
> >tracking facility also wants to use it.
> >
> >So instead of relying on glibc's backtrace_symbols[_fd] which misses
> >some (static) functions , use our own symbol searching mechanism. To
> >do that, add perf_thread global variable to keep its maps and symbols.
>
> Hmm, I doubt that this can work for debugging situation, because
> sometimes backtrace facilities has to debug itself by itself.
That is a valid point, possibly we can have both and when we think that
the code we rely on for resolving symbols has issues, activate the
other, more expensive, binutils/elfutils spawned command line utilities
to do compare the results?
> For the some (static) functions, I'd rather like to use glibc's
> backtrace_symbols and addr2line or even with raw address for
> reliability...
> Thank you,
>
> >
> >The backtrace output from TUI is changed like below. (I made a key
> >action to generate a segfault for testing):
> >
> >Before:
> > perf: Segmentation fault
> > -------- backtrace --------
> > perf[0x544a8b]
> > /usr/lib/libc.so.6(+0x33680)[0x7fc46420b680]
> > perf[0x54041b]
> > perf(perf_evlist__tui_browse_hists+0x91)[0x5432e1]
> > perf(cmd_report+0x1d20)[0x43cb10]
> > perf[0x487073]
> > perf(main+0x62f)[0x42cb1f]
> > /usr/lib/libc.so.6(__libc_start_main+0xf0)[0x7fc4641f8610]
> > perf(_start+0x29)[0x42cc39]
> > [0x0]
> >
> >After:
> > perf: Segmentation fault
> > -------- backtrace --------
> > perf_evsel__hists_browse(+0x43b) in perf [0x54066b]
> > perf_evlist__tui_browse_hists(+0x91) in perf [0x543531]
> > cmd_report(+0x1d20) in perf [0x43cb50]
> > run_builtin(+0x53) in perf [0x4870b3]
> > main(+0x634) in perf [0x42cb54]
> > __libc_start_main(+0xf0) in libc-2.22.so [0x7fea3577c610]
> > _start(+0x29) in perf [0x42cc79]
> > [0x0]
> >
> >Cc: Frederic Weisbecker <fweisbec@gmail.com>
> >Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> >Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> >---
> > tools/perf/perf.c | 7 +++++-
> > tools/perf/ui/tui/setup.c | 21 ++++++++++++++--
> > tools/perf/util/util.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++
> > tools/perf/util/util.h | 5 ++++
> > 4 files changed, 92 insertions(+), 3 deletions(-)
> >
> >diff --git a/tools/perf/perf.c b/tools/perf/perf.c
> >index 4bee53c3f796..f77eb440b05c 100644
> >--- a/tools/perf/perf.c
> >+++ b/tools/perf/perf.c
> >@@ -604,6 +604,8 @@ int main(int argc, const char **argv)
> > */
> > pthread__block_sigwinch();
> >
> >+ create_perf_thread();
> >+
> > while (1) {
> > static int done_help;
> > int was_alias = run_argv(&argc, &argv);
> >@@ -615,7 +617,7 @@ int main(int argc, const char **argv)
> > fprintf(stderr, "Expansion of alias '%s' failed; "
> > "'%s' is not a perf-command\n",
> > cmd, argv[0]);
> >- goto out;
> >+ goto out_destroy;
> > }
> > if (!done_help) {
> > cmd = argv[0] = help_unknown_cmd(cmd);
> >@@ -626,6 +628,9 @@ int main(int argc, const char **argv)
> >
> > fprintf(stderr, "Failed to run command '%s': %s\n",
> > cmd, strerror_r(errno, sbuf, sizeof(sbuf)));
> >+
> >+out_destroy:
> >+ destroy_perf_thread();
> > out:
> > return 1;
> > }
> >diff --git a/tools/perf/ui/tui/setup.c b/tools/perf/ui/tui/setup.c
> >index 7dfeba0a91f3..bc2da884e65a 100644
> >--- a/tools/perf/ui/tui/setup.c
> >+++ b/tools/perf/ui/tui/setup.c
> >@@ -13,6 +13,8 @@
> > #include "../libslang.h"
> > #include "../keysyms.h"
> > #include "tui.h"
> >+#include "../../util/symbol.h"
> >+#include "../../util/thread.h"
> >
> > static volatile int ui__need_resize;
> >
> >@@ -96,14 +98,29 @@ int ui__getch(int delay_secs)
> > static void ui__signal_backtrace(int sig)
> > {
> > void *stackdump[32];
> >- size_t size;
> >+ size_t size, i;
> >
> > ui__exit(false);
> > psignal(sig, "perf");
> >
> > printf("-------- backtrace --------\n");
> > size = backtrace(stackdump, ARRAY_SIZE(stackdump));
> >- backtrace_symbols_fd(stackdump, size, STDOUT_FILENO);
> >+ /* skip first two stack frame (for this function and signal stack) */
> >+ for (i = 2; i < size; i++) {
> >+ struct addr_location al = {
> >+ .sym = NULL,
> >+ };
> >+
> >+ thread__find_addr_location(perf_thread, PERF_RECORD_MISC_USER,
> >+ MAP__FUNCTION, (long)stackdump[i], &al);
> >+
> >+ if (al.sym)
> >+ printf("%s(+0x%"PRIx64") in ", al.sym->name,
> >+ map__map_ip(al.map, (u64)stackdump[i]) - al.sym->start);
> >+ if (al.map)
> >+ printf("%s ", al.map->dso->short_name);
> >+ printf("[0x%lx]\n", (unsigned long)stackdump[i]);
> >+ }
> >
> > exit(0);
> > }
> >diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
> >index 75759aebc7b8..f1a26ea14053 100644
> >--- a/tools/perf/util/util.c
> >+++ b/tools/perf/util/util.c
> >@@ -16,6 +16,9 @@
> > #include <linux/kernel.h>
> > #include <unistd.h>
> > #include "callchain.h"
> >+#include "machine.h"
> >+#include "thread.h"
> >+#include "thread_map.h"
> >
> > struct callchain_param callchain_param = {
> > .mode = CHAIN_GRAPH_ABS,
> >@@ -696,3 +699,62 @@ fetch_kernel_version(unsigned int *puint, char *str,
> > *puint = (version << 16) + (patchlevel << 8) + sublevel;
> > return 0;
> > }
> >+
> >+
> >+static int process_event(struct perf_tool *tool, union perf_event *event,
> >+ struct perf_sample *sample, struct machine *machine)
> >+{
> >+ switch (event->header.type) {
> >+ case PERF_RECORD_COMM:
> >+ return tool->comm(tool, event, sample, machine);
> >+ case PERF_RECORD_MMAP:
> >+ return tool->mmap(tool, event, sample, machine);
> >+ case PERF_RECORD_MMAP2:
> >+ return tool->mmap2(tool, event, sample, machine);
> >+ default:
> >+ break;
> >+ }
> >+ return 0;
> >+}
> >+
> >+struct thread *perf_thread;
> >+
> >+void create_perf_thread(void)
> >+{
> >+ struct perf_tool tool = {
> >+ .comm = perf_event__process_comm,
> >+ .mmap = perf_event__process_mmap,
> >+ .mmap2 = perf_event__process_mmap2,
> >+ };
> >+ struct thread_map *tm;
> >+ struct machine *machine;
> >+ int pid = getpid();
> >+
> >+ machine = machine__new_host();
> >+ if (machine == NULL)
> >+ return;
> >+
> >+ tm = thread_map__new_dummy();
> >+ if (tm == NULL) {
> >+ machine__delete(machine);
> >+ return;
> >+ }
> >+
> >+ thread_map__set_pid(tm, 0, pid);
> >+
> >+ perf_event__synthesize_thread_map(&tool, tm, process_event, machine,
> >+ false, 500);
> >+
> >+ perf_thread = machine__find_thread(machine, pid, pid);
> >+ BUG_ON(perf_thread == NULL);
> >+
> >+ thread_map__put(tm);
> >+}
> >+
> >+void destroy_perf_thread(void)
> >+{
> >+ struct machine *machine = perf_thread->mg->machine;
> >+
> >+ machine__delete_threads(machine);
> >+ machine__delete(machine);
> >+}
> >diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
> >index dcc659017976..630e145049aa 100644
> >--- a/tools/perf/util/util.h
> >+++ b/tools/perf/util/util.h
> >@@ -358,4 +358,9 @@ int fetch_kernel_version(unsigned int *puint,
> > #define KVER_FMT "%d.%d.%d"
> > #define KVER_PARAM(x) KVER_VERSION(x), KVER_PATCHLEVEL(x), KVER_SUBLEVEL(x)
> >
> >+extern struct thread *perf_thread;
> >+
> >+void create_perf_thread(void);
> >+void destroy_perf_thread(void);
> >+
> > #endif /* GIT_COMPAT_UTIL_H */
> >--
> >2.6.2
>
next prev parent reply other threads:[~2015-11-20 12:10 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-11-20 6:03 [RFC/PATCH] perf tools: Introduce perf_thread for backtrace Namhyung Kim
2015-11-20 9:05 ` 平松雅巳 / HIRAMATU,MASAMI
2015-11-20 12:10 ` Arnaldo Carvalho de Melo [this message]
2015-11-24 7:24 ` Namhyung Kim
2015-11-20 9:29 ` Jiri Olsa
2015-11-24 7:36 ` Namhyung Kim
2015-11-23 21:39 ` Arnaldo Carvalho de Melo
2015-11-24 7:34 ` Namhyung Kim
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20151120121044.GJ29361@kernel.org \
--to=acme@kernel.org \
--cc=a.p.zijlstra@chello.nl \
--cc=dsahern@gmail.com \
--cc=fweisbec@gmail.com \
--cc=jolsa@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=masami.hiramatsu.pt@hitachi.com \
--cc=mingo@kernel.org \
--cc=namhyung@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.