ftrace: to kill a daemon (small updates)
Subsystems (5)
Steven Rostedt
maintainerv2 cover ftrace: to kill a daemon (small updates)
[
Changes since v1:
regex fix in x86_64 recordmcount.pl. Now it can handle all
mcount+0x... mcount-0x... and mcount, where as the original only handled mcount+0x...
Made mcount on start-up to simply return. The current mcount
is set up to be replaced with a call to ftrace_record_ip. This is no longer necessary.
Note: This patch series is focusing on how calls to mcount in
the kernel are converted to nops. It does not address what kind of nop is used. That is a different topic, and should be in a different patch series.
Note 2: I have found that the changes here are more stable than
the current daemon method, and these patches should be used. It also solves the resume from suspend to ram bug that was reported:
http://lkml.org/lkml/2008/8/12/234
and
http://lkml.org/lkml/2008/8/12/451
Note 3: I have already ported this to PowerPC64, but I am waiting
for this to be accepted first before submitting those changes.
]
One of the things that bothered me about the latest ftrace code was this annoying daemon that would wake up once a second to see if it had work to do. If it did not, it would go to sleep, otherwise it would do its work and then go to sleep.
You see, the reason for this is that for ftrace to maintain performance when configured in but disabled, it would need to change all the locations that called "mcount" (enabled with the gcc -pg option) into nops. The "-pg" option in gcc sets up a function profiler to call this function called "mcount". If you simply have "mcount" return, it will still add 15 to 18% overhead in performance. Changing all the calls to nops moved the overhead into noise.
To get rid of this, I had the mcount code record the location that called it. Later, the "ftraced" daemon would wake up and look to see if any new functions were recorded. If so, it would call kstop_machine and convert the calls to "nops". We needed kstop_machine because bad things happen on SMP if you modify code that happens to be in the instruction cache of another CPU.
This "ftraced" kernel thread would be a happy little worker, but it caused some pains. One, is that it woke up once a second, and Ted Tso got mad at me because it would show up on PowerTop. I could easily make the default 5 seconds, and even have it runtime configurable, with a trivial patch. I have not got around to doing that yet.
The other annoying thing, and this one bothers me the most, is that we can not have this enabled on a production -rt kernel. The latency caused by the kstop_machine when doing work is lost in the noise on a non-rt kernel, but it can be up to 800 microseconds, and that would kill the -rt kernel. The reason this bothered me the most, is that -rt is where it came from, and ftraced was not treating its motherland very well.
Along came Gregory Haskins, who was bickering about having ftrace enabled on a production -rt kernel. I told him the reasons that this would be bad and then he started thinking out loud, and suggesting wild ideas, like patching gcc!
Since I have recently seen "The Dark Knight", Gregory's comments put me into an "evil" mood. I then thought of the idea about using the relocation entries of the mcount call sites, in a prelinked object file, and create a separate section with a list of these sites. On boot up, record them and change them into nops.
That's it! No kstop_machine for turning them into nops. We would only need stop_machine to enable or disable tracing, but a user not tracing will not have to deal with this annoying "ftraced" kernel thread waking up every second or ever running kstop_machine.
What's more, this means we can enable it on a production -rt kernel!
Now, this was no easy task. We needed to add a section to every object file with a list of pointers to the call sites to mcount. The idea I came up with was to make a tmp.s file for every object just after it is compiled. This tmp.s would then be compiled and relinked into the original object. The tmp.s file would have something like:
.section __mcount_loc,"a",@progbits
.quad location_of_mcount1
.quad location_of_mcount2
(etc)
By running objdump on the object file we can find the offsets into the sections that the functions are called.
For example, looking at hrtimer.o:
Disassembly of section .text:
0000000000000000 <hrtimer_init_sleeper>:
0: 55 push %rbp
1: 48 89 e5 mov %rsp,%rbp
4: e8 00 00 00 00 callq 9 <hrtimer_init_sleeper+0x9>
5: R_X86_64_PC32 mcount+0xfffffffffffffffc[...]
the '5' in the '5: R_X86_64_PC32' is the offset that the mcount relocation is to be done for the call site. This offset is from the .text section, and not necessarily, from the function. If we look further we see:
000000000000001e <ktime_add_safe>:
1e: 55 push %rbp
1f: 48 89 e5 mov %rsp,%rbp
22: e8 00 00 00 00 callq 27 <ktime_add_safe+0x9>
23: R_X86_64_PC32 mcount+0xfffffffffffffffcThis mcount call site is 0x23 from the .text section, and obviously not from the ktime_add_safe.
If we make a tmp.s that has the following:
.section __mcount_loc,"a",@progbits
.quad hrtimer_init_sleeper + 0x5
.quad hrtimer_init_sleeper + 0x23
We have a section with the locations of these two call sites. After the final linking, they will point to the actual address used.
All that would need to be done is:
gcc -c tmp.s -o tmp.o
ld -r tmp.o hrtimer.o -o tmp_hrtime.o
mv tmp_hrtimer.o hrtimer.o
Easy as that! Not quite. What happens if that first function in the section is a static function? That is, the symbol for the function is local to the object. If for some reason hrtimer_init_sleeper is static, the tmp_hrtimer.o would have two symbols for hrtimer_init_sleeper. One local and one global.
But we can be even more evil with this idea. We can do crazy things with objcopy to solve it for us.
objcopy --globalize-symbol hrtimer_init_sleeper hrtimer.o tmp_hrtimer.o
Now the hrtimer_init_sleeper would be global for linking.
ld -r tmp_hrtimer.o tmp.o -o tmp2_hrtimer.o
Now the tmp.o could use the same global hrtimer_init_sleeper symbol. But we have tmp2_hritmer.o that has the tmp.o and tmp_hrtimer.o symbols, but we cant just blindly convert local symbols to globals.
The solution is simply put it back to local.
objcopy --localize-symbol hrtimer_init_sleeper tmp2_hrtimer.o hrtimer.o
Now our hrtimer.o file has our __mcount_loc section and the reference to hrtimer_init_sleeper will be resolved.
This is a bit complex to do in shell scripting and Makefiles, so I wrote a well documented recordmcount.pl perl script, that will do the above all in one place.
With this new update, we can work to kill that kernel thread "ftraced"!
This patch set ports to x86_64 and i386, the other archs will still use the daemon until they are converted over.
I tested this on both x86_64 and i386 with and without CONFIG_RELOCATE set.
Steven Rostedt
maintainerPATCH v2 1/6 ftrace: create __mcount_loc section
[
Note: after this patch is applied, you need to run:
chmod +x scripts/recordmcount.pl
]
This patch creates a section in the kernel called "__mcount_loc". This will hold a list of pointers to the mcount relocation for each call site of mcount.
For example:
objdump -dr init/main.o
[...]
Disassembly of section .text:
0000000000000000 <do_one_initcall>:
0: 55 push %rbp
[...]000000000000017b <init_post>:
17b: 55 push %rbp
17c: 48 89 e5 mov %rsp,%rbp
17f: 53 push %rbx
180: 48 83 ec 08 sub $0x8,%rsp
184: e8 00 00 00 00 callq 189 <init_post+0xe>
185: R_X86_64_PC32 mcount+0xfffffffffffffffc[...]
We will add a section to point to each function call.
.section __mcount_loc,"a",@progbits
[...]
.quad .text + 0x185
[...]
The offset to of the mcount call site in init_post is an offset from the start of the section, and not the start of the function init_post. The mcount relocation is at the call site 0x185 from the start of the .text section.
.text + 0x185 == init_post + 0xa
We need a way to add this __mcount_loc section in a way that we do not lose the relocations after final link. The .text section here will be attached to all other .text sections after final link and the offsets will be meaningless. We need to keep track of where these .text sections are.
To do this, we use the start of the first function in the section. do_one_initcall. We can make a tmp.s file with this function as a reference to the start of the .text section.
.section __mcount_loc,"a",@progbits
[...]
.quad do_one_initcall + 0x185
[...]
Then we can compile the tmp.s into a tmp.o
gcc -c tmp.s -o tmp.o
And link it into back into main.o.
ld -r main.o tmp.o -o tmp_main.o
mv tmp_main.o main.o
But we have a problem. What happens if the first function in a section is not exported, and is a static function. The linker will not let the tmp.o use it. This case exists in main.o as well.
Disassembly of section .init.text:
0000000000000000 <set_reset_devices>:
0: 55 push %rbp
1: 48 89 e5 mov %rsp,%rbp
4: e8 00 00 00 00 callq 9 <set_reset_devices+0x9>
5: R_X86_64_PC32 mcount+0xfffffffffffffffcThe first function in .init.text is a static function.
# nm init/main.o | grep set_reset_devices00000000000000a8 t __setup_set_reset_devices000000000000105f t __setup_str_set_reset_devices
0000000000000000 t set_reset_devices
The lowercase 't' means that set_reset_devices is local and is not exported. If we simply try to link the tmp.o with the set_reset_devices we end up with two symbols: one local and one global.
# cat tmp.s .section __mcount_loc,"a",@progbits .quad set_reset_devices + 0x10
# gcc -c tmp.s -o tmp.o
# ld -r main.o tmp.o -o tmp_main.o
# nm tmp_main.o | grep set_reset_devices
00000000000000a8 t __setup_set_reset_devices
000000000000105f t __setup_str_set_reset_devices
0000000000000000 t set_reset_devices
U set_reset_devicesWe still have an undefined reference to set_reset_devices, and if we try to compile the kernel, we will end up with an undefined reference to set_reset_devices, or even worst, it could be exported someplace else, and then we will have a reference to the wrong location.
To handle this case, we make an intermediate step using objcopy. We convert set_reset_devices into a global exported symbol before linking it with tmp.o and set it back afterwards.
# objcopy --globalize-symbol set_reset_devices main.o tmp_main.o # nm tmp_main.o | grep set_reset_devices 00000000000000a8 t __setup_set_reset_devices 000000000000105f t __setup_str_set_reset_devices 0000000000000000 T set_reset_devices
# ld -r tmp_main.o tmp.o -o tmp2_main.o # nm tmp2_main.o | grep set_reset_devices 00000000000000a8 t __setup_set_reset_devices 000000000000105f t __setup_str_set_reset_devices 0000000000000000 T set_reset_devices
# objcopy --localize-symbol set_reset_devices tmp2_main.o main.o # nm main.o | grep set_reset_devices 00000000000000a8 t __setup_set_reset_devices 000000000000105f t __setup_str_set_reset_devices 0000000000000000 t set_reset_devices
Now we have a section in main.o called __mcount_loc that we can place somewhere in the kernel using vmlinux.ld.S and access it to convert all these locations that call mcount into nops before starting SMP and thus, eliminating the need to do this with kstop_machine.
Note, A well documented perl script (scripts/recordmcount.pl) is used to do all this in one location.
--- include/asm-generic/vmlinux.lds.h | 8 + kernel/trace/Kconfig | 8 + scripts/Makefile.build | 6 scripts/recordmcount.pl | 280 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 302 insertions(+)
include/asm-generic/vmlinux.lds.h +8 -0
@@ -37,6 +37,13 @@#define MEM_DISCARD (sec) *(.mem##sec)#endif +#ifdef CONFIG_FTRACE_MCOUNT_RECORD +#define MCOUNT_REC () VMLINUX_SYMBOL(__start_mcount_loc) = .; \ + *(__mcount_loc) \ + VMLINUX_SYMBOL(__stop_mcount_loc) = .; +#else +#define MCOUNT_REC () +#endif /* .data section */ #define DATA_DATA \
@@ -192,6 +199,7 @@/* __*init sections */ \ __init_rodata : AT(ADDR(__init_rodata) - LOAD_OFFSET) { \ *(.ref.rodata) \ + MCOUNT_REC() \ DEV_KEEP(init.rodata) \ DEV_KEEP(exit.rodata) \CPU_KEEP (init.rodata) \
scripts/Makefile.build +6 -0
@@ -198,10 +198,16 @@ cmd_modversions = \ fi; endif +ifdef CONFIG_FTRACE_MCOUNT_RECORD +cmd_record_mcount = scripts/recordmcount.pl "$(ARCH)" \ + "$(OBJDUMP)" "$(OBJCOPY)" "$(CC)" "$(LD)" "$(NM)" "$(RM)" "$(MV)" "$(@)"; +endif + define rule_cc_o_c $(call echo-cmd,checksrc) $(cmd_checksrc) \ $(call echo-cmd,cc_o_c) $(cmd_cc_o_c); \ $(cmd_modversions) \ + $(cmd_record_mcount) \ scripts/basic/fixdep $(depfile) $@ '$(call make-cmd,cc_o_c)' > \ $(dot-target).tmp; \ rm -f $(depfile); \
scripts/recordmcount.pl new file +280 -0
280 lines not on this page · raw message
kernel/trace/Kconfig +8 -0
@@ -7,6 +7,9 @@ config HAVE_FTRACE config HAVE_DYNAMIC_FTRACE bool +config HAVE_FTRACE_MCOUNT_RECORD + bool + config TRACER_MAX_TRACE bool
@@ -122,6 +125,11 @@ config DYNAMIC_FTRACE were made. If so, it runs stop_machine (stops all CPUS) and modifies the code to jump over the call to ftrace. +config FTRACE_MCOUNT_RECORD + def_bool y + depends on DYNAMIC_FTRACE + depends on HAVE_FTRACE_MCOUNT_RECORD + config FTRACE_SELFTEST bool
Steven Rostedt
maintainerPATCH v2 2/6 ftrace: mcount call site on boot nops core
In reply to Steven RostedtThis is the infrastructure to the converting the mcount call sites recorded by the __mcount_loc section into nops on boot. It also allows for using these sites to enable tracing as normal. When the __mcount_loc section is used, the "ftraced" kernel thread is disabled.
This uses the current infrastructure to record the mcount call sites as well as convert them to nops. The mcount function is kept as a stub on boot up and not converted to the ftrace_record_ip function. We use the ftrace_record_ip to only record from the table.
This patch does not handle modules. That comes with a later patch.
--- include/asm-x86/ftrace.h | 10 +++ include/linux/ftrace.h | 6 + init/main.c | 3 kernel/trace/ftrace.c | 148 +++++++++++++++++++++++++++++++++-------------- 4 files changed, 124 insertions(+), 43 deletions(-)
include/asm-x86/ftrace.h +10 -0
@@ -7,6 +7,16 @@#ifndef __ASSEMBLY__extern void mcount (void ); + +static inline unsigned long ftrace_call_adjust (unsigned long addr) +{ +/* +* call mcount is "e8 <4 byte offset>" +* The addr points to the 4 byte offset and the caller of this +* function wants the pointer to e8. Simply subtract one. +*/ +return addr -1 ; +}#endif #endif /* CONFIG_FTRACE */
include/linux/ftrace.h +6 -0
@@ -141,4 +141,10 @@ static inline void ftrace_special(unsigned long arg1, unsigned long arg2, unsigned long arg3) { }#endif +#ifdef CONFIG_FTRACE_MCOUNT_RECORD +extern void ftrace_init (void ); +#else +static inline void ftrace_init (void ) { } +#endif +#endif /* _LINUX_FTRACE_H */
init/main.c +3 -0
@@ -62,6 +62,7 @@#include <linux/signal.h> #include <linux/idr.h> #include <linux/kmemcheck.h> +#include <linux/ftrace.h> #include <asm/io.h> #include <asm/bugs.h>
@@ -820,6 +821,8 @@ asmlinkage void __init start_kernel(voidacpi_early_init ();/* before LAPIC and SMP init */ +ftrace_init (); +/* Do the rest non-__init'ed, we're now alive */ rest_init (); }
kernel/trace/ftrace.c +105 -43
178 lines not on this page · raw message
Steven Rostedt
maintainerPATCH v2 3/6 ftrace: enable mcount recording for modules
In reply to Steven RostedtThis patch enables the loading of the __mcount_section of modules and changing all the callers of mcount into nops.
The modification is done before the init_module function is called, so again, we do not need to use kstop_machine to make these changes.
--- include/linux/ftrace.h | 3 +++ kernel/module.c | 11 +++++++++++ kernel/trace/ftrace.c | 5 +++++ 3 files changed, 19 insertions(+)
include/linux/ftrace.h +3 -0
@@ -143,8 +143,11 @@ ftrace_special(unsigned long arg1, unsig#ifdef CONFIG_FTRACE_MCOUNT_RECORD extern void ftrace_init (void ); +extern void ftrace_init_module (unsigned long *start,unsigned long *end);#else static inline void ftrace_init (void ) { } +static inline void +ftrace_init_module (unsigned long *start,unsigned long *end) { }#endif #endif /* _LINUX_FTRACE_H */
kernel/module.c +11 -0
@@ -48,6 +48,7 @@#include <linux/license.h> #include <asm/sections.h> #include <linux/tracepoint.h> +#include <linux/ftrace.h> #if 0 #define DEBUGP printk
@@ -1835,6 +1836,7 @@ static struct module *load_module(void _unsigned int markersstringsindex;unsigned int tracepointsindex;unsigned int tracepointsstringsindex; +unsigned int mcountindex;struct module *mod;long err =0 ;void *percpu =NULL , *ptr =NULL ;/* Stops spurious gcc warning */
@@ -2125,6 +2127,9 @@ static struct module *load_module(void _ tracepointsstringsindex =find_sec (hdr, sechdrs, secstrings,"__tracepoints_strings" ); + mcountindex =find_sec (hdr, sechdrs, secstrings, +"__mcount_loc" ); +/* Now do relocations. */ for (i =1 ; i < hdr->e_shnum; i++) {const char *strtab = (char *)sechdrs[strindex].sh_addr;
@@ -2185,6 +2190,12 @@ static struct module *load_module(void _ mod->tracepoints + mod->num_tracepoints);#endif } + +if (mcountindex) { +void *mseg = (void *)sechdrs[mcountindex].sh_addr; +ftrace_init_module (mseg, mseg + sechdrs[mcountindex].sh_size); + } + err =module_finalize (hdr, sechdrs, mod);if (err <0 ) goto cleanup;
kernel/trace/ftrace.c +5 -0
@@ -1548,6 +1548,11 @@ static int ftrace_convert_nops(unsignedreturn 0 ; } +void ftrace_init_module (unsigned long *start,unsigned long *end) +{ +ftrace_convert_nops (start, end); +} +extern unsigned long __start_mcount_loc[];extern unsigned long __stop_mcount_loc[];
Re: [PATCH v2 3/6] ftrace: enable mcount recording for modules
Quoted 2 lines · On Friday 15 August 2008 05:45:09 Steven Rostedt wrote:
This patch enables the loading of the
__mcount_sectionof modules and changing all the callers of mcount into nops.
...
kernel/module.c static struct module *load_module(void _ · line 2196
Quoted 3 lines
kernel/module.c +3
-0+if (mcountindex) { +void *mseg = (void *)sechdrs[mcountindex].sh_addr; +ftrace_init_module (mseg, mseg + sechdrs[mcountindex].sh_size);
You don't actually need the if. The size of section 0 is 0.
Rusty.
Re: [PATCH v2 3/6] ftrace: enable mcount recording for modules
Quoted 5 lines · Rusty Russell
Quoted 3 lines · On Friday 15 August 2008 05:45:09 Steven Rostedt wrote:
This patch enables the loading of the
__mcount_sectionof modules and changing all the callers of mcount into nops.
Quoted 9 lines
...
kernel/module.c static struct module *load_module(void _ · line 2196
Quoted 4 lines
kernel/module.c +3
-0+if (mcountindex) { +void *mseg = (void *)sechdrs[mcountindex].sh_addr; +ftrace_init_module (mseg, mseg + sechdrs[mcountindex].sh_size);You don't actually need the if. The size of section 0 is 0.
Thanks,
But I find the if is a bit more clear. It may confuse code reviewers if we expect them to know that sechdrs[0].sh_size is zero. This isn't a hot path.
-- Steve
Steven Rostedt
maintainerPATCH v2 4/6 ftrace: rebuild everything on change to FTRACE_MCOUNT_RECORD
In reply to Steven RostedtWhen enabling or disabling CONFIG_FTRACE_MCOUNT_RECORD, we want a full kernel compile to handle the adding of the __mcount_loc sections.
--- include/linux/kernel.h | 5 +++++ 1 file changed, 5 insertions(+)
include/linux/kernel.h +5 -0
@@ -489,4 +489,9 @@ struct sysinfo {#define NUMA_BUILD 0#endif +/* Rebuild everything on CONFIG_FTRACE_MCOUNT_RECORD */ +#ifdef CONFIG_FTRACE_MCOUNT_RECORD +# define REBUILD_DUE_TO_FTRACE_MCOUNT_RECORD +#endif +#endif
Steven Rostedt
maintainerPATCH v2 5/6 ftrace: enable using mcount recording on x86
In reply to Steven RostedtEnable the use of the __mcount_loc infrastructure on x86_64 and i386.
--- arch/x86/Kconfig | 1 + 1 file changed, 1 insertion(+)
arch/x86/Kconfig +1 -0
@@ -25,6 +25,7 @@ config X86 select HAVE_KPROBES select ARCH_WANT_OPTIONAL_GPIOLIB select HAVE_KRETPROBES + select HAVE_FTRACE_MCOUNT_RECORD select HAVE_DYNAMIC_FTRACE select HAVE_FTRACE select HAVE_KVM if ((X86_32 && !X86_VOYAGER && !X86_VISWS && !X86_NUMAQ) || X86_64)
Steven Rostedt
maintainerPATCH v2 6/6 ftrace: x86 mcount stub
In reply to Steven Rostedtx86 now sets up the mcount locations through the build and no longer needs to record the ip when the function is executed. This patch changes the initial mcount to simply return. There's no need to do any other work. If the ftrace start up test fails, the original mcount will be what everything will use, so having this as fast as possible is a good thing.
--- arch/x86/kernel/entry_32.S | 14 -------------- arch/x86/kernel/entry_64.S | 26 -------------------------- arch/x86/kernel/ftrace.c | 14 ++------------ 3 files changed, 2 insertions(+), 52 deletions(-)
arch/x86/kernel/entry_32.S +0 -14
@@ -1189,20 +1189,6 @@ ENDPROC(xen_failsafe_callback) #ifdef CONFIG_DYNAMIC_FTRACE ENTRY(mcount) - pushl %eax - pushl %ecx - pushl %edx - movl 0xc(%esp), %eax - subl $MCOUNT_INSN_SIZE, %eax - -.globl mcount_call -mcount_call: - call ftrace_stub - - popl %edx - popl %ecx - popl %eax - ret END(mcount)
arch/x86/kernel/entry_64.S +0 -26
@@ -64,32 +64,6 @@ #ifdef CONFIG_FTRACE #ifdef CONFIG_DYNAMIC_FTRACE ENTRY(mcount) - - subq $0x38, %rsp - movq %rax, (%rsp) - movq %rcx, 8(%rsp) - movq %rdx, 16(%rsp) - movq %rsi, 24(%rsp) - movq %rdi, 32(%rsp) - movq %r8, 40(%rsp) - movq %r9, 48(%rsp) - - movq 0x38(%rsp), %rdi - subq $MCOUNT_INSN_SIZE, %rdi - -.globl mcount_call -mcount_call: - call ftrace_stub - - movq 48(%rsp), %r9 - movq 40(%rsp), %r8 - movq 32(%rsp), %rdi - movq 24(%rsp), %rsi - movq 16(%rsp), %rdx - movq 8(%rsp), %rcx - movq (%rsp), %rax - addq $0x38, %rsp - retq END(mcount)
arch/x86/kernel/ftrace.c +2 -12
@@ -112,18 +112,8 @@ notrace int ftrace_update_ftrace_func(ft notrace int ftrace_mcount_set(unsigned long *data) { -unsigned long ip = (long )(&mcount_call); -unsigned long *addr = data; -unsigned char old[MCOUNT_INSN_SIZE ], *new; - -/* -* Replace the mcount stub with a pointer to the -* ip recorder function. -*/ -memcpy (old, &mcount_call,MCOUNT_INSN_SIZE ); - new =ftrace_call_replace (ip, *addr); - *addr =ftrace_modify_code (ip, old, new); - +/* mcount is initialized as a nop */ + *data =0 ;return 0 ; }
Quoted 25 lines · Steven Rostedt
[
Changes since v1:regex fix in
x86_64recordmcount.pl. Now it can handle all
mcount+0x... mcount-0x... and mcount, where as the original only handled mcount+0x...Made mcount on start-up to simply return. The current mcount
is set up to be replaced with a call toftrace_record_ip. This is no longer necessary.Note: This patch series is focusing on how calls to mcount in
the kernel are converted to nops. It does not address what kind of nop is used. That is a different topic, and should be in a different patch series.Note 2: I have found that the changes here are more stable than
the current daemon method, and these patches should be used. It also solves the resume from suspend to ram bug that was reported:http://lkml.org/lkml/2008/8/12/234
and
http://lkml.org/lkml/2008/8/12/451
Quoted 8 lines
Note 3: I have already ported this to PowerPC64, but I am waiting
for this to be accepted first before submitting those changes.
]One of the things that bothered me about the latest ftrace code was this annoying daemon that would wake up once a second to see if it had work to do. If it did not, it would go to sleep, otherwise it would do its work and then go to sleep.
i like this concept alot - i've applied the whole lot to tip/tracing/ftrace.
Eventually gcc should be extended to provide a separate section for instrumentation patch sites, instead of us having to disassemble the object code. That would also make the ftrace build faster. Any gcc folks interested in that?
Ingo
(From the original mail)
Quoted 6 lines · Steven Rostedt
Along came Gregory Haskins, who was bickering about having ftrace enabled on a production -rt kernel. I told him the reasons that this would be bad and then he started thinking out loud, and suggesting wild ideas, like patching gcc!
^^^^^^^^^^
Quoted 1 line · Ingo Molnar
Eventually gcc should be extended to provide a separate section for instrumentation patch sites, instead of us having to disassemble the object code.
:)
I obviously agree with this, so +1
Though, tbh, at the time I suggested it I didn't think of Steve's idea to post-process which was quite clever. But I do agree that having gcc do it will probably save some build time since it will probably be trivial for it to do this when already processing -pg. It would have the added benefit of letting the arch specific toolchain do the arch specific work (though I think Steve's solution capitalizes on the toolchain extensively as it is).
The biggest downside is that we would have an external dependency on gcc for the feature, but I guess the kernel already has some of those anyway (e.g. the stack overflow guard feature, etc). We could always fall back on Steve's post-processing if the toolchain lacks the feature.
-Greg
Quoted 8 lines · Steven Rostedt
Note 2: I have found that the changes here are more stable than
the current daemon method, and these patches should be used. It also solves the resume from suspend to ram bug that was reported:http://lkml.org/lkml/2008/8/12/234
and
http://lkml.org/lkml/2008/8/12/451
I don't remember testing any patch for my bug... ;)
For which tree this patchset is?
I tried to apply it on rc3, rc3+tip/tip and rc3+tip/master and all failed.
Marcin
Steven Rostedt
maintainerQuoted 6 lines · Marcin Slusarz
Quoted 5 lines · Steven Rostedt
Note 2: I have found that the changes here are more stable than
the current daemon method, and these patches should be used. It also solves the resume from suspend to ram bug that was reported:
Quoted 9 lines
Quoted 3 lines
http://lkml.org/lkml/2008/8/12/234 and http://lkml.org/lkml/2008/8/12/451I don't remember testing any patch for my bug... ;)
For which tree this patchset is?
I tried to apply it on rc3, rc3+tip/tip and rc3+tip/master and all failed.
Ah, Bruce reported the same error, and said this patch series fixed it. I assumed that this would fix your bug too. But if it does not, then I need to look deeper into what you are reporting.
Could you recheck-out the latest tip/master and see if it works for you.
Thanks,
-- Steve
Quoted 19 lines · Steven Rostedt
Quoted 6 lines · Marcin Slusarz
Quoted 5 lines · Steven Rostedt
Note 2: I have found that the changes here are more stable than
the current daemon method, and these patches should be used. It also solves the resume from suspend to ram bug that was reported:Quoted 9 lines
Quoted 3 lines
http://lkml.org/lkml/2008/8/12/234 and http://lkml.org/lkml/2008/8/12/451I don't remember testing any patch for my bug... ;)
For which tree this patchset is?
I tried to apply it on rc3, rc3+tip/tip and rc3+tip/master and all failed.
Quoted 5 lines
Ah, Bruce reported the same error, and said this patch series fixed it. I assumed that this would fix your bug too. But if it does not, then I need to look deeper into what you are reporting.
Could you recheck-out the latest tip/master and see if it works for you.
Ok, I tried tip/master and resume from suspend to ram works again. Thanks!
Marcin
Quoted 20 lines · Marcin Slusarz
Quoted 19 lines · Steven Rostedt
Quoted 15 lines · Marcin Slusarz
Quoted 8 lines · Steven Rostedt
Note 2: I have found that the changes here are more stable than
the current daemon method, and these patches should be used. It also solves the resume from suspend to ram bug that was reported:http://lkml.org/lkml/2008/8/12/234
and
http://lkml.org/lkml/2008/8/12/451I don't remember testing any patch for my bug... ;)
For which tree this patchset is?
I tried to apply it on rc3, rc3+tip/tip and rc3+tip/master and all failed.
Quoted 8 lines
Quoted 5 lines
Ah, Bruce reported the same error, and said this patch series fixed it. I assumed that this would fix your bug too. But if it does not, then I need to look deeper into what you are reporting.
Could you recheck-out the latest tip/master and see if it works for you.
Ok, I tried tip/master and resume from suspend to ram works again. Thanks!
I assume this patchset won't go into .27. So is there any plan to fix it in .27?
Marcin
Steven Rostedt
maintainerQuoted 25 lines · Marcin Slusarz
Quoted 24 lines · Marcin Slusarz
Quoted 23 lines · Steven Rostedt
Quoted 6 lines · Marcin Slusarz
Quoted 5 lines · Steven Rostedt
Note 2: I have found that the changes here are more stable than
the current daemon method, and these patches should be used. It also solves the resume from suspend to ram bug that was reported:Quoted 9 lines
Quoted 3 lines
http://lkml.org/lkml/2008/8/12/234 and http://lkml.org/lkml/2008/8/12/451I don't remember testing any patch for my bug... ;)
For which tree this patchset is?
I tried to apply it on rc3, rc3+tip/tip and rc3+tip/master and all failed.Ah, Bruce reported the same error, and said this patch series fixed it. I assumed that this would fix your bug too. But if it does not, then I need to look deeper into what you are reporting.
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
Could you recheck-out the latest tip/master and see if it works for you.
Ok, I tried tip/master and resume from suspend to ram works again. Thanks!
I assume this patchset won't go into .27. So is there any plan to fix it in .27?
I'm hoping that this does go into 27, since it is more reliable than the current daemon approach.
-- Steve
Quoted 5 lines · Steven Rostedt
Quoted 2 lines
I assume this patchset won't go into .27. So is there any plan to fix it in .27?
I'm hoping that this does go into 27, since it is more reliable than the current daemon approach.
it cannot really go into .27 - there's a lot of ftrace stuff piled up in tip/tracing/ftrace. (the script is also pretty hacky and unproven. All this stuff was written after the merge window.)
Ingo