[2.6.40] ftrace: cleanups
Steven Rostedt
maintainerIngo,
I'm still working on the accounting of ftrace function tracer to be able to have multiple users that do not conflict with each other in selecting functions to trace. It's going well and is almost done.
But in doing this, there were several cleanups that had to be made first. This patch series contains those cleanups and I've already run my tests on them and wanted to get them out now, instead of sending them with the major changes. This will let reviews focus on the major changes and not have to worry about these cleanups. Reviewers can examine these cleanups now if they want.
One nice thing about this patch set is the stats:
141 insertions(+), 214 deletions(-)
Please pull the latest tip/perf/core tree, which can be found at:
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
tip/perf/core
Steven Rostedt (8):
ftrace: Make FTRACE_WARN_ON() work in if condition
ftrace: Remove unnecessary disabling of irqs
ftrace: Remove failures file
ftrace: Remove FTRACE_FL_FAILED flag
ftrace: Remove FTRACE_FL_CONVERTED flag
ftrace: Move record update for normal and modules into a separate function
ftrace: Consolidate updating of ftrace_trace_function
ftrace: Consolidate the function match routines for normal and mods
----
include/linux/ftrace.h | 8 +- kernel/trace/ftrace.c | 347 +++++++++++++++++++----------------------------- 2 files changed, 141 insertions(+), 214 deletions(-)
Steven Rostedt
maintainerPATCH 1/8 ftrace: Make FTRACE_WARN_ON() work in if condition
From: Steven Rostedt <[email protected]>
Let FTRACE_WARN_ON() be used as a stand alone statement or inside a conditional: if (FTRACE_WARN_ON(x))
--- kernel/trace/ftrace.c | 16 ++++++++++------ 1 files changed, 10 insertions(+), 6 deletions(-)
kernel/trace/ftrace.c +10 -6
@@ -39,16 +39,20 @@#include "trace_stat.h" #define FTRACE_WARN_ON (cond) \ - do { \ - if (WARN_ON(cond)) \ + ({ \ + int ___r = cond; \ + if (WARN_ON(___r)) \ ftrace_kill(); \ - } while (0) + ___r; \ + })#define FTRACE_WARN_ON_ONCE (cond) \ - do { \ - if (WARN_ON_ONCE(cond)) \ + ({ \ + int ___r = cond; \ + if (WARN_ON_ONCE(___r)) \ ftrace_kill(); \ - } while (0) + ___r; \ + })/* hash bits for specific function selection */ #define FTRACE_HASH_BITS 7
Signature · 1 line
1.7.2.3
Steven Rostedt
maintainerPATCH 2/8 ftrace: Remove unnecessary disabling of irqs
In reply to Steven RostedtFrom: Steven Rostedt <[email protected]>
The disabling of interrupts around ftrace_update_code() was used to protect against the evil ftrace daemon from years past. But that daemon has long been killed. It is safe to keep interrupts enabled while updating the initial mcount into nops.
The ftrace_mutex is also held which keeps other users at bay.
--- kernel/trace/ftrace.c | 4 ---- 1 files changed, 0 insertions(+), 4 deletions(-)
kernel/trace/ftrace.c +0 -4
@@ -2707,7 +2707,6 @@ static int ftrace_process_locs(struct module *mod, {unsigned long *p;unsigned long addr; -unsigned long flags;mutex_lock (&ftrace_lock); p = start;
@@ -2724,10 +2723,7 @@ static int ftrace_process_locs(struct module *mod,ftrace_record_ip (addr); } -/* disable interrupts to prevent kstop machine */ -local_irq_save (flags);ftrace_update_code (mod); -local_irq_restore (flags);mutex_unlock (&ftrace_lock);return 0 ;
Signature · 1 line
1.7.2.3
Steven Rostedt
maintainerPATCH 3/8 ftrace: Remove failures file
In reply to Steven RostedtFrom: Steven Rostedt <[email protected]>
The failures file in the debugfs tracing directory would list the functions that failed to convert when the old dead ftrace daemon tried to update code but failed. Since this code is now dead along with the daemon the failures file is useless. Remove it.
--- kernel/trace/ftrace.c | 39 ++------------------------------------- 1 files changed, 2 insertions(+), 37 deletions(-)
kernel/trace/ftrace.c +2 -37
@@ -1355,9 +1355,8 @@ static int __init ftrace_dyn_table_alloc(unsigned long num_to_init)enum {FTRACE_ITER_FILTER = (1 <<0 ),FTRACE_ITER_NOTRACE = (1 <<1 ), -FTRACE_ITER_FAILURES = (1 <<2 ), -FTRACE_ITER_PRINTALL = (1 <<3 ), -FTRACE_ITER_HASH = (1 <<4 ), +FTRACE_ITER_PRINTALL = (1 <<2 ), +FTRACE_ITER_HASH = (1 <<3 ), };#define FTRACE_BUFF_MAX (KSYM_SYMBOL_LEN+4)/* room for wildcards */
@@ -1487,12 +1486,6 @@ t_next(struct seq_file *m, void *v, loff_t *pos) rec = &iter->pg->records[iter->idx++]; if ((rec->flags & FTRACE_FL_FREE) || - (!(iter->flags & FTRACE_ITER_FAILURES) && - (rec->flags & FTRACE_FL_FAILED)) || - - ((iter->flags & FTRACE_ITER_FAILURES) && - !(rec->flags & FTRACE_FL_FAILED)) || - ((iter->flags & FTRACE_ITER_FILTER) && !(rec->flags & FTRACE_FL_FILTER)) ||
@@ -1633,24 +1626,6 @@ ftrace_avail_open(struct inode *inode, struct file *file)return ret; } -static int -ftrace_failures_open (struct inode *inode,struct file *file) -{ -int ret; -struct seq_file *m; -struct ftrace_iterator *iter; - - ret =ftrace_avail_open (inode, file); -if (!ret) { - m = file->private_data; - iter = m->private; - iter->flags =FTRACE_ITER_FAILURES ; - } - -return ret; -} - -static void ftrace_filter_reset (int enable) {struct ftrace_page *pg;
@@ -2448,13 +2423,6 @@ static const struct file_operations ftrace_avail_fops = { .release = seq_release_private, }; -static const struct file_operations ftrace_failures_fops = { - .open = ftrace_failures_open, - .read = seq_read, - .llseek = seq_lseek, - .release = seq_release_private, -}; -static const struct file_operations ftrace_filter_fops = { .open = ftrace_filter_open, .read = seq_read,
@@ -2683,9 +2651,6 @@ static __init int ftrace_init_dyn_debugfs(struct dentry *d_tracer)trace_create_file ("available_filter_functions" ,0444 , d_tracer,NULL , &ftrace_avail_fops); -trace_create_file ("failures" ,0444 , - d_tracer,NULL , &ftrace_failures_fops); -trace_create_file ("set_ftrace_filter" ,0644 , d_tracer,NULL , &ftrace_filter_fops);
Signature · 1 line
1.7.2.3
Steven Rostedt
maintainerPATCH 4/8 ftrace: Remove FTRACE_FL_FAILED flag
In reply to Steven RostedtFrom: Steven Rostedt <[email protected]>
Since we disable all function tracer processing if we detect that a modification of a instruction had failed, we do not need to track that the record has failed. No more ftrace processing is allowed, and the FTRACE_FL_FAILED flag is pointless.
Removing this flag simplifies some of the code, but some ftrace_disabled checks needed to be added or move around a little.
--- include/linux/ftrace.h | 9 ++--- kernel/trace/ftrace.c | 76 +++++++++++++++++++++++++++++------------------ 2 files changed, 51 insertions(+), 34 deletions(-)
include/linux/ftrace.h +4 -5
@@ -147,11 +147,10 @@ extern int ftrace_text_reserved(void *start, void *end);enum {FTRACE_FL_FREE = (1 <<0 ), -FTRACE_FL_FAILED = (1 <<1 ), -FTRACE_FL_FILTER = (1 <<2 ), -FTRACE_FL_ENABLED = (1 <<3 ), -FTRACE_FL_NOTRACE = (1 <<4 ), -FTRACE_FL_CONVERTED = (1 <<5 ), +FTRACE_FL_FILTER = (1 <<1 ), +FTRACE_FL_ENABLED = (1 <<2 ), +FTRACE_FL_NOTRACE = (1 <<3 ), +FTRACE_FL_CONVERTED = (1 <<4 ), };struct dyn_ftrace {
kernel/trace/ftrace.c +47 -29
201 lines not on this page · raw message
Signature · 1 line
1.7.2.3
Steven Rostedt
maintainerPATCH 5/8 ftrace: Remove FTRACE_FL_CONVERTED flag
In reply to Steven RostedtFrom: Steven Rostedt <[email protected]>
Since we disable all function tracer processing if we detect that a modification of a instruction had failed, we do not need to track that the record has failed. No more ftrace processing is allowed, and the FTRACE_FL_CONVERTED flag is pointless.
The FTRACE_FL_CONVERTED flag was used to denote records that were successfully converted from mcount calls into nops. But if a single record fails, all of ftrace is disabled.
--- include/linux/ftrace.h | 1 - kernel/trace/ftrace.c | 12 ++++-------- 2 files changed, 4 insertions(+), 9 deletions(-)
include/linux/ftrace.h +0 -1
@@ -150,7 +150,6 @@ enum {FTRACE_FL_FILTER = (1 <<1 ),FTRACE_FL_ENABLED = (1 <<2 ), FTRACE_FL_NOTRACE = (1 << 3), - FTRACE_FL_CONVERTED = (1 << 4), }; struct dyn_ftrace {
kernel/trace/ftrace.c +4 -8
@@ -1087,12 +1087,8 @@ static void ftrace_replace_code(int enable)return ;do_for_each_ftrace_rec (pg, rec) { -/* -* Skip over free records, records that have -* failed and not converted. -*/ -if (rec->flags &FTRACE_FL_FREE || - !(rec->flags &FTRACE_FL_CONVERTED )) +/* Skip over free records */ +if (rec->flags &FTRACE_FL_FREE )continue ; failed =__ftrace_replace_code (rec, enable);
@@ -1280,10 +1276,10 @@ static int ftrace_update_code(struct module *mod)*/ if (!ftrace_code_disable (mod, p)) {ftrace_free_rec (p); -continue ; +/* Game over */ +break ; } - p->flags |=FTRACE_FL_CONVERTED ; ftrace_update_cnt++;/*
Signature · 1 line
1.7.2.3
Steven Rostedt
maintainerPATCH 6/8 ftrace: Move record update for normal and modules into a separate function
In reply to Steven RostedtFrom: Steven Rostedt <[email protected]>
The updating of a function record is moved to a single function. This will allow us to add specific changes in one location for both modules and kernel functions.
Later patches will determine if the function record itself needs to be updated (which enables the mcount caller), or just the ftrace_ops needs the update.
--- kernel/trace/ftrace.c | 19 +++++++++++-------- 1 files changed, 11 insertions(+), 8 deletions(-)
kernel/trace/ftrace.c +11 -8
@@ -1743,6 +1743,15 @@ static int ftrace_match(char *str, char *regex, int len, int type)return matched; } +static void +update_record (struct dyn_ftrace *rec,unsigned long flag,int not) +{ +if (not) + rec->flags &= ~flag; +else + rec->flags |= flag; +} +static int ftrace_match_record (struct dyn_ftrace *rec,char *regex,int len,int type) {
@@ -1772,10 +1781,7 @@ static int ftrace_match_records(char *buff, int len, int enable)do_for_each_ftrace_rec (pg, rec) {if (ftrace_match_record (rec, search, search_len, type)) { -if (not) - rec->flags &= ~flag; -else - rec->flags |= flag; +update_record (rec, flag, not); found =1 ; }/*
@@ -1846,10 +1852,7 @@ static int ftrace_match_module_records(char *buff, char *mod, int enable)if (ftrace_match_module_record (rec, mod, search, search_len, type)) { -if (not) - rec->flags &= ~flag; -else - rec->flags |= flag; +update_record (rec, flag, not); found =1 ; }if (enable && (rec->flags &FTRACE_FL_FILTER ))
Signature · 1 line
1.7.2.3
Steven Rostedt
maintainerPATCH 7/8 ftrace: Consolidate updating of ftrace_trace_function
In reply to Steven RostedtFrom: Steven Rostedt <[email protected]>
There are three locations that perform almost identical functions in order to update the ftrace_trace_function (the ftrace function variable that gets called by mcount).
Consolidate these into a single function called update_ftrace_function().
--- kernel/trace/ftrace.c | 95 +++++++++++++++++------------------------------- 1 files changed, 34 insertions(+), 61 deletions(-)
kernel/trace/ftrace.c +34 -61
@@ -151,6 +151,34 @@ static void ftrace_test_stop_func(unsigned long ip, unsigned long parent_ip) }#endif +static void update_ftrace_function (void ) +{ +ftrace_func_t func; + +/* +* If there's only one function registered, then call that +* function directly. Otherwise, we need to iterate over the +* registered callers. +*/ +if (ftrace_list == &ftrace_list_end || + ftrace_list->next == &ftrace_list_end) + func = ftrace_list->func; +else + func = ftrace_list_func; + +/* If we filter on pids, update to use the pid function */ +if (!list_empty (&ftrace_pids)) { +set_ftrace_pid_function (func); + func = ftrace_pid_func; + } +#ifdef CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST + ftrace_trace_function = func; +#else + __ftrace_trace_function = func; + ftrace_trace_function = ftrace_test_stop_func; +#endif +} +static int __register_ftrace_function (struct ftrace_ops *ops) { ops->next = ftrace_list;
@@ -162,30 +190,8 @@ static int __register_ftrace_function(struct ftrace_ops *ops)*/ rcu_assign_pointer (ftrace_list, ops); -if (ftrace_enabled) { -ftrace_func_t func; - -if (ops->next == &ftrace_list_end) - func = ops->func; -else - func = ftrace_list_func; - -if (!list_empty (&ftrace_pids)) { -set_ftrace_pid_function (func); - func = ftrace_pid_func; - } - -/* -* For one func, simply call it directly. -* For more than one func, call the chain. -*/ -#ifdef CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST - ftrace_trace_function = func; -#else - __ftrace_trace_function = func; - ftrace_trace_function = ftrace_test_stop_func; -#endif - } +if (ftrace_enabled) +update_ftrace_function ();return 0 ; }
@@ -213,52 +219,19 @@ static int __unregister_ftrace_function(struct ftrace_ops *ops) *p = (*p)->next; -if (ftrace_enabled) { -/* If we only have one func left, then call that directly */ -if (ftrace_list->next == &ftrace_list_end) { -ftrace_func_t func = ftrace_list->func; - -if (!list_empty (&ftrace_pids)) { -set_ftrace_pid_function (func); - func = ftrace_pid_func; - } -#ifdef CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST - ftrace_trace_function = func; -#else - __ftrace_trace_function = func; -#endif - } - } +if (ftrace_enabled) +update_ftrace_function ();return 0 ; }static void ftrace_update_pid_func (void ) { -ftrace_func_t func; - +/* Only do something if we are tracing something */ if (ftrace_trace_function == ftrace_stub)return ; -#ifdef CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST - func = ftrace_trace_function; -#else - func = __ftrace_trace_function; -#endif - -if (!list_empty (&ftrace_pids)) { -set_ftrace_pid_function (func); - func = ftrace_pid_func; - }else { -if (func == ftrace_pid_func) - func = ftrace_pid_function; - } - -#ifdef CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST - ftrace_trace_function = func; -#else - __ftrace_trace_function = func; -#endif +update_ftrace_function (); }#ifdef CONFIG_FUNCTION_PROFILER
Signature · 1 line
1.7.2.3
Steven Rostedt
maintainerPATCH 8/8 ftrace: Consolidate the function match routines for normal and mods
In reply to Steven RostedtFrom: Steven Rostedt <[email protected]>
The code used for matching functions is almost identical between normal selecting of functions and using the :mod: feature of set_ftrace_notrace.
Consolidate the two users into one function.
--- kernel/trace/ftrace.c | 98 ++++++++++++++++++------------------------------- 1 files changed, 36 insertions(+), 62 deletions(-)
kernel/trace/ftrace.c +36 -62
@@ -1726,34 +1726,52 @@ update_record(struct dyn_ftrace *rec, unsigned long flag, int not) }static int -ftrace_match_record (struct dyn_ftrace *rec,char *regex,int len,int type) +ftrace_match_record (struct dyn_ftrace *rec,char *mod, +char *regex,int len,int type) {char str[KSYM_SYMBOL_LEN ]; +char *modname; + +kallsyms_lookup (rec->ip,NULL ,NULL , &modname, str); + +if (mod) { +/* module lookup requires matching the module */ +if (!modname ||strcmp (modname, mod)) +return 0 ; + +/* blank search means to match all funcs in the mod */ +if (!len) +return 1 ; + } -kallsyms_lookup (rec->ip,NULL ,NULL ,NULL , str);return ftrace_match (str, regex, len, type); } -static int ftrace_match_records (char *buff,int len,int enable) +static int match_records (char *buff,int len,char *mod,int enable,int not) { -unsigned int search_len; +unsigned search_len =0 ;struct ftrace_page *pg;struct dyn_ftrace *rec; +int type =MATCH_FULL ; +char *search = buff;unsigned long flag; -char *search; -int type; -int not;int found =0 ; - flag = enable ?FTRACE_FL_FILTER :FTRACE_FL_NOTRACE ; - type =filter_parse_regex (buff, len, &search, ¬); +if (len) { + type =filter_parse_regex (buff, len, &search, ¬); + search_len =strlen (search); + } - search_len =strlen (search); + flag = enable ?FTRACE_FL_FILTER :FTRACE_FL_NOTRACE ;mutex_lock (&ftrace_lock); + +if (unlikely (ftrace_disabled)) + goto out_unlock; +do_for_each_ftrace_rec (pg, rec) { -if (ftrace_match_record (rec, search, search_len, type)) { +if (ftrace_match_record (rec, mod, search, search_len, type)) {update_record (rec, flag, not); found =1 ; }
@@ -1763,43 +1781,23 @@ static int ftrace_match_records(char *buff, int len, int enable)*/ if (enable && (rec->flags &FTRACE_FL_FILTER )) ftrace_filtered =1 ; + }while_for_each_ftrace_rec (); + out_unlock:mutex_unlock (&ftrace_lock);return found; }static int -ftrace_match_module_record (struct dyn_ftrace *rec,char *mod, -char *regex,int len,int type) +ftrace_match_records (char *buff,int len,int enable) { -char str[KSYM_SYMBOL_LEN ]; -char *modname; - -kallsyms_lookup (rec->ip,NULL ,NULL , &modname, str); - -if (!modname ||strcmp (modname, mod)) -return 0 ; - -/* blank search means to match all funcs in the mod */ -if (len) -return ftrace_match (str, regex, len, type); -else -return 1 ; +return match_records (buff, len,NULL , enable,0 ); }static int ftrace_match_module_records (char *buff,char *mod,int enable) { -unsigned search_len =0 ; -struct ftrace_page *pg; -struct dyn_ftrace *rec; -int type =MATCH_FULL ; -char *search = buff; -unsigned long flag;int not =0 ; -int found =0 ; - - flag = enable ?FTRACE_FL_FILTER :FTRACE_FL_NOTRACE ;/* blank or '*' mean the same */ if (strcmp (buff,"*" ) ==0 )
@@ -1811,31 +1809,7 @@ static int ftrace_match_module_records(char *buff, char *mod, int enable) not =1 ; } -if (strlen (buff)) { - type =filter_parse_regex (buff,strlen (buff), &search, ¬); - search_len =strlen (search); - } - -mutex_lock (&ftrace_lock); - -if (unlikely (ftrace_disabled)) - goto out_unlock; - -do_for_each_ftrace_rec (pg, rec) { - -if (ftrace_match_module_record (rec, mod, - search, search_len, type)) { -update_record (rec, flag, not); - found =1 ; - } -if (enable && (rec->flags &FTRACE_FL_FILTER )) - ftrace_filtered =1 ; - - }while_for_each_ftrace_rec (); - out_unlock: -mutex_unlock (&ftrace_lock); - -return found; +return match_records (buff,strlen (buff), mod, enable, not); }/*
@@ -1993,7 +1967,7 @@ register_ftrace_function_probe(char *glob, struct ftrace_probe_ops *ops,do_for_each_ftrace_rec (pg, rec) { -if (!ftrace_match_record (rec, search, len, type)) +if (!ftrace_match_record (rec,NULL , search, len, type))continue ; entry =kmalloc (sizeof (*entry),GFP_KERNEL );
@@ -2548,7 +2522,7 @@ ftrace_set_func(unsigned long *array, int *idx, char *buffer)if (rec->flags &FTRACE_FL_FREE )continue ; -if (ftrace_match_record (rec, search, search_len, type)) { +if (ftrace_match_record (rec,NULL , search, search_len, type)) {/* if it is in the array */ exists = false;for (i =0 ; i < *idx; i++) {
Signature · 1 line
1.7.2.3
Quoted 32 lines · Steven Rostedt
Ingo,
I'm still working on the accounting of ftrace function tracer to be able to have multiple users that do not conflict with each other in selecting functions to trace. It's going well and is almost done.
But in doing this, there were several cleanups that had to be made first. This patch series contains those cleanups and I've already run my tests on them and wanted to get them out now, instead of sending them with the major changes. This will let reviews focus on the major changes and not have to worry about these cleanups. Reviewers can examine these cleanups now if they want.
One nice thing about this patch set is the stats:
141 insertions(+), 214 deletions(-)Please pull the latest tip/perf/core tree, which can be found at:
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
tip/perf/coreSteven Rostedt (8):
ftrace: MakeFTRACE_WARN_ON()work in if condition
ftrace: Remove unnecessary disabling of irqs
ftrace: Remove failures file
ftrace: RemoveFTRACE_FL_FAILEDflag
ftrace: RemoveFTRACE_FL_CONVERTEDflag
ftrace: Move record update for normal and modules into a separate function
ftrace: Consolidate updating offtrace_trace_function
ftrace: Consolidate the function match routines for normal and mods
Quoted 4 lines
----
include/linux/ftrace.h | 8 +- kernel/trace/ftrace.c | 347 +++++++++++++++++++----------------------------- 2 files changed, 141 insertions(+), 214 deletions(-)
Pulled, thanks Steve!
Ingo