* [PATCH 1/2] perf symbols: Ignore mapping symbols on aarch64 @ 2015-01-13 16:59 Victor Kamensky 2015-01-13 16:59 ` [PATCH 2/2] perf symbols: debuglink should take symfs option into account Victor Kamensky 2015-01-14 11:22 ` [PATCH 1/2] perf symbols: Ignore mapping symbols on aarch64 Will Deacon 0 siblings, 2 replies; 13+ messages in thread From: Victor Kamensky @ 2015-01-13 16:59 UTC (permalink / raw) To: linux-arm-kernel Aarch64 ELF files use mapping symbols with special names $x, $d to identify regions of Aarch64 code (see Aarch64 ELF ABI - "ARM IHI 0056B", section "4.5.4 Mapping symbols"). The patch filters out these symbols at load time, similar to "696b97a perf symbols: Ignore mapping symbols on ARM" changes done for ARM before V8. Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl> Cc: Paul Mackerras <paulus@samba.org> Cc: Ingo Molnar <mingo@redhat.com> Cc: Arnaldo Carvalho de Melo <acme@kernel.org> Cc: Adrian Hunter <adrian.hunter@intel.com> Cc: Jiri Olsa <jolsa@redhat.com> Cc: Namhyung Kim <namhyung@kernel.org> Cc: Avi Kivity <avi@cloudius-systems.com> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> Cc: Anton Blanchard <anton@samba.org> Cc: David Ahern <dsahern@gmail.com> Cc: Will Deacon <will.deacon@arm.com> Cc: Dave Martin <Dave.Martin@arm.com> --- tools/perf/util/symbol-elf.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c index 06fcd1b..1e188dd 100644 --- a/tools/perf/util/symbol-elf.c +++ b/tools/perf/util/symbol-elf.c @@ -862,6 +862,14 @@ int dso__load_sym(struct dso *dso, struct map *map, !strcmp(elf_name, "$t")) continue; } + /* Reject Aarch64 ELF "mapping symbols": these aren't unique and + * don't identify functions, so will confuse the profile + * output: */ + if (ehdr.e_machine == EM_AARCH64) { + if (!strcmp(elf_name, "$x") || + !strcmp(elf_name, "$d")) + continue; + } if (runtime_ss->opdsec && sym.st_shndx == runtime_ss->opdidx) { u32 offset = sym.st_value - syms_ss->opdshdr.sh_addr; -- 1.9.3 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/2] perf symbols: debuglink should take symfs option into account 2015-01-13 16:59 [PATCH 1/2] perf symbols: Ignore mapping symbols on aarch64 Victor Kamensky @ 2015-01-13 16:59 ` Victor Kamensky 2015-01-19 17:50 ` Victor Kamensky 2015-01-14 11:22 ` [PATCH 1/2] perf symbols: Ignore mapping symbols on aarch64 Will Deacon 1 sibling, 1 reply; 13+ messages in thread From: Victor Kamensky @ 2015-01-13 16:59 UTC (permalink / raw) To: linux-arm-kernel Currently code that tries to read corresponding debug symbol file from .gnu_debuglink section (DSO_BINARY_TYPE__DEBUGLINK) does not take in account symfs option, so filename__read_debuglink function cannot open ELF file, if symfs option is used. Fix is to add proper handling of symfs as it is done in other places: use __symbol__join_symfs function to get real file name of target ELF file. Created malloced copy of target filename in symfs before passing it to __symbol__join_symfs function because filename will be modified by it if corresponding debuglink is found. Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl> Cc: Paul Mackerras <paulus@samba.org> Cc: Ingo Molnar <mingo@redhat.com> Cc: Arnaldo Carvalho de Melo <acme@kernel.org> Cc: Jiri Olsa <jolsa@kernel.org> Cc: Namhyung Kim <namhyung@kernel.org> Cc: Adrian Hunter <adrian.hunter@intel.com> Cc: Waiman Long <Waiman.Long@hp.com> Cc: David Ahern <dsahern@gmail.com> --- tools/perf/util/dso.c | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c index 45be944..6a2f663 100644 --- a/tools/perf/util/dso.c +++ b/tools/perf/util/dso.c @@ -42,19 +42,30 @@ int dso__read_binary_type_filename(const struct dso *dso, size_t len; switch (type) { - case DSO_BINARY_TYPE__DEBUGLINK: { + case DSO_BINARY_TYPE__DEBUGLINK: + { char *debuglink; - - strncpy(filename, dso->long_name, size); - debuglink = filename + dso->long_name_len; - while (debuglink != filename && *debuglink != '/') - debuglink--; - if (*debuglink == '/') - debuglink++; - ret = filename__read_debuglink(dso->long_name, debuglink, - size - (debuglink - filename)); - } + char *filename_copy; + + filename_copy = malloc(PATH_MAX); + if (filename_copy) { + len = __symbol__join_symfs(filename, size, + dso->long_name); + strncpy(filename_copy, filename, PATH_MAX); + debuglink = filename + len; + while (debuglink != filename && *debuglink != '/') + debuglink--; + if (*debuglink == '/') + debuglink++; + ret = filename__read_debuglink(filename_copy, debuglink, + size - (debuglink - + filename)); + free(filename_copy); + } else + ret = -1; break; + } + case DSO_BINARY_TYPE__BUILD_ID_CACHE: /* skip the locally configured cache if a symfs is given */ if (symbol_conf.symfs[0] || -- 1.9.3 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/2] perf symbols: debuglink should take symfs option into account 2015-01-13 16:59 ` [PATCH 2/2] perf symbols: debuglink should take symfs option into account Victor Kamensky @ 2015-01-19 17:50 ` Victor Kamensky 2015-01-21 22:00 ` David Ahern 0 siblings, 1 reply; 13+ messages in thread From: Victor Kamensky @ 2015-01-19 17:50 UTC (permalink / raw) To: linux-arm-kernel Hi Guys, If it helps to look at below patch, here is a test case for failure that below patch addresses. Run on target with root NFS mounted in this case. Current target is ARM TC2 board. But I am pretty sure the same sequence will fail on any other device that is built in cross compiled way, with .gnu_debuglink split debug symbols and with and perf used with --symfs option: On target device built split debug executable with .gnu_debuglink section and collect perf data: root at genericarmv7a:~/tests/hang# cat hang.c void hang(void) { while(1); } int main (void) { hang(); return 0; } root at genericarmv7a:~/tests/hang# gcc -o hang -g hang.c root at genericarmv7a:~/tests/hang# cp hang hang.debug root at genericarmv7a:~/tests/hang# strip --only-keep-debug hang.debug root at genericarmv7a:~/tests/hang# objcopy --add-gnu-debuglink=hang.debug hang root at genericarmv7a:~/tests/hang# strip hang root at genericarmv7a:~/tests/hang# readelf -S hang | grep gnu_debuglink [27] .gnu_debuglink PROGBITS 00000000 00060b 000010 00 0 0 1 root at genericarmv7a:~/tests/hang# readelf -x 27 hang Hex dump of section '.gnu_debuglink': 0x00000000 68616e67 2e646562 75670000 ee7e9545 hang.debug...~.E root at genericarmv7a:~/tests/hang# perf record -e cycles --no-buildid-cache ./hang ^C[ perf record: Woken up 6 times to write data ] [ perf record: Captured and wrote 1.448 MB perf.data (~63258 samples) ] root at genericarmv7a:~/tests/hang# chmod a+r perf.data On the host trying to decode perf result without proposed fix using --symfs option to point to target rootfs. Note failure of decode symbol for hang function. [kamensky at coreos-lnx2 hang]$ ls -a . .. hang hang.c hang.debug perf.data [kamensky at coreos-lnx2 hang]$ $L/tools/perf/perf report --stdio --symfs /wd2/nfs/rootfs_32_le_tc2_20141218 | head -10 [kernel.kallsyms] with build id c70bcdc8d97df0dcded9984e3ada2b6ff5a69510 not found, continuing without symbols no symbols found in /home/root/tests/hang/hang, maybe install a debug package? # To display the perf.data header info, please use --header/--header-only options. # # Samples: 37K of event 'cycles' # Event count (approx.): 9469365389 # # Overhead Command Shared Object Symbol # ........ ....... ................. ...................... # 99.97% hang hang [.] 0x00000000000003ec <-- failed to decode symbol 0.01% hang [kernel.kallsyms] [k] 0x00000000c08c5438 [kamensky@coreos-lnx2 hang]$ strace -o /tmp/perf.trace.txt $L/tools/perf/perf report --stdio --symfs /wd2/nfs/rootfs_32_le_tc2_20141218 >& /dev/null [kamensky at coreos-lnx2 hang]$ grep open /tmp/perf.trace.txt | grep hang open("/home/root/tests/hang/hang", O_RDONLY) = -1 ENOENT (No such file or directory) <--- filename__read_debuglink tries to open file open("/wd2/nfs/rootfs_32_le_tc2_20141218//usr/lib/debug/home/root/tests/hang/hang.debug", O_RDONLY) = -1 ENOENT (No such file or directory) open("/wd2/nfs/rootfs_32_le_tc2_20141218//usr/lib/debug/home/root/tests/hang/hang", O_RDONLY) = -1 ENOENT (No such file or directory) open("/wd2/nfs/rootfs_32_le_tc2_20141218//home/root/tests/hang/hang", O_RDONLY) = 4 open("/wd2/nfs/rootfs_32_le_tc2_20141218//home/root/tests/hang/.debug/hang", O_RDONLY) = -1 ENOENT (No such file or directory) Running version of perf with proposed fix, now perf picks up right hang.debug symbols file: [kamensky at coreos-lnx2 hang]$ $L/tools/perf/perf report --stdio --symfs /wd2/nfs/rootfs_32_le_tc2_20141218 | head -10 [kernel.kallsyms] with build id c70bcdc8d97df0dcded9984e3ada2b6ff5a69510 not found, continuing without symbols # To display the perf.data header info, please use --header/--header-only options. # # Samples: 37K of event 'cycles' # Event count (approx.): 9469365389 # # Overhead Command Shared Object Symbol # ........ ....... ................. ...................... # 99.97% hang hang [.] hang 0.01% hang [kernel.kallsyms] [k] 0x00000000c08c5438 Thanks, Victor On 13 January 2015 at 08:59, Victor Kamensky <victor.kamensky@linaro.org> wrote: > Currently code that tries to read corresponding debug symbol > file from .gnu_debuglink section (DSO_BINARY_TYPE__DEBUGLINK) > does not take in account symfs option, so filename__read_debuglink > function cannot open ELF file, if symfs option is used. > > Fix is to add proper handling of symfs as it is done in other > places: use __symbol__join_symfs function to get real file name > of target ELF file. > > Created malloced copy of target filename in symfs before passing > it to __symbol__join_symfs function because filename will be > modified by it if corresponding debuglink is found. > > Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org> > Cc: Peter Zijlstra <a.p.zijlstra@chello.nl> > Cc: Paul Mackerras <paulus@samba.org> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: Arnaldo Carvalho de Melo <acme@kernel.org> > Cc: Jiri Olsa <jolsa@kernel.org> > Cc: Namhyung Kim <namhyung@kernel.org> > Cc: Adrian Hunter <adrian.hunter@intel.com> > Cc: Waiman Long <Waiman.Long@hp.com> > Cc: David Ahern <dsahern@gmail.com> > --- > tools/perf/util/dso.c | 33 ++++++++++++++++++++++----------- > 1 file changed, 22 insertions(+), 11 deletions(-) > > diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c > index 45be944..6a2f663 100644 > --- a/tools/perf/util/dso.c > +++ b/tools/perf/util/dso.c > @@ -42,19 +42,30 @@ int dso__read_binary_type_filename(const struct dso *dso, > size_t len; > > switch (type) { > - case DSO_BINARY_TYPE__DEBUGLINK: { > + case DSO_BINARY_TYPE__DEBUGLINK: > + { > char *debuglink; > - > - strncpy(filename, dso->long_name, size); > - debuglink = filename + dso->long_name_len; > - while (debuglink != filename && *debuglink != '/') > - debuglink--; > - if (*debuglink == '/') > - debuglink++; > - ret = filename__read_debuglink(dso->long_name, debuglink, > - size - (debuglink - filename)); > - } > + char *filename_copy; > + > + filename_copy = malloc(PATH_MAX); > + if (filename_copy) { > + len = __symbol__join_symfs(filename, size, > + dso->long_name); > + strncpy(filename_copy, filename, PATH_MAX); > + debuglink = filename + len; > + while (debuglink != filename && *debuglink != '/') > + debuglink--; > + if (*debuglink == '/') > + debuglink++; > + ret = filename__read_debuglink(filename_copy, debuglink, > + size - (debuglink - > + filename)); > + free(filename_copy); > + } else > + ret = -1; > break; > + } > + > case DSO_BINARY_TYPE__BUILD_ID_CACHE: > /* skip the locally configured cache if a symfs is given */ > if (symbol_conf.symfs[0] || > -- > 1.9.3 > ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/2] perf symbols: debuglink should take symfs option into account 2015-01-19 17:50 ` Victor Kamensky @ 2015-01-21 22:00 ` David Ahern 2015-01-22 0:34 ` Victor Kamensky 0 siblings, 1 reply; 13+ messages in thread From: David Ahern @ 2015-01-21 22:00 UTC (permalink / raw) To: linux-arm-kernel On 1/19/15 10:50 AM, Victor Kamensky wrote: >> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c >> index 45be944..6a2f663 100644 >> --- a/tools/perf/util/dso.c >> +++ b/tools/perf/util/dso.c >> @@ -42,19 +42,30 @@ int dso__read_binary_type_filename(const struct dso *dso, >> size_t len; >> >> switch (type) { >> - case DSO_BINARY_TYPE__DEBUGLINK: { >> + case DSO_BINARY_TYPE__DEBUGLINK: >> + { >> char *debuglink; >> - >> - strncpy(filename, dso->long_name, size); >> - debuglink = filename + dso->long_name_len; >> - while (debuglink != filename && *debuglink != '/') >> - debuglink--; >> - if (*debuglink == '/') >> - debuglink++; >> - ret = filename__read_debuglink(dso->long_name, debuglink, >> - size - (debuglink - filename)); >> - } >> + char *filename_copy; >> + >> + filename_copy = malloc(PATH_MAX); >> + if (filename_copy) { >> + len = __symbol__join_symfs(filename, size, >> + dso->long_name); >> + strncpy(filename_copy, filename, PATH_MAX); >> + debuglink = filename + len; >> + while (debuglink != filename && *debuglink != '/') >> + debuglink--; >> + if (*debuglink == '/') >> + debuglink++; >> + ret = filename__read_debuglink(filename_copy, debuglink, >> + size - (debuglink - >> + filename)); >> + free(filename_copy); >> + } else >> + ret = -1; >> break; >> + } >> + I do not believe the filename_copy is needed; just add the symfs path to filename and pass filename to read_debuglink David ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/2] perf symbols: debuglink should take symfs option into account 2015-01-21 22:00 ` David Ahern @ 2015-01-22 0:34 ` Victor Kamensky 2015-01-22 0:53 ` David Ahern 0 siblings, 1 reply; 13+ messages in thread From: Victor Kamensky @ 2015-01-22 0:34 UTC (permalink / raw) To: linux-arm-kernel David, Thank you for response! On 21 January 2015 at 14:00, David Ahern <dsahern@gmail.com> wrote: > On 1/19/15 10:50 AM, Victor Kamensky wrote: >>> >>> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c >>> index 45be944..6a2f663 100644 >>> --- a/tools/perf/util/dso.c >>> +++ b/tools/perf/util/dso.c >>> @@ -42,19 +42,30 @@ int dso__read_binary_type_filename(const struct dso >>> *dso, >>> size_t len; >>> >>> switch (type) { >>> - case DSO_BINARY_TYPE__DEBUGLINK: { >>> + case DSO_BINARY_TYPE__DEBUGLINK: >>> + { >>> char *debuglink; >>> - >>> - strncpy(filename, dso->long_name, size); >>> - debuglink = filename + dso->long_name_len; >>> - while (debuglink != filename && *debuglink != '/') >>> - debuglink--; >>> - if (*debuglink == '/') >>> - debuglink++; >>> - ret = filename__read_debuglink(dso->long_name, debuglink, >>> - size - (debuglink - >>> filename)); >>> - } >>> + char *filename_copy; >>> + >>> + filename_copy = malloc(PATH_MAX); >>> + if (filename_copy) { >>> + len = __symbol__join_symfs(filename, size, >>> + dso->long_name); >>> + strncpy(filename_copy, filename, PATH_MAX); >>> + debuglink = filename + len; >>> + while (debuglink != filename && *debuglink != >>> '/') >>> + debuglink--; >>> + if (*debuglink == '/') >>> + debuglink++; >>> + ret = filename__read_debuglink(filename_copy, >>> debuglink, >>> + size - (debuglink >>> - >>> + >>> filename)); >>> + free(filename_copy); >>> + } else >>> + ret = -1; >>> break; >>> + } >>> + > > > I do not believe the filename_copy is needed; just add the symfs path to > filename and pass filename to read_debuglink My first version of the fix did not create filename_copy and looked like as patch below. But then I noticed that filename__read_debuglink function receives two pointers: 'filename' first parameter pointer to executable path from which .gnu_debuglink sections content will be read 'debuglink' path to directory that will be updated by the function (note strncpy at line 531) to point to debug symbol file. Before my change offset within 'filename' parameter of dso__read_binary_type_filename function is passed as 'debuglink' parameter and it will be updated, and dso->long_name is separate, passed as 'filename' parameter to filename__read_debuglink function. When in the first version of patch, as below, I pass filename buffer to both (filename to open first and pointer within filename to be updated as debuglink) as in patch below, it will work with current implementation because open happens first but update happens after that. But that is specific dependency on current implementation of filename__read_debuglink function. It did not feel quite right to me, but practically it could be OK. Here is the first version of the without filename_copy. Practically I am OK with it. Please let me know if you prefer this version: >From cafc06d95886f1d82f7b127af58a51384c0fe931 Mon Sep 17 00:00:00 2001 From: Victor Kamensky <victor.kamensky@linaro.org> Date: Mon, 12 Jan 2015 17:33:06 -0800 Subject: [PATCH 2/2] perf symbols: debuglink should take symfs option into account Currently code that tries to read corresponding debug symbol file from .gnu_debuglink section (DSO_BINARY_TYPE__DEBUGLINK) does not take in account symfs option, so filename__read_debuglink function cannot open ELF file, if symfs option is used. Fix is to add proper handling of symfs as it is done in other places: use __symbol__join_symfs function to get real file name of target ELF file. Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org> --- tools/perf/util/dso.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c index 45be944..ca8d8d5 100644 --- a/tools/perf/util/dso.c +++ b/tools/perf/util/dso.c @@ -45,13 +45,13 @@ int dso__read_binary_type_filename(const struct dso *dso, case DSO_BINARY_TYPE__DEBUGLINK: { char *debuglink; - strncpy(filename, dso->long_name, size); - debuglink = filename + dso->long_name_len; + len = __symbol__join_symfs(filename, size, dso->long_name); + debuglink = filename + len; while (debuglink != filename && *debuglink != '/') debuglink--; if (*debuglink == '/') debuglink++; - ret = filename__read_debuglink(dso->long_name, debuglink, + ret = filename__read_debuglink(filename, debuglink, size - (debuglink - filename)); } break; -- 1.9.3 Thanks, Victor > David > ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/2] perf symbols: debuglink should take symfs option into account 2015-01-22 0:34 ` Victor Kamensky @ 2015-01-22 0:53 ` David Ahern 2015-01-22 1:32 ` Victor Kamensky 0 siblings, 1 reply; 13+ messages in thread From: David Ahern @ 2015-01-22 0:53 UTC (permalink / raw) To: linux-arm-kernel On 1/21/15 5:34 PM, Victor Kamensky wrote: > diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c > index 45be944..ca8d8d5 100644 > --- a/tools/perf/util/dso.c > +++ b/tools/perf/util/dso.c > @@ -45,13 +45,13 @@ int dso__read_binary_type_filename(const struct dso *dso, > case DSO_BINARY_TYPE__DEBUGLINK: { > char *debuglink; > > - strncpy(filename, dso->long_name, size); > - debuglink = filename + dso->long_name_len; > + len = __symbol__join_symfs(filename, size, dso->long_name); > + debuglink = filename + len; > while (debuglink != filename && *debuglink != '/') > debuglink--; > if (*debuglink == '/') > debuglink++; > - ret = filename__read_debuglink(dso->long_name, debuglink, > + ret = filename__read_debuglink(filename, debuglink, > size - (debuglink - filename)); > } > break; > I do not see any reason this will not work. Essentially after filename__read_debuglink filename contains symfs + dso path + debuglink read from .gnu_debuglink section which is what is wanted. Thanks for the example. I used it with both a symfs and non-symfs example and both times this change worked properly -- the correct hang.debug file is read. Arnaldo? David ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/2] perf symbols: debuglink should take symfs option into account 2015-01-22 0:53 ` David Ahern @ 2015-01-22 1:32 ` Victor Kamensky 2015-01-22 3:53 ` David Ahern 0 siblings, 1 reply; 13+ messages in thread From: Victor Kamensky @ 2015-01-22 1:32 UTC (permalink / raw) To: linux-arm-kernel On 21 January 2015 at 16:53, David Ahern <dsahern@gmail.com> wrote: > On 1/21/15 5:34 PM, Victor Kamensky wrote: >> >> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c >> index 45be944..ca8d8d5 100644 >> --- a/tools/perf/util/dso.c >> +++ b/tools/perf/util/dso.c >> @@ -45,13 +45,13 @@ int dso__read_binary_type_filename(const struct dso >> *dso, >> case DSO_BINARY_TYPE__DEBUGLINK: { >> char *debuglink; >> >> - strncpy(filename, dso->long_name, size); >> - debuglink = filename + dso->long_name_len; >> + len = __symbol__join_symfs(filename, size, dso->long_name); >> + debuglink = filename + len; >> while (debuglink != filename && *debuglink != '/') >> debuglink--; >> if (*debuglink == '/') >> debuglink++; >> - ret = filename__read_debuglink(dso->long_name, debuglink, >> + ret = filename__read_debuglink(filename, debuglink, >> size - (debuglink - filename)); >> } >> break; >> > > I do not see any reason this will not work. Essentially after > filename__read_debuglink filename contains symfs + dso path + debuglink read > from .gnu_debuglink section which is what is wanted. OK, I am good with this. Let me repost the whole mini-series based on mailing list review comments. May I use 'Acked-by' (maybe 'Tested-by' as well) with your name for the shorter (no filename_copy) as above version of the patch for .gnu_debuglink issue fix. ARM/Aarch64 specific change also needs update based on discussion with Will and Russell. I'll post updated version 2 latter tonight. Thanks, Victor > Thanks for the example. I used it with both a symfs and non-symfs example > and both times this change worked properly -- the correct hang.debug file is > read. > > Arnaldo? > > David ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/2] perf symbols: debuglink should take symfs option into account 2015-01-22 1:32 ` Victor Kamensky @ 2015-01-22 3:53 ` David Ahern 0 siblings, 0 replies; 13+ messages in thread From: David Ahern @ 2015-01-22 3:53 UTC (permalink / raw) To: linux-arm-kernel On 1/21/15 6:32 PM, Victor Kamensky wrote: > May I use 'Acked-by' (maybe 'Tested-by' as well) with > your name for the shorter (no filename_copy) as above > version of the patch for .gnu_debuglink issue fix. You can add both to your second patch -- the short version. Acked-and-Tested-by: David Ahern <dsahern@gmail.com> ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/2] perf symbols: Ignore mapping symbols on aarch64 2015-01-13 16:59 [PATCH 1/2] perf symbols: Ignore mapping symbols on aarch64 Victor Kamensky 2015-01-13 16:59 ` [PATCH 2/2] perf symbols: debuglink should take symfs option into account Victor Kamensky @ 2015-01-14 11:22 ` Will Deacon 2015-01-14 18:38 ` Victor Kamensky 1 sibling, 1 reply; 13+ messages in thread From: Will Deacon @ 2015-01-14 11:22 UTC (permalink / raw) To: linux-arm-kernel On Tue, Jan 13, 2015 at 04:59:03PM +0000, Victor Kamensky wrote: > Aarch64 ELF files use mapping symbols with special names $x, $d > to identify regions of Aarch64 code (see Aarch64 ELF ABI - "ARM > IHI 0056B", section "4.5.4 Mapping symbols"). > > The patch filters out these symbols at load time, similar to > "696b97a perf symbols: Ignore mapping symbols on ARM" changes > done for ARM before V8. > > Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org> > Cc: Peter Zijlstra <a.p.zijlstra@chello.nl> > Cc: Paul Mackerras <paulus@samba.org> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: Arnaldo Carvalho de Melo <acme@kernel.org> > Cc: Adrian Hunter <adrian.hunter@intel.com> > Cc: Jiri Olsa <jolsa@redhat.com> > Cc: Namhyung Kim <namhyung@kernel.org> > Cc: Avi Kivity <avi@cloudius-systems.com> > Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> > Cc: Anton Blanchard <anton@samba.org> > Cc: David Ahern <dsahern@gmail.com> > Cc: Will Deacon <will.deacon@arm.com> > Cc: Dave Martin <Dave.Martin@arm.com> > --- > tools/perf/util/symbol-elf.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c > index 06fcd1b..1e188dd 100644 > --- a/tools/perf/util/symbol-elf.c > +++ b/tools/perf/util/symbol-elf.c > @@ -862,6 +862,14 @@ int dso__load_sym(struct dso *dso, struct map *map, > !strcmp(elf_name, "$t")) > continue; > } > + /* Reject Aarch64 ELF "mapping symbols": these aren't unique and > + * don't identify functions, so will confuse the profile > + * output: */ > + if (ehdr.e_machine == EM_AARCH64) { > + if (!strcmp(elf_name, "$x") || > + !strcmp(elf_name, "$d")) > + continue; > + } Do we need to skip $x.* and $d.* too? I doubt GCC generates them, but they are permitted by the ELF ABI. Will ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/2] perf symbols: Ignore mapping symbols on aarch64 2015-01-14 11:22 ` [PATCH 1/2] perf symbols: Ignore mapping symbols on aarch64 Will Deacon @ 2015-01-14 18:38 ` Victor Kamensky 2015-01-15 0:03 ` Russell King - ARM Linux 0 siblings, 1 reply; 13+ messages in thread From: Victor Kamensky @ 2015-01-14 18:38 UTC (permalink / raw) To: linux-arm-kernel On 14 January 2015 at 03:22, Will Deacon <will.deacon@arm.com> wrote: > On Tue, Jan 13, 2015 at 04:59:03PM +0000, Victor Kamensky wrote: >> Aarch64 ELF files use mapping symbols with special names $x, $d >> to identify regions of Aarch64 code (see Aarch64 ELF ABI - "ARM >> IHI 0056B", section "4.5.4 Mapping symbols"). >> >> The patch filters out these symbols at load time, similar to >> "696b97a perf symbols: Ignore mapping symbols on ARM" changes >> done for ARM before V8. >> >> Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org> >> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl> >> Cc: Paul Mackerras <paulus@samba.org> >> Cc: Ingo Molnar <mingo@redhat.com> >> Cc: Arnaldo Carvalho de Melo <acme@kernel.org> >> Cc: Adrian Hunter <adrian.hunter@intel.com> >> Cc: Jiri Olsa <jolsa@redhat.com> >> Cc: Namhyung Kim <namhyung@kernel.org> >> Cc: Avi Kivity <avi@cloudius-systems.com> >> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> >> Cc: Anton Blanchard <anton@samba.org> >> Cc: David Ahern <dsahern@gmail.com> >> Cc: Will Deacon <will.deacon@arm.com> >> Cc: Dave Martin <Dave.Martin@arm.com> >> --- >> tools/perf/util/symbol-elf.c | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c >> index 06fcd1b..1e188dd 100644 >> --- a/tools/perf/util/symbol-elf.c >> +++ b/tools/perf/util/symbol-elf.c >> @@ -862,6 +862,14 @@ int dso__load_sym(struct dso *dso, struct map *map, >> !strcmp(elf_name, "$t")) >> continue; >> } >> + /* Reject Aarch64 ELF "mapping symbols": these aren't unique and >> + * don't identify functions, so will confuse the profile >> + * output: */ >> + if (ehdr.e_machine == EM_AARCH64) { >> + if (!strcmp(elf_name, "$x") || >> + !strcmp(elf_name, "$d")) >> + continue; >> + } > > Do we need to skip $x.* and $d.* too? I doubt GCC generates them, but they > are permitted by the ELF ABI. Fair enough. But I think it would need to be done for both EM_ARM and EM_AARCH64. My above patch follows EM_ARM current case. Also it seems that it would be quite rare case, as ABI suggests symbols with dot notation should be used in manual asm where assembler does not support multiple definitions of the same symbol. Will something like the following suffice? I can add that as separate follow up patch in this small series. Tested with explicit "$x.func1" symbol introduced in manual asm. >From fed6caab410ddcaf487ff23a3908eca129e50b89 Mon Sep 17 00:00:00 2001 From: Victor Kamensky <victor.kamensky@linaro.org> Date: Wed, 14 Jan 2015 07:42:41 -0800 Subject: [PATCH 3/3] perf symbols: improve abi compliance in arm mapping symbols handling Both Arm and Aarch64 ELF ABI allow mapping symbols be in from either "$d" or "$d.<any>". But current code that handles mapping symbols only deals with the first, dollar character and a single letter, case. The patch adds handling of the second case with period followed by any characters. Suggested-by: Will Deacon <will.deacon@arm.com> Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org> --- tools/perf/util/symbol-elf.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c index 1e188dd..ae92c27 100644 --- a/tools/perf/util/symbol-elf.c +++ b/tools/perf/util/symbol-elf.c @@ -859,7 +859,10 @@ int dso__load_sym(struct dso *dso, struct map *map, if (ehdr.e_machine == EM_ARM) { if (!strcmp(elf_name, "$a") || !strcmp(elf_name, "$d") || - !strcmp(elf_name, "$t")) + !strcmp(elf_name, "$t") || + !strncmp(elf_name, "$a.", 3) || + !strncmp(elf_name, "$d.", 3) || + !strncmp(elf_name, "$t.", 3)) continue; } /* Reject Aarch64 ELF "mapping symbols": these aren't unique and @@ -867,7 +870,9 @@ int dso__load_sym(struct dso *dso, struct map *map, * output: */ if (ehdr.e_machine == EM_AARCH64) { if (!strcmp(elf_name, "$x") || - !strcmp(elf_name, "$d")) + !strcmp(elf_name, "$d") || + !strncmp(elf_name, "$x.", 3) || + !strncmp(elf_name, "$d.", 3)) continue; } -- 1.9.3 Thanks, Victor > Will ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 1/2] perf symbols: Ignore mapping symbols on aarch64 2015-01-14 18:38 ` Victor Kamensky @ 2015-01-15 0:03 ` Russell King - ARM Linux 2015-01-15 2:51 ` Victor Kamensky 0 siblings, 1 reply; 13+ messages in thread From: Russell King - ARM Linux @ 2015-01-15 0:03 UTC (permalink / raw) To: linux-arm-kernel On Wed, Jan 14, 2015 at 10:38:38AM -0800, Victor Kamensky wrote: > >From fed6caab410ddcaf487ff23a3908eca129e50b89 Mon Sep 17 00:00:00 2001 > From: Victor Kamensky <victor.kamensky@linaro.org> > Date: Wed, 14 Jan 2015 07:42:41 -0800 > Subject: [PATCH 3/3] perf symbols: improve abi compliance in arm mapping > symbols handling > > Both Arm and Aarch64 ELF ABI allow mapping symbols be in from > either "$d" or "$d.<any>". But current code that handles mapping > symbols only deals with the first, dollar character and a single > letter, case. > > The patch adds handling of the second case with period > followed by any characters. > > Suggested-by: Will Deacon <will.deacon@arm.com> > Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org> I wonder if it would make more sense to re-use the "is_arm_mapping_symbol" thing which we have in kernel/module.c and scripts/kallsyms.c - it seems silly to re-invent code which we already have to detect these symbols. -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/2] perf symbols: Ignore mapping symbols on aarch64 2015-01-15 0:03 ` Russell King - ARM Linux @ 2015-01-15 2:51 ` Victor Kamensky 2015-01-16 9:56 ` Will Deacon 0 siblings, 1 reply; 13+ messages in thread From: Victor Kamensky @ 2015-01-15 2:51 UTC (permalink / raw) To: linux-arm-kernel On 14 January 2015 at 16:03, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Wed, Jan 14, 2015 at 10:38:38AM -0800, Victor Kamensky wrote: >> >From fed6caab410ddcaf487ff23a3908eca129e50b89 Mon Sep 17 00:00:00 2001 >> From: Victor Kamensky <victor.kamensky@linaro.org> >> Date: Wed, 14 Jan 2015 07:42:41 -0800 >> Subject: [PATCH 3/3] perf symbols: improve abi compliance in arm mapping >> symbols handling >> >> Both Arm and Aarch64 ELF ABI allow mapping symbols be in from >> either "$d" or "$d.<any>". But current code that handles mapping >> symbols only deals with the first, dollar character and a single >> letter, case. >> >> The patch adds handling of the second case with period >> followed by any characters. >> >> Suggested-by: Will Deacon <will.deacon@arm.com> >> Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org> > > I wonder if it would make more sense to re-use the "is_arm_mapping_symbol" > thing which we have in kernel/module.c and scripts/kallsyms.c - it > seems silly to re-invent code which we already have to detect these > symbols. Thanks for pointing this out. I did not know about "is_arm_mapping_symbol" function. Do you suggest we copy one of those functions into tools/perf? Since tools/perf is separate user-land utility I don't see easy way how can we reuse those directly. Also those functions check for mapping symbols seems to be more efficient that one I come with, for example one from scripts/kallsyms.c static inline int is_arm_mapping_symbol(const char *str) { return str[0] == '$' && strchr("axtd", str[1]) && (str[2] == '\0' || str[2] == '.'); } But it seems that they are somewhat accurate: because they bundle EM_ARM and EM_AARCH64 into one case. According to ABIs for EM_ARM we have $a, $t, $d, $a.<any>, $t.<any>, $d.<any>; and for EM_AARCH64 we have $x, $d, $x.<any>, $d.<any>. How about the following two variants of the patch. It follows the same mapping handling logic as in other 3 copies of is_arm_mapping_symbol function in kernel, but it still separate copy in tools/perf code. Personally I prefer variant 1, but I am fine with variant 2 too, because practically it will be OK. Variant 1 (as addition to this patch, as above): >From e08d348bd72d406d8939993d266729d805577c4b Mon Sep 17 00:00:00 2001 From: Victor Kamensky <victor.kamensky@linaro.org> Date: Wed, 14 Jan 2015 07:42:41 -0800 Subject: [PATCH 3/3] perf symbols: improve abi compliance in arm mapping symbols handling Both Arm and Aarch64 ELF ABI allow mapping symbols be in from either "$d" or "$d.<any>". But current code that handles mapping symbols only deals with the first, dollar character and a single letter, case. The patch adds handling of the second case with period followed by any characters. Suggested-by: Russell King <linux@arm.linux.org.uk> Suggested-by: Will Deacon <will.deacon@arm.com> Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org> --- tools/perf/util/symbol-elf.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c index 1e188dd..a038c98 100644 --- a/tools/perf/util/symbol-elf.c +++ b/tools/perf/util/symbol-elf.c @@ -857,17 +857,16 @@ int dso__load_sym(struct dso *dso, struct map *map, * don't identify functions, so will confuse the profile * output: */ if (ehdr.e_machine == EM_ARM) { - if (!strcmp(elf_name, "$a") || - !strcmp(elf_name, "$d") || - !strcmp(elf_name, "$t")) + if (elf_name[0] == '$' && strchr("adt", elf_name[1]) + && (elf_name[2] == '\0' || elf_name[2] == '.')) continue; } /* Reject Aarch64 ELF "mapping symbols": these aren't unique and * don't identify functions, so will confuse the profile * output: */ if (ehdr.e_machine == EM_AARCH64) { - if (!strcmp(elf_name, "$x") || - !strcmp(elf_name, "$d")) + if (elf_name[0] == '$' && strchr("dx", elf_name[1]) + && (elf_name[2] == '\0' || elf_name[2] == '.')) continue; } -- 1.9.3 Variant 2 instead of patch posted with current subject: From c8d08ebddc61203daf21b17c891c26c1d08e14f1 Mon Sep 17 00:00:00 2001 From: Victor Kamensky <victor.kamensky@linaro.org> Date: Mon, 12 Jan 2015 14:13:36 -0800 Subject: [PATCH 1/2] perf symbols: Ignore mapping symbols on aarch64 Aarch64 ELF files use mapping symbols with special names $x, $d to identify regions of Aarch64 code (see Aarch64 ELF ABI - "ARM IHI 0056B", section "4.5.4 Mapping symbols"). The patch filters out these symbols at load time, similar to "696b97a perf symbols: Ignore mapping symbols on ARM" changes done for ARM before V8. Also added handling of mapping symbols that has format "$d.<any>" and similar for both cases. Note we are not making difference between EM_ARM and EM_AARCH64 mapping symbols instead code handles superset of both. Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl> Cc: Paul Mackerras <paulus@samba.org> Cc: Ingo Molnar <mingo@redhat.com> Cc: Arnaldo Carvalho de Melo <acme@kernel.org> Cc: Adrian Hunter <adrian.hunter@intel.com> Cc: Jiri Olsa <jolsa@redhat.com> Cc: Namhyung Kim <namhyung@kernel.org> Cc: Avi Kivity <avi@cloudius-systems.com> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> Cc: Anton Blanchard <anton@samba.org> Cc: David Ahern <dsahern@gmail.com> Cc: Will Deacon <will.deacon@arm.com> Cc: Russell King <linux@arm.linux.org.uk> Cc: Dave Martin <Dave.Martin@arm.com> --- tools/perf/util/symbol-elf.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c index 06fcd1b..b2eb0f9 100644 --- a/tools/perf/util/symbol-elf.c +++ b/tools/perf/util/symbol-elf.c @@ -856,10 +856,9 @@ int dso__load_sym(struct dso *dso, struct map *map, /* Reject ARM ELF "mapping symbols": these aren't unique and * don't identify functions, so will confuse the profile * output: */ - if (ehdr.e_machine == EM_ARM) { - if (!strcmp(elf_name, "$a") || - !strcmp(elf_name, "$d") || - !strcmp(elf_name, "$t")) + if (ehdr.e_machine == EM_ARM || ehdr.e_machine == EM_AARCH64) { + if (elf_name[0] == '$' && strchr("adtx", elf_name[1]) + && (elf_name[2] == '\0' || elf_name[2] == '.')) continue; } -- 1.9.3 Thanks, Victor > -- > FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up > according to speedtest.net. ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 1/2] perf symbols: Ignore mapping symbols on aarch64 2015-01-15 2:51 ` Victor Kamensky @ 2015-01-16 9:56 ` Will Deacon 0 siblings, 0 replies; 13+ messages in thread From: Will Deacon @ 2015-01-16 9:56 UTC (permalink / raw) To: linux-arm-kernel On Thu, Jan 15, 2015 at 02:51:06AM +0000, Victor Kamensky wrote: > On 14 January 2015 at 16:03, Russell King - ARM Linux > <linux@arm.linux.org.uk> wrote: > > On Wed, Jan 14, 2015 at 10:38:38AM -0800, Victor Kamensky wrote: > >> >From fed6caab410ddcaf487ff23a3908eca129e50b89 Mon Sep 17 00:00:00 2001 > >> From: Victor Kamensky <victor.kamensky@linaro.org> > >> Date: Wed, 14 Jan 2015 07:42:41 -0800 > >> Subject: [PATCH 3/3] perf symbols: improve abi compliance in arm mapping > >> symbols handling > >> > >> Both Arm and Aarch64 ELF ABI allow mapping symbols be in from > >> either "$d" or "$d.<any>". But current code that handles mapping > >> symbols only deals with the first, dollar character and a single > >> letter, case. > >> > >> The patch adds handling of the second case with period > >> followed by any characters. > >> > >> Suggested-by: Will Deacon <will.deacon@arm.com> > >> Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org> > > > > I wonder if it would make more sense to re-use the "is_arm_mapping_symbol" > > thing which we have in kernel/module.c and scripts/kallsyms.c - it > > seems silly to re-invent code which we already have to detect these > > symbols. > > Thanks for pointing this out. I did not know about > "is_arm_mapping_symbol" function. > > Do you suggest we copy one of those functions into tools/perf? > Since tools/perf is separate user-land utility I don't see easy way > how can we reuse those directly. > > Also those functions check for mapping symbols > seems to be more efficient that one I come with, for example > one from scripts/kallsyms.c > > static inline int is_arm_mapping_symbol(const char *str) > { > return str[0] == '$' && strchr("axtd", str[1]) > && (str[2] == '\0' || str[2] == '.'); > } > > But it seems that they are somewhat accurate: because they bundle > EM_ARM and EM_AARCH64 into one case. According to ABIs > for EM_ARM we have $a, $t, $d, $a.<any>, $t.<any>, $d.<any>; > and for EM_AARCH64 we have $x, $d, $x.<any>, $d.<any>. > > How about the following two variants of the patch. It follows the same > mapping handling logic as in other 3 copies of is_arm_mapping_symbol > function in kernel, but it still separate copy in tools/perf code. Personally I > prefer variant 1, but I am fine with variant 2 too, because practically it > will be OK. They both look fine to me; pick your favourite. Acked-by: Will Deacon <will.deacon@arm.com> Will > Variant 1 (as addition to this patch, as above): > > From e08d348bd72d406d8939993d266729d805577c4b Mon Sep 17 00:00:00 2001 > From: Victor Kamensky <victor.kamensky@linaro.org> > Date: Wed, 14 Jan 2015 07:42:41 -0800 > Subject: [PATCH 3/3] perf symbols: improve abi compliance in arm mapping > symbols handling > > Both Arm and Aarch64 ELF ABI allow mapping symbols be in from > either "$d" or "$d.<any>". But current code that handles mapping > symbols only deals with the first, dollar character and a single > letter, case. > > The patch adds handling of the second case with period > followed by any characters. > > Suggested-by: Russell King <linux@arm.linux.org.uk> > Suggested-by: Will Deacon <will.deacon@arm.com> > Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org> > --- > tools/perf/util/symbol-elf.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c > index 1e188dd..a038c98 100644 > --- a/tools/perf/util/symbol-elf.c > +++ b/tools/perf/util/symbol-elf.c > @@ -857,17 +857,16 @@ int dso__load_sym(struct dso *dso, struct map *map, > * don't identify functions, so will confuse the profile > * output: */ > if (ehdr.e_machine == EM_ARM) { > - if (!strcmp(elf_name, "$a") || > - !strcmp(elf_name, "$d") || > - !strcmp(elf_name, "$t")) > + if (elf_name[0] == '$' && strchr("adt", elf_name[1]) > + && (elf_name[2] == '\0' || elf_name[2] == '.')) > continue; > } > /* Reject Aarch64 ELF "mapping symbols": these aren't unique and > * don't identify functions, so will confuse the profile > * output: */ > if (ehdr.e_machine == EM_AARCH64) { > - if (!strcmp(elf_name, "$x") || > - !strcmp(elf_name, "$d")) > + if (elf_name[0] == '$' && strchr("dx", elf_name[1]) > + && (elf_name[2] == '\0' || elf_name[2] == '.')) > continue; > } > > -- > 1.9.3 > > Variant 2 instead of patch posted with current subject: > > From c8d08ebddc61203daf21b17c891c26c1d08e14f1 Mon Sep 17 00:00:00 2001 > From: Victor Kamensky <victor.kamensky@linaro.org> > Date: Mon, 12 Jan 2015 14:13:36 -0800 > Subject: [PATCH 1/2] perf symbols: Ignore mapping symbols on aarch64 > > Aarch64 ELF files use mapping symbols with special names $x, $d > to identify regions of Aarch64 code (see Aarch64 ELF ABI - "ARM > IHI 0056B", section "4.5.4 Mapping symbols"). > > The patch filters out these symbols at load time, similar to > "696b97a perf symbols: Ignore mapping symbols on ARM" changes > done for ARM before V8. > > Also added handling of mapping symbols that has format > "$d.<any>" and similar for both cases. > > Note we are not making difference between EM_ARM and > EM_AARCH64 mapping symbols instead code handles superset > of both. > > Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org> > Cc: Peter Zijlstra <a.p.zijlstra@chello.nl> > Cc: Paul Mackerras <paulus@samba.org> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: Arnaldo Carvalho de Melo <acme@kernel.org> > Cc: Adrian Hunter <adrian.hunter@intel.com> > Cc: Jiri Olsa <jolsa@redhat.com> > Cc: Namhyung Kim <namhyung@kernel.org> > Cc: Avi Kivity <avi@cloudius-systems.com> > Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> > Cc: Anton Blanchard <anton@samba.org> > Cc: David Ahern <dsahern@gmail.com> > Cc: Will Deacon <will.deacon@arm.com> > Cc: Russell King <linux@arm.linux.org.uk> > Cc: Dave Martin <Dave.Martin@arm.com> > --- > tools/perf/util/symbol-elf.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c > index 06fcd1b..b2eb0f9 100644 > --- a/tools/perf/util/symbol-elf.c > +++ b/tools/perf/util/symbol-elf.c > @@ -856,10 +856,9 @@ int dso__load_sym(struct dso *dso, struct map *map, > /* Reject ARM ELF "mapping symbols": these aren't unique and > * don't identify functions, so will confuse the profile > * output: */ > - if (ehdr.e_machine == EM_ARM) { > - if (!strcmp(elf_name, "$a") || > - !strcmp(elf_name, "$d") || > - !strcmp(elf_name, "$t")) > + if (ehdr.e_machine == EM_ARM || ehdr.e_machine == EM_AARCH64) { > + if (elf_name[0] == '$' && strchr("adtx", elf_name[1]) > + && (elf_name[2] == '\0' || elf_name[2] == '.')) > continue; > } > > -- > 1.9.3 > > Thanks, > Victor > > > -- > > FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up > > according to speedtest.net. > ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2015-01-22 3:53 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-01-13 16:59 [PATCH 1/2] perf symbols: Ignore mapping symbols on aarch64 Victor Kamensky 2015-01-13 16:59 ` [PATCH 2/2] perf symbols: debuglink should take symfs option into account Victor Kamensky 2015-01-19 17:50 ` Victor Kamensky 2015-01-21 22:00 ` David Ahern 2015-01-22 0:34 ` Victor Kamensky 2015-01-22 0:53 ` David Ahern 2015-01-22 1:32 ` Victor Kamensky 2015-01-22 3:53 ` David Ahern 2015-01-14 11:22 ` [PATCH 1/2] perf symbols: Ignore mapping symbols on aarch64 Will Deacon 2015-01-14 18:38 ` Victor Kamensky 2015-01-15 0:03 ` Russell King - ARM Linux 2015-01-15 2:51 ` Victor Kamensky 2015-01-16 9:56 ` Will Deacon
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).