ftrace: clean ups and fixes
Subsystems (6)
Steven Rostedt
maintainerv3 cover ftrace: clean ups and fixes
[
Changes since v2:
Typo corrections suggested by Wenji Huang.
Removed adding notrace to init sections. Right now it should not be an issue. If we deem that it is, we can add the patch later. It's a performance patch, but I have yet to measure if the improvement is even measurable.
]
This is a series of patches to make ftrace more robust and clean ups.
The first couple of patches fix the recordmount.pl script and changes it to only record the .text section functions. This means that the init sections will not be processed.
A FTRACE_WARN_ON is added to change all WARN_ONS to not only print a warning, but also to disable ftrace as well.
The later patches are a bit more drastic. Since the daemon is error prone, I stripped it out. In doing so, I have to disable dynamic ftrace from all archs that use it. The archs can get dynamic ftrace reenabled when they are ported to the recordmcount.pl method. (Arch maintainers, please contact me if you want help. I can do it with some information about your arch).
Since the hash was created to work with the daemon, that too was stripped out, making the remaining code smaller and cleaner. The kprobe hooks in ftrace may need to be reworked.
Thanks goes to Andrew Morton for his comments on the code as well as the spelling corrections made by Wenji Huang.
-- Steve
Steven Rostedt
maintainerPATCH v3 1/12 ftrace: handle generic arch calls
The recordmcount script requires that the actual arch is passed in. This works well when ARCH=i386 or ARCH=x86_64 but does not handle the case of ARCH=x86.
This patch adds a parameter to the function to pass in the number of bits of the architecture. So that it can determine if x86 should be run for x86_64 or i386 archs.
Signed-off-by Steven Rostedt <[email protected]>
--- scripts/Makefile.build | 10 ++++++++-- scripts/recordmcount.pl | 11 ++++++++++- 2 files changed, 18 insertions(+), 3 deletions(-)
scripts/Makefile.build +8 -2
@@ -198,10 +198,16 @@ cmd_modversions = \ fi; endif +ifdef CONFIG_64BIT +arch_bits = 64 +else +arch_bits = 32 +endif + ifdef CONFIG_FTRACE_MCOUNT_RECORD cmd_record_mcount = perl $(srctree)/scripts/recordmcount.pl \ - "$(ARCH)" "$(OBJDUMP)" "$(OBJCOPY)" "$(CC)" "$(LD)" "$(NM)" "$(RM)" \ - "$(MV)" "$(@)"; + "$(ARCH)" "$(arch_bits)" "$(OBJDUMP)" "$(OBJCOPY)" "$(CC)" "$(LD)" \ + "$(NM)" "$(RM)" "$(MV)" "$(@)"; endif define rule_cc_o_c
scripts/recordmcount.pl +10 -1
@@ -106,7 +106,8 @@ if ($#ARGV < 6) { exit(1); } -my ($arch, $objdump, $objcopy, $cc, $ld, $nm, $rm, $mv, $inputfile) = @ARGV; +my ($arch, $bits, $objdump, $objcopy, $cc, + $ld, $nm, $rm, $mv, $inputfile) = @ARGV; $objdump = "objdump" if ((length $objdump) == 0); $objcopy = "objcopy" if ((length $objcopy) == 0);
@@ -129,6 +130,14 @@ my $function_regex; # Find the name of a # (return offset and func name) my $mcount_regex; # Find the call site to mcount (return offset) +if ($arch eq "x86") { + if ($bits == 64) { + $arch = "x86_64"; + } else { + $arch = "i386"; + } +} + if ($arch eq "x86_64") { $section_regex = "Disassembly of section"; $function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:";
Steven Rostedt
maintainerPATCH v3 2/12 ftrace: dynamic ftrace process only text section
In reply to Steven RostedtThe text section stays in memory without ever leaving. With the exception of modules, but modules know how to handle that case. With the dynamic ftrace tracer, we need to make sure that it does not try to modify code that no longer exists. The only safe section is .text.
This patch changes the recordmcount script to only record the mcount calls in the .text sections.
Signed-off-by Steven Rostedt <[email protected]>
--- scripts/recordmcount.pl | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)
scripts/recordmcount.pl +14 -3
@@ -109,6 +109,11 @@ if ($#ARGV < 6) { my ($arch, $bits, $objdump, $objcopy, $cc, $ld, $nm, $rm, $mv, $inputfile) = @ARGV; +# Acceptable sections to record. +my %text_sections = ( + ".text" => 1, +); + $objdump = "objdump" if ((length $objdump) == 0); $objcopy = "objcopy" if ((length $objcopy) == 0); $cc = "gcc" if ((length $cc) == 0);
@@ -139,7 +144,7 @@ if ($arch eq "x86") { } if ($arch eq "x86_64") { - $section_regex = "Disassembly of section"; + $section_regex = "Disassembly of section\\s+(\\S+):"; $function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:"; $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\smcount([+-]0x[0-9a-zA-Z]+)?\$"; $type = ".quad";
@@ -151,7 +156,7 @@ if ($arch eq "x86_64") { $cc .= " -m64"; } elsif ($arch eq "i386") { - $section_regex = "Disassembly of section"; + $section_regex = "Disassembly of section\\s+(\\S+):"; $function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:"; $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\smcount\$"; $type = ".long";
@@ -298,7 +303,13 @@ my $text; while (<IN>) { # is it a section? if (/$section_regex/) { - $read_function = 1; + + # Only record text sections that we know are safe + if (defined($text_sections{$1})) { + $read_function = 1; + } else { + $read_function = 0; + } # print out any recorded offsets update_funcs() if ($text_found);
Steven Rostedt
maintainerPATCH v3 3/12 ftrace: return error on failed modified text.
In reply to Steven RostedtHave the ftrace_modify_code return error values:
-EFAULT on error of reading the address
-EINVAL if what is read does not match what it expected
-EPERM if the write fails to update after a successful match.
Signed-off-by Steven Rostedt <[email protected]>
--- arch/x86/kernel/ftrace.c | 12 +++++++----- include/linux/ftrace.h | 24 ++++++++++++++++++++++-- kernel/trace/ftrace.c | 21 +++++++++++++++------ 3 files changed, 44 insertions(+), 13 deletions(-)
arch/x86/kernel/ftrace.c +7 -5
@@ -71,14 +71,16 @@ ftrace_modify_code(unsigned long ip, uns* No real locking needed, this code is run through * kstop_machine, or before SMP starts. */ -if (__copy_from_user_inatomic (replaced, (char __user *)ip,MCOUNT_INSN_SIZE )) -return 1 ; +if (__copy_from_user_inatomic (replaced, (char __user *)ip, +MCOUNT_INSN_SIZE )) +return -EFAULT ;if (memcmp (replaced, old_code,MCOUNT_INSN_SIZE ) !=0 ) -return 2 ; +return -EINVAL ; -WARN_ON_ONCE (__copy_to_user_inatomic ((char __user *)ip, new_code, -MCOUNT_INSN_SIZE )); +if (__copy_to_user_inatomic ((char __user *)ip, new_code, +MCOUNT_INSN_SIZE )) +return -EPERM ;sync_core ();
kernel/trace/ftrace.c +15 -6
@@ -591,22 +591,22 @@ ftrace_code_disable(struct dyn_ftrace *r {unsigned long ip;unsigned char *nop, *call; -int failed; +int ret; ip = rec->ip; nop =ftrace_nop_replace (); call =ftrace_call_replace (ip, mcount_addr); - failed =ftrace_modify_code (ip, call, nop); -if (failed) { -switch (failed) { -case 1 : + ret =ftrace_modify_code (ip, call, nop); +if (ret) { +switch (ret) { +case -EFAULT :WARN_ON_ONCE (1 );pr_info ("ftrace faulted on modifying " );print_ip_sym (ip);break ; -case 2 : +case -EINVAL :WARN_ON_ONCE (1 );pr_info ("ftrace failed to modify " );print_ip_sym (ip);
@@ -615,6 +615,15 @@ ftrace_code_disable(struct dyn_ftrace *rprint_ip_ins (" replace: " , nop);printk (KERN_CONT "\n" );break ; +case -EPERM : +WARN_ON_ONCE (1 ); +pr_info ("ftrace faulted on writing " ); +print_ip_sym (ip); +break ; +default : +WARN_ON_ONCE (1 ); +pr_info ("ftrace faulted on unknown error " ); +print_ip_sym (ip); } rec->flags |=FTRACE_FL_FAILED ;
include/linux/ftrace.h +22 -2
@@ -72,13 +72,33 @@ extern unsigned char *ftrace_nop_replaceextern unsigned char *ftrace_call_replace (unsigned long ip,unsigned long addr);extern int ftrace_dyn_arch_init (void *data);extern int ftrace_mcount_set (unsigned long *data); -extern int ftrace_modify_code (unsigned long ip,unsigned char *old_code, -unsigned char *new_code);extern int ftrace_update_ftrace_func (ftrace_func_t func);extern void ftrace_caller (void );extern void ftrace_call (void );extern void mcount_call (void ); +/** +* ftrace_modify_code - modify code segment +* @ip: the address of the code segment +* @old_code: the contents of what is expected to be there +* @new_code: the code to patch in +* +* This is a very sensitive operation and great care needs +* to be taken by the arch. The operation should carefully +* read the location, check to see if what is read is indeed +* what we expect it to be, and then on success of the compare, +* it should write to the location. +* +* Return must be: +* 0 on success +* -EFAULT on error reading the location +* -EINVAL on a failed compare of the contents +* -EPERM on error writing to the location +* Any other value will be considered a failure. +*/ +extern int ftrace_modify_code (unsigned long ip,unsigned char *old_code, +unsigned char *new_code); +extern int skip_trace (unsigned long ip);extern void ftrace_release (void *start,unsigned long size);
Steven Rostedt
maintainerPATCH v3 4/12 ftrace: comment arch ftrace code
In reply to Steven RostedtAdd comments to explain what is happening in the x86 arch ftrace code.
Signed-off-by Steven Rostedt <[email protected]>
--- arch/x86/kernel/ftrace.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
arch/x86/kernel/ftrace.c +6 -1
@@ -66,18 +66,23 @@ ftrace_modify_code(unsigned long ip, uns/* * Note: Due to modules and __init, code can * disappear and change, we need to protect against faulting -* as well as code changing. +* as well as code changing. We do this by using the +* __copy_*_user functions. * * No real locking needed, this code is run through * kstop_machine, or before SMP starts. */ + +/* read the text we want to modify */ if (__copy_from_user_inatomic (replaced, (char __user *)ip,MCOUNT_INSN_SIZE ))return -EFAULT ; +/* Make sure it is what we expect it to be */ if (memcmp (replaced, old_code,MCOUNT_INSN_SIZE ) !=0 )return -EINVAL ; +/* replace the text with the new text */ if (__copy_to_user_inatomic ((char __user *)ip, new_code,MCOUNT_INSN_SIZE ))return -EPERM ;
Steven Rostedt
maintainerPATCH v3 5/12 ftrace: use probe_kernel
In reply to Steven RostedtAndrew Morton suggested using the proper API for reading and writing kernel areas that might fault.
Signed-off-by Steven Rostedt <[email protected]>
--- arch/x86/kernel/ftrace.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
arch/x86/kernel/ftrace.c +3 -5
@@ -67,15 +67,14 @@ ftrace_modify_code(unsigned long ip, uns* Note: Due to modules and __init, code can * disappear and change, we need to protect against faulting * as well as code changing. We do this by using the -* __copy_*_user functions. +* probe_kernel_* functions. * * No real locking needed, this code is run through * kstop_machine, or before SMP starts. */ /* read the text we want to modify */ -if (__copy_from_user_inatomic (replaced, (char __user *)ip, -MCOUNT_INSN_SIZE )) +if (probe_kernel_read (replaced, (void *)ip,MCOUNT_INSN_SIZE ))return -EFAULT ;/* Make sure it is what we expect it to be */
@@ -83,8 +82,7 @@ ftrace_modify_code(unsigned long ip, unsreturn -EINVAL ;/* replace the text with the new text */ -if (__copy_to_user_inatomic ((char __user *)ip, new_code, -MCOUNT_INSN_SIZE )) +if (probe_kernel_write ((void *)ip, new_code,MCOUNT_INSN_SIZE ))return -EPERM ;sync_core ();
Steven Rostedt
maintainerPATCH v3 6/12 ftrace: only have ftrace_kill atomic
In reply to Steven RostedtWhen an anomaly is detected, we need a way to completely disable ftrace. Right now we have two functions: ftrace_kill and ftrace_kill_atomic. The ftrace_kill tries to do it in a "nice" way by converting everything back to a nop.
The "nice" way is dangerous itself, so this patch removes it and only has the "atomic" version, which is all that is needed.
Signed-off-by Steven Rostedt <[email protected]>
--- include/linux/ftrace.h | 3 +-- kernel/trace/ftrace.c | 42 ++---------------------------------------- kernel/trace/trace.c | 2 +- 3 files changed, 4 insertions(+), 43 deletions(-)
include/linux/ftrace.h +1 -2
@@ -40,7 +40,7 @@ extern void ftrace_stub(unsigned long a0# define register_ftrace_function (ops) do { } while (0)# define unregister_ftrace_function (ops) do { } while (0)# define clear_ftrace_function (ops) do { } while (0) -static inline void ftrace_kill_atomic (void ) { } +static inline void ftrace_kill (void ) { }#endif /* CONFIG_FTRACE */ #ifdef CONFIG_DYNAMIC_FTRACE
@@ -117,7 +117,6 @@ static inline void ftrace_release(void */* totally disable ftrace - can not re-enable after this */ void ftrace_kill (void ); -void ftrace_kill_atomic (void );static inline void tracer_disable (void ) {
kernel/trace/ftrace.c +2 -40
68 lines not on this page · raw message
kernel/trace/trace.c +1 -1
@@ -3097,7 +3097,7 @@ void ftrace_dump(void) dump_ran =1 ;/* No turning back! */ -ftrace_kill_atomic (); +ftrace_kill ();for_each_tracing_cpu (cpu) {atomic_inc (&global_trace.data[cpu]->disabled);
Steven Rostedt
maintainerPATCH v3 7/12 ftrace: add ftrace warn on to disable ftrace
In reply to Steven RostedtAdd ftrace warn on to disable ftrace as well as report a warning.
[ Thanks to Andrew Morton for suggesting using the WARN_ON return value ]
Signed-off-by Steven Rostedt <[email protected]>
--- kernel/trace/ftrace.c | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-)
kernel/trace/ftrace.c +19 -10
@@ -32,6 +32,18 @@#include "trace.h" +#define FTRACE_WARN_ON (cond) \ + do { \ + if (WARN_ON(cond)) \ + ftrace_kill(); \ + } while (0) + +#define FTRACE_WARN_ON_ONCE (cond) \ + do { \ + if (WARN_ON_ONCE(cond)) \ + ftrace_kill(); \ + } while (0) +/* ftrace_enabled is a method to turn ftrace on or off */ int ftrace_enabled__read_mostly ;static int last_ftrace_enabled;
@@ -358,10 +370,8 @@ static struct dyn_ftrace *ftrace_alloc_d rec = ftrace_free_records;if (unlikely (!(rec->flags &FTRACE_FL_FREE ))) { -WARN_ON_ONCE (1 ); +FTRACE_WARN_ON_ONCE (1 ); ftrace_free_records =NULL ; - ftrace_disabled =1 ; - ftrace_enabled =0 ;return NULL ; }
@@ -410,7 +420,7 @@ ftrace_record_ip(unsigned long ip) key =hash_long (ip,FTRACE_HASHBITS ); -WARN_ON_ONCE (key >=FTRACE_HASHSIZE ); +FTRACE_WARN_ON_ONCE (key >=FTRACE_HASHSIZE );if (ftrace_ip_in_hash (ip, key)) goto out;
@@ -602,12 +612,12 @@ ftrace_code_disable(struct dyn_ftrace *rif (ret) {switch (ret) {case -EFAULT : -WARN_ON_ONCE (1 ); +FTRACE_WARN_ON_ONCE (1 );pr_info ("ftrace faulted on modifying " );print_ip_sym (ip);break ;case -EINVAL : -WARN_ON_ONCE (1 ); +FTRACE_WARN_ON_ONCE (1 );pr_info ("ftrace failed to modify " );print_ip_sym (ip);print_ip_ins (" expected: " , call);
@@ -616,12 +626,12 @@ ftrace_code_disable(struct dyn_ftrace *rprintk (KERN_CONT "\n" );break ;case -EPERM : -WARN_ON_ONCE (1 ); +FTRACE_WARN_ON_ONCE (1 );pr_info ("ftrace faulted on writing " );print_ip_sym (ip);break ;default : -WARN_ON_ONCE (1 ); +FTRACE_WARN_ON_ONCE (1 );pr_info ("ftrace faulted on unknown error " );print_ip_sym (ip); }
@@ -1679,8 +1689,7 @@ static int ftraced(void *ignore) ftrace_update_cnt !=1 ?"s" :"" , ftrace_update_tot_cnt, usecs, usecs != 1 ? "s" : ""); - ftrace_disabled =1 ; -WARN_ON_ONCE (1 ); +FTRACE_WARN_ON_ONCE (1 ); } }mutex_unlock (&ftraced_lock);
Steven Rostedt
maintainerPATCH v3 8/12 ftrace: disable dynamic ftrace for all archs that use daemon
In reply to Steven RostedtThe ftrace daemon is complex and can cause nasty races if something goes wrong. Since it affects all of the kernel, this patch disables dynamic ftrace from any arch that depends on the daemon. Until the archs are ported over to the new MCOUNT_RECORD method, I am disabling dynamic ftrace from them.
Note: I am leaving in the arch/<arch>/kernel/ftrace.c code alone since that can be used when the arch is ported to MCOUNT_RECORD. To port the arch to MCOUNT_RECORD, the scripts/recordmcount.pl needs to be updated. I will make that easier to do for 2.6.29. For 28, we will keep the archs disabled.
Signed-off-by Steven Rostedt <[email protected]>
--- arch/arm/Kconfig | 1 - arch/powerpc/Kconfig | 1 - arch/sparc64/Kconfig | 1 - 3 files changed, 3 deletions(-)
arch/arm/Kconfig +0 -1
@@ -17,7 +17,6 @@ config ARM select HAVE_KPROBES if (!XIP_KERNEL) select HAVE_KRETPROBES if (HAVE_KPROBES) select HAVE_FTRACE if (!XIP_KERNEL) - select HAVE_DYNAMIC_FTRACE if (HAVE_FTRACE) select HAVE_GENERIC_DMA_COHERENT help The ARM series is a line of low-power-consumption RISC chip designs
arch/powerpc/Kconfig +0 -1
@@ -111,7 +111,6 @@ config ARCH_NO_VIRT_TO_BUS config PPC bool default y - select HAVE_DYNAMIC_FTRACE select HAVE_FTRACE select ARCH_WANT_OPTIONAL_GPIOLIB select HAVE_IDE
arch/sparc64/Kconfig +0 -1
@@ -11,7 +11,6 @@ config SPARC config SPARC64 bool default y - select HAVE_DYNAMIC_FTRACE select HAVE_FTRACE select HAVE_IDE select HAVE_LMB
Steven Rostedt
maintainerPATCH v3 9/12 ftrace: remove daemon
In reply to Steven RostedtThe ftrace daemon is complex and error prone. This patch strips it out of the code.
Signed-off-by Steven Rostedt <[email protected]>
--- kernel/trace/ftrace.c | 280 ++++-------------------------------------- kernel/trace/trace_selftest.c | 14 -- 2 files changed, 28 insertions(+), 266 deletions(-)
kernel/trace/ftrace.c +28 -252
@@ -165,21 +165,8 @@ static int __unregister_ftrace_function( }#ifdef CONFIG_DYNAMIC_FTRACE -#ifndef CONFIG_FTRACE_MCOUNT_RECORD -/* -* The hash lock is only needed when the recording of the mcount -* callers are dynamic. That is, by the caller themselves and -* not recorded via the compilation. -*/ -static DEFINE_SPINLOCK (ftrace_hash_lock); -#define ftrace_hash_lock (flags) spin_lock_irqsave(&ftrace_hash_lock, flags) -#define ftrace_hash_unlock (flags) \ - spin_unlock_irqrestore(&ftrace_hash_lock, flags) -#else -/* This is protected via the ftrace_lock with MCOUNT_RECORD. */ -#define ftrace_hash_lock (flags) do { (void)(flags); } while (0) -#define ftrace_hash_unlock (flags) do { } while(0) +# error Dynamic ftrace depends on MCOUNT_RECORD#endif /*
@@ -190,8 +177,6 @@ static DEFINE_SPINLOCK(ftrace_hash_lock)*/ static unsigned long mcount_addr =MCOUNT_ADDR ; -static struct task_struct *ftraced_task; -enum {FTRACE_ENABLE_CALLS = (1 <<0 ),FTRACE_DISABLE_CALLS = (1 <<1 ),
@@ -208,7 +193,6 @@ static struct hlist_head ftrace_hash[FTR static DEFINE_PER_CPU(int, ftrace_shutdown_disable_cpu); -static DEFINE_MUTEX (ftraced_lock);static DEFINE_MUTEX (ftrace_regex_lock);struct ftrace_page {
@@ -226,10 +210,6 @@ struct ftrace_page {static struct ftrace_page *ftrace_pages_start;static struct ftrace_page *ftrace_pages; -static int ftraced_trigger; -static int ftraced_suspend; -static int ftraced_stop; -static int ftrace_record_suspend;static struct dyn_ftrace *ftrace_free_records;
@@ -393,7 +373,6 @@ static void ftrace_record_ip(unsigned long ip) {struct dyn_ftrace *node; -unsigned long flags;unsigned long key;int resched;int cpu;
@@ -425,24 +404,18 @@ ftrace_record_ip(unsigned long ip)if (ftrace_ip_in_hash (ip, key)) goto out; -ftrace_hash_lock (flags); -/* This ip may have hit the hash before the lock */ if (ftrace_ip_in_hash (ip, key)) - goto out_unlock; + goto out; node =ftrace_alloc_dyn_node (ip);if (!node) - goto out_unlock; + goto out; node->ip = ip;ftrace_add_hash (node, key); - ftraced_trigger =1 ; - - out_unlock: -ftrace_hash_unlock (flags); out:per_cpu (ftrace_shutdown_disable_cpu, cpu)--;
@@ -642,7 +615,7 @@ ftrace_code_disable(struct dyn_ftrace *rreturn 1 ; } -static int __ftrace_update_code (void *ignore); +static int ftrace_update_code (void *ignore);static int __ftrace_modify_code (void *data) {
@@ -654,7 +627,7 @@ static int __ftrace_modify_code(void *da* Update any recorded ips now that we have the * machine stopped */ -__ftrace_update_code (NULL ); +ftrace_update_code (NULL );ftrace_replace_code (1 ); tracing_on =1 ; }else if (*command &FTRACE_DISABLE_CALLS ) {
@@ -681,26 +654,9 @@ static void ftrace_run_update_code(int cstop_machine (__ftrace_modify_code, &command,NULL ); } -void ftrace_disable_daemon (void ) -{ -/* Stop the daemon from calling kstop_machine */ -mutex_lock (&ftraced_lock); - ftraced_stop =1 ; -mutex_unlock (&ftraced_lock); - -ftrace_force_update (); -} - -void ftrace_enable_daemon (void ) -{ -mutex_lock (&ftraced_lock); - ftraced_stop =0 ; -mutex_unlock (&ftraced_lock); - -ftrace_force_update (); -} -static ftrace_func_t saved_ftrace_func; +static int ftrace_start; +static DEFINE_MUTEX (ftrace_start_lock);static void ftrace_startup (void ) {
@@ -709,9 +665,9 @@ static void ftrace_startup(void)if (unlikely (ftrace_disabled))return ; -mutex_lock (&ftraced_lock); - ftraced_suspend++; -if (ftraced_suspend ==1 ) +mutex_lock (&ftrace_start_lock); + ftrace_start++; +if (ftrace_start ==1 ) command |=FTRACE_ENABLE_CALLS ;if (saved_ftrace_func != ftrace_trace_function) {
@@ -724,7 +680,7 @@ static void ftrace_startup(void)ftrace_run_update_code (command); out: -mutex_unlock (&ftraced_lock); +mutex_unlock (&ftrace_start_lock); }static void ftrace_shutdown (void )
@@ -734,9 +690,9 @@ static void ftrace_shutdown(void)if (unlikely (ftrace_disabled))return ; -mutex_lock (&ftraced_lock); - ftraced_suspend--; -if (!ftraced_suspend) +mutex_lock (&ftrace_start_lock); + ftrace_start--; +if (!ftrace_start) command |=FTRACE_DISABLE_CALLS ;if (saved_ftrace_func != ftrace_trace_function) {
@@ -749,7 +705,7 @@ static void ftrace_shutdown(void)ftrace_run_update_code (command); out: -mutex_unlock (&ftraced_lock); +mutex_unlock (&ftrace_start_lock); }static void ftrace_startup_sysctl (void )
@@ -759,15 +715,15 @@ static void ftrace_startup_sysctl(void)if (unlikely (ftrace_disabled))return ; -mutex_lock (&ftraced_lock); +mutex_lock (&ftrace_start_lock);/* Force update next time */ saved_ftrace_func =NULL ; -/* ftraced_suspend is true if we want ftrace running */ -if (ftraced_suspend) +/* ftrace_start is true if we want ftrace running */ +if (ftrace_start) command |=FTRACE_ENABLE_CALLS ;ftrace_run_update_code (command); -mutex_unlock (&ftraced_lock); +mutex_unlock (&ftrace_start_lock); }static void ftrace_shutdown_sysctl (void )
@@ -777,20 +733,20 @@ static void ftrace_shutdown_sysctl(void)if (unlikely (ftrace_disabled))return ; -mutex_lock (&ftraced_lock); -/* ftraced_suspend is true if ftrace is running */ -if (ftraced_suspend) +mutex_lock (&ftrace_start_lock); +/* ftrace_start is true if ftrace is running */ +if (ftrace_start) command |=FTRACE_DISABLE_CALLS ;ftrace_run_update_code (command); -mutex_unlock (&ftraced_lock); +mutex_unlock (&ftrace_start_lock); }static cycle_t ftrace_update_time;static unsigned long ftrace_update_cnt;unsigned long ftrace_update_tot_cnt; -static int __ftrace_update_code (void *ignore) +static int ftrace_update_code (void *ignore) {int i, save_ftrace_enabled;cycle_t start, stop;
@@ -864,7 +820,6 @@ static int __ftrace_update_code(void *ig stop =ftrace_now (raw_smp_processor_id ()); ftrace_update_time = stop - start; ftrace_update_tot_cnt += ftrace_update_cnt; - ftraced_trigger =0 ; ftrace_enabled = save_ftrace_enabled; ftrace_record_suspend--;
@@ -872,17 +827,6 @@ static int __ftrace_update_code(void *igreturn 0 ; } -static int ftrace_update_code (void ) -{ -if (unlikely (ftrace_disabled) || - !ftrace_enabled || !ftraced_trigger) -return 0 ; - -stop_machine (__ftrace_update_code,NULL ,NULL ); - -return 1 ; -} -static int __init ftrace_dyn_table_alloc (unsigned long num_to_init) {struct ftrace_page *pg;
@@ -1420,10 +1364,10 @@ ftrace_regex_release(struct inode *inode }mutex_lock (&ftrace_sysctl_lock); -mutex_lock (&ftraced_lock); -if (iter->filtered && ftraced_suspend && ftrace_enabled) +mutex_lock (&ftrace_start_lock); +if (iter->filtered && ftrace_start && ftrace_enabled)ftrace_run_update_code (FTRACE_ENABLE_CALLS ); -mutex_unlock (&ftraced_lock); +mutex_unlock (&ftrace_start_lock);mutex_unlock (&ftrace_sysctl_lock);kfree (iter);
@@ -1443,55 +1387,6 @@ ftrace_notrace_release(struct inode *inoreturn ftrace_regex_release (inode, file,0 ); } -static ssize_t -ftraced_read (struct file *filp,char __user *ubuf, -size_t cnt,loff_t *ppos) -{ -/* don't worry about races */ -char *buf = ftraced_stop ?"disabled\n" :"enabled\n" ; -int r =strlen (buf); - -return simple_read_from_buffer (ubuf, cnt, ppos, buf, r); -} - -static ssize_t -ftraced_write (struct file *filp,const char __user *ubuf, -size_t cnt,loff_t *ppos) -{ -char buf[64 ]; -long val; -int ret; - -if (cnt >=sizeof (buf)) -return -EINVAL ; - -if (copy_from_user (&buf, ubuf, cnt)) -return -EFAULT ; - -if (strncmp (buf,"enable" ,6 ) ==0 ) - val =1 ; -else if (strncmp (buf,"disable" ,7 ) ==0 ) - val =0 ; -else { - buf[cnt] =0 ; - - ret =strict_strtoul (buf,10 , &val); -if (ret <0 ) -return ret; - - val = !!val; - } - -if (val) -ftrace_enable_daemon (); -else -ftrace_disable_daemon (); - - filp->f_pos += cnt; - -return cnt; -} -static struct file_operations ftrace_avail_fops = { .open = ftrace_avail_open, .read = seq_read,
@@ -1522,38 +1417,6 @@ static struct file_operations ftrace_not .release = ftrace_notrace_release, }; -static struct file_operations ftraced_fops = { - .open = tracing_open_generic, - .read = ftraced_read, - .write = ftraced_write, -}; - -/** -* ftrace_force_update - force an update to all recording ftrace functions -*/ -int ftrace_force_update (void ) -{ -int ret =0 ; - -if (unlikely (ftrace_disabled)) -return -ENODEV ; - -mutex_lock (&ftrace_sysctl_lock); -mutex_lock (&ftraced_lock); - -/* -* If ftraced_trigger is not set, then there is nothing -* to update. -*/ -if (ftraced_trigger && !ftrace_update_code ()) - ret = -EBUSY ; - -mutex_unlock (&ftraced_lock); -mutex_unlock (&ftrace_sysctl_lock); - -return ret; -} -static __init int ftrace_init_debugfs (void ) {struct dentry *d_tracer;
@@ -1584,17 +1447,11 @@ static __init int ftrace_init_debugfs(vopr_warning ("Could not create debugfs " "'set_ftrace_notrace' entry\n" ); - entry =debugfs_create_file ("ftraced_enabled" ,0644 , d_tracer, -NULL , &ftraced_fops); -if (!entry) -pr_warning ("Could not create debugfs " -"'ftraced_enabled' entry\n" );return 0 ; }fs_initcall (ftrace_init_debugfs); -#ifdef CONFIG_FTRACE_MCOUNT_RECORD static int ftrace_convert_nops (unsigned long *start,unsigned long *end) {
@@ -1614,7 +1471,7 @@ static int ftrace_convert_nops(unsigned/* p is ignored */ local_irq_save (flags); -__ftrace_update_code (p); +ftrace_update_code (p);local_irq_restore (flags);return 0 ;
@@ -1661,84 +1518,6 @@ void __init ftrace_init(void) failed: ftrace_disabled = 1; } -#else /* CONFIG_FTRACE_MCOUNT_RECORD */ -static int ftraced (void *ignore) -{ -unsigned long usecs; - -while (!kthread_should_stop ()) { - -set_current_state (TASK_INTERRUPTIBLE ); - -/* check once a second */ -schedule_timeout (HZ ); - -if (unlikely (ftrace_disabled)) -continue ; - -mutex_lock (&ftrace_sysctl_lock); -mutex_lock (&ftraced_lock); -if (!ftraced_suspend && !ftraced_stop && -ftrace_update_code ()) { - usecs =nsecs_to_usecs (ftrace_update_time); -if (ftrace_update_tot_cnt >100000 ) { - ftrace_update_tot_cnt =0 ; -pr_info ("hm, dftrace overflow: %lu change%s" -" (%lu total) in %lu usec%s\n" , - ftrace_update_cnt, - ftrace_update_cnt !=1 ?"s" :"" , - ftrace_update_tot_cnt, - usecs, usecs !=1 ?"s" :"" ); -FTRACE_WARN_ON_ONCE (1 ); - } - } -mutex_unlock (&ftraced_lock); -mutex_unlock (&ftrace_sysctl_lock); - -ftrace_shutdown_replenish (); - } -__set_current_state (TASK_RUNNING ); -return 0 ; -} - -static int __init ftrace_dynamic_init (void ) -{ -struct task_struct *p; -unsigned long addr; -int ret; - - addr = (unsigned long )ftrace_record_ip; - -stop_machine (ftrace_dyn_arch_init, &addr,NULL ); - -/* ftrace_dyn_arch_init places the return code in addr */ -if (addr) { - ret = (int )addr; - goto failed; - } - - ret =ftrace_dyn_table_alloc (NR_TO_INIT ); -if (ret) - goto failed; - - p =kthread_run (ftraced,NULL ,"ftraced" ); -if (IS_ERR (p)) { - ret =-1 ; - goto failed; - } - - last_ftrace_enabled = ftrace_enabled =1 ; - ftraced_task = p; - -return 0 ; - - failed: - ftrace_disabled =1 ; -return ret; -} - -core_initcall (ftrace_dynamic_init); -#endif /* CONFIG_FTRACE_MCOUNT_RECORD */ #else # define ftrace_startup () do { } while (0)
@@ -1758,9 +1537,6 @@ void ftrace_kill(void) { ftrace_disabled =1 ; ftrace_enabled =0 ; -#ifdef CONFIG_DYNAMIC_FTRACE - ftraced_suspend =-1 ; -#endif clear_ftrace_function (); }
kernel/trace/trace_selftest.c +0 -14
@@ -99,13 +99,6 @@ int trace_selftest_startup_dynamic_traci/* passed in by parameter to fool gcc from optimizing */ func (); -/* update the records */ - ret =ftrace_force_update (); -if (ret) { -printk (KERN_CONT ".. ftraced failed .. " ); -return ret; - } -/* * Some archs *cough*PowerPC*cough* add charachters to the * start of the function names. We simply put a '*' to
@@ -183,13 +176,6 @@ trace_selftest_startup_function(struct t/* make sure msleep has been recorded */ msleep (1 ); -/* force the recorded functions to be traced */ - ret =ftrace_force_update (); -if (ret) { -printk (KERN_CONT ".. ftraced failed .. " ); -return ret; - } -/* start the tracing */ ftrace_enabled =1 ; tracer_enabled =1 ;
Steven Rostedt
maintainerPATCH v3 10/12 ftrace: remove mcount set
In reply to Steven RostedtThe arch dependent function ftrace_mcount_set was only used by the daemon start up code. This patch removes it.
Signed-off-by Steven Rostedt <[email protected]>
--- arch/arm/kernel/ftrace.c | 13 ------------- arch/powerpc/kernel/ftrace.c | 17 ----------------- arch/sparc64/kernel/ftrace.c | 18 ------------------ arch/x86/kernel/ftrace.c | 7 ------- include/linux/ftrace.h | 1 - kernel/trace/ftrace.c | 9 --------- 6 files changed, 65 deletions(-)
arch/arm/kernel/ftrace.c +0 -13
@@ -95,19 +95,6 @@ int ftrace_update_ftrace_func(ftrace_funreturn ret; } -int ftrace_mcount_set (unsigned long *data) -{ -unsigned long pc, old; -unsigned long *addr = data; -unsigned char *new; - - pc = (unsigned long )&mcount_call; -memcpy (&old, &mcount_call,MCOUNT_INSN_SIZE ); - new =ftrace_call_replace (pc, *addr); - *addr =ftrace_modify_code (pc, (unsigned char *)&old, new); -return 0 ; -} -/* run from kstop_machine */ int __init ftrace_dyn_arch_init (void *data) {
arch/powerpc/kernel/ftrace.c +0 -17
@@ -126,23 +126,6 @@ notrace int ftrace_update_ftrace_func(ftreturn ret; } -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); - -return 0 ; -} -int __init ftrace_dyn_arch_init (void *data) {/* This is running in kstop_machine */
arch/x86/kernel/ftrace.c +0 -7
@@ -103,13 +103,6 @@ notrace int ftrace_update_ftrace_func(ftreturn ret; } -notrace int ftrace_mcount_set(unsigned long *data) -{ -/* mcount is initialized as a nop */ - *data =0 ; -return 0 ; -} -int __init ftrace_dyn_arch_init (void *data) {extern const unsigned char ftrace_test_p6nop[];
include/linux/ftrace.h +0 -1
@@ -71,7 +71,6 @@ extern int ftrace_ip_converted(unsignedextern unsigned char *ftrace_nop_replace (void );extern unsigned char *ftrace_call_replace (unsigned long ip,unsigned long addr);extern int ftrace_dyn_arch_init (void *data); -extern int ftrace_mcount_set (unsigned long *data);extern int ftrace_update_ftrace_func (ftrace_func_t func);extern void ftrace_caller (void );extern void ftrace_call (void );
kernel/trace/ftrace.c +0 -9
@@ -619,7 +619,6 @@ static int ftrace_update_code(void *ignostatic int __ftrace_modify_code (void *data) { -unsigned long addr;int *command = data;if (*command &FTRACE_ENABLE_CALLS ) {
@@ -638,14 +637,6 @@ static int __ftrace_modify_code(void *daif (*command &FTRACE_UPDATE_TRACE_FUNC )ftrace_update_ftrace_func (ftrace_trace_function); -if (*command &FTRACE_ENABLE_MCOUNT ) { - addr = (unsigned long )ftrace_record_ip; -ftrace_mcount_set (&addr); - }else if (*command &FTRACE_DISABLE_MCOUNT ) { - addr = (unsigned long )ftrace_stub; -ftrace_mcount_set (&addr); - } -return 0 ; }
arch/sparc64/kernel/ftrace.c +0 -18
@@ -69,24 +69,6 @@ notrace int ftrace_update_ftrace_func(ftreturn ftrace_modify_code (ip, old, new); } -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); - -return 0 ; -} - -int __init ftrace_dyn_arch_init (void *data) {ftrace_mcount_set (data);
Steven Rostedt
maintainerPATCH v3 11/12 ftrace: remove ftrace hash
In reply to Steven RostedtThe ftrace hash was used by the ftrace_daemon code. The record ip function would place the calling address (ip) into the hash. The daemon would later read the hash and modify that code.
The hash complicates the code. This patch removes it.
Signed-off-by Steven Rostedt <[email protected]>
--- include/linux/ftrace.h | 8 - kernel/trace/ftrace.c | 247 +++++++------------------------------------------ kernel/trace/trace.c | 3 3 files changed, 40 insertions(+), 218 deletions(-)
include/linux/ftrace.h +3 -5
@@ -44,8 +44,6 @@ static inline void ftrace_kill(void) { }#endif /* CONFIG_FTRACE */ #ifdef CONFIG_DYNAMIC_FTRACE -# define FTRACE_HASHBITS 10 -# define FTRACE_HASHSIZE (1<<FTRACE_HASHBITS)enum { FTRACE_FL_FREE = (1 << 0),
@@ -58,9 +56,9 @@ enum { };struct dyn_ftrace { -struct hlist_node node; -unsigned long ip;/* address of mcount call-site */ -unsigned long flags; +struct list_head list; +unsigned long ip;/* address of mcount call-site */ +unsigned long flags; };int ftrace_force_update (void );
kernel/trace/ftrace.c +37 -210
353 lines not on this page · raw message
kernel/trace/trace.c +0 -3
@@ -865,9 +865,6 @@ function_trace_call(unsigned long ip, unif (unlikely (!ftrace_function_enabled))return ; -if (skip_trace (ip)) -return ; - pc =preempt_count (); resched =need_resched ();preempt_disable_notrace ();
Steven Rostedt
maintainerPATCH v3 12/12 ftrace: remove notrace from arch ftrace file
In reply to Steven RostedtThe entire file of ftrace.c in the arch code needs to be marked as notrace. It is much cleaner to do this from the Makefile with CFLAGS_REMOVE_ftrace.o.
Signed-off-by Steven Rostedt <[email protected]>
--- arch/powerpc/kernel/ftrace.c | 10 +++++----- arch/sparc64/kernel/Makefile | 2 ++ arch/sparc64/kernel/ftrace.c | 8 ++++---- arch/x86/kernel/Makefile | 1 + arch/x86/kernel/ftrace.c | 10 +++++----- 5 files changed, 17 insertions(+), 14 deletions(-)
arch/powerpc/kernel/ftrace.c +5 -5
@@ -28,17 +28,17 @@ static unsigned int ftrace_nop = 0x60000#endif -static unsigned int notrace ftrace_calc_offset(long ip, long addr) +static unsigned int ftrace_calc_offset (long ip,long addr) {return (int )(addr - ip); } -notrace unsigned char *ftrace_nop_replace(void) +unsigned char *ftrace_nop_replace (void ) {return (char *)&ftrace_nop; } -notrace unsigned char *ftrace_call_replace(unsigned long ip, unsigned long addr) +unsigned char *ftrace_call_replace (unsigned long ip,unsigned long addr) {static unsigned int op;
@@ -68,7 +68,7 @@ notrace unsigned char *ftrace_call_repla# define _ASM_PTR " .long "#endif -notrace int +int ftrace_modify_code (unsigned long ip,unsigned char *old_code,unsigned char *new_code) {
@@ -113,7 +113,7 @@ ftrace_modify_code(unsigned long ip, unsreturn faulted; } -notrace int ftrace_update_ftrace_func(ftrace_func_t func) +int ftrace_update_ftrace_func (ftrace_func_t func) {unsigned long ip = (unsigned long )(&ftrace_call);unsigned char old[MCOUNT_INSN_SIZE ], *new;
arch/sparc64/kernel/Makefile +2 -0
@@ -5,6 +5,8 @@ EXTRA_AFLAGS := -ansi EXTRA_CFLAGS := -Werror +CFLAGS_REMOVE_ftrace.o = -pg + extra-y := head.o init_task.o vmlinux.lds obj-y := process.o setup.o cpu.o idprom.o reboot.o \
arch/sparc64/kernel/ftrace.c +4 -4
@@ -9,12 +9,12 @@static const u32 ftrace_nop =0x01000000 ; -notrace unsigned char *ftrace_nop_replace(void) +unsigned char *ftrace_nop_replace (void ) {return (char *)&ftrace_nop; } -notrace unsigned char *ftrace_call_replace(unsigned long ip, unsigned long addr) +unsigned char *ftrace_call_replace (unsigned long ip,unsigned long addr) {static u32 call;s32 off;
@@ -25,7 +25,7 @@ notrace unsigned char *ftrace_call_replareturn (unsigned char *) &call; } -notrace int +int ftrace_modify_code (unsigned long ip,unsigned char *old_code,unsigned char *new_code) {
@@ -59,7 +59,7 @@ ftrace_modify_code(unsigned long ip, unsreturn faulted; } -notrace int ftrace_update_ftrace_func(ftrace_func_t func) +int ftrace_update_ftrace_func (ftrace_func_t func) {unsigned long ip = (unsigned long )(&ftrace_call);unsigned char old[MCOUNT_INSN_SIZE ], *new;
arch/x86/kernel/Makefile +1 -0
@@ -11,6 +11,7 @@ ifdef CONFIG_FTRACE CFLAGS_REMOVE_tsc.o = -pg CFLAGS_REMOVE_rtc.o = -pg CFLAGS_REMOVE_paravirt-spinlocks.o = -pg +CFLAGS_REMOVE_ftrace.o = -pg endif #
arch/x86/kernel/ftrace.c +5 -5
@@ -33,17 +33,17 @@ union ftrace_code_union { }; -static int notrace ftrace_calc_offset(long ip, long addr) +static int ftrace_calc_offset (long ip,long addr) {return (int )(addr - ip); } -notrace unsigned char *ftrace_nop_replace(void) +unsigned char *ftrace_nop_replace (void ) {return (char *)ftrace_nop; } -notrace unsigned char *ftrace_call_replace(unsigned long ip, unsigned long addr) +unsigned char *ftrace_call_replace (unsigned long ip,unsigned long addr) {static union ftrace_code_union calc;
@@ -57,7 +57,7 @@ notrace unsigned char *ftrace_call_replareturn calc.code; } -notrace int +int ftrace_modify_code (unsigned long ip,unsigned char *old_code,unsigned char *new_code) {
@@ -90,7 +90,7 @@ ftrace_modify_code(unsigned long ip, unsreturn 0 ; } -notrace int ftrace_update_ftrace_func(ftrace_func_t func) +int ftrace_update_ftrace_func (ftrace_func_t func) {unsigned long ip = (unsigned long )(&ftrace_call);unsigned char old[MCOUNT_INSN_SIZE ], *new;
Ingo Molnar
AppliedQuoted 5 lines · Steven Rostedt
[
Changes since v2:Typo corrections suggested by Wenji Huang.
Quoted 7 lines
Removed adding notrace to init sections. Right now it should not be an issue. If we deem that it is, we can add the patch later. It's a performance patch, but I have yet to measure if the improvement is even measurable.
]This is a series of patches to make ftrace more robust and clean ups.
i've merged them to tip/tracing/urgent, thanks Steve. There were a handful of conflicts i had to resolve so please double check the end result.
in general, the direction you've taken with these patches is very good!
Ingo