linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] powerpc/perf: Enable linking with libunwind
@ 2014-03-06  4:41 Sukadev Bhattiprolu
  2014-03-06  4:41 ` [RFC][PATCH 1/3] power: perf: Enable saving the user stack in a sample Sukadev Bhattiprolu
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Sukadev Bhattiprolu @ 2014-03-06  4:41 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Michael Ellerman, linux-kernel, Stephane Eranian, linuxppc-dev,
	Paul Mackerras, Jiri Olsa

When we try to create backtraces (call-graphs) with the perf tool

	perf record -g /tmp/sprintft

we get backtraces with duplicate arcs for sprintft[1]:

    14.61%  sprintft  libc-2.18.so       [.] __random                            
            |
            --- __random
               |
               |--61.09%-- __random
               |          |
               |          |--97.18%-- rand
               |          |          do_my_sprintf
               |          |          main
               |          |          generic_start_main.isra.0
               |          |          __libc_start_main
               |          |          0x0
               |          |
               |           --2.82%-- do_my_sprintf
               |                     main
               |                     generic_start_main.isra.0
               |                     __libc_start_main
               |                     0x0
               |
                --38.91%-- rand
                          |          
                          |--92.90%-- rand
                          |          |
                          |          |--99.87%-- do_my_sprintf
                          |          |          main
                          |          |          generic_start_main.isra.0
                          |          |          __libc_start_main
                          |          |          0x0
                          |           --0.13%-- [...]
                          |
                           --7.10%-- do_my_sprintf
                                     main
                                     generic_start_main.isra.0
                                     __libc_start_main
                                     0x0

(where the two arcs both have the same backtrace but are not merged).

Linking with libunwind seems to create better backtraces. While x86 and
ARM processors have support for linking with libunwind but Power does not.
This patchset is an RFC for linking with libunwind.

With this patchset and running:

	/tmp/perf record --call-graph=dwarf,8192 /tmp/sprintft

the backtrace is:

    14.94%  sprintft  libc-2.18.so       [.] __random                            
            |
            --- __random
                rand
                do_my_sprintf
                main
                generic_start_main.isra.0
                __libc_start_main
                (nil)

This appears better.

One downside is that we now need the kernel to save the entire user stack
(the 8192 in the command line is the default user stack size).

A second issue is that this invocation of perf (with --call-graph=dwarf,8192)
seems to fail for backtraces involving tail-calls[2]

	/tmp/perf record -g ./tailcall
gives 

    20.00%  tailcall  tailcall           [.] work2
            |
            --- work2
                work

shows the tail function 'work2' as "called from" 'work()'

But with libunwind:

	/tmp/perf record --call-graph=dwarf,8192 ./tailcall
we get:

   20.50%  tailcall  tailcall           [.] work2
            |
            --- work2

the caller of 'work' is not shown.

I am debugging this, but would appreciate any feedback/pointers on the
patchset/direction:

	- Does libunwind need the entire user stack to work or are there
	  optimizations we can do to save the minimal entries for it to
	  perform the unwind.

	- Does libunwind work with tailcalls like the one above ?

	- Are there benefits to linking with libunwind (even if it does not
	  yet solve the tailcall problem)

	- Are there any examples of using libdwarf to solve the tailcall
	  issue ?

[1] sprintft (excerpt from a test program by Maynard Johnson).

	char * do_my_sprintf(char * strx, int num)
	{
		int i;
		for (i = 0; i < inner_iterations; i++) {
			int r = rand() % 10;
			sprintf(my_string, "%s ...%d\n", strx+r, num);
			if (strlen(my_string) > 15)
				num = 15;
		}
		return my_string;
	}

[2] tailcall (Powerpc assembly, from Anton Blanchard)

    Build with: gcc -O2 --static -nostdlib -o tailcall tailcall.S

	#define ITERATIONS 1000000000

		.align 2
		.globl _start
		.globl ._start
		.section ".opd","aw"
	_start:
		.quad ._start
		.quad .TOC.@tocbase
		.quad 0;

		.text;
	._start:
		lis     4,ITERATIONS@h
		ori     4,4,ITERATIONS@l
		mtctr   4

	1:      bl      work
		bdnz    1b

		li 0,1  /* sys_exit */
		sc

	work:
		mflr    30
		bl      work2
		mtlr    30
		blr

	work2:
		blr


Sukadev Bhattiprolu (3):
  power: perf: Enable saving the user stack in a sample.
  power: perf tool: Add libunwind support for Power
  perf: Use 64-bit value when comparing sample_regs

 arch/powerpc/Kconfig                        |    2 +
 arch/powerpc/include/uapi/asm/perf_regs.h   |   70 ++++++++++++++++++
 arch/powerpc/perf/Makefile                  |    1 +
 arch/powerpc/perf/perf-regs.c               |  104 +++++++++++++++++++++++++++
 tools/perf/arch/powerpc/Makefile            |    5 ++
 tools/perf/arch/powerpc/include/perf_regs.h |   69 ++++++++++++++++++
 tools/perf/arch/powerpc/util/unwind.c       |   63 ++++++++++++++++
 tools/perf/config/Makefile                  |    7 ++
 tools/perf/util/unwind.c                    |    4 +-
 9 files changed, 323 insertions(+), 2 deletions(-)
 create mode 100644 arch/powerpc/include/uapi/asm/perf_regs.h
 create mode 100644 arch/powerpc/perf/perf-regs.c
 create mode 100644 tools/perf/arch/powerpc/include/perf_regs.h
 create mode 100644 tools/perf/arch/powerpc/util/unwind.c

-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [RFC][PATCH 1/3] power: perf: Enable saving the user stack in a sample.
  2014-03-06  4:41 [PATCH 0/3] powerpc/perf: Enable linking with libunwind Sukadev Bhattiprolu
@ 2014-03-06  4:41 ` Sukadev Bhattiprolu
  2014-03-06  4:41 ` [RFC][PATCH 2/3] power: perf tool: Add libunwind support for Power Sukadev Bhattiprolu
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Sukadev Bhattiprolu @ 2014-03-06  4:41 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Michael Ellerman, linux-kernel, Stephane Eranian, linuxppc-dev,
	Paul Mackerras, Jiri Olsa

When requested, have the kernel save the user stack in each perf sample
so 'perf report' can use libunwind and produce better backtraces.

The downside of course is that the kernel has to copy the user-stack
on each sample which has both performance and file-size implications
(of the perf.data file).

But we save the user-stack only when user explicitly requests it:

	perf record --call-graph=dwarf,8192 <application>

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
 arch/powerpc/Kconfig                      |    2 +
 arch/powerpc/include/uapi/asm/perf_regs.h |   70 +++++++++++++++++++
 arch/powerpc/perf/Makefile                |    1 +
 arch/powerpc/perf/perf-regs.c             |  104 +++++++++++++++++++++++++++++
 4 files changed, 177 insertions(+)
 create mode 100644 arch/powerpc/include/uapi/asm/perf_regs.h
 create mode 100644 arch/powerpc/perf/perf-regs.c

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 957bf34..e79ce6e 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -113,6 +113,8 @@ config PPC
 	select GENERIC_ATOMIC64 if PPC32
 	select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
 	select HAVE_PERF_EVENTS
+	select HAVE_PERF_REGS
+	select HAVE_PERF_USER_STACK_DUMP
 	select HAVE_REGS_AND_STACK_ACCESS_API
 	select HAVE_HW_BREAKPOINT if PERF_EVENTS && PPC_BOOK3S_64
 	select ARCH_WANT_IPC_PARSE_VERSION
diff --git a/arch/powerpc/include/uapi/asm/perf_regs.h b/arch/powerpc/include/uapi/asm/perf_regs.h
new file mode 100644
index 0000000..b6120dc
--- /dev/null
+++ b/arch/powerpc/include/uapi/asm/perf_regs.h
@@ -0,0 +1,70 @@
+#ifndef _ASM_POWERPC_PERF_REGS_H
+#define _ASM_POWERPC_PERF_REGS_H
+
+#ifndef __powerpc64__
+#error Support for 32bit processors is TBD.
+#endif
+
+enum perf_event_powerpc_regs {
+	/*
+	 * The order of these values are based on the corresponding
+	 * macros in arch/powerpc/include/uapi/asm/ptrace.h .
+	 */
+	PERF_REG_POWERPC_GPR0,
+	PERF_REG_POWERPC_GPR1,
+	PERF_REG_POWERPC_GPR2,
+	PERF_REG_POWERPC_GPR3,
+	PERF_REG_POWERPC_GPR4,
+	PERF_REG_POWERPC_GPR5,
+	PERF_REG_POWERPC_GPR6,
+	PERF_REG_POWERPC_GPR7,
+	PERF_REG_POWERPC_GPR8,
+	PERF_REG_POWERPC_GPR9,
+
+	PERF_REG_POWERPC_GPR10,
+	PERF_REG_POWERPC_GPR11,
+	PERF_REG_POWERPC_GPR12,
+	PERF_REG_POWERPC_GPR13,
+	PERF_REG_POWERPC_GPR14,
+	PERF_REG_POWERPC_GPR15,
+	PERF_REG_POWERPC_GPR16,
+	PERF_REG_POWERPC_GPR17,
+	PERF_REG_POWERPC_GPR18,
+	PERF_REG_POWERPC_GPR19,
+
+	PERF_REG_POWERPC_GPR20,
+	PERF_REG_POWERPC_GPR21,
+	PERF_REG_POWERPC_GPR22,
+	PERF_REG_POWERPC_GPR23,
+	PERF_REG_POWERPC_GPR24,
+	PERF_REG_POWERPC_GPR25,
+	PERF_REG_POWERPC_GPR26,
+	PERF_REG_POWERPC_GPR27,
+	PERF_REG_POWERPC_GPR28,
+	PERF_REG_POWERPC_GPR29,
+
+	PERF_REG_POWERPC_GPR30,
+	PERF_REG_POWERPC_GPR31,
+
+	PERF_REG_POWERPC_NIP,
+	PERF_REG_POWERPC_MSR,
+	PERF_REG_POWERPC_ORIG_GPR3,
+	PERF_REG_POWERPC_CTR,		/* 35 */
+
+	PERF_REG_POWERPC_LINK,
+	PERF_REG_POWERPC_XER,
+	PERF_REG_POWERPC_CCR,
+#ifdef __powerpc64__
+	PERF_REG_POWERPC_SOFTE,
+#else
+	PERF_REG_POWERPC_MQ, 
+#endif
+	PERF_REG_POWERPC_TRAP,		/* 40 */
+
+	PERF_REG_POWERPC_DAR,
+	PERF_REG_POWERPC_DSISR,
+	PERF_REG_POWERPC_RESULT,
+	PERF_REG_POWERPC_DSCR,
+	PERF_REG_POWERPC_MAX
+};
+#endif
diff --git a/arch/powerpc/perf/Makefile b/arch/powerpc/perf/Makefile
index 60d71ee..44fec45 100644
--- a/arch/powerpc/perf/Makefile
+++ b/arch/powerpc/perf/Makefile
@@ -2,6 +2,7 @@ subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
 
 obj-$(CONFIG_PERF_EVENTS)	+= callchain.o
 
+obj-$(CONFIG_HAVE_PERF_REGS)	+= perf-regs.o
 obj-$(CONFIG_PPC_PERF_CTRS)	+= core-book3s.o bhrb.o
 obj64-$(CONFIG_PPC_PERF_CTRS)	+= power4-pmu.o ppc970-pmu.o power5-pmu.o \
 				   power5+-pmu.o power6-pmu.o power7-pmu.o \
diff --git a/arch/powerpc/perf/perf-regs.c b/arch/powerpc/perf/perf-regs.c
new file mode 100644
index 0000000..3963038
--- /dev/null
+++ b/arch/powerpc/perf/perf-regs.c
@@ -0,0 +1,104 @@
+#include <linux/errno.h>
+#include <linux/sched.h>
+#include <linux/types.h>
+#include <linux/stddef.h>
+#include <asm/perf_regs.h>
+
+#define PT_REGS_GPR_OFFSET(g)	\
+	[PERF_REG_POWERPC_GPR##g] = offsetof(struct pt_regs, gpr[g])
+
+#define PT_REGS_OFFSET(n, r) 	\
+	[PERF_REG_POWERPC_##n] = offsetof(struct pt_regs, r)
+
+/*
+ * An enum in arch/powerpc/include/uapi/asm/perf_regs.h assigns an "id" to
+ * each register in Power. Build a table mapping each register id to its
+ * offset in 'struct pt_regs', that we can use to quickly read-from or
+ * write-to the register in pt_regs.
+ */
+static unsigned int pt_regs_offset[PERF_REG_POWERPC_MAX] = {
+	PT_REGS_GPR_OFFSET(0),
+	PT_REGS_GPR_OFFSET(1),
+	PT_REGS_GPR_OFFSET(2),
+	PT_REGS_GPR_OFFSET(3),
+	PT_REGS_GPR_OFFSET(4),
+	PT_REGS_GPR_OFFSET(5),
+	PT_REGS_GPR_OFFSET(6),
+	PT_REGS_GPR_OFFSET(7),
+	PT_REGS_GPR_OFFSET(8),
+	PT_REGS_GPR_OFFSET(9),
+	PT_REGS_GPR_OFFSET(10),
+
+	PT_REGS_GPR_OFFSET(11),
+	PT_REGS_GPR_OFFSET(12),
+	PT_REGS_GPR_OFFSET(13),
+	PT_REGS_GPR_OFFSET(14),
+	PT_REGS_GPR_OFFSET(15),
+	PT_REGS_GPR_OFFSET(16),
+	PT_REGS_GPR_OFFSET(17),
+	PT_REGS_GPR_OFFSET(18),
+	PT_REGS_GPR_OFFSET(19),
+	PT_REGS_GPR_OFFSET(20),
+
+	PT_REGS_GPR_OFFSET(21),
+	PT_REGS_GPR_OFFSET(22),
+	PT_REGS_GPR_OFFSET(23),
+	PT_REGS_GPR_OFFSET(24),
+	PT_REGS_GPR_OFFSET(25),
+	PT_REGS_GPR_OFFSET(26),
+	PT_REGS_GPR_OFFSET(27),
+	PT_REGS_GPR_OFFSET(28),
+	PT_REGS_GPR_OFFSET(29),
+	PT_REGS_GPR_OFFSET(30),
+
+	PT_REGS_GPR_OFFSET(31),
+
+	PT_REGS_OFFSET(NIP, nip),
+	PT_REGS_OFFSET(MSR, msr),
+	PT_REGS_OFFSET(ORIG_GPR3, orig_gpr3),
+	PT_REGS_OFFSET(CTR, ctr),
+
+	PT_REGS_OFFSET(LINK, link),
+	PT_REGS_OFFSET(XER, xer),
+	PT_REGS_OFFSET(CCR, ccr),
+#ifdef __powerpc64__
+	PT_REGS_OFFSET(SOFTE, softe),
+#else
+	PT_REGS_OFFSET(MQ, mq),
+#endif
+
+	PT_REGS_OFFSET(TRAP, trap),
+	PT_REGS_OFFSET(DAR, dar),
+	PT_REGS_OFFSET(DSISR, dsisr),
+	PT_REGS_OFFSET(RESULT, result),
+};
+
+u64 perf_reg_value(struct pt_regs *regs, int idx)
+{
+	if (WARN_ON_ONCE(idx >= ARRAY_SIZE(pt_regs_offset)))
+		return 0;
+
+	return regs_get_register(regs, pt_regs_offset[idx]);
+}
+
+u64 perf_reg_validate(u64 mask)
+{
+	/*
+	 * TODO: Are there any registers to ignore/check here ?
+	 */
+	if (!mask)
+		return -EINVAL;
+
+	return 0;
+}
+
+u64 perf_reg_abi(struct task_struct *task)
+{
+	/*
+	 * TODO: WHAT SHOULD WE RETURN HERE ????
+	 *
+	 * 	x86 returns PERF_SAMPLE_REGS_ABI_32
+	 * 	perf tool needs this to be non-zero to process registers.
+	 */
+        return 1;
+}
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [RFC][PATCH 2/3] power: perf tool: Add libunwind support for Power
  2014-03-06  4:41 [PATCH 0/3] powerpc/perf: Enable linking with libunwind Sukadev Bhattiprolu
  2014-03-06  4:41 ` [RFC][PATCH 1/3] power: perf: Enable saving the user stack in a sample Sukadev Bhattiprolu
@ 2014-03-06  4:41 ` Sukadev Bhattiprolu
  2014-03-06  4:41 ` [PATCH 3/3] perf: Use 64-bit value when comparing sample_regs Sukadev Bhattiprolu
  2014-03-06 17:49 ` [PATCH 0/3] powerpc/perf: Enable linking with libunwind Jiri Olsa
  3 siblings, 0 replies; 8+ messages in thread
From: Sukadev Bhattiprolu @ 2014-03-06  4:41 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Michael Ellerman, linux-kernel, Stephane Eranian, linuxppc-dev,
	Paul Mackerras, Jiri Olsa

Add ability to link with libunwind on Power.

This is based on similar changes in x86 and arm. Basically, implement
accessor functions that libunwind can call into while building the
backtrace from the user stack (which the kernel saved in a perf sample
- in previous commit).

Tested on Fedora-20 with libunwind and libunwind-devel libdwarf packages
installed.

TODO:
	- Not sure if we need to list all the Power registers or restrict
	  to only those that libunwind needs.
	- Check about perf_reg_abi()
	- Build for 32-bit

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
 tools/perf/arch/powerpc/Makefile            |    5 ++
 tools/perf/arch/powerpc/include/perf_regs.h |   69 +++++++++++++++++++++++++++
 tools/perf/arch/powerpc/util/unwind.c       |   63 ++++++++++++++++++++++++
 tools/perf/config/Makefile                  |    7 +++
 4 files changed, 144 insertions(+)
 create mode 100644 tools/perf/arch/powerpc/include/perf_regs.h
 create mode 100644 tools/perf/arch/powerpc/util/unwind.c

diff --git a/tools/perf/arch/powerpc/Makefile b/tools/perf/arch/powerpc/Makefile
index 744e629..9d3b8ce 100644
--- a/tools/perf/arch/powerpc/Makefile
+++ b/tools/perf/arch/powerpc/Makefile
@@ -2,4 +2,9 @@ ifndef NO_DWARF
 PERF_HAVE_DWARF_REGS := 1
 LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/dwarf-regs.o
 endif
+
+ifndef NO_LIBUNWIND
+LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/unwind.o
+endif
+
 LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/header.o
diff --git a/tools/perf/arch/powerpc/include/perf_regs.h b/tools/perf/arch/powerpc/include/perf_regs.h
new file mode 100644
index 0000000..d5667da
--- /dev/null
+++ b/tools/perf/arch/powerpc/include/perf_regs.h
@@ -0,0 +1,69 @@
+#ifndef ARCH_PERF_REGS_H
+#define ARCH_PERF_REGS_H
+
+#include <asm/perf_regs.h>
+
+#define PERF_REGS_MASK	((1ULL << PERF_REG_POWERPC_MAX) - 1)
+#define PERF_REG_IP	PERF_REG_POWERPC_NIP
+#define PERF_REG_SP	PERF_REG_POWERPC_GPR1
+
+static inline const char *perf_reg_name(int id)
+{
+	switch (id) {
+		case PERF_REG_POWERPC_GPR0: 		return "R0";
+		case PERF_REG_POWERPC_GPR1: 		return "R1";
+		case PERF_REG_POWERPC_GPR2: 		return "R2";
+		case PERF_REG_POWERPC_GPR3: 		return "R3";
+		case PERF_REG_POWERPC_GPR4: 		return "R4";
+		case PERF_REG_POWERPC_GPR5: 		return "R5";
+		case PERF_REG_POWERPC_GPR6: 		return "R6";
+		case PERF_REG_POWERPC_GPR7: 		return "R7";
+		case PERF_REG_POWERPC_GPR8: 		return "R8";
+		case PERF_REG_POWERPC_GPR9: 		return "R9";
+		case PERF_REG_POWERPC_GPR10: 		return "R10";
+		case PERF_REG_POWERPC_GPR11: 		return "R11";
+		case PERF_REG_POWERPC_GPR12: 		return "R12";
+		case PERF_REG_POWERPC_GPR13: 		return "R13";
+		case PERF_REG_POWERPC_GPR14: 		return "R14";
+		case PERF_REG_POWERPC_GPR15: 		return "R15";
+		case PERF_REG_POWERPC_GPR16: 		return "R16";
+		case PERF_REG_POWERPC_GPR17: 		return "R17";
+		case PERF_REG_POWERPC_GPR18: 		return "R18";
+		case PERF_REG_POWERPC_GPR19: 		return "R19";
+		case PERF_REG_POWERPC_GPR20: 		return "R20";
+		case PERF_REG_POWERPC_GPR21: 		return "R21";
+		case PERF_REG_POWERPC_GPR22: 		return "R22";
+		case PERF_REG_POWERPC_GPR23: 		return "R23";
+		case PERF_REG_POWERPC_GPR24: 		return "R24";
+		case PERF_REG_POWERPC_GPR25: 		return "R25";
+		case PERF_REG_POWERPC_GPR26: 		return "R26";
+		case PERF_REG_POWERPC_GPR27: 		return "R27";
+		case PERF_REG_POWERPC_GPR28: 		return "R28";
+		case PERF_REG_POWERPC_GPR29: 		return "R29";
+		case PERF_REG_POWERPC_GPR30: 		return "R30";
+		case PERF_REG_POWERPC_GPR31: 		return "R31";
+
+		case PERF_REG_POWERPC_NIP: 		return "NIP";
+		case PERF_REG_POWERPC_MSR: 		return "MSR";
+		case PERF_REG_POWERPC_ORIG_GPR3:	return "ORIG_GPR3";
+		case PERF_REG_POWERPC_CTR: 		return "CTR";
+		case PERF_REG_POWERPC_LINK: 		return "LINK";
+		case PERF_REG_POWERPC_XER: 		return "XER";
+		case PERF_REG_POWERPC_CCR: 		return "CCR";
+
+#ifdef __powerpc64__
+		case PERF_REG_POWERPC_SOFTE: 		return "SOFTE";
+#else
+		case PERF_REG_POWERPC_MQ: 		return "MQ";
+#endif
+		case PERF_REG_POWERPC_TRAP: 		return "TRAP";
+		case PERF_REG_POWERPC_DAR: 		return "DAR";
+		case PERF_REG_POWERPC_DSISR: 		return "DSISR";
+		case PERF_REG_POWERPC_RESULT: 		return "RESULT";
+
+		default: 				return NULL;
+	}
+	return NULL;
+}
+
+#endif /* ARCH_PERF_REGS_H */
diff --git a/tools/perf/arch/powerpc/util/unwind.c b/tools/perf/arch/powerpc/util/unwind.c
new file mode 100644
index 0000000..ba73399
--- /dev/null
+++ b/tools/perf/arch/powerpc/util/unwind.c
@@ -0,0 +1,63 @@
+#ifdef HAVE_ARCH_POWERPC_SUPPORT
+#include <libunwind.h>			/* UNW_PPC64* macros */
+#include <asm/perf_regs.h>		/* PERF_REG* macros */
+#include "../../util/unwind.h"		/* unwind__arch_reg_id() prototype */
+#include <errno.h>
+
+/*
+ * Map Libunwind's "UNWIND" register ids to "PERF" register ids. 
+ *
+ * One place where this mapping is used is when unwinding the user
+ * stack saved in a perf-sample by the kernel. The kernel uses the
+ * "PERF" register ids to find offset of the register in 'struct
+ * pt_regs'. When using libunwind, we find the "PERF" register id
+ * and use it to retrieve the register contents from the sample.
+ */
+int unwind__arch_reg_id(int regnum)
+{
+	switch (regnum) {
+	case UNW_PPC64_R0: return PERF_REG_POWERPC_GPR0;
+	case UNW_PPC64_R1: return PERF_REG_POWERPC_GPR1;
+	case UNW_PPC64_R2: return PERF_REG_POWERPC_GPR2;
+	case UNW_PPC64_R3: return PERF_REG_POWERPC_GPR3;
+	case UNW_PPC64_R4: return PERF_REG_POWERPC_GPR4;
+	case UNW_PPC64_R5: return PERF_REG_POWERPC_GPR5;
+	case UNW_PPC64_R6: return PERF_REG_POWERPC_GPR6;
+	case UNW_PPC64_R7: return PERF_REG_POWERPC_GPR7;
+	case UNW_PPC64_R8: return PERF_REG_POWERPC_GPR8;
+	case UNW_PPC64_R9: return PERF_REG_POWERPC_GPR9;
+	case UNW_PPC64_R10: return PERF_REG_POWERPC_GPR9;
+	case UNW_PPC64_R11: return PERF_REG_POWERPC_GPR11;
+	case UNW_PPC64_R12: return PERF_REG_POWERPC_GPR12;
+	case UNW_PPC64_R13: return PERF_REG_POWERPC_GPR13;
+	case UNW_PPC64_R14: return PERF_REG_POWERPC_GPR14;
+	case UNW_PPC64_R15: return PERF_REG_POWERPC_GPR15;
+	case UNW_PPC64_R16: return PERF_REG_POWERPC_GPR16;
+	case UNW_PPC64_R17: return PERF_REG_POWERPC_GPR17;
+	case UNW_PPC64_R18: return PERF_REG_POWERPC_GPR18;
+	case UNW_PPC64_R19: return PERF_REG_POWERPC_GPR19;
+	case UNW_PPC64_R20: return PERF_REG_POWERPC_GPR20;
+	case UNW_PPC64_R21: return PERF_REG_POWERPC_GPR21;
+	case UNW_PPC64_R22: return PERF_REG_POWERPC_GPR22;
+	case UNW_PPC64_R23: return PERF_REG_POWERPC_GPR23;
+	case UNW_PPC64_R24: return PERF_REG_POWERPC_GPR24;
+	case UNW_PPC64_R25: return PERF_REG_POWERPC_GPR25;
+	case UNW_PPC64_R26: return PERF_REG_POWERPC_GPR26;
+	case UNW_PPC64_R27: return PERF_REG_POWERPC_GPR27;
+	case UNW_PPC64_R28: return PERF_REG_POWERPC_GPR28;
+	case UNW_PPC64_R29: return PERF_REG_POWERPC_GPR29;
+	case UNW_PPC64_R30: return PERF_REG_POWERPC_GPR30;
+	case UNW_PPC64_R31: return PERF_REG_POWERPC_GPR31;
+
+	case UNW_PPC64_LR: return PERF_REG_POWERPC_LINK;
+	case UNW_PPC64_CTR: return PERF_REG_POWERPC_CTR;
+	case UNW_PPC64_XER: return PERF_REG_POWERPC_XER;
+	case UNW_PPC64_NIP: return PERF_REG_POWERPC_NIP;
+
+	default:
+		pr_err("unwind: invalid reg id %d\n", regnum);
+		return -EINVAL;
+	}
+	return -EINVAL;
+}
+#endif /* HAVE_ARCH_POWERPC_SUPPORT */
diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index c48d449..6446afd 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -29,11 +29,18 @@ ifeq ($(ARCH),x86)
   endif
   NO_PERF_REGS := 0
 endif
+
 ifeq ($(ARCH),arm)
   NO_PERF_REGS := 0
   LIBUNWIND_LIBS = -lunwind -lunwind-arm
 endif
 
+ifeq ($(ARCH),powerpc)
+  CFLAGS += -DHAVE_ARCH_POWERPC_SUPPORT
+  NO_PERF_REGS := 0
+  LIBUNWIND_LIBS = -lunwind -lunwind-ppc64
+endif
+
 ifeq ($(LIBUNWIND_LIBS),)
   NO_LIBUNWIND := 1
 else
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/3] perf: Use 64-bit value when comparing sample_regs
  2014-03-06  4:41 [PATCH 0/3] powerpc/perf: Enable linking with libunwind Sukadev Bhattiprolu
  2014-03-06  4:41 ` [RFC][PATCH 1/3] power: perf: Enable saving the user stack in a sample Sukadev Bhattiprolu
  2014-03-06  4:41 ` [RFC][PATCH 2/3] power: perf tool: Add libunwind support for Power Sukadev Bhattiprolu
@ 2014-03-06  4:41 ` Sukadev Bhattiprolu
  2014-03-06  9:44   ` David Laight
  2014-03-06 17:49 ` [PATCH 0/3] powerpc/perf: Enable linking with libunwind Jiri Olsa
  3 siblings, 1 reply; 8+ messages in thread
From: Sukadev Bhattiprolu @ 2014-03-06  4:41 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Michael Ellerman, linux-kernel, Stephane Eranian, linuxppc-dev,
	Paul Mackerras, Jiri Olsa

When checking whether a bit representing a register is set in
sample_regs, a 64-bit mask, use 64-bit value (1LL).

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
 tools/perf/util/unwind.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/unwind.c b/tools/perf/util/unwind.c
index 742f23b..2b888c6 100644
--- a/tools/perf/util/unwind.c
+++ b/tools/perf/util/unwind.c
@@ -396,11 +396,11 @@ static int reg_value(unw_word_t *valp, struct regs_dump *regs, int id,
 {
 	int i, idx = 0;
 
-	if (!(sample_regs & (1 << id)))
+	if (!(sample_regs & (1LL << id)))
 		return -EINVAL;
 
 	for (i = 0; i < id; i++) {
-		if (sample_regs & (1 << i))
+		if (sample_regs & (1LL << i))
 			idx++;
 	}
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* RE: [PATCH 3/3] perf: Use 64-bit value when comparing sample_regs
  2014-03-06  4:41 ` [PATCH 3/3] perf: Use 64-bit value when comparing sample_regs Sukadev Bhattiprolu
@ 2014-03-06  9:44   ` David Laight
  2014-03-06 11:33     ` Gabriel Paubert
  0 siblings, 1 reply; 8+ messages in thread
From: David Laight @ 2014-03-06  9:44 UTC (permalink / raw)
  To: 'Sukadev Bhattiprolu', Arnaldo Carvalho de Melo
  Cc: Michael Ellerman, linux-kernel@vger.kernel.org, Stephane Eranian,
	linuxppc-dev@ozlabs.org, Paul Mackerras, Jiri Olsa

RnJvbTogU3VrYWRldiBCaGF0dGlwcm9sdQ0KPiBXaGVuIGNoZWNraW5nIHdoZXRoZXIgYSBiaXQg
cmVwcmVzZW50aW5nIGEgcmVnaXN0ZXIgaXMgc2V0IGluDQo+IHNhbXBsZV9yZWdzLCBhIDY0LWJp
dCBtYXNrLCB1c2UgNjQtYml0IHZhbHVlICgxTEwpLg0KPiANCj4gU2lnbmVkLW9mZi1ieTogU3Vr
YWRldiBCaGF0dGlwcm9sdSA8c3VrYWRldkBsaW51eC52bmV0LmlibS5jb20+DQo+IC0tLQ0KPiAg
dG9vbHMvcGVyZi91dGlsL3Vud2luZC5jIHwgICAgNCArKy0tDQo+ICAxIGZpbGUgY2hhbmdlZCwg
MiBpbnNlcnRpb25zKCspLCAyIGRlbGV0aW9ucygtKQ0KPiANCj4gZGlmZiAtLWdpdCBhL3Rvb2xz
L3BlcmYvdXRpbC91bndpbmQuYyBiL3Rvb2xzL3BlcmYvdXRpbC91bndpbmQuYw0KPiBpbmRleCA3
NDJmMjNiLi4yYjg4OGM2IDEwMDY0NA0KPiAtLS0gYS90b29scy9wZXJmL3V0aWwvdW53aW5kLmMN
Cj4gKysrIGIvdG9vbHMvcGVyZi91dGlsL3Vud2luZC5jDQo+IEBAIC0zOTYsMTEgKzM5NiwxMSBA
QCBzdGF0aWMgaW50IHJlZ192YWx1ZSh1bndfd29yZF90ICp2YWxwLCBzdHJ1Y3QgcmVnc19kdW1w
ICpyZWdzLCBpbnQgaWQsDQo+ICB7DQo+ICAJaW50IGksIGlkeCA9IDA7DQo+IA0KPiAtCWlmICgh
KHNhbXBsZV9yZWdzICYgKDEgPDwgaWQpKSkNCj4gKwlpZiAoIShzYW1wbGVfcmVncyAmICgxTEwg
PDwgaWQpKSkNCj4gIAkJcmV0dXJuIC1FSU5WQUw7DQo+IA0KPiAgCWZvciAoaSA9IDA7IGkgPCBp
ZDsgaSsrKSB7DQo+IC0JCWlmIChzYW1wbGVfcmVncyAmICgxIDw8IGkpKQ0KPiArCQlpZiAoc2Ft
cGxlX3JlZ3MgJiAoMUxMIDw8IGkpKQ0KPiAgCQkJaWR4Kys7DQo+ICAJfQ0KDQpUaGVyZSBhcmUg
bXVjaCBmYXN0ZXIgd2F5cyB0byBjb3VudCB0aGUgbnVtYmVyIG9mIHNldCBiaXRzLCBlc3BlY2lh
bGx5DQppZiB5b3UgbWlnaHQgbmVlZCB0byBjaGVjayBhIHNpZ25pZmljYW50IG51bWJlciBvZiBi
aXRzLg0KVGhlcmUgbWlnaHQgZXZlbiBiZSBhIGZ1bmN0aW9uIGRlZmluZWQgc29tZXdoZXJlIHRv
IGRvIGl0Lg0KQmFzaWNhbGx5IHlvdSBqdXN0IGFkZCB1cCB0aGUgYml0cywgZm9yIDE2IGJpdCBp
dCB3b3VsZCBiZToNCgl2YWwgPSAodmFsICYgMHg1NTU1KSArICh2YWwgPj4gMSkgJiAweDU1NTU7
DQoJdmFsID0gKHZhbCAmIDB4MzMzMykgKyAodmFsID4+IDIpICYgMHgzMzMzOw0KCXZhbCA9ICh2
YWwgJiAweDBmMGYpICsgKHZhbCA+PiA0KSAmIDB4MGYwZjsNCgl2YWwgPSAodmFsICYgMHgwMGZm
KSArICh2YWwgPj4gOCkgJiAweDAwZmY7DQpBcyB0aGUgc2l6ZSBvZiB0aGUgd29yayBpbmNyZWFz
ZXMgdGhlIGltcHJvdmVtZW50IGlzIG1vcmUgc2lnbmlmaWNhbnQuDQooU29tZSBvZiB0aGUgbGF0
ZXIgbWFza2luZyBjYW4gcHJvYmFibHkgYmUgcHJvdmVuIHVubmVjZXNzYXJ5LikNCg0KCURhdmlk
DQoNCg==

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/3] perf: Use 64-bit value when comparing sample_regs
  2014-03-06  9:44   ` David Laight
@ 2014-03-06 11:33     ` Gabriel Paubert
  2014-03-06 17:06       ` Jiri Olsa
  0 siblings, 1 reply; 8+ messages in thread
From: Gabriel Paubert @ 2014-03-06 11:33 UTC (permalink / raw)
  To: David Laight
  Cc: Michael Ellerman, linux-kernel@vger.kernel.org, Stephane Eranian,
	linuxppc-dev@ozlabs.org, Paul Mackerras, Arnaldo Carvalho de Melo,
	'Sukadev Bhattiprolu', Jiri Olsa

On Thu, Mar 06, 2014 at 09:44:47AM +0000, David Laight wrote:
> From: Sukadev Bhattiprolu
> > When checking whether a bit representing a register is set in
> > sample_regs, a 64-bit mask, use 64-bit value (1LL).
> > 
> > Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> > ---
> >  tools/perf/util/unwind.c |    4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/perf/util/unwind.c b/tools/perf/util/unwind.c
> > index 742f23b..2b888c6 100644
> > --- a/tools/perf/util/unwind.c
> > +++ b/tools/perf/util/unwind.c
> > @@ -396,11 +396,11 @@ static int reg_value(unw_word_t *valp, struct regs_dump *regs, int id,
> >  {
> >  	int i, idx = 0;
> > 
> > -	if (!(sample_regs & (1 << id)))
> > +	if (!(sample_regs & (1LL << id)))
> >  		return -EINVAL;
> > 
> >  	for (i = 0; i < id; i++) {
> > -		if (sample_regs & (1 << i))
> > +		if (sample_regs & (1LL << i))
> >  			idx++;
> >  	}
> 
> There are much faster ways to count the number of set bits, especially
> if you might need to check a significant number of bits.
> There might even be a function defined somewhere to do it.

Indeed, look for Hamming weight (hweight family of functions)
in asm/hweight.h and what is included from there.

Besides that, many modern processors also have a machine instruction
to perform this task. In the processor manuals the instruction is 
described as population count and the mnemonic starts with "popcnt"
on x86 and ppc.

	Gabriel

> Basically you just add up the bits, for 16 bit it would be:
> 	val = (val & 0x5555) + (val >> 1) & 0x5555;
> 	val = (val & 0x3333) + (val >> 2) & 0x3333;
> 	val = (val & 0x0f0f) + (val >> 4) & 0x0f0f;
> 	val = (val & 0x00ff) + (val >> 8) & 0x00ff;
> As the size of the work increases the improvement is more significant.
> (Some of the later masking can probably be proven unnecessary.)
> 
> 	David
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/3] perf: Use 64-bit value when comparing sample_regs
  2014-03-06 11:33     ` Gabriel Paubert
@ 2014-03-06 17:06       ` Jiri Olsa
  0 siblings, 0 replies; 8+ messages in thread
From: Jiri Olsa @ 2014-03-06 17:06 UTC (permalink / raw)
  To: Gabriel Paubert
  Cc: Michael Ellerman, linux-kernel@vger.kernel.org, Stephane Eranian,
	linuxppc-dev@ozlabs.org, David Laight, Paul Mackerras,
	Arnaldo Carvalho de Melo, 'Sukadev Bhattiprolu'

On Thu, Mar 06, 2014 at 12:33:32PM +0100, Gabriel Paubert wrote:
> On Thu, Mar 06, 2014 at 09:44:47AM +0000, David Laight wrote:
> > From: Sukadev Bhattiprolu
> > > When checking whether a bit representing a register is set in
> > > sample_regs, a 64-bit mask, use 64-bit value (1LL).
> > > 
> > > Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> > > ---
> > >  tools/perf/util/unwind.c |    4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/tools/perf/util/unwind.c b/tools/perf/util/unwind.c
> > > index 742f23b..2b888c6 100644
> > > --- a/tools/perf/util/unwind.c
> > > +++ b/tools/perf/util/unwind.c
> > > @@ -396,11 +396,11 @@ static int reg_value(unw_word_t *valp, struct regs_dump *regs, int id,
> > >  {
> > >  	int i, idx = 0;
> > > 
> > > -	if (!(sample_regs & (1 << id)))
> > > +	if (!(sample_regs & (1LL << id)))
> > >  		return -EINVAL;
> > > 
> > >  	for (i = 0; i < id; i++) {
> > > -		if (sample_regs & (1 << i))
> > > +		if (sample_regs & (1LL << i))
> > >  			idx++;
> > >  	}
> > 
> > There are much faster ways to count the number of set bits, especially
> > if you might need to check a significant number of bits.
> > There might even be a function defined somewhere to do it.
> 
> Indeed, look for Hamming weight (hweight family of functions)
> in asm/hweight.h and what is included from there.
> 
> Besides that, many modern processors also have a machine instruction
> to perform this task. In the processor manuals the instruction is 
> described as population count and the mnemonic starts with "popcnt"
> on x86 and ppc.
> 
> 	Gabriel
> 
> > Basically you just add up the bits, for 16 bit it would be:
> > 	val = (val & 0x5555) + (val >> 1) & 0x5555;
> > 	val = (val & 0x3333) + (val >> 2) & 0x3333;
> > 	val = (val & 0x0f0f) + (val >> 4) & 0x0f0f;
> > 	val = (val & 0x00ff) + (val >> 8) & 0x00ff;
> > As the size of the work increases the improvement is more significant.
> > (Some of the later masking can probably be proven unnecessary.)

right I think the loop could be replaced by:

  idx = hweight(mask & ((1 << id) - 1))

Sukadev,
please also rebase against latest Arnaldo's perf/core,
this code has changed just recently, it's now in:
  util/perf_regs.c:perf_reg_value

thanks,
jirka

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/3] powerpc/perf: Enable linking with libunwind
  2014-03-06  4:41 [PATCH 0/3] powerpc/perf: Enable linking with libunwind Sukadev Bhattiprolu
                   ` (2 preceding siblings ...)
  2014-03-06  4:41 ` [PATCH 3/3] perf: Use 64-bit value when comparing sample_regs Sukadev Bhattiprolu
@ 2014-03-06 17:49 ` Jiri Olsa
  3 siblings, 0 replies; 8+ messages in thread
From: Jiri Olsa @ 2014-03-06 17:49 UTC (permalink / raw)
  To: Sukadev Bhattiprolu
  Cc: Michael Ellerman, linux-kernel, Stephane Eranian, linuxppc-dev,
	Paul Mackerras, Arnaldo Carvalho de Melo, Jean Pihet

On Wed, Mar 05, 2014 at 08:41:56PM -0800, Sukadev Bhattiprolu wrote:
> When we try to create backtraces (call-graphs) with the perf tool
> 
> 	perf record -g /tmp/sprintft
> 
> we get backtraces with duplicate arcs for sprintft[1]:
> 
>     14.61%  sprintft  libc-2.18.so       [.] __random                            
>             |
>             --- __random
>                |
>                |--61.09%-- __random
>                |          |
>                |          |--97.18%-- rand
>                |          |          do_my_sprintf
>                |          |          main
>                |          |          generic_start_main.isra.0
>                |          |          __libc_start_main
>                |          |          0x0
>                |          |
>                |           --2.82%-- do_my_sprintf
>                |                     main
>                |                     generic_start_main.isra.0
>                |                     __libc_start_main
>                |                     0x0
>                |
>                 --38.91%-- rand
>                           |          
>                           |--92.90%-- rand
>                           |          |
>                           |          |--99.87%-- do_my_sprintf
>                           |          |          main
>                           |          |          generic_start_main.isra.0
>                           |          |          __libc_start_main
>                           |          |          0x0
>                           |           --0.13%-- [...]
>                           |
>                            --7.10%-- do_my_sprintf
>                                      main
>                                      generic_start_main.isra.0
>                                      __libc_start_main
>                                      0x0
> 
> (where the two arcs both have the same backtrace but are not merged).
> 
> Linking with libunwind seems to create better backtraces. While x86 and
> ARM processors have support for linking with libunwind but Power does not.
> This patchset is an RFC for linking with libunwind.
> 
> With this patchset and running:
> 
> 	/tmp/perf record --call-graph=dwarf,8192 /tmp/sprintft
> 
> the backtrace is:
> 
>     14.94%  sprintft  libc-2.18.so       [.] __random                            
>             |
>             --- __random
>                 rand
>                 do_my_sprintf
>                 main
>                 generic_start_main.isra.0
>                 __libc_start_main
>                 (nil)
> 
> This appears better.
> 
> One downside is that we now need the kernel to save the entire user stack
> (the 8192 in the command line is the default user stack size).
> 
> A second issue is that this invocation of perf (with --call-graph=dwarf,8192)
> seems to fail for backtraces involving tail-calls[2]
> 
> 	/tmp/perf record -g ./tailcall
> gives 
> 
>     20.00%  tailcall  tailcall           [.] work2
>             |
>             --- work2
>                 work
> 
> shows the tail function 'work2' as "called from" 'work()'
> 
> But with libunwind:
> 
> 	/tmp/perf record --call-graph=dwarf,8192 ./tailcall
> we get:
> 
>    20.50%  tailcall  tailcall           [.] work2
>             |
>             --- work2
> 
> the caller of 'work' is not shown.
> 
> I am debugging this, but would appreciate any feedback/pointers on the
> patchset/direction:
> 
> 	- Does libunwind need the entire user stack to work or are there
> 	  optimizations we can do to save the minimal entries for it to
> 	  perform the unwind.

AFAIK you dont need to provide whole stack, but the more
you have the bigger chance you'll get full(er) backtrace

> 
> 	- Does libunwind work with tailcalls like the one above ?

not sure, but if you have x86 alternative to your tailcall (i cannot
read ppc assembly) I could try on x86 ;-)

CC-ing Jean, as he might have seen this issue..


> 
> 	- Are there benefits to linking with libunwind (even if it does not
> 	  yet solve the tailcall problem)

provides backtrace for binaries/distros/archs compiled without framepointer

> 
> 	- Are there any examples of using libdwarf to solve the tailcall
> 	  issue ?


btw there's now remote unwinder in elfutils (version 0.158)
the perf supprot is in Arnaldo's perf/core tree

jirka

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2014-03-06 19:31 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-06  4:41 [PATCH 0/3] powerpc/perf: Enable linking with libunwind Sukadev Bhattiprolu
2014-03-06  4:41 ` [RFC][PATCH 1/3] power: perf: Enable saving the user stack in a sample Sukadev Bhattiprolu
2014-03-06  4:41 ` [RFC][PATCH 2/3] power: perf tool: Add libunwind support for Power Sukadev Bhattiprolu
2014-03-06  4:41 ` [PATCH 3/3] perf: Use 64-bit value when comparing sample_regs Sukadev Bhattiprolu
2014-03-06  9:44   ` David Laight
2014-03-06 11:33     ` Gabriel Paubert
2014-03-06 17:06       ` Jiri Olsa
2014-03-06 17:49 ` [PATCH 0/3] powerpc/perf: Enable linking with libunwind Jiri Olsa

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).