diff mbox

[2/3] perf tests: Add dwarf unwind test on ARM

Message ID 1399389979-11279-3-git-send-email-jean.pihet@linaro.org
State New
Headers show

Commit Message

Jean Pihet May 6, 2014, 3:26 p.m. UTC
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

Comments

Jiri Olsa May 7, 2014, 12:06 p.m. UTC | #1
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Jean Pihet May 7, 2014, 12:24 p.m. UTC | #2
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Jiri Olsa May 7, 2014, 12:36 p.m. UTC | #3
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
diff mbox

Patch

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;