tracing: Updates for 4.10
Subsystems (5)
Steven Rostedt
maintainercover tracing: Updates for 4.10
The first two patches have already been sent to Linus. I'm just including them here as I based the other patches on top.
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
for-next
Head SHA1: 8d414bd2f77ce858f6b9d119c63b9ce29cf0b75d
Alexei Starovoitov (1):
ftrace: Ignore FTRACE_FL_DISABLED while walking dyn_ftrace records
Masami Hiramatsu (1):
ftrace: Support full glob matching
Steven Rostedt (Red Hat) (3):
ftrace: Add more checks for FTRACE_FL_DISABLED in processing ip records
tracing: Optimise comparison filters and fix binary and for 64 bit tracing/filter: Define op as the enum that it is
Zhou Chengming (1):
tracing: Allow wakeup_dl tracer to be used by instances
----
Documentation/trace/events.txt | 9 +-- Documentation/trace/ftrace.txt | 9 +-- kernel/trace/Kconfig | 2 + kernel/trace/ftrace.c | 28 ++++++++- kernel/trace/trace.c | 2 +- kernel/trace/trace.h | 2 + kernel/trace/trace_events_filter.c | 119 +++++++++++++++++++++++-------------- kernel/trace/trace_sched_wakeup.c | 1 + 8 files changed, 115 insertions(+), 57 deletions(-)
Steven Rostedt
maintainerPATCH 1/6 ftrace: Ignore FTRACE_FL_DISABLED while walking dyn_ftrace records
From: Alexei Starovoitov <[email protected]>
ftrace_shutdown() checks for sanity of ftrace records
and if dyn_ftrace->flags is not zero, it will warn.
It can happen that 'flags' are set to FTRACE_FL_DISABLED at this point, since some module was loaded, but before ftrace_module_enable() cleared the flags for this module.
In other words the module.c is doing:ftrace_module_init(mod); // calls ftrace_update_code() that sets flags=FTRACE_FL_DISABLED ... // here ftrace_shutdown() is called that warns, since
err = prepare_coming_module(mod); // didn't have a chance to clear FTRACE_FL_DISABLED
Fix it by ignoring disabled records.
It's similar to what __ftrace_hash_rec_update() is already doing.
--- kernel/trace/ftrace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/trace/ftrace.c +1 -1
@@ -2763,7 +2763,7 @@ static int ftrace_shutdown(struct ftrace_ops *ops, int command)struct dyn_ftrace *rec;do_for_each_ftrace_rec (pg, rec) { -if (FTRACE_WARN_ON_ONCE (rec->flags)) +if (FTRACE_WARN_ON_ONCE (rec->flags & ~FTRACE_FL_DISABLED ))pr_warn (" %pS flags:%lx\n" , (void *)rec->ip, rec->flags); }while_for_each_ftrace_rec ();
Signature · 1 line
2.10.2
Steven Rostedt
maintainerPATCH 2/6 ftrace: Add more checks for FTRACE_FL_DISABLED in processing ip records
In reply to Steven RostedtFrom: "Steven Rostedt (Red Hat)" <[email protected]>
When a module is first loaded and its function ip records are added to the ftrace list of functions to modify, they are set to DISABLED, as their text is still in a read only state. When the module is fully loaded, and can be updated, the flag is cleared, and if their's any functions that should be tracing them, it is updated at that moment.
But there's several locations that do record accounting and should ignore records that are marked as disabled, or they can cause issues.
Alexei already fixed one location, but others need to be addressed.
--- kernel/trace/ftrace.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)
kernel/trace/ftrace.c +22 -0
@@ -1862,6 +1862,10 @@ static int __ftrace_hash_update_ipmodify(struct ftrace_ops *ops,/* Update rec->flags */ do_for_each_ftrace_rec (pg, rec) { + +if (rec->flags &FTRACE_FL_DISABLED ) +continue ; +/* We need to update only differences of filter_hash */ in_old = !!ftrace_lookup_ip (old_hash, rec->ip); in_new = !!ftrace_lookup_ip (new_hash, rec->ip);
@@ -1884,6 +1888,10 @@ static int __ftrace_hash_update_ipmodify(struct ftrace_ops *ops,/* Roll back what we did above */ do_for_each_ftrace_rec (pg, rec) { + +if (rec->flags &FTRACE_FL_DISABLED ) +continue ; +if (rec == end) goto err_out;
@@ -2397,6 +2405,10 @@ void __weak ftrace_replace_code(int enable)return ;do_for_each_ftrace_rec (pg, rec) { + +if (rec->flags &FTRACE_FL_DISABLED ) +continue ; + failed =__ftrace_replace_code (rec, enable);if (failed) {ftrace_bug (failed, rec);
@@ -3598,6 +3610,10 @@ match_records(struct ftrace_hash *hash, char *func, int len, char *mod) goto out_unlock;do_for_each_ftrace_rec (pg, rec) { + +if (rec->flags &FTRACE_FL_DISABLED ) +continue ; +if (ftrace_match_record (rec, &func_g, mod_match, exclude_mod)) { ret =enter_record (hash, rec, clear_filter);if (ret <0 ) {
@@ -3793,6 +3809,9 @@ register_ftrace_function_probe(char *glob, struct ftrace_probe_ops *ops,do_for_each_ftrace_rec (pg, rec) { +if (rec->flags &FTRACE_FL_DISABLED ) +continue ; +if (!ftrace_match_record (rec, &func_g,NULL ,0 ))continue ;
@@ -4685,6 +4704,9 @@ ftrace_set_func(unsigned long *array, int *idx, int size, char *buffer)do_for_each_ftrace_rec (pg, rec) { +if (rec->flags &FTRACE_FL_DISABLED ) +continue ; +if (ftrace_match_record (rec, &func_g,NULL ,0 )) {/* if it is in the array */ exists = false;
Signature · 1 line
2.10.2
Steven Rostedt
maintainerPATCH 3/6 ftrace: Support full glob matching
In reply to Steven RostedtFrom: Masami Hiramatsu <[email protected]>
Use glob_match() to support flexible glob wildcards (*,?) and character classes ([) for ftrace.
Since the full glob matching is slower than the current partial matching routines(*pat, pat*, *pat*), this leaves those routines and just add MATCH_GLOB for complex glob expression.
e.g.
----
[root@localhost tracing]# echo 'sched*group' > set_ftrace_filter [root@localhost tracing]# cat set_ftrace_filter sched_free_groupsched_change_groupsched_create_groupsched_online_groupsched_destroy_groupsched_offline_group
[root@localhost tracing]# echo '[Ss]y[Ss]_*' > set_ftrace_filter [root@localhost tracing]# head set_ftrace_filter sys_arch_prctlsys_rt_sigreturnsys_iopermSyS_ioplsys_modify_ldtSyS_mmapSyS_set_thread_areaSyS_get_thread_areaSyS_set_tid_addresssys_fork
----
--- Documentation/trace/events.txt | 9 +++------ Documentation/trace/ftrace.txt | 9 +++------ kernel/trace/Kconfig | 2 ++ kernel/trace/ftrace.c | 4 ++++ kernel/trace/trace.c | 2 +- kernel/trace/trace.h | 2 ++ kernel/trace/trace_events_filter.c | 17 ++++++++++++++++- 7 files changed, 31 insertions(+), 14 deletions(-)
Documentation/trace/events.txt +3 -6
@@ -189,16 +189,13 @@ And for string fields they are: ==, !=, ~ -The glob (~) only accepts a wild card character (*) at the start and or -end of the string. For example: +The glob (~) accepts a wild card character (*,?) and character classes +([). For example: prev_comm ~ "*sh" prev_comm ~ "sh*" prev_comm ~ "*sh*" - -But does not allow for it to be within the string: - - prev_comm ~ "ba*sh" <-- is invalid + prev_comm ~ "ba*sh" 5.2 Setting filters -------------------
Documentation/trace/ftrace.txt +3 -6
@@ -2218,16 +2218,13 @@ hrtimer_interrupt sys_nanosleep -Perhaps this is not enough. The filters also allow simple wild -cards. Only the following are currently available +Perhaps this is not enough. The filters also allow glob(7) matching. <match>* - will match functions that begin with <match> *<match> - will match functions that end with <match> *<match>* - will match functions that have <match> in it - -These are the only wild cards which are supported. - - <match>*<match> will not work. + <match1>*<match2> - will match functions that begin with + <match1> and end with <match2> Note: It is better to use quotes to enclose the wild cards, otherwise the shell may expand the parameters into names
kernel/trace/Kconfig +2 -0
@@ -70,6 +70,7 @@ config FTRACE_NMI_ENTER config EVENT_TRACING select CONTEXT_SWITCH_TRACER + select GLOB bool config CONTEXT_SWITCH_TRACER
@@ -133,6 +134,7 @@ config FUNCTION_TRACER select KALLSYMS select GENERIC_TRACER select CONTEXT_SWITCH_TRACER + select GLOB help Enable the kernel to trace every kernel function. This is done by using a compiler feature to insert a small, 5-byte No-Operation
kernel/trace/ftrace.c +4 -0
@@ -3511,6 +3511,10 @@ static int ftrace_match(char *str, struct ftrace_glob *g) memcmp(str + slen - g->len, g->search, g->len) == 0) matched =1 ;break ; +case MATCH_GLOB : +if (glob_match (g->search, str)) + matched =1 ; +break ; }return matched;
kernel/trace/trace.c +1 -1
@@ -4065,7 +4065,7 @@ static const char readme_msg[] ="\n available_filter_functions - list of functions that can be filtered on\n" " set_ftrace_filter\t- echo function name in here to only trace these\n" "\t\t\t functions\n" -"\t accepts: func_full_name, *func_end, func_begin*, *func_middle*\n" +"\t accepts: func_full_name or glob-matching-pattern\n" "\t modules: Can select a group via module\n" "\t Format: :mod:<module-name>\n" "\t example: echo :mod:ext3 > set_ftrace_filter\n"
kernel/trace/trace.h +2 -0
@@ -15,6 +15,7 @@#include <linux/trace_events.h> #include <linux/compiler.h> #include <linux/trace_seq.h> +#include <linux/glob.h> #ifdef CONFIG_FTRACE_SYSCALLS #include <asm/unistd.h> /* For NR_SYSCALLS */
@@ -1257,6 +1258,7 @@ enum regex_type {MATCH_FRONT_ONLY ,MATCH_MIDDLE_ONLY ,MATCH_END_ONLY , + MATCH_GLOB, }; struct regex {
kernel/trace/trace_events_filter.c +16 -1
@@ -344,6 +344,12 @@ static int regex_match_end(char *str, struct regex *r, int len)return 0 ; } +static int regex_match_glob (char *str,struct regex *r,int len__maybe_unused ) +{ +if (glob_match (r->pattern, str)) +return 1 ; +return 0 ; +}/** * filter_parse_regex - parse a basic regex * @buff: the raw regex
@@ -380,14 +386,20 @@ enum regex_type filter_parse_regex(char *buff, int len, char **search, int *not)if (!i) { *search = buff +1 ; type =MATCH_END_ONLY ; - }else { + }else if (i == len -1 ) {if (type ==MATCH_END_ONLY ) type =MATCH_MIDDLE_ONLY ;else type =MATCH_FRONT_ONLY ; buff[i] =0 ;break ; + }else {/* pattern continues, use full glob */ + type =MATCH_GLOB ; +break ; } + }else if (strchr ("[?\\" , buff[i])) { + type =MATCH_GLOB ; +break ; } }
@@ -420,6 +432,9 @@ static void filter_build_regex(struct filter_pred *pred)case MATCH_END_ONLY : r->match = regex_match_end;break ; +case MATCH_GLOB : + r->match = regex_match_glob; +break ; } pred->not ^= not;
Signature · 1 line
2.10.2
Steven Rostedt
maintainerPATCH 4/6 tracing: Optimise comparison filters and fix binary and for 64 bit
In reply to Steven RostedtFrom: "Steven Rostedt (Red Hat)" <[email protected]>
Currently the filter logic for comparisons (like greater-than and less-than) are used, they share the same function and a switch statement is used to jump to the comparison type to perform. This is done in the extreme hot path of the tracing code, and it does not take much more space to create a unique comparison function to perform each type of comparison and remove the switch statement.
Also, a bug was found where the binary and operation for 64 bits could fail if the resulting bits were greater than 32 bits, because the result was passed into a 32 bit variable. This was fixed when adding the separate binary and function.
--- kernel/trace/trace_events_filter.c | 80 +++++++++++++++++++++++--------------- 1 file changed, 48 insertions(+), 32 deletions(-)
kernel/trace/trace_events_filter.c +48 -32
@@ -145,34 +145,50 @@ struct pred_stack {/* If not of not match is equal to not of not, then it is a match */ #define DEFINE_COMPARISON_PRED (type) \ -static int filter_pred_##type(struct filter_pred *pred, void *event) \ +static int filter_pred_LT_##type(struct filter_pred *pred, void *event) \ { \ type *addr = (type *)(event + pred->offset); \ type val = (type)pred->val; \ - int match = 0; \ - \ - switch (pred->op) { \ - case OP_LT: \ - match = (*addr < val); \ - break; \ - case OP_LE: \ - match = (*addr <= val); \ - break; \ - case OP_GT: \ - match = (*addr > val); \ - break; \ - case OP_GE: \ - match = (*addr >= val); \ - break; \ - case OP_BAND: \ - match = (*addr & val); \ - break; \ - default: \ - break; \ - } \ - \ + int match = (*addr < val); \ return !!match == !pred->not; \ -} +} \ +static int filter_pred_LE_##type(struct filter_pred *pred, void *event) \ +{ \ + type *addr = (type *)(event + pred->offset); \ + type val = (type)pred->val; \ + int match = (*addr <= val); \ + return !!match == !pred->not; \ +} \ +static int filter_pred_GT_##type(struct filter_pred *pred, void *event) \ +{ \ + type *addr = (type *)(event + pred->offset); \ + type val = (type)pred->val; \ + int match = (*addr > val); \ + return !!match == !pred->not; \ +} \ +static int filter_pred_GE_##type(struct filter_pred *pred, void *event) \ +{ \ + type *addr = (type *)(event + pred->offset); \ + type val = (type)pred->val; \ + int match = (*addr >= val); \ + return !!match == !pred->not; \ +} \ +static int filter_pred_BAND_##type(struct filter_pred *pred, void *event) \ +{ \ + type *addr = (type *)(event + pred->offset); \ + type val = (type)pred->val; \ + int match = !!(*addr & val); \ + return match == !pred->not; \ +} \ +static const filter_pred_fn_t pred_funcs_##type[] = { \ + filter_pred_LT_##type, \ + filter_pred_LE_##type, \ + filter_pred_GT_##type, \ + filter_pred_GE_##type, \ + filter_pred_BAND_##type, \ +}; + +#define PRED_FUNC_START OP_LT#define DEFINE_EQUALITY_PRED (size) \ static int filter_pred_##size(struct filter_pred *pred, void *event) \
@@ -982,33 +998,33 @@ static filter_pred_fn_t select_comparison_fn(int op, int field_size,if (op ==OP_EQ || op ==OP_NE ) fn = filter_pred_64;else if (field_is_signed) - fn = filter_pred_s64; + fn = pred_funcs_s64[op -PRED_FUNC_START ];else - fn = filter_pred_u64; + fn = pred_funcs_u64[op -PRED_FUNC_START ];break ;case 4 :if (op ==OP_EQ || op ==OP_NE ) fn = filter_pred_32;else if (field_is_signed) - fn = filter_pred_s32; + fn = pred_funcs_s32[op -PRED_FUNC_START ];else - fn = filter_pred_u32; + fn = pred_funcs_u32[op -PRED_FUNC_START ];break ;case 2 :if (op ==OP_EQ || op ==OP_NE ) fn = filter_pred_16;else if (field_is_signed) - fn = filter_pred_s16; + fn = pred_funcs_s16[op -PRED_FUNC_START ];else - fn = filter_pred_u16; + fn = pred_funcs_u16[op -PRED_FUNC_START ];break ;case 1 :if (op ==OP_EQ || op ==OP_NE ) fn = filter_pred_8;else if (field_is_signed) - fn = filter_pred_s8; + fn = pred_funcs_s8[op -PRED_FUNC_START ];else - fn = filter_pred_u8; + fn = pred_funcs_u8[op -PRED_FUNC_START ];break ; }
Signature · 1 line
2.10.2
Steven Rostedt
maintainerPATCH 5/6 tracing/filter: Define op as the enum that it is
In reply to Steven RostedtFrom: "Steven Rostedt (Red Hat)" <[email protected]>
The trace_events_file.c filter logic can be a bit complex. I copy this into a userspace program where I can debug it a bit easier. One issue is the op is defined in most places as an int instead of as an enum, and gdb just gives the value when debugging. Having the actual op name shown in gdb is more useful.
This has no functionality change, but helps in debugging when the file is debugged in user space.
--- kernel/trace/trace_events_filter.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-)
kernel/trace/trace_events_filter.c +12 -10
@@ -108,12 +108,12 @@ static char *err_text[] = { };struct opstack_op { -int op; +enum filter_op_ids op;struct list_head list; };struct postfix_elt { -int op; +enum filter_op_ids op;char *operand;struct list_head list; };
@@ -977,7 +977,7 @@ int filter_assign_type(const char *type)return FILTER_OTHER ; } -static bool is_legal_op (struct ftrace_event_field *field,int op) +static bool is_legal_op (struct ftrace_event_field *field,enum filter_op_ids op) {if (is_string_field (field) && (op !=OP_EQ && op !=OP_NE && op !=OP_GLOB ))
@@ -988,8 +988,8 @@ static bool is_legal_op(struct ftrace_event_field *field, int op)return true; } -static filter_pred_fn_t select_comparison_fn (int op,int field_size, -int field_is_signed) +static filter_pred_fn_t select_comparison_fn (enum filter_op_ids op, +int field_size,int field_is_signed) {filter_pred_fn_t fn =NULL ;
@@ -1197,7 +1197,8 @@ static inline int append_operand_char(struct filter_parse_state *ps, char c)return 0 ; } -static int filter_opstack_push (struct filter_parse_state *ps,int op) +static int filter_opstack_push (struct filter_parse_state *ps, +enum filter_op_ids op) {struct opstack_op *opstack_op;
@@ -1231,7 +1232,7 @@ static int filter_opstack_top(struct filter_parse_state *ps)static int filter_opstack_pop (struct filter_parse_state *ps) {struct opstack_op *opstack_op; -int op; +enum filter_op_ids op;if (filter_opstack_empty (ps))return OP_NONE ;
@@ -1276,7 +1277,7 @@ static int postfix_append_operand(struct filter_parse_state *ps, char *operand)return 0 ; } -static int postfix_append_op (struct filter_parse_state *ps,int op) +static int postfix_append_op (struct filter_parse_state *ps,enum filter_op_ids op) {struct postfix_elt *elt;
@@ -1306,8 +1307,8 @@ static void postfix_clear(struct filter_parse_state *ps)static int filter_parse (struct filter_parse_state *ps) { +enum filter_op_ids op, top_op;int in_string =0 ; -int op, top_op;char ch;while ((ch =infix_next (ps))) {
@@ -1398,7 +1399,8 @@ static int filter_parse(struct filter_parse_state *ps)static struct filter_pred *create_pred (struct filter_parse_state *ps,struct trace_event_call *call, -int op,char *operand1,char *operand2) +enum filter_op_ids op, +char *operand1,char *operand2) {struct ftrace_event_field *field;static struct filter_pred pred;
Signature · 1 line
2.10.2
Steven Rostedt
maintainerPATCH 6/6 tracing: Allow wakeup_dl tracer to be used by instances
In reply to Steven RostedtFrom: Zhou Chengming <[email protected]>
Allow wakeup_dl tracer to be used by instances, like wakeup tracer and wakeup_rt tracer.
--- kernel/trace/trace_sched_wakeup.c | 1 + 1 file changed, 1 insertion(+)
kernel/trace/trace_sched_wakeup.c +1 -0
@@ -790,6 +790,7 @@ static struct tracer wakeup_dl_tracer __read_mostly =#endif .open = wakeup_trace_open, .close = wakeup_trace_close, + .allow_instances = true, .use_max_tr = true, };
Signature · 1 line
2.10.2