All of lore.kernel.org
 help / color / mirror / Atom feed
From: jolsa@redhat.com (Jiri Olsa)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 2/3] perf tests: Add dwarf unwind test on ARM
Date: Wed, 7 May 2014 14:36:36 +0200	[thread overview]
Message-ID: <20140507123636.GD2019@krava.brq.redhat.com> (raw)
In-Reply-To: <CAFrcx1=-Q7t3ShLy=Ky9RNFfvMb=tdgSLh6bqFDXp59auXetFA@mail.gmail.com>

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

WARNING: multiple messages have this Message-ID (diff)
From: Jiri Olsa <jolsa@redhat.com>
To: Jean Pihet <jean.pihet@linaro.org>
Cc: Arnaldo Carvalho de Melo <acme@infradead.org>,
	Will Deacon <will.deacon@arm.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linaro-kernel@lists.linaro.org" <linaro-kernel@lists.linaro.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	Corey Ashford <cjashfor@linux.vnet.ibm.com>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Ingo Molnar <mingo@kernel.org>,
	Namhyung Kim <namhyung@kernel.org>,
	Paul Mackerras <paulus@samba.org>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	David Ahern <dsahern@gmail.com>
Subject: Re: [PATCH 2/3] perf tests: Add dwarf unwind test on ARM
Date: Wed, 7 May 2014 14:36:36 +0200	[thread overview]
Message-ID: <20140507123636.GD2019@krava.brq.redhat.com> (raw)
In-Reply-To: <CAFrcx1=-Q7t3ShLy=Ky9RNFfvMb=tdgSLh6bqFDXp59auXetFA@mail.gmail.com>

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

  reply	other threads:[~2014-05-07 12:36 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-06 15:26 [PATCH 0/3] perf tools: Add libdw DWARF post unwind support for ARM Jean Pihet
2014-05-06 15:26 ` Jean Pihet
2014-05-06 15:26 ` [PATCH 1/3] perf tests: Introduce perf_regs_load function on ARM Jean Pihet
2014-05-06 15:26   ` Jean Pihet
2014-05-06 15:26 ` [PATCH 2/3] perf tests: Add dwarf unwind test " Jean Pihet
2014-05-06 15:26   ` Jean Pihet
2014-05-07 12:06   ` Jiri Olsa
2014-05-07 12:06     ` Jiri Olsa
2014-05-07 12:24     ` Jean Pihet
2014-05-07 12:24       ` Jean Pihet
2014-05-07 12:36       ` Jiri Olsa [this message]
2014-05-07 12:36         ` Jiri Olsa
2014-05-06 15:26 ` [PATCH 3/3] perf tools: Add libdw DWARF post unwind support for ARM Jean Pihet
2014-05-06 15:26   ` Jean Pihet
2014-05-06 17:56   ` Will Deacon
2014-05-06 17:56     ` Will Deacon
2014-05-07  9:52     ` Jean Pihet
2014-05-07  9:52       ` Jean Pihet
2014-05-07 10:00       ` Will Deacon
2014-05-07 10:00         ` Will Deacon
2014-05-07 10:14         ` Jean Pihet
2014-05-07 10:14           ` Jean Pihet
2014-05-16  8:41           ` [PATCH 1/4] tools: perf: consolidate types.h for ARM and ARM64 Jean Pihet
2014-05-16  8:41             ` Jean Pihet
2014-05-16  8:41             ` [PATCH 2/4] perf tests: Introduce perf_regs_load function on ARM Jean Pihet
2014-05-16  8:41               ` Jean Pihet
2014-05-21  5:55               ` [tip:perf/core] " tip-bot for Jean Pihet
2014-05-16  8:41             ` [PATCH 3/4] perf tests: Add dwarf unwind test " Jean Pihet
2014-05-16  8:41               ` Jean Pihet
2014-05-21  5:55               ` [tip:perf/core] " tip-bot for Jean Pihet
2014-05-16  8:41             ` [PATCH 4/4] perf tools: Add libdw DWARF post unwind support for ARM Jean Pihet
2014-05-16  8:41               ` Jean Pihet
2014-05-21  5:55               ` [tip:perf/core] " tip-bot for Jean Pihet
2014-05-21  5:55             ` [tip:perf/core] perf tools: Consolidate types.h for ARM and ARM64 tip-bot for Jean Pihet
  -- strict thread matches above, loose matches on Subject: below --
2014-03-03  9:53 [PATCH 0/3] perf tools: Add libdw DWARF post unwind support for ARM Jean Pihet
2014-03-03  9:53 ` [PATCH 2/3] perf tests: Add dwarf unwind test on ARM Jean Pihet
2014-03-03  9:53   ` Jean Pihet

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140507123636.GD2019@krava.brq.redhat.com \
    --to=jolsa@redhat.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.