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
>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?
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
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
>
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
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
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
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