On Tue, May 06, 2014 at 05:26:18PM +0200, Jean Pihet wrote:

SNIP

diff --git a/tools/perf/arch/arm/tests/dwarf-unwind.c b/tools/perf/arch/arm/tests/dwarf-unwind.c new file mode 100644 index 0000000..d618f5f --- /dev/null +++ b/tools/perf/arch/arm/tests/dwarf-unwind.c @@ -0,0 +1,59 @@ +#include <string.h> +#include "perf_regs.h" +#include "thread.h" +#include "map.h" +#include "event.h" +#include "tests/tests.h"

+#define STACK_SIZE 8192

+static int sample_ustack(struct perf_sample *sample,

• 	 struct thread *thread, u64 *regs)
  

+{

• struct stack_dump *stack = &sample->user_stack;
• struct map *map;
• unsigned long sp;
• u64 stack_size, *buf;
• buf = malloc(STACK_SIZE);
• if (!buf) {

• pr_debug("failed to allocate sample uregs data\n");
  

• return -1;
  

• }
• sp = (unsigned long) regs[PERF_REG_ARM_SP];
• map = map_groups__find(&thread->mg, MAP__FUNCTION, (u64) sp);
• if (!map) {

• pr_debug("failed to get stack map\n");
  

• return -1;
  

• }

there's a memory leak of 'buf' already fixed fox x86:

perf tests x86: Fix memory leak in sample_ustack() commit 763d7f5f2718f085bab5a9e63308349728f3ad12 Author: Masanari Iida standby24x7@gmail.com Date: Sun Apr 20 00:16:41 2014 +0900

jirka

On Wed, May 07, 2014 at 02:24:53PM +0200, Jean Pihet wrote:

Hi Jiri,

On 7 May 2014 14:06, Jiri Olsa jolsa@redhat.com wrote:

...

On Tue, May 06, 2014 at 05:26:18PM +0200, Jean Pihet wrote:

SNIP

...

there's a memory leak of 'buf' already fixed fox x86:

perf tests x86: Fix memory leak in sample_ustack() commit 763d7f5f2718f085bab5a9e63308349728f3ad12 Author: Masanari Iida standby24x7@gmail.com Date: Sun Apr 20 00:16:41 2014 +0900

jirka

Ok

Here is the diff between the x86 and the ARM implementations: $ diff -urN tools/perf/arch/arm64/tests/dwarf-unwind.c tools/perf/arch/x86/tests/dwarf-unwind.c --- tools/perf/arch/arm64/tests/dwarf-unwind.c 2014-05-06 17:31:17.507961045 +0200 +++ tools/perf/arch/x86/tests/dwarf-unwind.c 2014-05-06 16:52:00.589776839 +0200 @@ -21,11 +21,12 @@ return -1; }

• sp = (unsigned long) regs[PERF_REG_ARM64_SP];

• sp = (unsigned long) regs[PERF_REG_X86_SP];

• map = map_groups__find(&thread->mg, MAP__FUNCTION, (u64) sp);

• map = map_groups__find(thread->mg, MAP__VARIABLE, (u64) sp); if (!map) { pr_debug("failed to get stack map\n");

•    free(buf);
     return -1;
  

  }

Which leads to a few questions:

• the map_groups__find parameters need to be fixed too, right?

the reason for this is following commit: 6392b4e perf x86: Fix perf to use non-executable stack, again

which also adds global link flags: -Wl,-z,noexecstack so I'm guessing arm is affected too

• the free(buf) needs to be fixed,
• given that the remaining difference in the file is just a register

macro, it is worth to factor the code in a single file. Does that make sense? If worthwhile I can do that once the ARM and ARM64 support is merged in.

we can do the code factoring later here, np

thanks, jirka

Prevents a build breakage since commit d944c4eebcf4c0d5e5d9728fec110cbf0047ad7f 'tools: Consolidate types.h'

Signed-off-by: Jean Pihet jean.pihet@linaro.org --- tools/perf/arch/arm/include/perf_regs.h | 2 +- tools/perf/arch/arm64/include/perf_regs.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/arch/arm/include/perf_regs.h b/tools/perf/arch/arm/include/perf_regs.h index 2a1cfde..bcca511 100644 --- a/tools/perf/arch/arm/include/perf_regs.h +++ b/tools/perf/arch/arm/include/perf_regs.h @@ -2,7 +2,7 @@ #define ARCH_PERF_REGS_H

#include <stdlib.h> -#include "../../util/types.h" +#include <linux/types.h> #include <asm/perf_regs.h>

#define PERF_REGS_MASK ((1ULL << PERF_REG_ARM_MAX) - 1) diff --git a/tools/perf/arch/arm64/include/perf_regs.h b/tools/perf/arch/arm64/include/perf_regs.h index 2359546..e9441b9 100644 --- a/tools/perf/arch/arm64/include/perf_regs.h +++ b/tools/perf/arch/arm64/include/perf_regs.h @@ -2,7 +2,7 @@ #define ARCH_PERF_REGS_H

#include <stdlib.h> -#include "../../util/types.h" +#include <linux/types.h> #include <asm/perf_regs.h>

#define PERF_REGS_MASK ((1ULL << PERF_REG_ARM64_MAX) - 1)

Adding dwarf unwind test, that setups live machine data over the perf test thread and does the remote unwind.

Need to use -fno-optimize-sibling-calls for test compilation, otherwise 'krava_*' function calls are optimized into jumps and ommited from the stack unwind.

So far it was enabled only for x86.

Reviewed-by: Will Deacon will.deacon@arm.com Cc: Jiri Olsa jolsa@redhat.com Cc: Corey Ashford cjashfor@linux.vnet.ibm.com Cc: Frederic Weisbecker fweisbec@gmail.com Cc: Ingo Molnar mingo@kernel.org Cc: Namhyung Kim namhyung@kernel.org Cc: Paul Mackerras paulus@samba.org Cc: Peter Zijlstra a.p.zijlstra@chello.nl Cc: Arnaldo Carvalho de Melo acme@ghostprotocols.net Cc: David Ahern dsahern@gmail.com Signed-off-by: Jean Pihet jean.pihet@linaro.org --- tools/perf/Makefile.perf | 2 +- tools/perf/arch/arm/Makefile | 1 + tools/perf/arch/arm/include/perf_regs.h | 3 ++ tools/perf/arch/arm/tests/dwarf-unwind.c | 60 ++++++++++++++++++++++++++++++++ tools/perf/config/Makefile | 4 +-- tools/perf/tests/builtin-test.c | 2 +- tools/perf/tests/tests.h | 2 +- 7 files changed, 69 insertions(+), 5 deletions(-) create mode 100644 tools/perf/arch/arm/tests/dwarf-unwind.c

diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf index 2baf61c..dea2d633 100644 --- a/tools/perf/Makefile.perf +++ b/tools/perf/Makefile.perf @@ -411,7 +411,7 @@ LIB_OBJS += $(OUTPUT)tests/code-reading.o LIB_OBJS += $(OUTPUT)tests/sample-parsing.o LIB_OBJS += $(OUTPUT)tests/parse-no-sample-id-all.o ifndef NO_DWARF_UNWIND -ifeq ($(ARCH),x86) +ifeq ($(ARCH),$(filter $(ARCH),x86 arm)) LIB_OBJS += $(OUTPUT)tests/dwarf-unwind.o endif endif diff --git a/tools/perf/arch/arm/Makefile b/tools/perf/arch/arm/Makefile index 9b8f87e..221f21d 100644 --- a/tools/perf/arch/arm/Makefile +++ b/tools/perf/arch/arm/Makefile @@ -5,4 +5,5 @@ endif ifndef NO_LIBUNWIND LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/unwind-libunwind.o LIB_OBJS += $(OUTPUT)arch/$(ARCH)/tests/regs_load.o +LIB_OBJS += $(OUTPUT)arch/$(ARCH)/tests/dwarf-unwind.o endif diff --git a/tools/perf/arch/arm/include/perf_regs.h b/tools/perf/arch/arm/include/perf_regs.h index 33abcfa..f619c9c 100644 --- a/tools/perf/arch/arm/include/perf_regs.h +++ b/tools/perf/arch/arm/include/perf_regs.h @@ -8,6 +8,9 @@ void perf_regs_load(u64 *regs);

#define PERF_REGS_MASK ((1ULL << PERF_REG_ARM_MAX) - 1) +#define PERF_REGS_MAX PERF_REG_ARM_MAX +#define PERF_SAMPLE_REGS_ABI PERF_SAMPLE_REGS_ABI_32 + #define PERF_REG_IP PERF_REG_ARM_PC #define PERF_REG_SP PERF_REG_ARM_SP

diff --git a/tools/perf/arch/arm/tests/dwarf-unwind.c b/tools/perf/arch/arm/tests/dwarf-unwind.c new file mode 100644 index 0000000..9f870d2 --- /dev/null +++ b/tools/perf/arch/arm/tests/dwarf-unwind.c @@ -0,0 +1,60 @@ +#include <string.h> +#include "perf_regs.h" +#include "thread.h" +#include "map.h" +#include "event.h" +#include "tests/tests.h" + +#define STACK_SIZE 8192 + +static int sample_ustack(struct perf_sample *sample, + struct thread *thread, u64 *regs) +{ + struct stack_dump *stack = &sample->user_stack; + struct map *map; + unsigned long sp; + u64 stack_size, *buf; + + buf = malloc(STACK_SIZE); + if (!buf) { + pr_debug("failed to allocate sample uregs data\n"); + return -1; + } + + sp = (unsigned long) regs[PERF_REG_ARM_SP]; + + map = map_groups__find(thread->mg, MAP__VARIABLE, (u64) sp); + if (!map) { + pr_debug("failed to get stack map\n"); + free(buf); + return -1; + } + + stack_size = map->end - sp; + stack_size = stack_size > STACK_SIZE ? STACK_SIZE : stack_size; + + memcpy(buf, (void *) sp, stack_size); + stack->data = (char *) buf; + stack->size = stack_size; + return 0; +} + +int test__arch_unwind_sample(struct perf_sample *sample, + struct thread *thread) +{ + struct regs_dump *regs = &sample->user_regs; + u64 *buf; + + buf = calloc(1, sizeof(u64) * PERF_REGS_MAX); + if (!buf) { + pr_debug("failed to allocate sample uregs data\n"); + return -1; + } + + perf_regs_load(buf); + regs->abi = PERF_SAMPLE_REGS_ABI; + regs->regs = buf; + regs->mask = PERF_REGS_MASK; + + return sample_ustack(sample, thread, buf); +} diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile index f2edc59..729bbdf 100644 --- a/tools/perf/config/Makefile +++ b/tools/perf/config/Makefile @@ -40,11 +40,11 @@ ifeq ($(ARCH),arm64) LIBUNWIND_LIBS = -lunwind -lunwind-aarch64 endif

-# So far there's only x86 libdw unwind support merged in perf. +# So far there's only x86 and arm libdw unwind support merged in perf. # Disable it on all other architectures in case libdw unwind # support is detected in system. Add supported architectures # to the check. -ifneq ($(ARCH),x86) +ifneq ($(ARCH),$(filter $(ARCH),x86 arm)) NO_LIBDW_DWARF_UNWIND := 1 endif

diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c index 0d5afaf..5e0764b 100644 --- a/tools/perf/tests/builtin-test.c +++ b/tools/perf/tests/builtin-test.c @@ -115,7 +115,7 @@ static struct test { .desc = "Test parsing with no sample_id_all bit set", .func = test__parse_no_sample_id_all, }, -#if defined(__x86_64__) || defined(__i386__) +#if defined(__x86_64__) || defined(__i386__) || defined(__arm__) #ifdef HAVE_DWARF_UNWIND_SUPPORT { .desc = "Test dwarf unwind", diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h index a9d7cb0..8f91fb0 100644 --- a/tools/perf/tests/tests.h +++ b/tools/perf/tests/tests.h @@ -45,7 +45,7 @@ int test__hists_filter(void); int test__mmap_thread_lookup(void); int test__thread_mg_share(void);

-#if defined(__x86_64__) || defined(__i386__) +#if defined(__x86_64__) || defined(__i386__) || defined(__arm__) #ifdef HAVE_DWARF_UNWIND_SUPPORT struct thread; struct perf_sample;