Re: [PATCH] module: ppc64 module CRC relocation fix causes perf issues
On Fri, Jul 26, 2013 at 11:19:13AM +1000, Anton Blanchard wrote: Hi Neil, Sorry I'm a bit late to the thread, I've ben swamped. Has someone tested this with kexec/kdump? Thats why the origional patch was created, because when kexec loads the kernel at a different physical address, the relocations messed with the module crc's, and modules couldn't load during the kexec boot. Assuming that kernaddr_start gets set appropriately during boot, using PHYSICAL_START should be fine, but I wanted to check, and don't currently have access to a powerpc system to do so. Neil I tested a relocatable kernel forced to run at a non zero physical address (ie basically kdump). I verified CRCs were bad with your original patch backed out, and were good with this patch applied. Anton Perfect, sounds like a sufficient test to me. Acked-by: Neil Horman nhor...@tuxdriver.com ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] module: ppc64 module CRC relocation fix causes perf issues
On Thu, Jul 25, 2013 at 09:14:25AM +1000, Benjamin Herrenschmidt wrote: On Thu, 2013-07-25 at 08:34 +1000, Anton Blanchard wrote: Apart from the annoying colors, is there anything specific I should be looking for? Some sort of error message, or output that actually makes sense? Thanks for testing! Ben, I think the patch is good to go. Sent it yesterday to Linus, it's upstream already :-) Cheers, Ben. Sorry I'm a bit late to the thread, I've ben swamped. Has someone tested this with kexec/kdump? Thats why the origional patch was created, because when kexec loads the kernel at a different physical address, the relocations messed with the module crc's, and modules couldn't load during the kexec boot. Assuming that kernaddr_start gets set appropriately during boot, using PHYSICAL_START should be fine, but I wanted to check, and don't currently have access to a powerpc system to do so. Neil ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] module: ppc64 module CRC relocation fix causes perf issues
Hi Neil, Sorry I'm a bit late to the thread, I've ben swamped. Has someone tested this with kexec/kdump? Thats why the origional patch was created, because when kexec loads the kernel at a different physical address, the relocations messed with the module crc's, and modules couldn't load during the kexec boot. Assuming that kernaddr_start gets set appropriately during boot, using PHYSICAL_START should be fine, but I wanted to check, and don't currently have access to a powerpc system to do so. Neil I tested a relocatable kernel forced to run at a non zero physical address (ie basically kdump). I verified CRCs were bad with your original patch backed out, and were good with this patch applied. Anton ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] module: ppc64 module CRC relocation fix causes perf issues
On 07/23/2013 08:30:32 AM, Michael Ellerman wrote: On Fri, Jul 19, 2013 at 05:59:30PM -0500, Scott Wood wrote: On 07/17/2013 11:00:45 PM, Anton Blanchard wrote: Hi Scott, What specifically should I do to test it? Could you double check perf annotate works? I'm 99% sure it will but that is what was failing on ppc64. I'm not really sure what it's supposed to look like when perf annotate works. It spits a bunch of unreadable[1] dark-blue-on-black assembly code at me, all with 0.00 : in the left column. Oh, wait -- some lines have 100.00 : on the left, in even-more-unreadable dark-red-on-black. Apart from the annoying colors, is there anything specific I should be looking for? Some sort of error message, or output that actually makes sense? The colours look fine on my terminal, so I don't know what you've done there. It probably looks better if the terminal is configured to have a light background (which of course makes some other programs look worse), or (as I noted) if you've got your monitor set to be very bright. I now see that xfce4-terminal lets me redefine the standard colors, though, so that should help. If you care you can use --stdio to use the plainer interface, though it still uses colours. That output looks fine in terms of the bug Anton was chasing. As far as only ever hitting one instruction that does look weird. OK. I'll add investigate weird e500 perf annotate results to the TODO list... -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] module: ppc64 module CRC relocation fix causes perf issues
Hi Scott, I'm not really sure what it's supposed to look like when perf annotate works. It spits a bunch of unreadable[1] dark-blue-on-black assembly code at me, all with 0.00 : in the left column. Oh, wait -- some lines have 100.00 : on the left, in even-more-unreadable dark-red-on-black. Apart from the annoying colors, is there anything specific I should be looking for? Some sort of error message, or output that actually makes sense? Thanks for testing! Ben, I think the patch is good to go. Anton ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] module: ppc64 module CRC relocation fix causes perf issues
On Thu, 2013-07-25 at 08:34 +1000, Anton Blanchard wrote: Apart from the annoying colors, is there anything specific I should be looking for? Some sort of error message, or output that actually makes sense? Thanks for testing! Ben, I think the patch is good to go. Sent it yesterday to Linus, it's upstream already :-) Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] module: ppc64 module CRC relocation fix causes perf issues
On Fri, Jul 19, 2013 at 05:59:30PM -0500, Scott Wood wrote: On 07/17/2013 11:00:45 PM, Anton Blanchard wrote: Hi Scott, What specifically should I do to test it? Could you double check perf annotate works? I'm 99% sure it will but that is what was failing on ppc64. I'm not really sure what it's supposed to look like when perf annotate works. It spits a bunch of unreadable[1] dark-blue-on-black assembly code at me, all with 0.00 : in the left column. Oh, wait -- some lines have 100.00 : on the left, in even-more-unreadable dark-red-on-black. Apart from the annoying colors, is there anything specific I should be looking for? Some sort of error message, or output that actually makes sense? The colours look fine on my terminal, so I don't know what you've done there. If you care you can use --stdio to use the plainer interface, though it still uses colours. That output looks fine in terms of the bug Anton was chasing. As far as only ever hitting one instruction that does look weird. cheers ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] module: ppc64 module CRC relocation fix causes perf issues
On 07/17/2013 11:00:45 PM, Anton Blanchard wrote: Hi Scott, What specifically should I do to test it? Could you double check perf annotate works? I'm 99% sure it will but that is what was failing on ppc64. I'm not really sure what it's supposed to look like when perf annotate works. It spits a bunch of unreadable[1] dark-blue-on-black assembly code at me, all with 0.00 : in the left column. Oh, wait -- some lines have 100.00 : on the left, in even-more-unreadable dark-red-on-black. Apart from the annoying colors, is there anything specific I should be looking for? Some sort of error message, or output that actually makes sense? I've attached the output from perf annotate and perf report. perf.data was generated by perf record find /usr /dev/null on an NFS root (which took a few seconds to complete), so the large amount of __alloc_skb makes some sense, but the way perf annotate shows 100% on one instruction in each function seems odd. -Scott [1] ...unless I crank the brightness up on my monitor to the point where whites are blinding, or redirect the output to a file so the colors go away. Percent | Source code Disassembly of vmlinux : : : : Disassembly of section .text: : : c0097510 perf_event_comm: 0.00 : c0097510: 94 21 ff a0 stwur1,-96(r1) 0.00 : c0097514: 7c 08 02 a6 mflrr0 0.00 : c0097518: bf 01 00 40 stmwr24,64(r1) 0.00 : c009751c: 7c 7e 1b 78 mr r30,r3 0.00 : c0097520: 90 01 00 64 stw r0,100(r1) 0.00 : c0097524: 3b 80 00 00 li r28,0 0.00 : c0097528: 3b 63 04 68 addir27,r3,1128 0.00 : c009752c: 3b 40 00 00 li r26,0 0.00 : c0097530: 87 bb 00 04 lwzur29,4(r27) 0.00 : c0097534: 2f 9d 00 00 cmpwi cr7,r29,0 0.00 : c0097538: 41 9e 00 1c beq-cr7,c0097554 perf_event_comm+0x44 0.00 : c009753c: 7f 00 00 a6 mfmsr r24 0.00 : c0097540: 7c 00 01 46 .long 0x7c000146 0.00 : c0097544: 80 1d 00 3c lwz r0,60(r29) 0.00 : c0097548: 2f 80 00 00 cmpwi cr7,r0,0 0.00 : c009754c: 40 9e 00 c4 bne-cr7,c0097610 perf_event_comm+0x100 0.00 : c0097550: 7f 00 01 06 .long 0x7f000106 100.00 : c0097554: 2f 9c 00 01 cmpwi cr7,r28,1 0.00 : c0097558: 3b 9c 00 01 addir28,r28,1 0.00 : c009755c: 40 be ff d4 bne-cr7,c0097530 perf_event_comm+0x20 0.00 : c0097560: 3d 20 c0 72 lis r9,-16270 0.00 : c0097564: 80 09 99 50 lwz r0,-26288(r9) 0.00 : c0097568: 2f 80 00 00 cmpwi cr7,r0,0 0.00 : c009756c: 41 be 00 88 beq+cr7,c00975f4 perf_event_comm+0xe4 0.00 : c0097570: 3b e1 00 08 addir31,r1,8 0.00 : c0097574: 38 00 00 00 li r0,0 0.00 : c0097578: 39 20 00 03 li r9,3 0.00 : c009757c: 38 9e 01 d8 addir4,r30,472 0.00 : c0097580: 38 a0 00 10 li r5,16 0.00 : c0097584: 7f e3 fb 78 mr r3,r31 0.00 : c0097588: 90 01 00 1c stw r0,28(r1) 0.00 : c009758c: 90 01 00 20 stw r0,32(r1) 0.00 : c0097590: 90 01 00 28 stw r0,40(r1) 0.00 : c0097594: 90 01 00 2c stw r0,44(r1) 0.00 : c0097598: 90 01 00 30 stw r0,48(r1) 0.00 : c009759c: 91 21 00 24 stw r9,36(r1) 0.00 : c00975a0: 90 01 00 08 stw r0,8(r1) 0.00 : c00975a4: 90 01 00 0c stw r0,12(r1) 0.00 : c00975a8: 90 01 00 10 stw r0,16(r1) 0.00 : c00975ac: 90 01 00 14 stw r0,20(r1) 0.00 : c00975b0: 93 c1 00 18 stw r30,24(r1) 0.00 : c00975b4: 48 1a 05 8d bl c0237b40 strlcpy 0.00 : c00975b8: 7f e3 fb 78 mr r3,r31 0.00 : c00975bc: 4b f8 0e 89 bl c0018444 strlen 0.00 : c00975c0: 3c 80 c0 09 lis r4,-16375 0.00 : c00975c4: 39 23 00 08 addir9,r3,8 0.00 : c00975c8: 93 e1 00 1c stw r31,28(r1) 0.00 : c00975cc: 55 29 00 38 rlwinm r9,r9,0,0,28 0.00 : c00975d0: 3c 60 c0 09 lis r3,-16375 0.00 : c00975d4: 38 09 00 10 addir0,r9,16 0.00 : c00975d8: 91 21 00 20 stw r9,32(r1) 0.00 : c00975dc: 38 63 fd 80 addir3,r3,-640 0.00 : c00975e0: 38 84 65 50 addir4,r4,25936 0.00 : c00975e4: 38 a1
Re: [PATCH] module: ppc64 module CRC relocation fix causes perf issues
Hi Scott, What specifically should I do to test it? Could you double check perf annotate works? I'm 99% sure it will but that is what was failing on ppc64. Anton ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] module: ppc64 module CRC relocation fix causes perf issues
On 07/15/2013 03:47:06 AM, Benjamin Herrenschmidt wrote: On Mon, 2013-07-15 at 14:04 +1000, Anton Blanchard wrote: Module CRCs are implemented as absolute symbols that get resolved by a linker script. We build an intermediate .o that contains an unresolved symbol for each CRC. genksysms parses this .o, calculates the CRCs and writes a linker script that resolves the symbols to the calc Scott, can somebody from FSL test that on 32-bit and Ack it ? Thanks ! Cheers, Ben. It boots for me on e500mc and I can insert modules. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] module: ppc64 module CRC relocation fix causes perf issues
On Tue, 2013-07-16 at 17:40 -0500, Scott Wood wrote: On 07/15/2013 03:47:06 AM, Benjamin Herrenschmidt wrote: On Mon, 2013-07-15 at 14:04 +1000, Anton Blanchard wrote: Module CRCs are implemented as absolute symbols that get resolved by a linker script. We build an intermediate .o that contains an unresolved symbol for each CRC. genksysms parses this .o, calculates the CRCs and writes a linker script that resolves the symbols to the calc Scott, can somebody from FSL test that on 32-bit and Ack it ? Thanks ! Cheers, Ben. It boots for me on e500mc and I can insert modules. But does perf work ? :-) Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] module: ppc64 module CRC relocation fix causes perf issues
On 07/16/2013 07:04:05 PM, Benjamin Herrenschmidt wrote: On Tue, 2013-07-16 at 17:40 -0500, Scott Wood wrote: On 07/15/2013 03:47:06 AM, Benjamin Herrenschmidt wrote: On Mon, 2013-07-15 at 14:04 +1000, Anton Blanchard wrote: Module CRCs are implemented as absolute symbols that get resolved by a linker script. We build an intermediate .o that contains an unresolved symbol for each CRC. genksysms parses this .o, calculates the CRCs and writes a linker script that resolves the symbols to the calc Scott, can somebody from FSL test that on 32-bit and Ack it ? Thanks ! Cheers, Ben. It boots for me on e500mc and I can insert modules. But does perf work ? :-) What specifically should I do to test it? -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] module: ppc64 module CRC relocation fix causes perf issues
On Mon, 2013-07-15 at 14:04 +1000, Anton Blanchard wrote:
Module CRCs are implemented as absolute symbols that get resolved by
a linker script. We build an intermediate .o that contains an
unresolved symbol for each CRC. genksysms parses this .o, calculates
the CRCs and writes a linker script that resolves the symbols to
the calc
Scott, can somebody from FSL test that on 32-bit and Ack it ?
Thanks !
Cheers,
Ben.
Unfortunately the ppc64 relocatable kernel sees these CRCs as symbols
that need relocating and relocates them at boot. Commit d4703aef
(module: handle ppc64 relocating kcrctabs when CONFIG_RELOCATABLE=y)
added a hook to reverse the bogus relocations. Part of this patch
created a symbol at 0x0:
# head -2 /proc/kallsyms
T reloc_start
c000 T .__start
This reloc_start symbol is causing lots of confusion to perf. It
thinks reloc_start is a massive function that stretches from 0x0 to
0xc000 and we get various cryptic errors out of perf,
including:
problem incrementing symbol count, skipping event
This patch removes the reloc_start linker script label and instead
defines it as PHYSICAL_START. We also need to wrap it with
CONFIG_PPC64 because the ppc32 kernel can set a non zero
PHYSICAL_START at compile time and we wouldn't want to subtract
it from the CRCs in that case.
Signed-off-by: Anton Blanchard an...@samba.org
Cc: sta...@kernel.org
---
This bug was originally reported on Fedora 19 (3.9.x), so I've marked
it for stable.
Index: b/arch/powerpc/include/asm/module.h
===
--- a/arch/powerpc/include/asm/module.h
+++ b/arch/powerpc/include/asm/module.h
@@ -82,10 +82,9 @@ struct exception_table_entry;
void sort_ex_table(struct exception_table_entry *start,
struct exception_table_entry *finish);
-#ifdef CONFIG_MODVERSIONS
+#if defined(CONFIG_MODVERSIONS) defined(CONFIG_PPC64)
#define ARCH_RELOCATES_KCRCTAB
-
-extern const unsigned long reloc_start[];
+#define reloc_start PHYSICAL_START
#endif
#endif /* __KERNEL__ */
#endif /* _ASM_POWERPC_MODULE_H */
Index: b/arch/powerpc/kernel/vmlinux.lds.S
===
--- a/arch/powerpc/kernel/vmlinux.lds.S
+++ b/arch/powerpc/kernel/vmlinux.lds.S
@@ -38,9 +38,6 @@ jiffies = jiffies_64 + 4;
#endif
SECTIONS
{
- . = 0;
- reloc_start = .;
-
. = KERNELBASE;
/*
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] module: ppc64 module CRC relocation fix causes perf issues
Module CRCs are implemented as absolute symbols that get resolved by
a linker script. We build an intermediate .o that contains an
unresolved symbol for each CRC. genksysms parses this .o, calculates
the CRCs and writes a linker script that resolves the symbols to
the calculated CRC.
Unfortunately the ppc64 relocatable kernel sees these CRCs as symbols
that need relocating and relocates them at boot. Commit d4703aef
(module: handle ppc64 relocating kcrctabs when CONFIG_RELOCATABLE=y)
added a hook to reverse the bogus relocations. Part of this patch
created a symbol at 0x0:
# head -2 /proc/kallsyms
T reloc_start
c000 T .__start
This reloc_start symbol is causing lots of confusion to perf. It
thinks reloc_start is a massive function that stretches from 0x0 to
0xc000 and we get various cryptic errors out of perf,
including:
problem incrementing symbol count, skipping event
This patch removes the reloc_start linker script label and instead
defines it as PHYSICAL_START. We also need to wrap it with
CONFIG_PPC64 because the ppc32 kernel can set a non zero
PHYSICAL_START at compile time and we wouldn't want to subtract
it from the CRCs in that case.
Signed-off-by: Anton Blanchard an...@samba.org
Cc: sta...@kernel.org
---
This bug was originally reported on Fedora 19 (3.9.x), so I've marked
it for stable.
Index: b/arch/powerpc/include/asm/module.h
===
--- a/arch/powerpc/include/asm/module.h
+++ b/arch/powerpc/include/asm/module.h
@@ -82,10 +82,9 @@ struct exception_table_entry;
void sort_ex_table(struct exception_table_entry *start,
struct exception_table_entry *finish);
-#ifdef CONFIG_MODVERSIONS
+#if defined(CONFIG_MODVERSIONS) defined(CONFIG_PPC64)
#define ARCH_RELOCATES_KCRCTAB
-
-extern const unsigned long reloc_start[];
+#define reloc_start PHYSICAL_START
#endif
#endif /* __KERNEL__ */
#endif /* _ASM_POWERPC_MODULE_H */
Index: b/arch/powerpc/kernel/vmlinux.lds.S
===
--- a/arch/powerpc/kernel/vmlinux.lds.S
+++ b/arch/powerpc/kernel/vmlinux.lds.S
@@ -38,9 +38,6 @@ jiffies = jiffies_64 + 4;
#endif
SECTIONS
{
- . = 0;
- reloc_start = .;
-
. = KERNELBASE;
/*
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] module: ppc64 module CRC relocation fix causes perf issues
Anton Blanchard an...@samba.org writes: Module CRCs are implemented as absolute symbols that get resolved by a linker script. We build an intermediate .o that contains an unresolved symbol for each CRC. genksysms parses this .o, calculates the CRCs and writes a linker script that resolves the symbols to the calculated CRC. Unfortunately the ppc64 relocatable kernel sees these CRCs as symbols that need relocating and relocates them at boot. Commit d4703aef (module: handle ppc64 relocating kcrctabs when CONFIG_RELOCATABLE=y) added a hook to reverse the bogus relocations. Part of this patch created a symbol at 0x0: # head -2 /proc/kallsyms T reloc_start c000 T .__start This reloc_start symbol is causing lots of confusion to perf. It thinks reloc_start is a massive function that stretches from 0x0 to 0xc000 and we get various cryptic errors out of perf, including: problem incrementing symbol count, skipping event This patch removes the reloc_start linker script label and instead defines it as PHYSICAL_START. We also need to wrap it with CONFIG_PPC64 because the ppc32 kernel can set a non zero PHYSICAL_START at compile time and we wouldn't want to subtract it from the CRCs in that case. Signed-off-by: Anton Blanchard an...@samba.org Cc: sta...@kernel.org Acked-by: Rusty Russell ru...@rustcorp.com.au Ben? Cheers, Rusty. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
