powerpc/perf: Adjust callchain based on DWARF debug info

[PATCH v5 1/1] powerpc/perf: Adjust callchain based on DWARF debug info

When saving the callchain on Power, the kernel conservatively saves excess entries in the callchain. A few of these entries are needed in some cases but not others. We should use the DWARF debug information to determine when the entries are needed.

Eg: the value in the link register (LR) is needed only when it holds the return address of a function. At other times it must be ignored.

If the unnecessary entries are not ignored, we end up with duplicate arcs in the call-graphs.

Use the DWARF debug information to determine if any callchain entries should be ignored when building call-graphs.

Callgraph before the patch:

    14.67%          2234  sprintft  libc-2.18.so       [.] __random
            |
            --- __random
               |
               |--61.12%-- __random
               |          |
               |          |--97.15%-- rand
               |          |          do_my_sprintf
               |          |          main
               |          |          generic_start_main.isra.0
               |          |          __libc_start_main
               |          |          0x0
               |          |
               |           --2.85%-- do_my_sprintf
               |                     main
               |                     generic_start_main.isra.0
               |                     __libc_start_main
               |                     0x0
               |
                --38.88%-- rand
                          |
                          |--94.01%-- rand
                          |          do_my_sprintf
                          |          main
                          |          generic_start_main.isra.0
                          |          __libc_start_main
                          |          0x0
                          |
                           --5.99%-- do_my_sprintf
                                     main
                                     generic_start_main.isra.0
                                     __libc_start_main
                                     0x0

Callgraph after the patch:

    14.67%          2234  sprintft  libc-2.18.so       [.] __random
            |
            --- __random
               |
               |--95.93%-- rand
               |          do_my_sprintf
               |          main
               |          generic_start_main.isra.0
               |          __libc_start_main
               |          0x0
               |
                --4.07%-- do_my_sprintf
                          main
                          generic_start_main.isra.0
                          __libc_start_main
                          0x0

TODO: For split-debug info objects like glibc, we can only determine
the call-frame-address only when both .eh_frame and .debug_info sections are available. We should be able to determin the CFA even without the .eh_frame section.

Fix suggested by Anton Blanchard.

Thanks to valuable input on DWARF debug information from Ulrich Weigand.

---
Changelog[v5]
[Jiri Olsa] Avoid the new external symbol PERF_CONTEXT_IGNORE; Revert back to previous version and use #ifdef directly in machine__resolve_callchain_sample() to avoid performance impact on other architectures.

Changelog[v4]
Move Powerpc-specific code into a separate patch

Changelog[v3]
[Jiri Olsa] Rename function to arch_skip_callchain_idx() to be
consistent with behavior.
[Jiri Olsa] Add '__maybe_unused' tags for unused parameters.

Changelog[v2]:
Add missing dwfl_end()
Fix merge conflicts due to some unwind code

 tools/perf/arch/powerpc/Makefile                  |    1 +
 tools/perf/arch/powerpc/util/skip-callchain-idx.c |  266 +++++++++++++++++++++
 tools/perf/config/Makefile                        |    4 +
 tools/perf/util/callchain.h                       |   13 +
 tools/perf/util/machine.c                         |   18 +-
 5 files changed, 300 insertions(+), 2 deletions(-)
 create mode 100644 tools/perf/arch/powerpc/util/skip-callchain-idx.c

index 744e629..b92219b 100644
@@ -3,3 +3,4 @@ PERF_HAVE_DWARF_REGS := 1
 LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/dwarf-regs.o
 endif
 LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/header.o
+LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/skip-callchain-idx.o

index f30ac5e..346bdb6 100644
@@ -48,6 +48,10 @@ ifneq ($(ARCH),$(filter $(ARCH),x86 arm))
   NO_LIBDW_DWARF_UNWIND := 1
 endif
 
+ifeq ($(ARCH),powerpc)
+  CFLAGS += -DHAVE_SKIP_CALLCHAIN_IDX
+endif
+
 ifeq ($(LIBUNWIND_LIBS),)
   NO_LIBUNWIND := 1
 else

index 8f84423..da43619 100644
@@ -176,4 +176,17 @@ static inline void callchain_cursor_snapshot(struct callchain_cursor *dest,
 	dest->first = src->curr;
 	dest->nr -= src->pos;
 }
+
+#ifdef HAVE_SKIP_CALLCHAIN_IDX
+extern int arch_skip_callchain_idx(struct machine *machine,
+			struct thread *thread, struct ip_callchain *chain);
+#else
+static inline int arch_skip_callchain_idx(struct machine *machine __maybe_unused,
+			struct thread *thread __maybe_unused,
+			struct ip_callchain *chain __maybe_unused)
+{
+	return -1;
+}
+#endif
+
 #endif	/* __PERF_CALLCHAIN_H */

index 0e5fea9..6221d01 100644
@@ -1291,7 +1291,9 @@ static int machine__resolve_callchain_sample(struct machine *machine,
 	u8 cpumode = PERF_RECORD_MISC_USER;
 	int chain_nr = min(max_stack, (int)chain->nr);
 	int i;
+	int j;
 	int err;
+	int skip_idx;
 
 	callchain_cursor_reset(&callchain_cursor);
 

@@ -1300,14 +1302,26 @@ static int machine__resolve_callchain_sample(struct machine *machine,
 		return 0;
 	}
 
+	/*
+	 * Based on DWARF debug information, some architectures skip
+	 * a callchain entry saved by the kernel.
+	 */
+	skip_idx = arch_skip_callchain_idx(machine, thread, chain);
+
 	for (i = 0; i < chain_nr; i++) {
 		u64 ip;
 		struct addr_location al;
 
 		if (callchain_param.order == ORDER_CALLEE)
-			ip = chain->ips[i];
+			j = i;
 		else
-			ip = chain->ips[chain->nr - i - 1];
+			j = chain->nr - i - 1;
+
+#ifdef HAVE_SKIP_CALLCHAIN_IDX
+		if (j == skip_idx)
+			continue;
+#endif
+		ip = chain->ips[j];
 
 		if (ip >= PERF_CONTEXT_MAX) {
 			switch (ip) {

1.7.9.5

superflous ^^^

SNIP

got following compile error:

CC util/map.o
util/machine.c: In function ‘machine__resolve_callchain_sample’:
util/machine.c:1286:6: error: variable ‘skip_idx’ set but not used [-Werror=unused-but-set-variable]

  int skip_idx;
      ^

cc1: all warnings being treated as errors make[2]: *** [util/machine.o] Error 1 make[2]: *** Waiting for unfinished jobs.... make[1]: *** [all] Error 2
test: test -x ./perf
make: *** [make_pure] Error 1

you could use __maybe_unused for the 'skip_idx'

jirka

Jiri Olsa [[email protected]] wrote:
|
| you could use __maybe_unused for the 'skip_idx'

Yes, here is the updated patch.
---

powerpc/perf: Adjust callchain based on DWARF debug info

When saving the callchain on Power, the kernel conservatively saves excess entries in the callchain. A few of these entries are needed in some cases but not others. We should use the DWARF debug information to determine when the entries are needed.

Eg: the value in the link register (LR) is needed only when it holds the return address of a function. At other times it must be ignored.

If the unnecessary entries are not ignored, we end up with duplicate arcs in the call-graphs.

Use the DWARF debug information to determine if any callchain entries should be ignored when building call-graphs.

Callgraph before the patch:

    14.67%          2234  sprintft  libc-2.18.so       [.] __random
            |
            --- __random
               |
               |--61.12%-- __random
               |          |
               |          |--97.15%-- rand
               |          |          do_my_sprintf
               |          |          main
               |          |          generic_start_main.isra.0
               |          |          __libc_start_main
               |          |          0x0
               |          |
               |           --2.85%-- do_my_sprintf
               |                     main
               |                     generic_start_main.isra.0
               |                     __libc_start_main
               |                     0x0
               |
                --38.88%-- rand
                          |
                          |--94.01%-- rand
                          |          do_my_sprintf
                          |          main
                          |          generic_start_main.isra.0
                          |          __libc_start_main
                          |          0x0
                          |
                           --5.99%-- do_my_sprintf
                                     main
                                     generic_start_main.isra.0
                                     __libc_start_main
                                     0x0

Callgraph after the patch:

    14.67%          2234  sprintft  libc-2.18.so       [.] __random
            |
            --- __random
               |
               |--95.93%-- rand
               |          do_my_sprintf
               |          main
               |          generic_start_main.isra.0
               |          __libc_start_main
               |          0x0
               |
                --4.07%-- do_my_sprintf
                          main
                          generic_start_main.isra.0
                          __libc_start_main
                          0x0

TODO: For split-debug info objects like glibc, we can only determine
the call-frame-address only when both .eh_frame and .debug_info sections are available. We should be able to determin the CFA even without the .eh_frame section.

Fix suggested by Anton Blanchard.

Thanks to valuable input on DWARF debug information from Ulrich Weigand.

Changelog[v5]
[Jiri Olsa] Avoid the new external symbol PERF_CONTEXT_IGNORE; Revert back to previous version and to avoid impact on other architectures, use #ifdef in machine__resolve_callchain_sample().

Changelog[v4]
Move Powerpc-specific code into a separate patch

Changelog[v3]
[Jiri Olsa] Rename function to arch_skip_callchain_idx() to be
consistent with behavior.
[Jiri Olsa] Add '__maybe_unused' tags for unused parameters.

Changelog[v2]:
Add missing dwfl_end()
Fix merge conflicts due to some unwind code

---
 tools/perf/arch/powerpc/Makefile                  |    1 +
 tools/perf/arch/powerpc/util/skip-callchain-idx.c |  266 +++++++++++++++++++++
 tools/perf/config/Makefile                        |    4 +
 tools/perf/util/callchain.h                       |   13 +
 tools/perf/util/machine.c                         |   18 +-
 5 files changed, 300 insertions(+), 2 deletions(-)
 create mode 100644 tools/perf/arch/powerpc/util/skip-callchain-idx.c

index 744e629..b92219b 100644
@@ -3,3 +3,4 @@ PERF_HAVE_DWARF_REGS := 1
 LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/dwarf-regs.o
 endif
 LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/header.o
+LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/skip-callchain-idx.o

index f30ac5e..346bdb6 100644
@@ -48,6 +48,10 @@ ifneq ($(ARCH),$(filter $(ARCH),x86 arm))
   NO_LIBDW_DWARF_UNWIND := 1
 endif
 
+ifeq ($(ARCH),powerpc)
+  CFLAGS += -DHAVE_SKIP_CALLCHAIN_IDX
+endif
+
 ifeq ($(LIBUNWIND_LIBS),)
   NO_LIBUNWIND := 1
 else

index 8f84423..da43619 100644
@@ -176,4 +176,17 @@ static inline void callchain_cursor_snapshot(struct callchain_cursor *dest,
 	dest->first = src->curr;
 	dest->nr -= src->pos;
 }
+
+#ifdef HAVE_SKIP_CALLCHAIN_IDX
+extern int arch_skip_callchain_idx(struct machine *machine,
+			struct thread *thread, struct ip_callchain *chain);
+#else
+static inline int arch_skip_callchain_idx(struct machine *machine __maybe_unused,
+			struct thread *thread __maybe_unused,
+			struct ip_callchain *chain __maybe_unused)
+{
+	return -1;
+}
+#endif
+
 #endif	/* __PERF_CALLCHAIN_H */

index 0e5fea9..09c62b4 100644
@@ -1291,7 +1291,9 @@ static int machine__resolve_callchain_sample(struct machine *machine,
 	u8 cpumode = PERF_RECORD_MISC_USER;
 	int chain_nr = min(max_stack, (int)chain->nr);
 	int i;
+	int j;
 	int err;
+	int skip_idx __maybe_unused;
 
 	callchain_cursor_reset(&callchain_cursor);
 

@@ -1300,14 +1302,26 @@ static int machine__resolve_callchain_sample(struct machine *machine,
 		return 0;
 	}
 
+	/*
+	 * Based on DWARF debug information, some architectures skip
+	 * a callchain entry saved by the kernel.
+	 */
+	skip_idx = arch_skip_callchain_idx(machine, thread, chain);
+
 	for (i = 0; i < chain_nr; i++) {
 		u64 ip;
 		struct addr_location al;
 
 		if (callchain_param.order == ORDER_CALLEE)
-			ip = chain->ips[i];
+			j = i;
 		else
-			ip = chain->ips[chain->nr - i - 1];
+			j = chain->nr - i - 1;
+
+#ifdef HAVE_SKIP_CALLCHAIN_IDX
+		if (j == skip_idx)
+			continue;
+#endif
+		ip = chain->ips[j];
 
 		if (ip >= PERF_CONTEXT_MAX) {
 			switch (ip) {

1.7.9.5

Hi,

This patch breaks perf compilation if DWARF support is not present. DWARF support is auto-detected early in the build, so IMHO either user should be informed to install the needed stuff, or the build should succeed with limited functionality.

arch/powerpc/util/skip-callchain-idx.c:13:19: fatal error: dwarf.h: No such file or directory #include <dwarf.h>

                   ^

compilation terminated.

A.

looks like powerpc issue.. hows 'make -f tests/make' doing on powerpc? It tests above scenario.

jirka

Aaro Koskinen [[email protected]] wrote:

| Hi,
| 
| On Wed, Jun 25, 2014 at 08:49:03AM -0700, Sukadev Bhattiprolu wrote:
| > powerpc/perf: Adjust callchain based on DWARF debug info
| > 
| > When saving the callchain on Power, the kernel conservatively saves excess
| > entries in the callchain. A few of these entries are needed in some cases
| > but not others. We should use the DWARF debug information to determine
| > when the entries are  needed.
| 
| This patch breaks perf compilation if DWARF support is not present.
| DWARF support is auto-detected early in the build, so IMHO either user
| should be informed to install the needed stuff, or the build should
| succeed with limited functionality.
| 
| arch/powerpc/util/skip-callchain-idx.c:13:19: fatal error: dwarf.h: No such file or directory #include <dwarf.h>
|                    ^
| compilation terminated.

Yes. Sorry. Anton fixed this recently:

	https://lkml.org/lkml/2014/9/19/29

Sukadev