From: Ian Munsie <[email protected]>
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 <[email protected]>
---
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 <[email protected]>
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 <[email protected]>
---
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 generic code */
+int arch_sym_strcmp(const char *s1, const char *s2)
+{
+ const char *ts1, *ts2;
+ ts1 = (*s1 == '.' ? s1+1 : s1);
+ ts2 = (*s2 == '.' ? s2+1 : s2);
+ return strcmp(ts1, ts2);
+}
diff --git a/tools/perf/builtin-test.c b/tools/perf/builtin-test.c
index 035b9fa..9c0301b 100644
--- a/tools/perf/builtin-test.c
+++ b/tools/perf/builtin-test.c
@@ -126,7 +126,7 @@ static int test__vmlinux_matches_kallsyms(void)
if (pair && pair->start == sym->start) {
next_pair:
- if (strcmp(sym->name, pair->name) == 0) {
+ if (arch_sym_strcmp(sym->name, pair->name) == 0) {
/*
* kallsyms don't have the symbol end, so we
* set that by using the next symbol start - 1,
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index a348906..180683e 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -2473,3 +2473,8 @@ int machine__load_vmlinux_path(struct machine *self, enum map_type type,
return ret;
}
+
+int __attribute__((weak)) arch_sym_strcmp(const char *s1, const char *s2)
+{
+ return strcmp(s1, s2);
+}
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 038f220..6f4daae 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -229,4 +229,6 @@ bool symbol_type__is_a(char symbol_type, enum map_type map_type);
size_t machine__fprintf_vmlinux_path(struct machine *self, FILE *fp);
+int arch_sym_strcmp(const char *s1, const char *s2);
+
#endif /* __PERF_SYMBOL */
--
1.7.2.3
From: Ian Munsie <[email protected]>
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 <[email protected]>
---
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
Em Thu, Nov 25, 2010 at 03:12:54PM +1100, Ian Munsie escreveu:
> From: Ian Munsie <[email protected]>
>
> 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.
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
Commit-ID: 9d1faba5fe410558099f13cfada2eab03186769d
Gitweb: http://git.kernel.org/tip/9d1faba5fe410558099f13cfada2eab03186769d
Author: Ian Munsie <[email protected]>
AuthorDate: Thu, 25 Nov 2010 15:12:53 +1100
Committer: Arnaldo Carvalho de Melo <[email protected]>
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 <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ian Munsie <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
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)
Hi Arnaldo,
Excerpts from Arnaldo Carvalho de Melo's message of Sat Nov 27 08:18:36 +1100 2010:
> 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
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 <[email protected]>
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 <[email protected]>
---
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)' \
$(COMPAT_CFLAGS)
--
1.7.2.3
Excerpts from Ian Munsie's message of Mon Nov 29 11:53:07 +1100 2010:
> 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.
Hey Arnaldo,
Just wondering if you got around to testing this patch?
Thanks,
-Ian
Em Tue, Dec 07, 2010 at 05:42:58PM +1100, Ian Munsie escreveu:
> Excerpts from Ian Munsie's message of Mon Nov 29 11:53:07 +1100 2010:
> > 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.
>
> Hey Arnaldo,
>
> Just wondering if you got around to testing this patch?
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 <[email protected]>
AuthorDate: Mon, 29 Nov 2010 11:53:07 +1100
Committer: Arnaldo Carvalho de Melo <[email protected]>
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" <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ian Munsie <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
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)' \
$(COMPAT_CFLAGS)
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
Excerpts from Ian Munsie's message of Thu Nov 25 15:12:55 +1100 2010:
> From: Ian Munsie <[email protected]>
>
> 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.
Em Thu, Nov 25, 2010 at 03:12:55PM +1100, Ian Munsie escreveu:
> From: Ian Munsie <[email protected]>
>
> 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.
Hi Ian,
I guess we can apply this one now? Or do you have anything
newer?
- Arnaldo
Excerpts from Arnaldo Carvalho de Melo's message of Mon Dec 13 01:39:48 +1100 2010:
> Hi Ian,
>
> I guess we can apply this one now? Or do you have anything
> newer?
Yep, that's good to go in as is.
Thanks,
-Ian