2015-11-20 06:03:09

by Namhyung Kim

[permalink] [raw]
Subject: [RFC/PATCH] perf tools: Introduce perf_thread for backtrace

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.

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 <[email protected]>
Cc: Masami Hiramatsu <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
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


Subject: RE: [RFC/PATCH] perf tools: Introduce perf_thread for backtrace

>From: Namhyung Kim [mailto:[email protected]]
>
>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.

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 <[email protected]>
>Cc: Masami Hiramatsu <[email protected]>
>Signed-off-by: Namhyung Kim <[email protected]>
>---
> 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

????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2015-11-20 09:30:32

by Jiri Olsa

[permalink] [raw]
Subject: Re: [RFC/PATCH] perf tools: Introduce perf_thread for backtrace

On Fri, Nov 20, 2015 at 03:03:03PM +0900, Namhyung Kim wrote:
> 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.
>
> 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]

nice idea!

SNIP

> +
> +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;
> + }

I think we could treat errors the usual way in here..
if fail to alloc this early, something is terribly wrong anyway

jirka

2015-11-20 12:10:50

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [RFC/PATCH] perf tools: Introduce perf_thread for backtrace

Em Fri, Nov 20, 2015 at 09:05:23AM +0000, 平松雅巳 / HIRAMATU,MASAMI escreveu:
> >From: Namhyung Kim [mailto:[email protected]]
> >
> >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 <[email protected]>
> >Cc: Masami Hiramatsu <[email protected]>
> >Signed-off-by: Namhyung Kim <[email protected]>
> >---
> > 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
>

2015-11-23 21:39:42

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [RFC/PATCH] perf tools: Introduce perf_thread for backtrace

Em Fri, Nov 20, 2015 at 03:03:03PM +0900, Namhyung Kim escreveu:
> 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.
>
> The backtrace output from TUI is changed like below. (I made a key
> action to generate a segfault for testing):

This is a really nice use of what we have, I guess we could simplify
things further, not having to create a struct machine, as we have just
one thread, and we're not interested in kernel addresses, so no need for
kmaps, etc.

But as-is it already looks better than what we were using :-)

I'll try testing it further and will probably switch to using it if
nobody voices any problem we haven't realised with such approach.

- Arnaldo

> 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 <[email protected]>
> Cc: Masami Hiramatsu <[email protected]>
> Signed-off-by: Namhyung Kim <[email protected]>
> ---
> 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

2015-11-24 07:24:58

by Namhyung Kim

[permalink] [raw]
Subject: Re: [RFC/PATCH] perf tools: Introduce perf_thread for backtrace

Hi Arnaldo and Masami,

On Fri, Nov 20, 2015 at 09:10:44AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Nov 20, 2015 at 09:05:23AM +0000, 平松雅巳 / HIRAMATU,MASAMI escreveu:
> > >From: Namhyung Kim [mailto:[email protected]]
> > >
> > >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?

Yeah, that's a possible solution. We can start by using our own, and
if there's a certain amount of failure in symbol resolving, then
fallback to glibc's backtrace_symbols + addr2line.


>
> > For the some (static) functions, I'd rather like to use glibc's
> > backtrace_symbols and addr2line or even with raw address for
> > reliability...

I also printed the raw addresses in case of doubts, so you could
verify its correctness. :) And IMHO, if something is severely broken,
we might not rely on glibc too.

Having said that, I agree with your concern and it needs the fallback
method for possible malfunction. But I guess it'd work quite well for
most cases so it's worth trying to convert using it. I'll work on the
fallback method then..

Thanks,
Namhyung

2015-11-24 07:34:51

by Namhyung Kim

[permalink] [raw]
Subject: Re: [RFC/PATCH] perf tools: Introduce perf_thread for backtrace

On Mon, Nov 23, 2015 at 06:39:36PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Nov 20, 2015 at 03:03:03PM +0900, Namhyung Kim escreveu:
> > 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.
> >
> > The backtrace output from TUI is changed like below. (I made a key
> > action to generate a segfault for testing):
>
> This is a really nice use of what we have, I guess we could simplify
> things further, not having to create a struct machine, as we have just
> one thread, and we're not interested in kernel addresses, so no need for
> kmaps, etc.

Yes, I thought about it. But as adding the perf thread to an existing
machine can affect other thread(s), I didn't do it. Maybe we can set
the perf threads' machine pointer in a hacky way without adding the
thread into the machine, but I'd rather not doing that too because
it's fragile and current code is simple enough IMHO.


>
> But as-is it already looks better than what we were using :-)
>
> I'll try testing it further and will probably switch to using it if
> nobody voices any problem we haven't realised with such approach.

Thank you!
Namhyung

2015-11-24 07:36:59

by Namhyung Kim

[permalink] [raw]
Subject: Re: [RFC/PATCH] perf tools: Introduce perf_thread for backtrace

Hi Jiri,

On Fri, Nov 20, 2015 at 10:29:48AM +0100, Jiri Olsa wrote:
> On Fri, Nov 20, 2015 at 03:03:03PM +0900, Namhyung Kim wrote:
> > 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.
> >
> > 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]
>
> nice idea!
>
> SNIP
>
> > +
> > +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;
> > + }
>
> I think we could treat errors the usual way in here..
> if fail to alloc this early, something is terribly wrong anyway

OK, I'll change it to return error code and let perf fail with it.

Thanks,
Namhyung