| Message ID | 1399389979-11279-3-git-send-email-jean.pihet@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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 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. What do you think? Regards, Jean
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
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 1476ae7..c2cefff 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..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; + } + + 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 = malloc(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/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;
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. 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@infradead.org> 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 | 59 ++++++++++++++++++++++++++++++++ tools/perf/tests/builtin-test.c | 2 +- tools/perf/tests/tests.h | 2 +- 6 files changed, 66 insertions(+), 3 deletions(-) create mode 100644 tools/perf/arch/arm/tests/dwarf-unwind.c