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):
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]
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]
Cc: Frederic Weisbecker <[email protected]>
Cc: Masami Hiramatsu <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
v2) fallback to glibc's backtrace_symbols_fd() if failed
tools/perf/perf.c | 7 +++++-
tools/perf/ui/tui/setup.c | 31 ++++++++++++++++++++++--
tools/perf/util/util.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++
tools/perf/util/util.h | 5 ++++
4 files changed, 102 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..ba0ff1c4e6b7 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,39 @@ 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,
+ };
+
+ /* valid callchain ends with 0 address */
+ if (stackdump[i] == NULL && i == (size - 1))
+ continue;
+
+ thread__find_addr_location(perf_thread, PERF_RECORD_MISC_USER,
+ MAP__FUNCTION, (long)stackdump[i], &al);
+
+ if (!al.sym)
+ break;
+
+ printf("%s(+0x%"PRIx64") in %s [0x%lx]\n", al.sym->name,
+ map__map_ip(al.map, (u64)stackdump[i]) - al.sym->start,
+ al.map->dso->short_name, (unsigned long)stackdump[i]);
+ }
+
+ if (i != size) {
+ printf("\nperf backtrace seems broken, try again with glibc\n");
+ printf("-------- backtrace --------\n");
+ backtrace_symbols_fd(stackdump, size, STDOUT_FILENO);
+ }
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
On Wed, Dec 02, 2015 at 02:42:21PM +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):
>
> 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]
>
> 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]
>
> Cc: Frederic Weisbecker <[email protected]>
> Cc: Masami Hiramatsu <[email protected]>
> Signed-off-by: Namhyung Kim <[email protected]>
> ---
> v2) fallback to glibc's backtrace_symbols_fd() if failed
create_perf_thread still returns void
jirka
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):
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]
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]
Cc: Frederic Weisbecker <[email protected]>
Cc: Masami Hiramatsu <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
v3) check return value of create_perf_thread()
v2) fallback to glibc's backtrace_symbols_fd() if failed
tools/perf/perf.c | 10 +++++++-
tools/perf/ui/tui/setup.c | 31 +++++++++++++++++++++--
tools/perf/util/util.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++
tools/perf/util/util.h | 5 ++++
4 files changed, 106 insertions(+), 3 deletions(-)
diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index 4bee53c3f796..f27dc5fa9534 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -604,6 +604,11 @@ int main(int argc, const char **argv)
*/
pthread__block_sigwinch();
+ if (create_perf_thread() < 0) {
+ fprintf(stderr, "Failed to reserve information for backtrace\n");
+ goto out;
+ }
+
while (1) {
static int done_help;
int was_alias = run_argv(&argc, &argv);
@@ -615,7 +620,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 +631,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..ba0ff1c4e6b7 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,39 @@ 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,
+ };
+
+ /* valid callchain ends with 0 address */
+ if (stackdump[i] == NULL && i == (size - 1))
+ continue;
+
+ thread__find_addr_location(perf_thread, PERF_RECORD_MISC_USER,
+ MAP__FUNCTION, (long)stackdump[i], &al);
+
+ if (!al.sym)
+ break;
+
+ printf("%s(+0x%"PRIx64") in %s [0x%lx]\n", al.sym->name,
+ map__map_ip(al.map, (u64)stackdump[i]) - al.sym->start,
+ al.map->dso->short_name, (unsigned long)stackdump[i]);
+ }
+
+ if (i != size) {
+ printf("\nperf backtrace seems broken, try again with glibc\n");
+ printf("-------- backtrace --------\n");
+ backtrace_symbols_fd(stackdump, size, STDOUT_FILENO);
+ }
exit(0);
}
diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
index 75759aebc7b8..b1e591b13131 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,63 @@ 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;
+
+int 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 -1;
+
+ tm = thread_map__new_dummy();
+ if (tm == NULL) {
+ machine__delete(machine);
+ return -1;
+ }
+
+ 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);
+ return 0;
+}
+
+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..769986044a7a 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;
+
+int create_perf_thread(void);
+void destroy_perf_thread(void);
+
#endif /* GIT_COMPAT_UTIL_H */
--
2.6.2
On Thu, Dec 03, 2015 at 12:33:40AM +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):
>
> 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]
>
> 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]
>
> Cc: Frederic Weisbecker <[email protected]>
> Cc: Masami Hiramatsu <[email protected]>
> Signed-off-by: Namhyung Kim <[email protected]>
> ---
> v3) check return value of create_perf_thread()
Acked-by: Jiri Olsa <[email protected]>
thanks,
jirka
>From: Namhyung Kim [mailto:[email protected]] On Behalf Of Namhyung Kim
>
>Backtrace is a crucial info for debugging. And upcoming refcnt
>tracking facility also wants to use it.
Note that the refcnt backtrace symbol resolution will work at
exit. This means that it can not depend on the feature in perf
tools itself. (and of course, since the refcnt tries to find unused
objects in perf tools at exit, if we use perf_thread, it will
detect the objects related to the perf_thread are leaked)
>
>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):
>
>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]
>
>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]
>
>Cc: Frederic Weisbecker <[email protected]>
>Cc: Masami Hiramatsu <[email protected]>
>Signed-off-by: Namhyung Kim <[email protected]>
>---
>v3) check return value of create_perf_thread()
>
>v2) fallback to glibc's backtrace_symbols_fd() if failed
>
> tools/perf/perf.c | 10 +++++++-
> tools/perf/ui/tui/setup.c | 31 +++++++++++++++++++++--
> tools/perf/util/util.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++
> tools/perf/util/util.h | 5 ++++
> 4 files changed, 106 insertions(+), 3 deletions(-)
>
>diff --git a/tools/perf/perf.c b/tools/perf/perf.c
>index 4bee53c3f796..f27dc5fa9534 100644
>--- a/tools/perf/perf.c
>+++ b/tools/perf/perf.c
>@@ -604,6 +604,11 @@ int main(int argc, const char **argv)
> */
> pthread__block_sigwinch();
>
>+ if (create_perf_thread() < 0) {
>+ fprintf(stderr, "Failed to reserve information for backtrace\n");
>+ goto out;
>+ }
>+
> while (1) {
> static int done_help;
> int was_alias = run_argv(&argc, &argv);
>@@ -615,7 +620,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 +631,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..ba0ff1c4e6b7 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,39 @@ 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,
>+ };
>+
>+ /* valid callchain ends with 0 address */
>+ if (stackdump[i] == NULL && i == (size - 1))
>+ continue;
>+
>+ thread__find_addr_location(perf_thread, PERF_RECORD_MISC_USER,
>+ MAP__FUNCTION, (long)stackdump[i], &al);
>+
>+ if (!al.sym)
>+ break;
>+
>+ printf("%s(+0x%"PRIx64") in %s [0x%lx]\n", al.sym->name,
>+ map__map_ip(al.map, (u64)stackdump[i]) - al.sym->start,
>+ al.map->dso->short_name, (unsigned long)stackdump[i]);
>+ }
>+
>+ if (i != size) {
>+ printf("\nperf backtrace seems broken, try again with glibc\n");
>+ printf("-------- backtrace --------\n");
>+ backtrace_symbols_fd(stackdump, size, STDOUT_FILENO);
>+ }
>
> exit(0);
> }
>diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
>index 75759aebc7b8..b1e591b13131 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,63 @@ 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;
>+
>+int 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 -1;
>+
>+ tm = thread_map__new_dummy();
>+ if (tm == NULL) {
>+ machine__delete(machine);
>+ return -1;
>+ }
>+
>+ 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);
>+ return 0;
>+}
>+
>+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..769986044a7a 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;
>+
>+int 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 Thu, Dec 03, 2015 at 12:15:12AM +0000, 平松雅巳 / HIRAMATU，MASAMI wrote:
> >From: Namhyung Kim [mailto:[email protected]] On Behalf Of Namhyung Kim
> >
> >Backtrace is a crucial info for debugging. And upcoming refcnt
> >tracking facility also wants to use it.
>
> Note that the refcnt backtrace symbol resolution will work at
> exit. This means that it can not depend on the feature in perf
> tools itself. (and of course, since the refcnt tries to find unused
> objects in perf tools at exit, if we use perf_thread, it will
> detect the objects related to the perf_thread are leaked)
Hmm.. right.
I think we can leave the perf_thread outside of refcnt infrastructure.
IOW it should be created before refcnt debugging is activated and
released after refcnt is done. What do you think?
Thanks,
Namhyung
>
> >
> >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):
> >
> >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]
> >
> >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]
> >
> >Cc: Frederic Weisbecker <[email protected]>
> >Cc: Masami Hiramatsu <[email protected]>
> >Signed-off-by: Namhyung Kim <[email protected]>
> >---
> >v3) check return value of create_perf_thread()
> >
> >v2) fallback to glibc's backtrace_symbols_fd() if failed
> >
> > tools/perf/perf.c | 10 +++++++-
> > tools/perf/ui/tui/setup.c | 31 +++++++++++++++++++++--
> > tools/perf/util/util.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++
> > tools/perf/util/util.h | 5 ++++
> > 4 files changed, 106 insertions(+), 3 deletions(-)
> >
> >diff --git a/tools/perf/perf.c b/tools/perf/perf.c
> >index 4bee53c3f796..f27dc5fa9534 100644
> >--- a/tools/perf/perf.c
> >+++ b/tools/perf/perf.c
> >@@ -604,6 +604,11 @@ int main(int argc, const char **argv)
> > */
> > pthread__block_sigwinch();
> >
> >+ if (create_perf_thread() < 0) {
> >+ fprintf(stderr, "Failed to reserve information for backtrace\n");
> >+ goto out;
> >+ }
> >+
> > while (1) {
> > static int done_help;
> > int was_alias = run_argv(&argc, &argv);
> >@@ -615,7 +620,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 +631,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..ba0ff1c4e6b7 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,39 @@ 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,
> >+ };
> >+
> >+ /* valid callchain ends with 0 address */
> >+ if (stackdump[i] == NULL && i == (size - 1))
> >+ continue;
> >+
> >+ thread__find_addr_location(perf_thread, PERF_RECORD_MISC_USER,
> >+ MAP__FUNCTION, (long)stackdump[i], &al);
> >+
> >+ if (!al.sym)
> >+ break;
> >+
> >+ printf("%s(+0x%"PRIx64") in %s [0x%lx]\n", al.sym->name,
> >+ map__map_ip(al.map, (u64)stackdump[i]) - al.sym->start,
> >+ al.map->dso->short_name, (unsigned long)stackdump[i]);
> >+ }
> >+
> >+ if (i != size) {
> >+ printf("\nperf backtrace seems broken, try again with glibc\n");
> >+ printf("-------- backtrace --------\n");
> >+ backtrace_symbols_fd(stackdump, size, STDOUT_FILENO);
> >+ }
> >
> > exit(0);
> > }
> >diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
> >index 75759aebc7b8..b1e591b13131 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,63 @@ 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;
> >+
> >+int 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 -1;
> >+
> >+ tm = thread_map__new_dummy();
> >+ if (tm == NULL) {
> >+ machine__delete(machine);
> >+ return -1;
> >+ }
> >+
> >+ 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);
> >+ return 0;
> >+}
> >+
> >+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..769986044a7a 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;
> >+
> >+int create_perf_thread(void);
> >+void destroy_perf_thread(void);
> >+
> > #endif /* GIT_COMPAT_UTIL_H */
> >--
> >2.6.2
>
From: Namhyung Kim [mailto:[email protected]]
>
>On Thu, Dec 03, 2015 at 12:15:12AM +0000, 平松雅巳 / HIRAMATU，MASAMI wrote:
>> >From: Namhyung Kim [mailto:[email protected]] On Behalf Of Namhyung Kim
>> >
>> >Backtrace is a crucial info for debugging. And upcoming refcnt
>> >tracking facility also wants to use it.
>>
>> Note that the refcnt backtrace symbol resolution will work at
>> exit. This means that it can not depend on the feature in perf
>> tools itself. (and of course, since the refcnt tries to find unused
>> objects in perf tools at exit, if we use perf_thread, it will
>> detect the objects related to the perf_thread are leaked)
>
>Hmm.. right.
>
>I think we can leave the perf_thread outside of refcnt infrastructure.
>IOW it should be created before refcnt debugging is activated and
>released after refcnt is done. What do you think?
Would you mean we don't debug the objects related to a perf_thread?
It will mean that you don't debug anything, since perf_thread involves
most of refcnt using objects, like dso, map, map_groups etc. And some
bugs are actually found at where those objects are handled.
I would not like to care about the output quality of the backtrace_symbols.
I only need the top 2-3 addresses of the backtrace buffer, because I have
(eu-)addr2line command to find the actual source code lines from those
addresses :). If you need, I can also provide an address decoder awk/shell
script for that.
Instead, I prefer to avoid complexity on the "debugging feature"(because
it can introduce new bugs,) and make it more robust (e.g. if we failed to
get symbol, just shows the raw address)
BTW, the robustness is a key point for debugging. Please consider the case
that you hit an error on the objects in the perf_thread, it could cause
double fault(segv again) on the same object. That is what I actually don't
want.
Thank you,
>
>Thanks,
>Namhyung
>
>
>>
>> >
>> >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):
>> >
>> >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]
>> >
>> >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]
>> >
>> >Cc: Frederic Weisbecker <[email protected]>
>> >Cc: Masami Hiramatsu <[email protected]>
>> >Signed-off-by: Namhyung Kim <[email protected]>
>> >---
>> >v3) check return value of create_perf_thread()
>> >
>> >v2) fallback to glibc's backtrace_symbols_fd() if failed
>> >
>> > tools/perf/perf.c | 10 +++++++-
>> > tools/perf/ui/tui/setup.c | 31 +++++++++++++++++++++--
>> > tools/perf/util/util.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++
>> > tools/perf/util/util.h | 5 ++++
>> > 4 files changed, 106 insertions(+), 3 deletions(-)
>> >
>> >diff --git a/tools/perf/perf.c b/tools/perf/perf.c
>> >index 4bee53c3f796..f27dc5fa9534 100644
>> >--- a/tools/perf/perf.c
>> >+++ b/tools/perf/perf.c
>> >@@ -604,6 +604,11 @@ int main(int argc, const char **argv)
>> > */
>> > pthread__block_sigwinch();
>> >
>> >+ if (create_perf_thread() < 0) {
>> >+ fprintf(stderr, "Failed to reserve information for backtrace\n");
>> >+ goto out;
>> >+ }
>> >+
>> > while (1) {
>> > static int done_help;
>> > int was_alias = run_argv(&argc, &argv);
>> >@@ -615,7 +620,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 +631,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..ba0ff1c4e6b7 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,39 @@ 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,
>> >+ };
>> >+
>> >+ /* valid callchain ends with 0 address */
>> >+ if (stackdump[i] == NULL && i == (size - 1))
>> >+ continue;
>> >+
>> >+ thread__find_addr_location(perf_thread, PERF_RECORD_MISC_USER,
>> >+ MAP__FUNCTION, (long)stackdump[i], &al);
>> >+
>> >+ if (!al.sym)
>> >+ break;
>> >+
>> >+ printf("%s(+0x%"PRIx64") in %s [0x%lx]\n", al.sym->name,
>> >+ map__map_ip(al.map, (u64)stackdump[i]) - al.sym->start,
>> >+ al.map->dso->short_name, (unsigned long)stackdump[i]);
>> >+ }
>> >+
>> >+ if (i != size) {
>> >+ printf("\nperf backtrace seems broken, try again with glibc\n");
>> >+ printf("-------- backtrace --------\n");
>> >+ backtrace_symbols_fd(stackdump, size, STDOUT_FILENO);
>> >+ }
>> >
>> > exit(0);
>> > }
>> >diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
>> >index 75759aebc7b8..b1e591b13131 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,63 @@ 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;
>> >+
>> >+int 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 -1;
>> >+
>> >+ tm = thread_map__new_dummy();
>> >+ if (tm == NULL) {
>> >+ machine__delete(machine);
>> >+ return -1;
>> >+ }
>> >+
>> >+ 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);
>> >+ return 0;
>> >+}
>> >+
>> >+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..769986044a7a 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;
>> >+
>> >+int 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?
Hi Masami,
On Thu, Dec 03, 2015 at 07:13:15AM +0000, 平松雅巳 / HIRAMATU，MASAMI wrote:
> From: Namhyung Kim [mailto:[email protected]]
> >
> >On Thu, Dec 03, 2015 at 12:15:12AM +0000, 平松雅巳 / HIRAMATU，MASAMI wrote:
> >> >From: Namhyung Kim [mailto:[email protected]] On Behalf Of Namhyung Kim
> >> >
> >> >Backtrace is a crucial info for debugging. And upcoming refcnt
> >> >tracking facility also wants to use it.
> >>
> >> Note that the refcnt backtrace symbol resolution will work at
> >> exit. This means that it can not depend on the feature in perf
> >> tools itself. (and of course, since the refcnt tries to find unused
> >> objects in perf tools at exit, if we use perf_thread, it will
> >> detect the objects related to the perf_thread are leaked)
> >
> >Hmm.. right.
> >
> >I think we can leave the perf_thread outside of refcnt infrastructure.
> >IOW it should be created before refcnt debugging is activated and
> >released after refcnt is done. What do you think?
>
> Would you mean we don't debug the objects related to a perf_thread?
> It will mean that you don't debug anything, since perf_thread involves
> most of refcnt using objects, like dso, map, map_groups etc. And some
> bugs are actually found at where those objects are handled.
>
> I would not like to care about the output quality of the backtrace_symbols.
> I only need the top 2-3 addresses of the backtrace buffer, because I have
> (eu-)addr2line command to find the actual source code lines from those
> addresses :). If you need, I can also provide an address decoder awk/shell
> script for that.
>
> Instead, I prefer to avoid complexity on the "debugging feature"(because
> it can introduce new bugs,) and make it more robust (e.g. if we failed to
> get symbol, just shows the raw address)
>
> BTW, the robustness is a key point for debugging. Please consider the case
> that you hit an error on the objects in the perf_thread, it could cause
> double fault(segv again) on the same object. That is what I actually don't
> want.
I understand your point. If you object, I won't insist it strongly.
It's possible there's a bug in perf_thread symbol resolution. But
it's pretty straightforward and simple use case so if there's a bug in
that code, it should be found beforehand IMHO.
Thanks,
Namhyung
Hi Namhyung,
>From: Namhyung Kim [mailto:[email protected]] On Behalf Of Namhyung Kim
>
>Hi Masami,
>
>On Thu, Dec 03, 2015 at 07:13:15AM +0000, 平松雅巳 / HIRAMATU，MASAMI wrote:
>> From: Namhyung Kim [mailto:[email protected]]
>> >
>> >On Thu, Dec 03, 2015 at 12:15:12AM +0000, 平松雅巳 / HIRAMATU，MASAMI wrote:
>> >> >From: Namhyung Kim [mailto:[email protected]] On Behalf Of Namhyung Kim
>> >> >
>> >> >Backtrace is a crucial info for debugging. And upcoming refcnt
>> >> >tracking facility also wants to use it.
>> >>
>> >> Note that the refcnt backtrace symbol resolution will work at
>> >> exit. This means that it can not depend on the feature in perf
>> >> tools itself. (and of course, since the refcnt tries to find unused
>> >> objects in perf tools at exit, if we use perf_thread, it will
>> >> detect the objects related to the perf_thread are leaked)
>> >
>> >Hmm.. right.
>> >
>> >I think we can leave the perf_thread outside of refcnt infrastructure.
>> >IOW it should be created before refcnt debugging is activated and
>> >released after refcnt is done. What do you think?
>>
>> Would you mean we don't debug the objects related to a perf_thread?
>> It will mean that you don't debug anything, since perf_thread involves
>> most of refcnt using objects, like dso, map, map_groups etc. And some
>> bugs are actually found at where those objects are handled.
>>
>> I would not like to care about the output quality of the backtrace_symbols.
>> I only need the top 2-3 addresses of the backtrace buffer, because I have
>> (eu-)addr2line command to find the actual source code lines from those
>> addresses :). If you need, I can also provide an address decoder awk/shell
>> script for that.
>>
>> Instead, I prefer to avoid complexity on the "debugging feature"(because
>> it can introduce new bugs,) and make it more robust (e.g. if we failed to
>> get symbol, just shows the raw address)
>>
>> BTW, the robustness is a key point for debugging. Please consider the case
>> that you hit an error on the objects in the perf_thread, it could cause
>> double fault(segv again) on the same object. That is what I actually don't
>> want.
>
>I understand your point. If you object, I won't insist it strongly.
>
>It's possible there's a bug in perf_thread symbol resolution. But
>it's pretty straightforward and simple use case so if there's a bug in
>that code, it should be found beforehand IMHO.
Agreed, my concerning case may rarely happens, so beforehand we'll be able
to handle it. (anyway, we also can use gdb for that case)
I'm OK this only for the sigsegv backtrace. But at least I don't want to use
the perf_thread for the refcnt debugger, because gdb is useless for this kind
of debugging. I'd like to make it robust. That is my point.
Thank you,
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?