From: Ian Munsie <imunsie@au1.ibm.com> If a 32bit userspace perf is running on a 64bit kernel, the end of the final map in the kernel would incorrectly be set to 2^32-1 rather than 2^64-1. Signed-off-by: Ian Munsie <imunsie@au1.ibm.com> --- tools/perf/util/event.c | 2 +- tools/perf/util/symbol.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c index bc47446..4283417 100644 --- a/tools/perf/util/event.c +++ b/tools/perf/util/event.c @@ -392,7 +392,7 @@ static void event_set_kernel_mmap_len(struct map **maps, event_t *self) * a zero sized synthesized MMAP event for the kernel. */ if (maps[MAP__FUNCTION]->end == 0) - maps[MAP__FUNCTION]->end = ~0UL; + maps[MAP__FUNCTION]->end = ~0ULL; } static int event__process_kernel_mmap(event_t *self, diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c index 0500895..a348906 100644 --- a/tools/perf/util/symbol.c +++ b/tools/perf/util/symbol.c @@ -121,7 +121,7 @@ static void __map_groups__fixup_end(struct map_groups *self, enum map_type type) * We still haven't the actual symbols, so guess the * last map final address. */ - curr->end = ~0UL; + curr->end = ~0ULL; } static void map_groups__fixup_end(struct map_groups *self) -- 1.7.2.3 --
From: Ian Munsie <imunsie@au1.ibm.com> When we build perf we place all of the .o files from the library files (util, arch/x/util, etc) into libperf.a which is then linked into perf. The problem is that the linker will by default only consider .o files within the .a archive if they are necessary to satisfy an unresolved symbol. As weak functions are not unresolved, it will not consider a .o file from the archive containing the strong versions of weak functions unless it requires it for another reason. This patch adds the --whole-archive flags to the linker when passing in the libperf.a file to ensure that it will consider every .o file in the archive, not just what it believes that it needs. The end result is that weak functions can now be overridden by strong variants of them in the libperf.a file. Signed-off-by: Ian Munsie <imunsie@au1.ibm.com> --- tools/perf/Makefile | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/tools/perf/Makefile b/tools/perf/Makefile index 2d414b3..c5f7b7f 100644 --- a/tools/perf/Makefile +++ b/tools/perf/Makefile @@ -940,7 +940,8 @@ $(OUTPUT)perf.o: perf.c $(OUTPUT)common-cmds.h $(OUTPUT)PERF-CFLAGS $(OUTPUT)perf$X: $(OUTPUT)perf.o $(BUILTIN_OBJS) $(PERFLIBS) $(QUIET_LINK)$(CC) $(ALL_CFLAGS) $(ALL_LDFLAGS) $(OUTPUT)perf.o \ - $(BUILTIN_OBJS) $(LIBS) -o $@ + $(BUILTIN_OBJS) -Wl,--whole-archive $(LIBS) \ + -Wl,--no-whole-archive -o $@ @rm -f trace @ln perf$X trace 2>/dev/null -- 1.7.2.3 --
After I applied this patch I got this on a x86_64 Fedora 14 box:
LINK /home/acme/git/build/perf/perf
/usr/lib64/libpthread_nonshared.a(pthread_atfork.oS): In function `pthread_atfork':
(.text+0x0): multiple definition of `__pthread_atfork'
/usr/lib64/libpthread_nonshared.a(pthread_atfork.oS):(.text+0x0): first defined here
/usr/lib64/libpthread_nonshared.a(pthread_atfork.oS): In function `pthread_atfork':
(.text+0x0): multiple definition of `pthread_atfork'
/usr/lib64/libpthread_nonshared.a(pthread_atfork.oS):(.text+0x0): first defined here
/usr/lib64/libpthread_nonshared.a(pthread_atfork.oS): In function `pthread_atfork':
(.text+0x0): multiple definition of `__pthread_atfork'
/usr/lib64/libpthread_nonshared.a(pthread_atfork.oS):(.text+0x0): first defined here
/usr/lib64/libpthread_nonshared.a(pthread_atfork.oS): In function `pthread_atfork':
(.text+0x0): multiple definition of `pthread_atfork'
/usr/lib64/libpthread_nonshared.a(pthread_atfork.oS):(.text+0x0): first defined here
/usr/lib/gcc/x86_64-redhat-linux/4.5.1/../../../../lib64/libiberty.a(choose-temp.o): In function `choose_temp_base':
(.text+0x57): warning: the use of `mktemp' is dangerous, better use `mkstemp'
/usr/lib/gcc/x86_64-redhat-linux/4.5.1/../../../../lib64/libiberty.a(strerror.o): In function `errno_max':
(.text+0x151): warning: `sys_nerr' is deprecated; use `strerror' or `strerror_r' instead
/usr/lib/gcc/x86_64-redhat-linux/4.5.1/../../../../lib64/libiberty.a(xstrdup.o): In function `xstrdup':
(.text+0x0): multiple definition of `xstrdup'
/home/acme/git/build/perf/libperf.a(wrapper.o):/media/tbs/acme/git/linux/tools/perf/util/wrapper.c:15: first defined here
collect2: ld returned 1 exit status
make: *** [/home/acme/git/build/perf/perf] Error 1
make: Leaving directory `/media/tbs/acme/git/linux/tools/perf'
[acme@felicio linux]$
- Arnaldo
--
Hi Arnaldo, I'm not seeing any of those warnings or errors on either of the systems I have been testing on (x86_64 + PPC64), but it looks like the linker is trying to pull in too much from the system libraries. Can you test the below patch to see if the messages go away - it only applies the whole-archive flag to libperf.a, whereas before I was also applying it to pthread, libelf, etc. Cheers, -Ian From da1ff2cafce30343fba34b81935d0c7772f8c6ae Mon Sep 17 00:00:00 2001 From: Ian Munsie <imunsie@au1.ibm.com> Date: Thu, 25 Nov 2010 13:44:52 +1100 Subject: [PATCH] perf: Allow strong and weak functions in LIB_OBJS When we build perf we place all of the .o files from the library files (util, arch/x/util, etc) into libperf.a which is then linked into perf. The problem is that the linker will by default only consider .o files within the .a archive if they are necessary to satisfy an unresolved symbol. As weak functions are not unresolved, it will not consider a .o file from the archive containing the strong versions of weak functions unless it requires it for another reason. This patch adds the --whole-archive flags to the linker when passing in the libperf.a file to ensure that it will consider every .o file in the archive, not just what it believes that it needs. The end result is that weak functions can now be overridden by strong variants of them in the libperf.a file. Signed-off-by: Ian Munsie <imunsie@au1.ibm.com> --- tools/perf/Makefile | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/tools/perf/Makefile b/tools/perf/Makefile index 2d414b3..f760a2d 100644 --- a/tools/perf/Makefile +++ b/tools/perf/Makefile @@ -902,7 +902,7 @@ prefix_SQ = $(subst ','\'',$(prefix)) SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH)) PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH)) -LIBS = $(PERFLIBS) $(EXTLIBS) +LIBS = -Wl,--whole-archive $(PERFLIBS) -Wl,--no-whole-archive $(EXTLIBS) BASIC_CFLAGS += -DSHA1_HEADER='$(SHA1_HEADER_SQ)' \ ...
Hey Arnaldo, Just wondering if you got around to testing this patch? Thanks, -Ian --
Yeah, fell thru the cracks, but I tested it now on several systems and it now seems ok, applying to perf/core, thanks, - Arnaldo --
Commit-ID: b38aa89600be39b3e10c5b6529aed2e66518598e Gitweb: http://git.kernel.org/tip/b38aa89600be39b3e10c5b6529aed2e66518598e Author: Ian Munsie <imunsie@au1.ibm.com> AuthorDate: Mon, 29 Nov 2010 11:53:07 +1100 Committer: Arnaldo Carvalho de Melo <acme@redhat.com> CommitDate: Tue, 7 Dec 2010 11:58:50 -0200 perf makefile: Allow strong and weak functions in LIB_OBJS When we build perf we place all of the .o files from the library files (util, arch/x/util, etc) into libperf.a which is then linked into perf. The problem is that the linker will by default only consider .o files within the .a archive if they are necessary to satisfy an unresolved symbol. As weak functions are not unresolved, it will not consider a .o file from the archive containing the strong versions of weak functions unless it requires it for another reason. This patch adds the --whole-archive flags to the linker when passing in the libperf.a file to ensure that it will consider every .o file in the archive, not just what it believes that it needs. The end result is that weak functions can now be overridden by strong variants of them in the libperf.a file. Cc: "tom.leiming" <tom.leiming@gmail.com> Cc: Ingo Molnar <mingo@elte.hu> Cc: Paul Mackerras <paulus@samba.org> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl> LKML-Reference: <1290991642-sup-5890@au1.ibm.com> Signed-off-by: Ian Munsie <imunsie@au1.ibm.com> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- tools/perf/Makefile | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/tools/perf/Makefile b/tools/perf/Makefile index d88137a..ac6692c 100644 --- a/tools/perf/Makefile +++ b/tools/perf/Makefile @@ -901,7 +901,7 @@ prefix_SQ = $(subst ','\'',$(prefix)) SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH)) PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH)) -LIBS = $(PERFLIBS) $(EXTLIBS) +LIBS = -Wl,--whole-archive $(PERFLIBS) -Wl,--no-whole-archive $(EXTLIBS) BASIC_CFLAGS += -DSHA1_HEADER='$(SHA1_HEADER_SQ)' ...
From: Ian Munsie <imunsie@au1.ibm.com> The PowerPC ABI prefixes symbol names with periods, which causes perf test to fail when it compares the symbol from the vmlinux file with the symbol names from kallsyms. This patch adds the infrastructure to allow archs to override how symbol names are compared and implements the PowerPC function to disregard any prefixed periods, allowing perf test to pass. Signed-off-by: Ian Munsie <imunsie@au1.ibm.com> --- tools/perf/arch/powerpc/Makefile | 5 +++++ tools/perf/arch/powerpc/util/symbol.c | 22 ++++++++++++++++++++++ tools/perf/builtin-test.c | 2 +- tools/perf/util/symbol.c | 5 +++++ tools/perf/util/symbol.h | 2 ++ 5 files changed, 35 insertions(+), 1 deletions(-) create mode 100644 tools/perf/arch/powerpc/util/symbol.c diff --git a/tools/perf/arch/powerpc/Makefile b/tools/perf/arch/powerpc/Makefile index 15130b5..b3e211e 100644 --- a/tools/perf/arch/powerpc/Makefile +++ b/tools/perf/arch/powerpc/Makefile @@ -2,3 +2,8 @@ ifndef NO_DWARF PERF_HAVE_DWARF_REGS := 1 LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/dwarf-regs.o endif + +LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/symbol.o + +$(OUTPUT)arch/powerpc/util/%.o : arch/powerpc/util/%.c $(OUTPUT)PERF-CFLAGS + $(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) -I util/ $< diff --git a/tools/perf/arch/powerpc/util/symbol.c b/tools/perf/arch/powerpc/util/symbol.c new file mode 100644 index 0000000..db56913 --- /dev/null +++ b/tools/perf/arch/powerpc/util/symbol.c @@ -0,0 +1,22 @@ +/* + * Functions to handle PowerPC symbol naming + * + * Copyright © 2010 Ian Munsie, IBM Corporation. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version + * 2 of the License, or (at your option) any later version. + */ + +#include <string.h> +#include <symbol.h> + +/* Overrides the definition in the ...
Hi Paul, Can you take a look at this patch and ACK (or NACK) it and push it to Ingo? The two other patches in this series are already in tip/perf/core, we're just missing this one. Cheers, -Ian --
Hi Ian, I guess we can apply this one now? Or do you have anything newer? - Arnaldo --
Yep, that's good to go in as is. Thanks, -Ian --
Commit-ID: 9d1faba5fe410558099f13cfada2eab03186769d Gitweb: http://git.kernel.org/tip/9d1faba5fe410558099f13cfada2eab03186769d Author: Ian Munsie <imunsie@au1.ibm.com> AuthorDate: Thu, 25 Nov 2010 15:12:53 +1100 Committer: Arnaldo Carvalho de Melo <acme@redhat.com> CommitDate: Sat, 27 Nov 2010 01:32:53 -0200 perf symbols: Correct final kernel map guesses If a 32bit userspace perf is running on a 64bit kernel, the end of the final map in the kernel would incorrectly be set to 2^32-1 rather than 2^64-1. Cc: Ingo Molnar <mingo@elte.hu> Cc: Paul Mackerras <paulus@samba.org> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl> LKML-Reference: <1290658375-10342-1-git-send-email-imunsie@au1.ibm.com> Signed-off-by: Ian Munsie <imunsie@au1.ibm.com> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- tools/perf/util/event.c | 2 +- tools/perf/util/symbol.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c index dab9e75..7260db7 100644 --- a/tools/perf/util/event.c +++ b/tools/perf/util/event.c @@ -392,7 +392,7 @@ static void event_set_kernel_mmap_len(struct map **maps, event_t *self) * a zero sized synthesized MMAP event for the kernel. */ if (maps[MAP__FUNCTION]->end == 0) - maps[MAP__FUNCTION]->end = ~0UL; + maps[MAP__FUNCTION]->end = ~0ULL; } static int event__process_kernel_mmap(event_t *self, diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c index 0500895..a348906 100644 --- a/tools/perf/util/symbol.c +++ b/tools/perf/util/symbol.c @@ -121,7 +121,7 @@ static void __map_groups__fixup_end(struct map_groups *self, enum map_type type) * We still haven't the actual symbols, so guess the * last map final address. */ - curr->end = ~0UL; + curr->end = ~0ULL; } static void map_groups__fixup_end(struct map_groups *self) --
