* [PATCH -mm] ftrace : Fix section mismatch warning.
@ 2008-10-13 5:50 Rakib Mullick
2008-10-20 16:27 ` Ingo Molnar
0 siblings, 1 reply; 10+ messages in thread
From: Rakib Mullick @ 2008-10-13 5:50 UTC (permalink / raw)
To: linux-kernel; +Cc: rostedt, Ingo Molnar, Andrew Morton
WARNING: arch/x86/kernel/built-in.o(.text+0x11b41): Section mismatch
in reference from the variable ftrace_test_p6nop to the function
.init.text:ftrace_dyn_arch_init()
The function ftrace_test_p6nop() references
the function __init ftrace_dyn_arch_init().
This is often because ftrace_test_p6nop lacks a __init
annotation or the annotation of ftrace_dyn_arch_init is wrong.
WARNING: arch/x86/kernel/built-in.o(.text+0x11b4b): Section mismatch
in reference from the variable ftrace_test_nop5 to the function
.init.text:ftrace_dyn_arch_init()
The function ftrace_test_nop5() references
the function __init ftrace_dyn_arch_init().
This is often because ftrace_test_nop5 lacks a __init
annotation or the annotation of ftrace_dyn_arch_init is wrong.
This patch fixes the above warnings. Introduced by
'linux-next.patch'(2.6.27-rc5-mm1-broken-out).
Thanks.
Signed-off-by: Md.Rakib H. Mullick(rakib.mullick@gmail.com)
--- linux-2.6-mm.orig/arch/x86/kernel/ftrace.c 2008-10-11
18:55:52.000000000 +0600
+++ linux-2.6-mm/arch/x86/kernel/ftrace.c 2008-10-12 18:59:38.000000000 +0600
@@ -105,7 +105,7 @@ notrace int ftrace_mcount_set(unsigned l
return 0;
}
-int __init ftrace_dyn_arch_init(void *data)
+int ftrace_dyn_arch_init(void *data)
{
extern const unsigned char ftrace_test_p6nop[];
extern const unsigned char ftrace_test_nop5[];
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH -mm] ftrace : Fix section mismatch warning. 2008-10-13 5:50 [PATCH -mm] ftrace : Fix section mismatch warning Rakib Mullick @ 2008-10-20 16:27 ` Ingo Molnar 2008-10-22 7:03 ` Ingo Molnar 0 siblings, 1 reply; 10+ messages in thread From: Ingo Molnar @ 2008-10-20 16:27 UTC (permalink / raw) To: Rakib Mullick; +Cc: linux-kernel, rostedt, Andrew Morton * Rakib Mullick <rakib.mullick@gmail.com> wrote: > WARNING: arch/x86/kernel/built-in.o(.text+0x11b41): Section mismatch > in reference from the variable ftrace_test_p6nop to the function > .init.text:ftrace_dyn_arch_init() > The function ftrace_test_p6nop() references > the function __init ftrace_dyn_arch_init(). > This is often because ftrace_test_p6nop lacks a __init > annotation or the annotation of ftrace_dyn_arch_init is wrong. > > WARNING: arch/x86/kernel/built-in.o(.text+0x11b4b): Section mismatch > in reference from the variable ftrace_test_nop5 to the function > .init.text:ftrace_dyn_arch_init() > The function ftrace_test_nop5() references > the function __init ftrace_dyn_arch_init(). > This is often because ftrace_test_nop5 lacks a __init > annotation or the annotation of ftrace_dyn_arch_init is wrong. > > This patch fixes the above warnings. Introduced by > 'linux-next.patch'(2.6.27-rc5-mm1-broken-out). > Thanks. > > Signed-off-by: Md.Rakib H. Mullick(rakib.mullick@gmail.com) applied to tip/tracing/urgent, thanks! Ingo ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH -mm] ftrace : Fix section mismatch warning. 2008-10-20 16:27 ` Ingo Molnar @ 2008-10-22 7:03 ` Ingo Molnar 2008-10-24 11:09 ` Rakib Mullick 0 siblings, 1 reply; 10+ messages in thread From: Ingo Molnar @ 2008-10-22 7:03 UTC (permalink / raw) To: Rakib Mullick; +Cc: linux-kernel, rostedt, Andrew Morton * Ingo Molnar <mingo@elte.hu> wrote: > > * Rakib Mullick <rakib.mullick@gmail.com> wrote: > > > WARNING: arch/x86/kernel/built-in.o(.text+0x11b41): Section mismatch > > in reference from the variable ftrace_test_p6nop to the function > > .init.text:ftrace_dyn_arch_init() > > The function ftrace_test_p6nop() references > > the function __init ftrace_dyn_arch_init(). > > This is often because ftrace_test_p6nop lacks a __init > > annotation or the annotation of ftrace_dyn_arch_init is wrong. > > > > WARNING: arch/x86/kernel/built-in.o(.text+0x11b4b): Section mismatch > > in reference from the variable ftrace_test_nop5 to the function > > .init.text:ftrace_dyn_arch_init() > > The function ftrace_test_nop5() references > > the function __init ftrace_dyn_arch_init(). > > This is often because ftrace_test_nop5 lacks a __init > > annotation or the annotation of ftrace_dyn_arch_init is wrong. > > > > This patch fixes the above warnings. Introduced by > > 'linux-next.patch'(2.6.27-rc5-mm1-broken-out). > > Thanks. > > > > Signed-off-by: Md.Rakib H. Mullick(rakib.mullick@gmail.com) > > applied to tip/tracing/urgent, thanks! actually, this patch is wrong: the problem highlighted in that warning is that ftrace_test_p6nop is non-__init while ftrace_dyn_arch_init() is __init. So the proper solution is to mark ftrace_test_p6nop init as well. (it's an assembly label so thus slightly more complex than usual, but doable.) Ingo ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH -mm] ftrace : Fix section mismatch warning. 2008-10-22 7:03 ` Ingo Molnar @ 2008-10-24 11:09 ` Rakib Mullick 2008-10-24 11:50 ` Ingo Molnar 0 siblings, 1 reply; 10+ messages in thread From: Rakib Mullick @ 2008-10-24 11:09 UTC (permalink / raw) To: Ingo Molnar; +Cc: linux-kernel, rostedt, Andrew Morton On 10/22/08, Ingo Molnar <mingo@elte.hu> wrote: > > * Ingo Molnar <mingo@elte.hu> wrote: > >> >> * Rakib Mullick <rakib.mullick@gmail.com> wrote: >> >> > WARNING: arch/x86/kernel/built-in.o(.text+0x11b41): Section mismatch >> > in reference from the variable ftrace_test_p6nop to the function >> > .init.text:ftrace_dyn_arch_init() >> > The function ftrace_test_p6nop() references >> > the function __init ftrace_dyn_arch_init(). >> > This is often because ftrace_test_p6nop lacks a __init >> > annotation or the annotation of ftrace_dyn_arch_init is wrong. >> > >> > WARNING: arch/x86/kernel/built-in.o(.text+0x11b4b): Section mismatch >> > in reference from the variable ftrace_test_nop5 to the function >> > .init.text:ftrace_dyn_arch_init() >> > The function ftrace_test_nop5() references >> > the function __init ftrace_dyn_arch_init(). >> > This is often because ftrace_test_nop5 lacks a __init >> > annotation or the annotation of ftrace_dyn_arch_init is wrong. >> > >> > This patch fixes the above warnings. Introduced by >> > 'linux-next.patch'(2.6.27-rc5-mm1-broken-out). >> > Thanks. >> > >> > Signed-off-by: Md.Rakib H. Mullick(rakib.mullick@gmail.com) >> >> applied to tip/tracing/urgent, thanks! > > actually, this patch is wrong: the problem highlighted in that warning > is that ftrace_test_p6nop is non-__init while ftrace_dyn_arch_init() is > __init. So the proper solution is to mark ftrace_test_p6nop init as > well. (it's an assembly label so thus slightly more complex than usual, > but doable.) Does the following solves the problem? Thanks. --- linux-2.6-orig/arch/x86/kernel/ftrace.c 2008-10-24 16:35:48.000000000 +0600 +++ linux-2.6/arch/x86/kernel/ftrace.c 2008-10-24 16:40:09.000000000 +0600 @@ -129,7 +129,7 @@ int __init ftrace_dyn_arch_init(void *da asm volatile ( "jmp ftrace_test_jmp\n" /* This code needs to stay around */ - ".section .text, \"ax\"\n" + ".section .text.init, \"ax\"\n" "ftrace_test_jmp:" "jmp ftrace_test_p6nop\n" "nop\n" > > Ingo > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH -mm] ftrace : Fix section mismatch warning. 2008-10-24 11:09 ` Rakib Mullick @ 2008-10-24 11:50 ` Ingo Molnar 2008-10-24 12:19 ` Steven Rostedt 2008-10-24 13:12 ` [PATCH] ftrace: use a real variable for ftrace_nop in x86 Steven Rostedt 0 siblings, 2 replies; 10+ messages in thread From: Ingo Molnar @ 2008-10-24 11:50 UTC (permalink / raw) To: Rakib Mullick; +Cc: linux-kernel, rostedt, Andrew Morton * Rakib Mullick <rakib.mullick@gmail.com> wrote: > Does the following solves the problem? > Thanks. > > --- linux-2.6-orig/arch/x86/kernel/ftrace.c 2008-10-24 16:35:48.000000000 +0600 > +++ linux-2.6/arch/x86/kernel/ftrace.c 2008-10-24 16:40:09.000000000 +0600 > @@ -129,7 +129,7 @@ int __init ftrace_dyn_arch_init(void *da > asm volatile ( > "jmp ftrace_test_jmp\n" > /* This code needs to stay around */ > - ".section .text, \"ax\"\n" > + ".section .text.init, \"ax\"\n" how about the "This code needs to stay around" comment? Ingo ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH -mm] ftrace : Fix section mismatch warning. 2008-10-24 11:50 ` Ingo Molnar @ 2008-10-24 12:19 ` Steven Rostedt 2008-10-24 12:22 ` Steven Rostedt 2008-10-24 13:12 ` [PATCH] ftrace: use a real variable for ftrace_nop in x86 Steven Rostedt 1 sibling, 1 reply; 10+ messages in thread From: Steven Rostedt @ 2008-10-24 12:19 UTC (permalink / raw) To: Ingo Molnar; +Cc: Rakib Mullick, linux-kernel, Andrew Morton On Fri, 24 Oct 2008, Ingo Molnar wrote: > > * Rakib Mullick <rakib.mullick@gmail.com> wrote: > > > Does the following solves the problem? > > Thanks. > > > > --- linux-2.6-orig/arch/x86/kernel/ftrace.c 2008-10-24 16:35:48.000000000 +0600 > > +++ linux-2.6/arch/x86/kernel/ftrace.c 2008-10-24 16:40:09.000000000 +0600 > > @@ -129,7 +129,7 @@ int __init ftrace_dyn_arch_init(void *da > > asm volatile ( > > "jmp ftrace_test_jmp\n" > > /* This code needs to stay around */ > > - ".section .text, \"ax\"\n" > > + ".section .text.init, \"ax\"\n" > > how about the "This code needs to stay around" comment? Yes that is correct. That part can not be converted into .init. The code is used later at run time when the tracer is enabled and we patch the code back to a call to mcount. We use this code as to know what we should expect should be at the nop locations. But that code is used later as data and not exectude, which means that we do not need to worry about it calling back into the .init section. If you only want to get rid of the section mismatch warning. Just remove the __init from the "ftrace_dyn_arch_init" function. But then we are just wasting those bytes that will never be used again. The real solution should probably be this: (Compiled tested only) Signed-off-by: Steven Rostedt <srostedt@redhat.com> --- arch/x86/kernel/ftrace.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) Index: linux-tip.git/arch/x86/kernel/ftrace.c =================================================================== --- linux-tip.git.orig/arch/x86/kernel/ftrace.c 2008-10-23 19:23:28.000000000 -0400 +++ linux-tip.git/arch/x86/kernel/ftrace.c 2008-10-24 08:11:57.000000000 -0400 @@ -22,7 +22,7 @@ /* Long is fine, even if it is only 4 bytes ;-) */ -static unsigned long *ftrace_nop; +static unsigned long ftrace_nop[MCOUNT_INSN_SIZE]; union ftrace_code_union { char code[MCOUNT_INSN_SIZE]; @@ -126,8 +126,7 @@ int __init ftrace_dyn_arch_init(void *da */ asm volatile ( "jmp ftrace_test_jmp\n" - /* This code needs to stay around */ - ".section .text, \"ax\"\n" + ".section .text.init, \"ax\"\n" "ftrace_test_jmp:" "jmp ftrace_test_p6nop\n" "nop\n" @@ -154,15 +153,15 @@ int __init ftrace_dyn_arch_init(void *da switch (faulted) { case 0: pr_info("ftrace: converting mcount calls to 0f 1f 44 00 00\n"); - ftrace_nop = (unsigned long *)ftrace_test_p6nop; + memcpy(ftrace_nop, ftrace_test_p6nop, MCOUNT_INSN_SIZE); break; case 1: pr_info("ftrace: converting mcount calls to 66 66 66 66 90\n"); - ftrace_nop = (unsigned long *)ftrace_test_nop5; + memcpy(ftrace_nop, ftrace_test_nop5, MCOUNT_INSN_SIZE); break; case 2: pr_info("ftrace: converting mcount calls to jmp . + 5\n"); - ftrace_nop = (unsigned long *)ftrace_test_jmp; + memcpy(ftrace_nop, ftrace_test_jmp, MCOUNT_INSN_SIZE); break; } ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH -mm] ftrace : Fix section mismatch warning. 2008-10-24 12:19 ` Steven Rostedt @ 2008-10-24 12:22 ` Steven Rostedt 0 siblings, 0 replies; 10+ messages in thread From: Steven Rostedt @ 2008-10-24 12:22 UTC (permalink / raw) To: Ingo Molnar; +Cc: Rakib Mullick, linux-kernel, Andrew Morton On Fri, 24 Oct 2008, Steven Rostedt wrote: > > The real solution should probably be this: > > (Compiled tested only) > > Signed-off-by: Steven Rostedt <srostedt@redhat.com> > --- > arch/x86/kernel/ftrace.c | 11 +++++------ > 1 file changed, 5 insertions(+), 6 deletions(-) > > Index: linux-tip.git/arch/x86/kernel/ftrace.c > =================================================================== > --- linux-tip.git.orig/arch/x86/kernel/ftrace.c 2008-10-23 19:23:28.000000000 -0400 > +++ linux-tip.git/arch/x86/kernel/ftrace.c 2008-10-24 08:11:57.000000000 -0400 > @@ -22,7 +22,7 @@ > > > /* Long is fine, even if it is only 4 bytes ;-) */ > -static unsigned long *ftrace_nop; > +static unsigned long ftrace_nop[MCOUNT_INSN_SIZE]; I need to convert that to unsigned char ftrace_nop[]. I'll run some more tests, and produce a real patch later. -- Steve > > union ftrace_code_union { > char code[MCOUNT_INSN_SIZE]; > @@ -126,8 +126,7 @@ int __init ftrace_dyn_arch_init(void *da > */ > asm volatile ( > "jmp ftrace_test_jmp\n" > - /* This code needs to stay around */ > - ".section .text, \"ax\"\n" > + ".section .text.init, \"ax\"\n" > "ftrace_test_jmp:" > "jmp ftrace_test_p6nop\n" > "nop\n" > @@ -154,15 +153,15 @@ int __init ftrace_dyn_arch_init(void *da > switch (faulted) { > case 0: > pr_info("ftrace: converting mcount calls to 0f 1f 44 00 00\n"); > - ftrace_nop = (unsigned long *)ftrace_test_p6nop; > + memcpy(ftrace_nop, ftrace_test_p6nop, MCOUNT_INSN_SIZE); > break; > case 1: > pr_info("ftrace: converting mcount calls to 66 66 66 66 90\n"); > - ftrace_nop = (unsigned long *)ftrace_test_nop5; > + memcpy(ftrace_nop, ftrace_test_nop5, MCOUNT_INSN_SIZE); > break; > case 2: > pr_info("ftrace: converting mcount calls to jmp . + 5\n"); > - ftrace_nop = (unsigned long *)ftrace_test_jmp; > + memcpy(ftrace_nop, ftrace_test_jmp, MCOUNT_INSN_SIZE); > break; > } > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] ftrace: use a real variable for ftrace_nop in x86 2008-10-24 11:50 ` Ingo Molnar 2008-10-24 12:19 ` Steven Rostedt @ 2008-10-24 13:12 ` Steven Rostedt 2008-10-24 13:17 ` Steven Rostedt 1 sibling, 1 reply; 10+ messages in thread From: Steven Rostedt @ 2008-10-24 13:12 UTC (permalink / raw) To: Ingo Molnar; +Cc: Rakib Mullick, linux-kernel, Andrew Morton The dynamic ftrace determines which nop is safe to use at start up. When it finds a safe nop for patching, it sets a pointer called ftrace_nop to point to the code. All call sites are then patched to this nop. Later, when tracing is turned on, this ftrace_nop variable is again used to compare the location to make sure it is a nop before we update it to a mcount call. If this fails just once, a warning is outputed and ftrace is disabled. Rakib Mullick noted that the code that sets up the nop is a .init section where as the nop itself is in the .text section. This is needed because the nop is used later on after boot up. The problem is that the test of the nop jumps back to the setup code and causes a "section mismatch" warning. Rabik first recommended to convert the nop to .init.text, but as stated above, this would fail since that text is used later. The real solution is to extend Rabik's patch, and to make the ftrace_nop into an array, and just save the code from the assembly to this array. Now the section can stay as an init section, and we have a nop to use later on. Signed-off-by: Steven Rostedt <srostedt@redhat.com> --- arch/x86/kernel/ftrace.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) Index: linux-tip.git/arch/x86/kernel/ftrace.c =================================================================== --- linux-tip.git.orig/arch/x86/kernel/ftrace.c 2008-10-23 19:23:28.000000000 -0400 +++ linux-tip.git/arch/x86/kernel/ftrace.c 2008-10-24 08:51:59.000000000 -0400 @@ -21,8 +21,7 @@ #include <asm/nops.h> -/* Long is fine, even if it is only 4 bytes ;-) */ -static unsigned long *ftrace_nop; +static unsigned char ftrace_nop[MCOUNT_INSN_SIZE]; union ftrace_code_union { char code[MCOUNT_INSN_SIZE]; @@ -40,7 +39,7 @@ static int ftrace_calc_offset(long ip, l unsigned char *ftrace_nop_replace(void) { - return (char *)ftrace_nop; + return ftrace_nop; } unsigned char *ftrace_call_replace(unsigned long ip, unsigned long addr) @@ -125,9 +124,6 @@ int __init ftrace_dyn_arch_init(void *da * TODO: check the cpuid to determine the best nop. */ asm volatile ( - "jmp ftrace_test_jmp\n" - /* This code needs to stay around */ - ".section .text, \"ax\"\n" "ftrace_test_jmp:" "jmp ftrace_test_p6nop\n" "nop\n" @@ -138,8 +134,6 @@ int __init ftrace_dyn_arch_init(void *da "jmp 1f\n" "ftrace_test_nop5:" ".byte 0x66,0x66,0x66,0x66,0x90\n" - "jmp 1f\n" - ".previous\n" "1:" ".section .fixup, \"ax\"\n" "2: movl $1, %0\n" @@ -154,15 +148,15 @@ int __init ftrace_dyn_arch_init(void *da switch (faulted) { case 0: pr_info("ftrace: converting mcount calls to 0f 1f 44 00 00\n"); - ftrace_nop = (unsigned long *)ftrace_test_p6nop; + memcpy(ftrace_nop, ftrace_test_p6nop, MCOUNT_INSN_SIZE); break; case 1: pr_info("ftrace: converting mcount calls to 66 66 66 66 90\n"); - ftrace_nop = (unsigned long *)ftrace_test_nop5; + memcpy(ftrace_nop, ftrace_test_nop5, MCOUNT_INSN_SIZE); break; case 2: pr_info("ftrace: converting mcount calls to jmp . + 5\n"); - ftrace_nop = (unsigned long *)ftrace_test_jmp; + memcpy(ftrace_nop, ftrace_test_jmp, MCOUNT_INSN_SIZE); break; } ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ftrace: use a real variable for ftrace_nop in x86 2008-10-24 13:12 ` [PATCH] ftrace: use a real variable for ftrace_nop in x86 Steven Rostedt @ 2008-10-24 13:17 ` Steven Rostedt 2008-10-27 15:52 ` Ingo Molnar 0 siblings, 1 reply; 10+ messages in thread From: Steven Rostedt @ 2008-10-24 13:17 UTC (permalink / raw) To: Ingo Molnar; +Cc: Rakib Mullick, linux-kernel, Andrew Morton On Fri, 24 Oct 2008, Steven Rostedt wrote: > > The dynamic ftrace determines which nop is safe to use at start up. > When it finds a safe nop for patching, it sets a pointer called ftrace_nop > to point to the code. All call sites are then patched to this nop. > > Later, when tracing is turned on, this ftrace_nop variable is again used > to compare the location to make sure it is a nop before we update it to > a mcount call. If this fails just once, a warning is outputed and ftrace > is disabled. > > Rakib Mullick noted that the code that sets up the nop is a .init section > where as the nop itself is in the .text section. This is needed because > the nop is used later on after boot up. The problem is that the test of the > nop jumps back to the setup code and causes a "section mismatch" warning. > > Rabik first recommended to convert the nop to .init.text, but as stated > above, this would fail since that text is used later. > > The real solution is to extend Rabik's patch, and to make the ftrace_nop > into an array, and just save the code from the assembly to this array. Rereading what I sent, I see I misspelled Rakib's name twice. Ingo, could you fix the changelog here before commiting it. Thanks, -- Steve ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ftrace: use a real variable for ftrace_nop in x86 2008-10-24 13:17 ` Steven Rostedt @ 2008-10-27 15:52 ` Ingo Molnar 0 siblings, 0 replies; 10+ messages in thread From: Ingo Molnar @ 2008-10-27 15:52 UTC (permalink / raw) To: Steven Rostedt; +Cc: Rakib Mullick, linux-kernel, Andrew Morton * Steven Rostedt <rostedt@goodmis.org> wrote: > > On Fri, 24 Oct 2008, Steven Rostedt wrote: > > > > > The dynamic ftrace determines which nop is safe to use at start up. > > When it finds a safe nop for patching, it sets a pointer called ftrace_nop > > to point to the code. All call sites are then patched to this nop. > > > > Later, when tracing is turned on, this ftrace_nop variable is again used > > to compare the location to make sure it is a nop before we update it to > > a mcount call. If this fails just once, a warning is outputed and ftrace > > is disabled. > > > > Rakib Mullick noted that the code that sets up the nop is a .init section > > where as the nop itself is in the .text section. This is needed because > > the nop is used later on after boot up. The problem is that the test of the > > nop jumps back to the setup code and causes a "section mismatch" warning. > > > > Rabik first recommended to convert the nop to .init.text, but as stated > > above, this would fail since that text is used later. > > > > The real solution is to extend Rabik's patch, and to make the ftrace_nop > > into an array, and just save the code from the assembly to this array. > > Rereading what I sent, I see I misspelled Rakib's name twice. > > Ingo, could you fix the changelog here before commiting it. done - find the tip/tracing/urgent commit below. I also added an Impact: line. Ingo ----------------> >From 8115f3f0c939c5db0fe3c6c6c58911fd3a205b1e Mon Sep 17 00:00:00 2001 From: Steven Rostedt <rostedt@goodmis.org> Date: Fri, 24 Oct 2008 09:12:17 -0400 Subject: [PATCH] ftrace: use a real variable for ftrace_nop in x86 Impact: avoid section mismatch warning, clean up The dynamic ftrace determines which nop is safe to use at start up. When it finds a safe nop for patching, it sets a pointer called ftrace_nop to point to the code. All call sites are then patched to this nop. Later, when tracing is turned on, this ftrace_nop variable is again used to compare the location to make sure it is a nop before we update it to an mcount call. If this fails just once, a warning is printed and ftrace is disabled. Rakib Mullick noted that the code that sets up the nop is a .init section where as the nop itself is in the .text section. This is needed because the nop is used later on after boot up. The problem is that the test of the nop jumps back to the setup code and causes a "section mismatch" warning. Rakib first recommended to convert the nop to .init.text, but as stated above, this would fail since that text is used later. The real solution is to extend Rabik's patch, and to make the ftrace_nop into an array, and just save the code from the assembly to this array. Now the section can stay as an init section, and we have a nop to use later on. Reported-by: Rakib Mullick <rakib.mullick@gmail.com> Signed-off-by: Steven Rostedt <srostedt@redhat.com> Signed-off-by: Ingo Molnar <mingo@elte.hu> --- arch/x86/kernel/ftrace.c | 16 +++++----------- 1 files changed, 5 insertions(+), 11 deletions(-) diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c index b1e5e22..50ea0ac 100644 --- a/arch/x86/kernel/ftrace.c +++ b/arch/x86/kernel/ftrace.c @@ -21,8 +21,7 @@ #include <asm/nops.h> -/* Long is fine, even if it is only 4 bytes ;-) */ -static unsigned long *ftrace_nop; +static unsigned char ftrace_nop[MCOUNT_INSN_SIZE]; union ftrace_code_union { char code[MCOUNT_INSN_SIZE]; @@ -40,7 +39,7 @@ static int ftrace_calc_offset(long ip, long addr) unsigned char *ftrace_nop_replace(void) { - return (char *)ftrace_nop; + return ftrace_nop; } unsigned char *ftrace_call_replace(unsigned long ip, unsigned long addr) @@ -125,9 +124,6 @@ int __init ftrace_dyn_arch_init(void *data) * TODO: check the cpuid to determine the best nop. */ asm volatile ( - "jmp ftrace_test_jmp\n" - /* This code needs to stay around */ - ".section .text, \"ax\"\n" "ftrace_test_jmp:" "jmp ftrace_test_p6nop\n" "nop\n" @@ -138,8 +134,6 @@ int __init ftrace_dyn_arch_init(void *data) "jmp 1f\n" "ftrace_test_nop5:" ".byte 0x66,0x66,0x66,0x66,0x90\n" - "jmp 1f\n" - ".previous\n" "1:" ".section .fixup, \"ax\"\n" "2: movl $1, %0\n" @@ -154,15 +148,15 @@ int __init ftrace_dyn_arch_init(void *data) switch (faulted) { case 0: pr_info("ftrace: converting mcount calls to 0f 1f 44 00 00\n"); - ftrace_nop = (unsigned long *)ftrace_test_p6nop; + memcpy(ftrace_nop, ftrace_test_p6nop, MCOUNT_INSN_SIZE); break; case 1: pr_info("ftrace: converting mcount calls to 66 66 66 66 90\n"); - ftrace_nop = (unsigned long *)ftrace_test_nop5; + memcpy(ftrace_nop, ftrace_test_nop5, MCOUNT_INSN_SIZE); break; case 2: pr_info("ftrace: converting mcount calls to jmp . + 5\n"); - ftrace_nop = (unsigned long *)ftrace_test_jmp; + memcpy(ftrace_nop, ftrace_test_jmp, MCOUNT_INSN_SIZE); break; } ^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2008-10-27 15:53 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2008-10-13 5:50 [PATCH -mm] ftrace : Fix section mismatch warning Rakib Mullick 2008-10-20 16:27 ` Ingo Molnar 2008-10-22 7:03 ` Ingo Molnar 2008-10-24 11:09 ` Rakib Mullick 2008-10-24 11:50 ` Ingo Molnar 2008-10-24 12:19 ` Steven Rostedt 2008-10-24 12:22 ` Steven Rostedt 2008-10-24 13:12 ` [PATCH] ftrace: use a real variable for ftrace_nop in x86 Steven Rostedt 2008-10-24 13:17 ` Steven Rostedt 2008-10-27 15:52 ` Ingo Molnar
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).