tracing: Updates for 4.15
Subsystems (3)
Steven Rostedt
maintainercover tracing: Updates for 4.15
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
for-next
Head SHA1: 6171a0310a06a7a0cb83713fa7068bdd4192de19
Colin Ian King (1):
tracing: Remove redundant unread variable ret
Joel Fernandes (1):
tracing: Remove obsolete sched_switch tracer selftest
Steven Rostedt (VMware) (7):
tracing: Reverse the order of trace_types_lock and event_mutex
ring-buffer: Rewrite trace_recursive_(un)lock() to be simpler
ftrace: Add a ftrace_free_mem() function for modules to use
ftrace: Allow module init functions to be traced
ftrace: Save module init functions kallsyms symbols for tracing
ftrace: Add freeing algorithm to free ftrace_mod_maps ftrace/kallsyms: Have /proc/kallsyms show saved mod init functions
Tom Zanussi (7):
tracing: Exclude 'generic fields' from histograms
tracing: Remove lookups from tracing_map hitcount
tracing: Increase tracing map KEYS_MAX size
tracing: Make traceprobe parsing code reusable
tracing: Clean up hist_field_flags enum
tracing: Add hist_field_name() accessor
tracing: Reimplement log2
----
include/linux/ftrace.h | 27 +++++ include/linux/init.h | 4 +- kernel/kallsyms.c | 43 +++++++- kernel/module.c | 2 + kernel/trace/ftrace.c | 230 ++++++++++++++++++++++++++++++++++++++- kernel/trace/ring_buffer.c | 64 +++-------- kernel/trace/trace.c | 91 ++++++++++++++++ kernel/trace/trace.h | 9 +- kernel/trace/trace_events.c | 31 +++--- kernel/trace/trace_events_hist.c | 128 +++++++++++++++------- kernel/trace/trace_kprobe.c | 18 +-- kernel/trace/trace_probe.c | 86 --------------- kernel/trace/trace_probe.h | 7 -- kernel/trace/trace_selftest.c | 32 ------ kernel/trace/trace_uprobe.c | 2 +- kernel/trace/tracing_map.c | 3 +- kernel/trace/tracing_map.h | 2 +- 17 files changed, 521 insertions(+), 258 deletions(-)
Steven Rostedt
maintainerPATCH 1/16 tracing: Remove obsolete sched_switch tracer selftest
From: Joel Fernandes <[email protected]>
Since commit 87d80de2800d087ea833cb79bc13f85ff34ed49f ("tracing: Remove obsolete sched_switch tracer"), the sched_switch tracer selftest is no longer used. This patch removes the same.
--- kernel/trace/trace.h | 2 -- kernel/trace/trace_selftest.c | 32 -------------------------------- 2 files changed, 34 deletions(-)
kernel/trace/trace.h +0 -2
@@ -738,8 +738,6 @@ extern int trace_selftest_startup_wakeup(struct tracer *trace, struct trace_array *tr);extern int trace_selftest_startup_nop (struct tracer *trace,struct trace_array *tr); -extern int trace_selftest_startup_sched_switch (struct tracer *trace, -struct trace_array *tr);extern int trace_selftest_startup_branch (struct tracer *trace,struct trace_array *tr);/*
kernel/trace/trace_selftest.c +0 -32
@@ -1150,38 +1150,6 @@ trace_selftest_startup_wakeup(struct tracer *trace, struct trace_array *tr) }#endif /* CONFIG_SCHED_TRACER */ -#ifdef CONFIG_CONTEXT_SWITCH_TRACER -int -trace_selftest_startup_sched_switch (struct tracer *trace,struct trace_array *tr) -{ -unsigned long count; -int ret; - -/* start the tracing */ - ret =tracer_init (trace, tr); -if (ret) { -warn_failed_init_tracer (trace, ret); -return ret; - } - -/* Sleep for a 1/10 of a second */ -msleep (100 ); -/* stop the tracing. */ -tracing_stop (); -/* check the trace buffer */ - ret =trace_test_buffer (&tr->trace_buffer, &count); - trace->reset (tr); -tracing_start (); - -if (!ret && !count) { -printk (KERN_CONT ".. no entries found .." ); - ret =-1 ; - } - -return ret; -} -#endif /* CONFIG_CONTEXT_SWITCH_TRACER */ -#ifdef CONFIG_BRANCH_TRACER int trace_selftest_startup_branch (struct tracer *trace,struct trace_array *tr)
Signature · 1 line
2.13.2
Steven Rostedt
maintainerPATCH 2/16 tracing: Remove redundant unread variable ret
In reply to Steven RostedtFrom: Colin Ian King <[email protected]>
Integer ret is being assigned but never used and hence it is redundant. Remove it, fixes clang warning:
trace_events_hist.c:1077:3: warning: Value stored to 'ret' is never read
--- kernel/trace/trace_events_hist.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
kernel/trace/trace_events_hist.c +2 -4
@@ -1062,7 +1062,7 @@ static void hist_trigger_show(struct seq_file *m, struct event_trigger_data *data, int n) {struct hist_trigger_data *hist_data; -int n_entries, ret =0 ; +int n_entries;if (n >0 )seq_puts (m,"\n\n" );
@@ -1073,10 +1073,8 @@ static void hist_trigger_show(struct seq_file *m, hist_data = data->private_data; n_entries =print_entries (m, hist_data); -if (n_entries <0 ) { - ret = n_entries; +if (n_entries <0 ) n_entries =0 ; - } seq_printf(m, "\nTotals:\n Hits: %llu\n Entries: %u\n Dropped: %llu\n", (u64)atomic64_read(&hist_data->map->hits),
Signature · 1 line
2.13.2
Steven Rostedt
maintainerPATCH 3/16 tracing: Reverse the order of trace_types_lock and event_mutex
In reply to Steven RostedtFrom: "Steven Rostedt (VMware)" <[email protected]>
In order to make future changes where we need to call tracing_set_clock() from within an event command, the order of trace_types_lock and event_mutex must be reversed, as the event command will hold event_mutex and the trace_types_lock is taken from within tracing_set_clock().
--- kernel/trace/trace.c | 5 +++++ kernel/trace/trace_events.c | 31 +++++++++++++++---------------- 2 files changed, 20 insertions(+), 16 deletions(-)
kernel/trace/trace.c +5 -0
@@ -7687,6 +7687,7 @@ static int instance_mkdir(const char *name)struct trace_array *tr;int ret; +mutex_lock (&event_mutex);mutex_lock (&trace_types_lock); ret = -EEXIST ;
@@ -7742,6 +7743,7 @@ static int instance_mkdir(const char *name)list_add (&tr->list, &ftrace_trace_arrays);mutex_unlock (&trace_types_lock); +mutex_unlock (&event_mutex);return 0 ;
@@ -7753,6 +7755,7 @@ static int instance_mkdir(const char *name) out_unlock:mutex_unlock (&trace_types_lock); +mutex_unlock (&event_mutex);return ret;
@@ -7765,6 +7768,7 @@ static int instance_rmdir(const char *name)int ret;int i; +mutex_lock (&event_mutex);mutex_lock (&trace_types_lock); ret = -ENODEV ;
@@ -7810,6 +7814,7 @@ static int instance_rmdir(const char *name) out_unlock:mutex_unlock (&trace_types_lock); +mutex_unlock (&event_mutex);return ret; }
kernel/trace/trace_events.c +15 -16
@@ -1406,8 +1406,8 @@ static int subsystem_open(struct inode *inode, struct file *filp)return -ENODEV ;/* Make sure the system still exists */ -mutex_lock (&trace_types_lock);mutex_lock (&event_mutex); +mutex_lock (&trace_types_lock);list_for_each_entry (tr, &ftrace_trace_arrays, list) {list_for_each_entry (dir, &tr->systems, list) {if (dir == inode->i_private) {
@@ -1421,8 +1421,8 @@ static int subsystem_open(struct inode *inode, struct file *filp) } } exit_loop: -mutex_unlock (&event_mutex);mutex_unlock (&trace_types_lock); +mutex_unlock (&event_mutex);if (!system)return -ENODEV ;
@@ -2294,15 +2294,15 @@ static void __add_event_to_tracers(struct trace_event_call *call);int trace_add_event_call (struct trace_event_call *call) {int ret; -mutex_lock (&trace_types_lock);mutex_lock (&event_mutex); +mutex_lock (&trace_types_lock); ret =__register_event (call,NULL );if (ret >=0 )__add_event_to_tracers (call); -mutex_unlock (&event_mutex);mutex_unlock (&trace_types_lock); +mutex_unlock (&event_mutex);return ret; }
@@ -2356,13 +2356,13 @@ int trace_remove_event_call(struct trace_event_call *call) {int ret; -mutex_lock (&trace_types_lock);mutex_lock (&event_mutex); +mutex_lock (&trace_types_lock);down_write (&trace_event_sem); ret =probe_remove_event_call (call);up_write (&trace_event_sem); -mutex_unlock (&event_mutex);mutex_unlock (&trace_types_lock); +mutex_unlock (&event_mutex);return ret; }
@@ -2424,8 +2424,8 @@ static int trace_module_notify(struct notifier_block *self, {struct module *mod = data; -mutex_lock (&trace_types_lock);mutex_lock (&event_mutex); +mutex_lock (&trace_types_lock);switch (val) {case MODULE_STATE_COMING :trace_module_add_events (mod);
@@ -2434,8 +2434,8 @@ static int trace_module_notify(struct notifier_block *self,trace_module_remove_events (mod);break ; } -mutex_unlock (&event_mutex);mutex_unlock (&trace_types_lock); +mutex_unlock (&event_mutex);return 0 ; }
@@ -2950,24 +2950,24 @@ create_event_toplevel_files(struct dentry *parent, struct trace_array *tr)* creates the event hierachry in the @parent/events directory. * * Returns 0 on success. +* +* Must be called with event_mutex held. */ int event_trace_add_tracer (struct dentry *parent,struct trace_array *tr) {int ret; -mutex_lock (&event_mutex); +lockdep_assert_held (&event_mutex); ret =create_event_toplevel_files (parent, tr);if (ret) - goto out_unlock; + goto out;down_write (&trace_event_sem);__trace_add_event_dirs (tr);up_write (&trace_event_sem); - out_unlock: -mutex_unlock (&event_mutex); - + out:return ret; }
@@ -2996,9 +2996,10 @@ early_event_add_tracer(struct dentry *parent, struct trace_array *tr)return ret; } +/* Must be called with event_mutex held */ int event_trace_del_tracer (struct trace_array *tr) { -mutex_lock (&event_mutex); +lockdep_assert_held (&event_mutex);/* Disable any event triggers and associated soft-disabled events */ clear_event_triggers (tr);
@@ -3019,8 +3020,6 @@ int event_trace_del_tracer(struct trace_array *tr) tr->event_dir =NULL ; -mutex_unlock (&event_mutex); -return 0 ; }
Signature · 1 line
2.13.2
Steven Rostedt
maintainerPATCH 4/16 ring-buffer: Rewrite trace_recursive_(un)lock() to be simpler
In reply to Steven RostedtFrom: "Steven Rostedt (VMware)" <[email protected]>
The current method to prevent the ring buffer from entering into a recursize loop is to use a bitmask and set the bit that maps to the current context (normal, softirq, irq or NMI), and if that bit was already set, it is considered a recursive loop.
New code is being added that may require the ring buffer to be entered a second time in the current context. The recursive locking prevents that from happening. Instead of mapping a bitmask to the current context, just allow 4 levels of nesting in the ring buffer. This matches the 4 context levels that it can already nest. It is highly unlikely to have more than two levels, thus it should be fine when we add the second entry into the ring buffer. If that proves to be a problem, we can always up the number to 8.
An added benefit is that reading preempt_count() to get the current level adds a very slight but noticeable overhead. This removes that need.
--- kernel/trace/ring_buffer.c | 64 ++++++++++++---------------------------------- 1 file changed, 17 insertions(+), 47 deletions(-)
kernel/trace/ring_buffer.c +17 -47
@@ -2538,61 +2538,29 @@ rb_wakeups(struct ring_buffer *buffer, struct ring_buffer_per_cpu *cpu_buffer)* The lock and unlock are done within a preempt disable section. * The current_context per_cpu variable can only be modified * by the current task between lock and unlock. But it can -* be modified more than once via an interrupt. To pass this -* information from the lock to the unlock without having to -* access the 'in_interrupt()' functions again (which do show -* a bit of overhead in something as critical as function tracing, -* we use a bitmask trick. +* be modified more than once via an interrupt. There are four +* different contexts that we need to consider. * -* bit 0 = NMI context -* bit 1 = IRQ context -* bit 2 = SoftIRQ context -* bit 3 = normal context. +* Normal context. +* SoftIRQ context +* IRQ context +* NMI context * -* This works because this is the order of contexts that can -* preempt other contexts. A SoftIRQ never preempts an IRQ -* context. -* -* When the context is determined, the corresponding bit is -* checked and set (if it was set, then a recursion of that context -* happened). -* -* On unlock, we need to clear this bit. To do so, just subtract -* 1 from the current_context and AND it to itself. -* -* (binary) -* 101 - 1 = 100 -* 101 & 100 = 100 (clearing bit zero) -* -* 1010 - 1 = 1001 -* 1010 & 1001 = 1000 (clearing bit 1) -* -* The least significant bit can be cleared this way, and it -* just so happens that it is the same bit corresponding to -* the current context. +* If for some reason the ring buffer starts to recurse, we +* only allow that to happen at most 4 times (one for each +* context). If it happens 5 times, then we consider this a +* recusive loop and do not let it go further. */ static __always_inline inttrace_recursive_lock (struct ring_buffer_per_cpu *cpu_buffer) { -unsigned int val = cpu_buffer->current_context; -int bit; - -if (in_interrupt ()) { -if (in_nmi ()) - bit =RB_CTX_NMI ; -else if (in_irq ()) - bit =RB_CTX_IRQ ; -else - bit =RB_CTX_SOFTIRQ ; - }else - bit =RB_CTX_NORMAL ; - -if (unlikely (val & (1 << bit))) +if (cpu_buffer->current_context >=4 )return 1 ; - val |= (1 << bit); - cpu_buffer->current_context = val; + cpu_buffer->current_context++; +/* Interrupts must see this update */ +barrier ();return 0 ; }
@@ -2600,7 +2568,9 @@ trace_recursive_lock(struct ring_buffer_per_cpu *cpu_buffer) static __always_inline voidtrace_recursive_unlock (struct ring_buffer_per_cpu *cpu_buffer) { - cpu_buffer->current_context &= cpu_buffer->current_context -1 ; +/* Don't let the dec leak out */ +barrier (); + cpu_buffer->current_context--; }/**
Signature · 1 line
2.13.2
Steven Rostedt
maintainerPATCH 5/16 tracing: Exclude generic fields from histograms
In reply to Steven RostedtFrom: Tom Zanussi <[email protected]>
There are a small number of 'generic fields' (comm/COMM/cpu/CPU) that are found by trace_find_event_field() but are only meant for filtering. Specifically, they unlike normal fields, they have a size of 0 and thus wreak havoc when used as a histogram key.
Exclude these (return -EINVAL) when used as histogram keys.
--- kernel/trace/trace_events_hist.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
kernel/trace/trace_events_hist.c +2 -2
@@ -450,7 +450,7 @@ static int create_val_field(struct hist_trigger_data *hist_data, } field =trace_find_event_field (file->event_call, field_name); -if (!field) { +if (!field || !field->size) { ret = -EINVAL ; goto out; }
@@ -548,7 +548,7 @@ static int create_key_field(struct hist_trigger_data *hist_data, } field =trace_find_event_field (file->event_call, field_name); -if (!field) { +if (!field || !field->size) { ret = -EINVAL ; goto out; }
Signature · 1 line
2.13.2
Steven Rostedt
maintainerPATCH 6/16 tracing: Remove lookups from tracing_map hitcount
In reply to Steven RostedtFrom: Tom Zanussi <[email protected]>
Lookups inflate the hitcount, making it essentially useless. Only inserts and updates should really affect the hitcount anyway, so explicitly filter lookups out.
--- kernel/trace/tracing_map.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
kernel/trace/tracing_map.c +2 -1
@@ -428,7 +428,8 @@ __tracing_map_insert(struct tracing_map *map, void *key, bool lookup_only)if (test_key && test_key == key_hash && entry->val &&keys_match (key, entry->val->key, map->key_size)) { -atomic64_inc (&map->hits); +if (!lookup_only) +atomic64_inc (&map->hits);return entry->val; }
Signature · 1 line
2.13.2
Steven Rostedt
maintainerPATCH 7/16 tracing: Increase tracing map KEYS_MAX size
In reply to Steven RostedtFrom: Tom Zanussi <[email protected]>
The current default for the number of subkeys in a compound key is 2, which is too restrictive. Increase it to a more realistic value of 3.
--- kernel/trace/tracing_map.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/trace/tracing_map.h +1 -1
@@ -5,7 +5,7 @@#define TRACING_MAP_BITS_MAX 17#define TRACING_MAP_BITS_MIN 7 -#define TRACING_MAP_KEYS_MAX 2 +#define TRACING_MAP_KEYS_MAX 3#define TRACING_MAP_VALS_MAX 3#define TRACING_MAP_FIELDS_MAX (TRACING_MAP_KEYS_MAX + \ TRACING_MAP_VALS_MAX)
Signature · 1 line
2.13.2
Steven Rostedt
maintainerPATCH 8/16 tracing: Make traceprobe parsing code reusable
In reply to Steven RostedtFrom: Tom Zanussi <[email protected]>
traceprobe_probes_write() and traceprobe_command() actually contain nothing that ties them to kprobes - the code is generically useful for similar types of parsing elsewhere, so separate it out and move it to trace.c/trace.h.
Other than moving it, the only change is in naming:traceprobe_probes_write() becomes trace_parse_run_command() and traceprobe_command() becomes trace_run_command().
--- kernel/trace/trace.c | 86 +++++++++++++++++++++++++++++++++++++++++++++ kernel/trace/trace.h | 7 ++++ kernel/trace/trace_kprobe.c | 18 +++++----- kernel/trace/trace_probe.c | 86 --------------------------------------------- kernel/trace/trace_probe.h | 7 ---- kernel/trace/trace_uprobe.c | 2 +- 6 files changed, 103 insertions(+), 103 deletions(-)
kernel/trace/trace.c +86 -0
@@ -8281,6 +8281,92 @@ void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) }EXPORT_SYMBOL_GPL (ftrace_dump); +int trace_run_command (const char *buf,int (*createfn)(int ,char **)) +{ +char **argv; +int argc, ret; + + argc =0 ; + ret =0 ; + argv =argv_split (GFP_KERNEL , buf, &argc); +if (!argv) +return -ENOMEM ; + +if (argc) + ret =createfn (argc, argv); + +argv_free (argv); + +return ret; +} + +#define WRITE_BUFSIZE 4096 + +ssize_t trace_parse_run_command (struct file *file,const char __user *buffer, +size_t count,loff_t *ppos, +int (*createfn)(int ,char **)) +{ +char *kbuf, *buf, *tmp; +int ret =0 ; +size_t done =0 ; +size_t size; + + kbuf =kmalloc (WRITE_BUFSIZE ,GFP_KERNEL ); +if (!kbuf) +return -ENOMEM ; + +while (done < count) { + size = count - done; + +if (size >=WRITE_BUFSIZE ) + size =WRITE_BUFSIZE -1 ; + +if (copy_from_user (kbuf, buffer + done, size)) { + ret = -EFAULT ; + goto out; + } + kbuf[size] ='\0' ; + buf = kbuf; +do { + tmp =strchr (buf,'\n' ); +if (tmp) { + *tmp ='\0' ; + size = tmp - buf +1 ; + }else { + size =strlen (buf); +if (done + size < count) { +if (buf != kbuf) +break ; +/* This can accept WRITE_BUFSIZE - 2 ('\n' + '\0') */ +pr_warn ("Line length is too long: Should be less than %d\n" , +WRITE_BUFSIZE -2 ); + ret = -EINVAL ; + goto out; + } + } + done += size; + +/* Remove comments */ + tmp =strchr (buf,'#' ); + +if (tmp) + *tmp ='\0' ; + + ret =trace_run_command (buf, createfn); +if (ret) + goto out; + buf += size; + + }while (done < count); + } + ret = done; + +out: +kfree (kbuf); + +return ret; +} +__init static int tracer_alloc_buffers (void ) {int ring_buf_size;
kernel/trace/trace.h +7 -0
@@ -1752,6 +1752,13 @@ void trace_printk_start_comm(void);int trace_keep_overwrite (struct tracer *tracer,u32 mask,int set);int set_tracer_flag (struct trace_array *tr,unsigned int mask,int enabled); +#define MAX_EVENT_NAME_LEN 64 + +extern int trace_run_command (const char *buf,int (*createfn)(int ,char **)); +extern ssize_t trace_parse_run_command (struct file *file, +const char __user *buffer,size_t count,loff_t *ppos, +int (*createfn)(int ,char **)); +/* * Normal trace_printk() and friends allocates special buffers * to do the manipulation, as well as saves the print formats
kernel/trace/trace_kprobe.c +9 -9
@@ -907,8 +907,8 @@ static int probes_open(struct inode *inode, struct file *file)static ssize_t probes_write (struct file *file,const char __user *buffer,size_t count,loff_t *ppos) { -return traceprobe_probes_write (file, buffer, count, ppos, - create_trace_kprobe); +return trace_parse_run_command (file, buffer, count, ppos, + create_trace_kprobe); }static const struct file_operations kprobe_events_ops = {
@@ -1433,9 +1433,9 @@ static __init int kprobe_trace_self_tests_init(void)pr_info ("Testing kprobe tracing: " ); - ret =traceprobe_command ("p:testprobe kprobe_trace_selftest_target " -"$stack $stack0 +0($stack)" , - create_trace_kprobe); + ret =trace_run_command ("p:testprobe kprobe_trace_selftest_target " +"$stack $stack0 +0($stack)" , + create_trace_kprobe);if (WARN_ON_ONCE (ret)) {pr_warn ("error on probing function entry.\n" ); warn++;
@@ -1455,8 +1455,8 @@ static __init int kprobe_trace_self_tests_init(void) } } - ret =traceprobe_command ("r:testprobe2 kprobe_trace_selftest_target " -"$retval" , create_trace_kprobe); + ret =trace_run_command ("r:testprobe2 kprobe_trace_selftest_target " +"$retval" , create_trace_kprobe);if (WARN_ON_ONCE (ret)) {pr_warn ("error on probing function return.\n" ); warn++;
@@ -1526,13 +1526,13 @@ static __init int kprobe_trace_self_tests_init(void)disable_trace_kprobe (tk, file); } - ret =traceprobe_command ("-:testprobe" , create_trace_kprobe); + ret =trace_run_command ("-:testprobe" , create_trace_kprobe);if (WARN_ON_ONCE (ret)) {pr_warn ("error on deleting a probe.\n" ); warn++; } - ret =traceprobe_command ("-:testprobe2" , create_trace_kprobe); + ret =trace_run_command ("-:testprobe2" , create_trace_kprobe);if (WARN_ON_ONCE (ret)) {pr_warn ("error on deleting a probe.\n" ); warn++;
kernel/trace/trace_probe.c +0 -86
92 lines not on this page · raw message
kernel/trace/trace_probe.h +0 -7
@@ -42,7 +42,6 @@#define MAX_TRACE_ARGS 128#define MAX_ARGSTR_LEN 63 -#define MAX_EVENT_NAME_LEN 64#define MAX_STRING_SIZE PATH_MAX/* Reserved field names */
@@ -356,12 +355,6 @@ extern void traceprobe_free_probe_arg(struct probe_arg *arg);extern int traceprobe_split_symbol_offset (char *symbol,unsigned long *offset); -extern ssize_t traceprobe_probes_write (struct file *file, -const char __user *buffer,size_t count,loff_t *ppos, -int (*createfn)(int ,char **)); - -extern int traceprobe_command (const char *buf,int (*createfn)(int ,char **)); -/* Sum up total data length for dynamic arraies (strings) */ static nokprobe_inline int__get_data_size (struct trace_probe *tp,struct pt_regs *regs)
kernel/trace/trace_uprobe.c +1 -1
@@ -651,7 +651,7 @@ static int probes_open(struct inode *inode, struct file *file)static ssize_t probes_write (struct file *file,const char __user *buffer,size_t count,loff_t *ppos) { -return traceprobe_probes_write (file, buffer, count, ppos, create_trace_uprobe); +return trace_parse_run_command (file, buffer, count, ppos, create_trace_uprobe); }static const struct file_operations uprobe_events_ops = {
Signature · 1 line
2.13.2
Steven Rostedt
maintainerPATCH 9/16 tracing: Clean up hist_field_flags enum
In reply to Steven RostedtFrom: Tom Zanussi <[email protected]>
As we add more flags, specifying explicit integers for the flag values becomes more unwieldy and error-prone - switch them over to left-shift values.
--- kernel/trace/trace_events_hist.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)
kernel/trace/trace_events_hist.c +10 -10
@@ -110,16 +110,16 @@ DEFINE_HIST_FIELD_FN(u8);#define HIST_KEY_SIZE_MAX (MAX_FILTER_STR_VAL + HIST_STACKTRACE_SIZE)enum hist_field_flags { -HIST_FIELD_FL_HITCOUNT =1 , -HIST_FIELD_FL_KEY =2 , -HIST_FIELD_FL_STRING =4 , -HIST_FIELD_FL_HEX =8 , -HIST_FIELD_FL_SYM =16 , -HIST_FIELD_FL_SYM_OFFSET =32 , -HIST_FIELD_FL_EXECNAME =64 , -HIST_FIELD_FL_SYSCALL =128 , -HIST_FIELD_FL_STACKTRACE =256 , -HIST_FIELD_FL_LOG2 =512 , +HIST_FIELD_FL_HITCOUNT =1 <<0 , +HIST_FIELD_FL_KEY =1 <<1 , +HIST_FIELD_FL_STRING =1 <<2 , +HIST_FIELD_FL_HEX =1 <<3 , +HIST_FIELD_FL_SYM =1 <<4 , +HIST_FIELD_FL_SYM_OFFSET =1 <<5 , +HIST_FIELD_FL_EXECNAME =1 <<6 , +HIST_FIELD_FL_SYSCALL =1 <<7 , +HIST_FIELD_FL_STACKTRACE =1 <<8 , +HIST_FIELD_FL_LOG2 =1 <<9 , };struct hist_trigger_attrs {
Signature · 1 line
2.13.2
Steven Rostedt
maintainerPATCH 10/16 tracing: Add hist_field_name() accessor
In reply to Steven RostedtFrom: Tom Zanussi <[email protected]>
In preparation for hist_fields that won't be strictly based on trace_event_fields, add a new hist_field_name() accessor to allow that flexibility and update associated users.
--- kernel/trace/trace_events_hist.c | 67 +++++++++++++++++++++++++++------------- 1 file changed, 45 insertions(+), 22 deletions(-)
kernel/trace/trace_events_hist.c +45 -22
@@ -146,6 +146,23 @@ struct hist_trigger_data {struct tracing_map *map; }; +static const char *hist_field_name (struct hist_field *field, +unsigned int level) +{ +const char *field_name ="" ; + +if (level >1 ) +return field_name; + +if (field->field) + field_name = field->field->name; + +if (field_name ==NULL ) + field_name ="" ; + +return field_name; +} +static hist_field_fn_t select_value_fn (int field_size,int field_is_signed) {hist_field_fn_t fn =NULL ;
@@ -653,7 +670,6 @@ static int is_descending(const char *str)static int create_sort_keys (struct hist_trigger_data *hist_data) {char *fields_str = hist_data->attrs->sort_key_str; -struct ftrace_event_field *field =NULL ;struct tracing_map_sort_key *sort_key;int descending, ret =0 ;unsigned int i, j;
@@ -670,7 +686,9 @@ static int create_sort_keys(struct hist_trigger_data *hist_data) }for (i =0 ; i <TRACING_MAP_SORT_KEYS_MAX ; i++) { +struct hist_field *hist_field;char *field_str, *field_name; +const char *test_name; sort_key = &hist_data->sort_keys[i];
@@ -703,8 +721,10 @@ static int create_sort_keys(struct hist_trigger_data *hist_data) }for (j =1 ; j < hist_data->n_fields; j++) { - field = hist_data->fields[j]->field; -if (field && (strcmp (field_name, field->name) ==0 )) { + hist_field = hist_data->fields[j]; + test_name =hist_field_name (hist_field,0 ); + +if (strcmp (field_name, test_name) ==0 ) { sort_key->field_idx = j; descending =is_descending (field_str);if (descending <0 ) {
@@ -952,6 +972,7 @@ hist_trigger_entry_print(struct seq_file *m,struct hist_field *key_field;char str[KSYM_SYMBOL_LEN ];bool multiline = false; +const char *field_name;unsigned int i;u64 uval;
@@ -963,26 +984,27 @@ hist_trigger_entry_print(struct seq_file *m,if (i > hist_data->n_vals)seq_puts (m,", " ); + field_name =hist_field_name (key_field,0 ); +if (key_field->flags &HIST_FIELD_FL_HEX ) { uval = *(u64 *)(key + key_field->offset); -seq_printf (m,"%s: %llx" , - key_field->field->name, uval); +seq_printf (m,"%s: %llx" , field_name, uval); }else if (key_field->flags &HIST_FIELD_FL_SYM ) { uval = *(u64 *)(key + key_field->offset);sprint_symbol_no_offset (str, uval); -seq_printf (m,"%s: [%llx] %-45s" , - key_field->field->name, uval, str); +seq_printf (m,"%s: [%llx] %-45s" , field_name, + uval, str); }else if (key_field->flags &HIST_FIELD_FL_SYM_OFFSET ) { uval = *(u64 *)(key + key_field->offset);sprint_symbol (str, uval); -seq_printf (m,"%s: [%llx] %-55s" , - key_field->field->name, uval, str); +seq_printf (m,"%s: [%llx] %-55s" , field_name, + uval, str); }else if (key_field->flags &HIST_FIELD_FL_EXECNAME ) {char *comm = elt->private_data; uval = *(u64 *)(key + key_field->offset); -seq_printf (m,"%s: %-16s[%10llu]" , - key_field->field->name, comm, uval); +seq_printf (m,"%s: %-16s[%10llu]" , field_name, + comm, uval); }else if (key_field->flags &HIST_FIELD_FL_SYSCALL ) {const char *syscall_name;
@@ -991,8 +1013,8 @@ hist_trigger_entry_print(struct seq_file *m,if (!syscall_name) syscall_name ="unknown_syscall" ; -seq_printf (m,"%s: %-30s[%3llu]" , - key_field->field->name, syscall_name, uval); +seq_printf (m,"%s: %-30s[%3llu]" , field_name, + syscall_name, uval); }else if (key_field->flags &HIST_FIELD_FL_STACKTRACE ) {seq_puts (m,"stacktrace:\n" ); hist_trigger_stacktrace_print(m,
@@ -1000,15 +1022,14 @@ hist_trigger_entry_print(struct seq_file *m, HIST_STACKTRACE_DEPTH); multiline = true; }else if (key_field->flags &HIST_FIELD_FL_LOG2 ) { -seq_printf (m,"%s: ~ 2^%-2llu" , key_field->field->name, +seq_printf (m,"%s: ~ 2^%-2llu" , field_name, *(u64 *)(key + key_field->offset)); }else if (key_field->flags &HIST_FIELD_FL_STRING ) { -seq_printf (m,"%s: %-50s" , key_field->field->name, +seq_printf (m,"%s: %-50s" , field_name, (char *)(key + key_field->offset)); }else { uval = *(u64 *)(key + key_field->offset); -seq_printf (m,"%s: %10llu" , key_field->field->name, - uval); +seq_printf (m,"%s: %10llu" , field_name, uval); } }
@@ -1021,13 +1042,13 @@ hist_trigger_entry_print(struct seq_file *m, tracing_map_read_sum(elt, HITCOUNT_IDX));for (i =1 ; i < hist_data->n_vals; i++) { + field_name =hist_field_name (hist_data->fields[i],0 ); +if (hist_data->fields[i]->flags &HIST_FIELD_FL_HEX ) { -seq_printf (m," %s: %10llx" , - hist_data->fields[i]->field->name, +seq_printf (m," %s: %10llx" , field_name,tracing_map_read_sum (elt, i)); }else { -seq_printf (m," %s: %10llu" , - hist_data->fields[i]->field->name, +seq_printf (m," %s: %10llu" , field_name,tracing_map_read_sum (elt, i)); } }
@@ -1140,7 +1161,9 @@ static const char *get_hist_field_flags(struct hist_field *hist_field)static void hist_field_print (struct seq_file *m,struct hist_field *hist_field) { -seq_printf (m,"%s" , hist_field->field->name); +const char *field_name =hist_field_name (hist_field,0 ); + +seq_printf (m,"%s" , field_name);if (hist_field->flags) {const char *flags_str =get_hist_field_flags (hist_field);
Signature · 1 line
2.13.2
Steven Rostedt
maintainerPATCH 11/16 tracing: Reimplement log2
In reply to Steven RostedtFrom: Tom Zanussi <[email protected]>
log2 as currently implemented applies only to u64 trace_event_field derived fields, and assumes that anything it's applied to is a u64 field.
To prepare for synthetic fields like latencies, log2 should be applicable to those as well, so take the opportunity now to fix the current problems as well as expand to more general uses.
log2 should be thought of as a chaining function rather than a field type. To enable this as well as possible future function implementations, add a hist_field operand array into the hist_field definition for this purpose, and make use of it to implement the log2 'function'.
--- kernel/trace/trace_events_hist.c | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-)
kernel/trace/trace_events_hist.c +27 -4
@@ -28,12 +28,16 @@ struct hist_field;typedef u64 (*hist_field_fn_t ) (struct hist_field *field,void *event); +#define HIST_FIELD_OPERANDS_MAX 2 +struct hist_field {struct ftrace_event_field *field;unsigned long flags;hist_field_fn_t fn;unsigned int size;unsigned int offset; +unsigned int is_signed; +struct hist_field *operands[HIST_FIELD_OPERANDS_MAX ]; };static u64 hist_field_none (struct hist_field *field,void *event)
@@ -71,7 +75,9 @@ static u64 hist_field_pstring(struct hist_field *hist_field, void *event)static u64 hist_field_log2 (struct hist_field *hist_field,void *event) { -u64 val = *(u64 *)(event + hist_field->field->offset); +struct hist_field *operand = hist_field->operands[0 ]; + +u64 val = operand->fn (operand, event);return (u64 )ilog2 (roundup_pow_of_two (val)); }
@@ -156,6 +162,8 @@ static const char *hist_field_name(struct hist_field *field,if (field->field) field_name = field->field->name; +else if (field->flags &HIST_FIELD_FL_LOG2 ) + field_name =hist_field_name (field->operands[0 ], ++level);if (field_name ==NULL ) field_name ="" ;
@@ -357,8 +365,20 @@ static const struct tracing_map_ops hist_trigger_elt_comm_ops = { .elt_init = hist_trigger_elt_comm_init, }; -static void destroy_hist_field (struct hist_field *hist_field) +static void destroy_hist_field (struct hist_field *hist_field, +unsigned int level) { +unsigned int i; + +if (level >2 ) +return ; + +if (!hist_field) +return ; + +for (i =0 ; i <HIST_FIELD_OPERANDS_MAX ; i++) +destroy_hist_field (hist_field->operands[i], level +1 ); +kfree (hist_field); }
@@ -385,7 +405,10 @@ static struct hist_field *create_hist_field(struct ftrace_event_field *field, }if (flags &HIST_FIELD_FL_LOG2 ) { +unsigned long fl = flags & ~HIST_FIELD_FL_LOG2 ; hist_field->fn = hist_field_log2; + hist_field->operands[0 ] =create_hist_field (field, fl); + hist_field->size = hist_field->operands[0 ]->size; goto out; }
@@ -405,7 +428,7 @@ static struct hist_field *create_hist_field(struct ftrace_event_field *field, hist_field->fn =select_value_fn (field->size, field->is_signed);if (!hist_field->fn) { -destroy_hist_field (hist_field); +destroy_hist_field (hist_field,0 );return NULL ; } }
@@ -422,7 +445,7 @@ static void destroy_hist_fields(struct hist_trigger_data *hist_data)for (i =0 ; i <TRACING_MAP_FIELDS_MAX ; i++) {if (hist_data->fields[i]) { -destroy_hist_field (hist_data->fields[i]); +destroy_hist_field (hist_data->fields[i],0 ); hist_data->fields[i] =NULL ; } }
Signature · 1 line
2.13.2
Steven Rostedt
maintainerPATCH 12/16 ftrace: Add a ftrace_free_mem() function for modules to use
In reply to Steven RostedtFrom: "Steven Rostedt (VMware)" <[email protected]>
In order to be able to trace module init functions, the module code needs to tell ftrace what is being freed when the init sections are freed. Use the code that the main init calls to tell ftrace to free the main init sections. This requires passing in a start and end address to free.
--- include/linux/ftrace.h | 2 ++ kernel/trace/ftrace.c | 14 +++++++++++--- 2 files changed, 13 insertions(+), 3 deletions(-)
include/linux/ftrace.h +2 -0
@@ -151,8 +151,10 @@ struct ftrace_ops_hash { };void ftrace_free_init_mem (void ); +void ftrace_free_mem (void *start,void *end);#else static inline void ftrace_free_init_mem (void ) { } +static inline void ftrace_free_mem (void *start,void *end) { }#endif /*
kernel/trace/ftrace.c +11 -3
@@ -5868,10 +5868,10 @@ void ftrace_module_init(struct module *mod) }#endif /* CONFIG_MODULES */ -void __init ftrace_free_init_mem (void ) +void ftrace_free_mem (void *start_ptr,void *end_ptr) { -unsigned long start = (unsigned long )(&__init_begin); -unsigned long end = (unsigned long )(&__init_end); +unsigned long start = (unsigned long )(start_ptr); +unsigned long end = (unsigned long )(end_ptr);struct ftrace_page **last_pg = &ftrace_pages_start;struct ftrace_page *pg;struct dyn_ftrace *rec;
@@ -5913,6 +5913,14 @@ void __init ftrace_free_init_mem(void)mutex_unlock (&ftrace_lock); } +void __init ftrace_free_init_mem (void ) +{ +void *start = (void *)(&__init_begin); +void *end = (void *)(&__init_end); + +ftrace_free_mem (start, end); +} +void __init ftrace_init (void ) {extern unsigned long __start_mcount_loc[];
Signature · 1 line
2.13.2
Steven Rostedt
maintainerPATCH 13/16 ftrace: Allow module init functions to be traced
In reply to Steven RostedtFrom: "Steven Rostedt (VMware)" <[email protected]>
Allow for module init sections to be traced as well as core kernel init sections. Now that filtering modules functions can be stored, for when they are loaded, it makes sense to be able to trace them.
--- include/linux/init.h | 4 +--- kernel/module.c | 2 ++ kernel/trace/ftrace.c | 6 ++++-- 3 files changed, 7 insertions(+), 5 deletions(-)
include/linux/init.h +1 -3
@@ -39,7 +39,7 @@/* These are for everybody (although not all archs will actually discard it in modules) */ -#define __init __section(.init.text) __cold __inittrace __latent_entropy +#define __init __section(.init.text) __cold __latent_entropy#define __initdata __section(.init.data)#define __initconst __section(.init.rodata)#define __exitdata __section(.exit.data)
@@ -68,10 +68,8 @@#ifdef MODULE #define __exitused -#define __inittrace notrace#else #define __exitused __used -#define __inittrace#endif #define __exit __section(.exit.text) __exitused __cold notrace
kernel/module.c +2 -0
@@ -3473,6 +3473,8 @@ static noinline int do_init_module(struct module *mod)if (!mod->async_probe_requested && (current->flags &PF_USED_ASYNC ))async_synchronize_full (); +ftrace_free_mem (mod->init_layout.base, mod->init_layout.base + + mod->init_layout.size);mutex_lock (&module_mutex);/* Drop initial reference. */ module_put (mod);
kernel/trace/ftrace.c +4 -2
@@ -5752,7 +5752,8 @@ void ftrace_release_mod(struct module *mod) last_pg = &ftrace_pages_start;for (pg = ftrace_pages_start; pg; pg = *last_pg) { rec = &pg->records[0 ]; -if (within_module_core (rec->ip, mod)) { +if (within_module_core (rec->ip, mod) || +within_module_init (rec->ip, mod)) {/* * As core pages are first, the first * page should never be a module page.
@@ -5821,7 +5822,8 @@ void ftrace_module_enable(struct module *mod)* not part of this module, then skip this pg, * which the "break" will do. */ -if (!within_module_core (rec->ip, mod)) +if (!within_module_core (rec->ip, mod) && + !within_module_init (rec->ip, mod))break ; cnt =0 ;
Signature · 1 line
2.13.2
Steven Rostedt
maintainerPATCH 14/16 ftrace: Save module init functions kallsyms symbols for tracing
In reply to Steven RostedtFrom: "Steven Rostedt (VMware)" <[email protected]>
If function tracing is active when the module init functions are freed, then store them to be referenced by kallsyms. As module init functions can now be traced on module load, they were useless:
Quoted 4 lines
# echo ':mod:snd_seq' > set_ftrace_filter # echo function > current_tracer # modprobe snd_seq # cat trace
# tracer: function
#
# _-----=> irqs-off
# / _----=> need-resched
# | / _---=> hardirq/softirq
# || / _--=> preempt-depth
# ||| / delay
# TASK-PID CPU# |||| TIMESTAMP FUNCTION
# | | | |||| | |
modprobe-2786 [000] .... 3189.037874: 0xffffffffa0860000 <-do_one_initcall
modprobe-2786 [000] .... 3189.037876: 0xffffffffa086004d <-0xffffffffa086000f
modprobe-2786 [000] .... 3189.037876: 0xffffffffa086010d <-0xffffffffa0860018
modprobe-2786 [000] .... 3189.037877: 0xffffffffa086011a <-0xffffffffa0860021
modprobe-2786 [000] .... 3189.037877: 0xffffffffa0860080 <-0xffffffffa086002a
modprobe-2786 [000] .... 3189.039523: 0xffffffffa0860400 <-0xffffffffa0860033
modprobe-2786 [000] .... 3189.039523: 0xffffffffa086038a <-0xffffffffa086041c
modprobe-2786 [000] .... 3189.039591: 0xffffffffa086038a <-0xffffffffa0860436
modprobe-2786 [000] .... 3189.039657: 0xffffffffa086038a <-0xffffffffa0860450
modprobe-2786 [000] .... 3189.039719: 0xffffffffa0860127 <-0xffffffffa086003c
modprobe-2786 [000] .... 3189.039742: snd_seq_create_kernel_client <-0xffffffffa08601f6When the output is shown, the kallsyms for the module init functions have already been freed, and the output of the trace can not convert them to their function names.
Now this looks like this:
# tracer: function
#
# _-----=> irqs-off
# / _----=> need-resched
# | / _---=> hardirq/softirq
# || / _--=> preempt-depth
# ||| / delay
# TASK-PID CPU# |||| TIMESTAMP FUNCTION
# | | | |||| | |
modprobe-2463 [002] .... 174.243237: alsa_seq_init <-do_one_initcall
modprobe-2463 [002] .... 174.243239: client_init_data <-alsa_seq_init
modprobe-2463 [002] .... 174.243240: snd_sequencer_memory_init <-alsa_seq_init
modprobe-2463 [002] .... 174.243240: snd_seq_queues_init <-alsa_seq_init
modprobe-2463 [002] .... 174.243240: snd_sequencer_device_init <-alsa_seq_init
modprobe-2463 [002] .... 174.244860: snd_seq_info_init <-alsa_seq_init
modprobe-2463 [002] .... 174.244861: create_info_entry <-snd_seq_info_init
modprobe-2463 [002] .... 174.244936: create_info_entry <-snd_seq_info_init
modprobe-2463 [002] .... 174.245003: create_info_entry <-snd_seq_info_init
modprobe-2463 [002] .... 174.245072: snd_seq_system_client_init <-alsa_seq_init
modprobe-2463 [002] .... 174.245094: snd_seq_create_kernel_client <-snd_seq_system_client_init--- include/linux/ftrace.h | 20 ++++++- kernel/kallsyms.c | 5 ++ kernel/module.c | 2 +- kernel/trace/ftrace.c | 146 ++++++++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 168 insertions(+), 5 deletions(-)
include/linux/ftrace.h +18 -2
@@ -51,6 +51,21 @@ static inline void early_trace_init(void) { }struct module ;struct ftrace_hash ; +#if defined(CONFIG_FUNCTION_TRACER ) && defined(CONFIG_MODULES ) && \ + defined(CONFIG_DYNAMIC_FTRACE ) +const char * +ftrace_mod_address_lookup (unsigned long addr,unsigned long *size, +unsigned long *off,char **modname,char *sym); +#else +static inline const char * +ftrace_mod_address_lookup (unsigned long addr,unsigned long *size, +unsigned long *off,char **modname,char *sym) +{ +return NULL ; +} +#endif + +#ifdef CONFIG_FUNCTION_TRACER extern int ftrace_enabled;
@@ -151,10 +166,10 @@ struct ftrace_ops_hash { };void ftrace_free_init_mem (void ); -void ftrace_free_mem (void *start,void *end); +void ftrace_free_mem (struct module *mod,void *start,void *end);#else static inline void ftrace_free_init_mem (void ) { } -static inline void ftrace_free_mem (void *start,void *end) { } +static inline void ftrace_free_mem (struct module *mod,void *start,void *end) { }#endif /*
@@ -272,6 +287,7 @@ static inline int ftrace_nr_registered_ops(void)static inline void clear_ftrace_function (void ) { }static inline void ftrace_kill (void ) { }static inline void ftrace_free_init_mem (void ) { } +static inline void ftrace_free_mem (struct module *mod,void *start,void *end) { }#endif /* CONFIG_FUNCTION_TRACER */ #ifdef CONFIG_STACK_TRACER
kernel/kallsyms.c +5 -0
@@ -24,6 +24,7 @@#include <linux/ctype.h> #include <linux/slab.h> #include <linux/filter.h> +#include <linux/ftrace.h> #include <linux/compiler.h> #include <asm/sections.h>
@@ -337,6 +338,10 @@ const char *kallsyms_lookup(unsigned long addr,if (!ret) ret =bpf_address_lookup (addr, symbolsize, offset, modname, namebuf); + +if (!ret) + ret =ftrace_mod_address_lookup (addr, symbolsize, + offset, modname, namebuf);return ret; }
kernel/module.c +1 -1
@@ -3473,7 +3473,7 @@ static noinline int do_init_module(struct module *mod)if (!mod->async_probe_requested && (current->flags &PF_USED_ASYNC ))async_synchronize_full (); -ftrace_free_mem (mod->init_layout.base, mod->init_layout.base + +ftrace_free_mem (mod, mod->init_layout.base, mod->init_layout.base + mod->init_layout.size);mutex_lock (&module_mutex);/* Drop initial reference. */
kernel/trace/ftrace.c +144 -2
184 lines not on this page · raw message
Signature · 1 line
2.13.2
Steven Rostedt
maintainerPATCH 15/16 ftrace: Add freeing algorithm to free ftrace_mod_maps
In reply to Steven RostedtFrom: "Steven Rostedt (VMware)" <[email protected]>
The ftrace_mod_map is a descriptor to save module init function names in case they were traced, and the trace output needs to reference the function name from the function address. But after the function is unloaded, it the maps should be freed, as the rest of the function names are as well.
--- kernel/trace/ftrace.c | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-)
kernel/trace/ftrace.c +30 -2
@@ -5683,6 +5683,7 @@ struct ftrace_mod_func { };struct ftrace_mod_map { +struct rcu_head rcu;struct list_head list;struct module *mod;unsigned long start_addr;
@@ -5694,6 +5695,8 @@ struct ftrace_mod_map {#define next_to_ftrace_page (p) container_of(p, struct ftrace_page, next) +static LIST_HEAD (ftrace_mod_maps); +static int referenced_filters (struct dyn_ftrace *rec) {struct ftrace_ops *ops;
@@ -5747,8 +5750,26 @@ static void clear_mod_from_hashes(struct ftrace_page *pg)mutex_unlock (&trace_types_lock); } +static void ftrace_free_mod_map (struct rcu_head *rcu) +{ + struct ftrace_mod_map *mod_map = container_of(rcu, struct ftrace_mod_map, rcu); +struct ftrace_mod_func *mod_func; +struct ftrace_mod_func *n; + +/* All the contents of mod_map are now not visible to readers */ +list_for_each_entry_safe (mod_func, n, &mod_map->funcs, list) { +kfree (mod_func->name); +list_del (&mod_func->list); +kfree (mod_func); + } + +kfree (mod_map); +} +void ftrace_release_mod (struct module *mod) { +struct ftrace_mod_map *mod_map; +struct ftrace_mod_map *n;struct dyn_ftrace *rec;struct ftrace_page **last_pg;struct ftrace_page *tmp_page =NULL ;
@@ -5760,6 +5781,14 @@ void ftrace_release_mod(struct module *mod)if (ftrace_disabled) goto out_unlock; +list_for_each_entry_safe (mod_map, n, &ftrace_mod_maps, list) { +if (mod_map->mod == mod) { +list_del_rcu (&mod_map->list); +call_rcu_sched (&mod_map->rcu, ftrace_free_mod_map); +break ; + } + } +/* * Each module has its own ftrace_pages, remove * them from the list.
@@ -5914,8 +5943,6 @@ static void save_ftrace_mod_rec(struct ftrace_mod_map *mod_map,list_add_rcu (&mod_func->list, &mod_map->funcs); } -static LIST_HEAD (ftrace_mod_maps); -static struct ftrace_mod_map *allocate_ftrace_mod_map (struct module *mod,unsigned long start,unsigned long end)
@@ -5974,6 +6001,7 @@ ftrace_mod_address_lookup(unsigned long addr, unsigned long *size,struct ftrace_mod_map *mod_map;const char *ret =NULL ; +/* mod_map is freed via call_rcu_sched() */ preempt_disable ();list_for_each_entry_rcu (mod_map, &ftrace_mod_maps, list) { ret =ftrace_func_address_lookup (mod_map, addr, size, off, sym);
Signature · 1 line
2.13.2
Re: [for-next][PATCH 15/16] ftrace: Add freeing algorithm to free ftrace_mod_maps
Hi Steve,
Quoted 6 lines · Steven Rostedt
From: "Steven Rostedt (VMware)" <[email protected]>
The
ftrace_mod_mapis a descriptor to save module init function names in case they were traced, and the trace output needs to reference the function name from the function address. But after the function is unloaded, it the maps should be freed, as the rest of the function names are as well.
Just checking for my understanding of this patch - wouldn't this also mean that if there were any look ups of the init functions that may be needed at trace output time, then those look ups wont be possible any more after module is unloaded?
I guess having a reference somehow on the ftrace_mod_map descriptor if there are any entries in the trace buffer that need it can help prevent that but it could be too expensive for not much return since most likely the user wouldn't unload modules before trace collection in normal usage.
thanks,
- Joel
Steven Rostedt
maintainerRe: [for-next][PATCH 15/16] ftrace: Add freeing algorithm to free ftrace_mod_maps
Quoted 10 lines · On Fri, 6 Oct 2017 23:41:25 -0700 "Joel Fernandes (Google)"…
Hi Steve,
Quoted 6 lines · Steven Rostedt
From: "Steven Rostedt (VMware)" <[email protected]>
The
ftrace_mod_mapis a descriptor to save module init function names in case they were traced, and the trace output needs to reference the function name from the function address. But after the function is unloaded, it the maps should be freed, as the rest of the function names are as well.
Quoted 4 lines
Just checking for my understanding of this patch - wouldn't this also mean that if there were any look ups of the init functions that may be needed at trace output time, then those look ups wont be possible any more after module is unloaded?
Yes. That's true for all functions in the module. When a module is unloaded, all references to it in kallsyms is also freed. Try it on a current kernel. Trace a function in a module, then unload that module. The trace data will just show the ip hex address of the module function after that.
Quoted 6 lines
I guess having a reference somehow on the
ftrace_mod_mapdescriptor if there are any entries in the trace buffer that need it can help prevent that but it could be too expensive for not much return since most likely the user wouldn't unload modules before trace collection in normal usage.
Right, I have thought about this, and I haven't come up with an inexpensive way to do this. As this has been the default operation of all module functions, and I haven't heard much complaining about it (I think I may have had a single complaint), I didn't put too much effort into it.
I need to look at the approach that Jessica sent me. Perhaps there's ways to have all module function names be saved by a trace. But mapping which trace buffer has these names may be difficult.
-- Steve
Re: [for-next][PATCH 15/16] ftrace: Add freeing algorithm to free ftrace_mod_maps
Hi Steve,
Quoted 18 lines · Steven Rostedt
Quoted 10 lines · On Fri, 6 Oct 2017 23:41:25 -0700 "Joel Fernandes (Google)"…
Hi Steve,
Quoted 6 lines · Steven Rostedt
From: "Steven Rostedt (VMware)" <[email protected]>
The
ftrace_mod_mapis a descriptor to save module init function names in case they were traced, and the trace output needs to reference the function name from the function address. But after the function is unloaded, it the maps should be freed, as the rest of the function names are as well.Quoted 4 lines
Just checking for my understanding of this patch - wouldn't this also mean that if there were any look ups of the init functions that may be needed at trace output time, then those look ups wont be possible any more after module is unloaded?
Quoted 6 lines
Yes. That's true for all functions in the module. When a module is unloaded, all references to it in kallsyms is also freed. Try it on a current kernel. Trace a function in a module, then unload that module. The trace data will just show the ip hex address of the module function after that.
I tried your core branch and I see some weirdness with the filters:
Here's the code for the module:
================================================
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/init.h>
void bar(void)
{
printk(KERN_INFO "bar!\n");
}void foo(void)
{
printk(KERN_INFO "foo!\n");
bar();
}static int __init hello_init(void)
{
printk(KERN_INFO "Hello world!\n");
foo();
return 0;
}static void __exit hello_cleanup(void)
{
printk(KERN_INFO "Cleaning up module.\n");
}module_init(hello_init);module_exit(hello_cleanup);
================================================
Following is a run with function tracing, during the first run I see foo and bar are setup in the filters. After that when I unload and load the module, I see that only "bar" is in the filters:
bash-4.3# echo '*:mod:test' > /d/tracing/set_ftrace_filter bash-4.3# echo function > /d/tracing/current_tracer bash-4.3# cat /d/tracing/set_ftrace_filter *:mod:test
bash-4.3# modprobe test
bash-4.3# cat /d/tracing/trace
# tracer: function
#
# _-----=> irqs-off
# / _----=> need-resched
# | / _---=> hardirq/softirq
# || / _--=> preempt-depth
# ||| / delay
# TASK-PID CPU# |||| TIMESTAMP FUNCTION
# | | | |||| | |
modprobe-1050 [003] .... 1.653000: hello_init <-do_one_initcallbash-4.3# cat /d/tracing/set_ftrace_filter
bar [test]
foo [test]
bash-4.3# rmmod test
bash-4.3# cat /d/tracing/set_ftrace_filter
bash-4.3# sleep 2
bash-4.3# modprobe test
bash-4.3# cat /d/tracing/trace
# tracer: function
#
# _-----=> irqs-off
# / _----=> need-resched
# | / _---=> hardirq/softirq
# || / _--=> preempt-depth
# ||| / delay
# TASK-PID CPU# |||| TIMESTAMP FUNCTION
# | | | |||| | |
modprobe-1050 [003] .... 1.653000: bar <-do_one_initcallbash-4.3# cat /d/tracing/set_ftrace_filter
bar [test] <--- bar is still in
the filter list
bash-4.3#
Interestingly this also shows that the symbol conversion can be unreliable if another module was loaded into the same address, in this case 'bar' was loaded at the same address as 'hello_init' the second time so during the second cat of the trace file, it shows a different symbol name.
Also could you let me know what is the correct behavior of the filters after a module being traced is unloaded, are the filters supposed to be setup again after the module is unloaded, before the module can be loaded again?
Also I wasn't able to see the call from hello_init -> bar and bar -> foo so I'm not fully sure if that's a bug or I did something wrong. I'm happy to try out anything you suggest.
Quoted 6 lines
Quoted 5 lines
I guess having a reference somehow on the
ftrace_mod_mapdescriptor if there are any entries in the trace buffer that need it can help prevent that but it could be too expensive for not much return since most likely the user wouldn't unload modules before trace collection in normal usage.
Quoted 7 lines
Right, I have thought about this, and I haven't come up with an inexpensive way to do this. As this has been the default operation of all module functions, and I haven't heard much complaining about it (I think I may have had a single complaint), I didn't put too much effort into it.
I need to look at the approach that Jessica sent me. Perhaps there's
Yes, I think that approach is better. But I think it would have the same issue (that if the init sections are freed and say used by other loaded modules), then the trace output would incorrectly show a different symbol name than the one from the module at the time the trace record was generated.
thanks,
- Joel
Re: [for-next][PATCH 15/16] ftrace: Add freeing algorithm to free ftrace_mod_maps
Hi Steve,
Quoted 1 line
"Joel Fernandes (Google)" <[email protected]> wrote:
[..]
Quoted 4 lines
Also could you let me know what is the correct behavior of the filters after a module being traced is unloaded, are the filters supposed to be setup again after the module is unloaded, before the module can be loaded again?
Actually I learnt that it doesn't get preserved and wrote a patch to fix that, let me know what you think, thanks. (its only lightly tested - checked that the filters are preserved, will do more testing tomorrow):
---8<---
From: Joel Fernandes <[email protected]>
Subject: [PATCH RFC] ftrace: Preserve filters across loading/unloading of module
When a module is removed, the filters associated with it is cleared, however it doesn't make sense to clear it in the following work flow:
1. setup filters (:mod:<mod-name>)
2. load module
3. unload module
4. load module
In step 4, the filters would have gone. Since the recently added mechanism to do step 1 before 2 (deferred execution of the module command string) is already present, we can reuse the same mechanism to preserve the filters. This patch does that and avoids losing the filters and having to set them up again.
--- kernel/trace/ftrace.c | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-)
kernel/trace/ftrace.c +20 -5
@@ -5693,7 +5693,9 @@ static int referenced_filters(struct dyn_ftrace *rec) }static void -clear_mod_from_hash (struct ftrace_page *pg,struct ftrace_hash *hash) +deactivate_mod_from_hash (struct module *mod,struct ftrace_page *pg, +struct trace_array *tr,int enable, +struct ftrace_hash *hash) {struct ftrace_func_entry *entry;struct dyn_ftrace *rec;
@@ -5712,11 +5714,22 @@ clear_mod_from_hash(struct ftrace_page *pg, struct ftrace_hash *hash)*/ if (entry) entry->ip =0 ; + +/* +* The hash has been made cleared, however lets save it as a +* string for the next time the module is loaded. +*/ +if (entry) { +char func[KSYM_SYMBOL_LEN ]; +kallsyms_lookup (rec->ip,NULL ,NULL ,NULL , func); +ftrace_add_mod (tr, func, mod->name, enable); + } } }/* Clear any records from hashs */ -static void clear_mod_from_hashes (struct ftrace_page *pg) +static void deactivate_mod_from_hashes (struct module *mod, +struct ftrace_page *pg) {struct trace_array *tr;
@@ -5725,8 +5738,10 @@ static void clear_mod_from_hashes(struct ftrace_page *pg)if (!tr->ops || !tr->ops->func_hash)continue ;mutex_lock (&tr->ops->func_hash->regex_lock); -clear_mod_from_hash (pg, tr->ops->func_hash->filter_hash); -clear_mod_from_hash (pg, tr->ops->func_hash->notrace_hash); +deactivate_mod_from_hash (mod, pg, tr,1 , + tr->ops->func_hash->filter_hash); +deactivate_mod_from_hash (mod, pg, tr,0 , + tr->ops->func_hash->notrace_hash);mutex_unlock (&tr->ops->func_hash->regex_lock); }mutex_unlock (&trace_types_lock);
@@ -5778,7 +5793,7 @@ void ftrace_release_mod(struct module *mod)for (pg = tmp_page; pg; pg = tmp_page) {/* Needs to be called outside of ftrace_lock */ -clear_mod_from_hashes (pg); +deactivate_mod_from_hashes (mod, pg); order =get_count_order (pg->size /ENTRIES_PER_PAGE );free_pages ((unsigned long )pg->records, order);
Signature · 1 line
2.14.2.920.gcf0c67979c-goog
Steven Rostedt
maintainerRe: [for-next][PATCH 15/16] ftrace: Add freeing algorithm to free ftrace_mod_maps
Quoted 4 lines · On Sun, 8 Oct 2017 01:42:15 -0700 Joel Fernandes <[email protected]> wrote:
Hi Steve,
Quoted 1 line
"Joel Fernandes (Google)" <[email protected]> wrote:
[..]
Quoted 8 lines
Quoted 4 lines
Also could you let me know what is the correct behavior of the filters after a module being traced is unloaded, are the filters supposed to be setup again after the module is unloaded, before the module can be loaded again?
Actually I learnt that it doesn't get preserved and wrote a patch to fix that, let me know what you think, thanks. (its only lightly tested - checked that the filters are preserved, will do more testing tomorrow):
They should not be preserved, it's too non-deterministic.
I'm wondering why the setting of the ip to zero didn't keep it from showing up again. I'll have to investigate that.
-- Steve
Re: [for-next][PATCH 15/16] ftrace: Add freeing algorithm to free ftrace_mod_maps
Hi Steve,
Quoted 1 line · Steven Rostedt
On Sun, 8 Oct 2017 01:42:15 -0700
[..]
Quoted 7 lines
Quoted 7 lines
Quoted 1 line
"Joel Fernandes (Google)" <[email protected]> wrote:
[..]
Quoted 4 lines
Also could you let me know what is the correct behavior of the filters after a module being traced is unloaded, are the filters supposed to be setup again after the module is unloaded, before the module can be loaded again?
Quoted 5 lines
Quoted 3 lines
Actually I learnt that it doesn't get preserved and wrote a patch to fix that, let me know what you think, thanks. (its only lightly tested - checked that the filters are preserved, will do more testing tomorrow):
They should not be preserved, it's too non-deterministic.
Could you elaborate more on non-deterministic?
Say a user setup filters with '*:mod:<mod-name>' and unloads and loads, then the filter carrying forward for the second load helps avoid having them to set it up again.
OTOH I agree its not a big deal to set up the filters again, and probably not worth save it if it complicates the code too much to handle all cases.
Quoted 2 lines
I'm wondering why the setting of the ip to zero didn't keep it from showing up again. I'll have to investigate that.
Ok. Happy to help test anything you want to try :) I *think* the 'bar' function that showed up the second time around is actually the address of the init function in the previous load, and probably the new code you added switched it on but I have yet to debug it (you'll probably beat me to it since you wrote this code ;)).
thanks,
- Joel
Re: [for-next][PATCH 15/16] ftrace: Add freeing algorithm to free ftrace_mod_maps
In reply to Steven RostedtHi Steve,
Quoted 6 lines · Steven Rostedt
Quoted 6 lines
Quoted 5 lines
"Joel Fernandes (Google)" <[email protected]> wrote:
Also could you let me know what is the correct behavior of the filters after a module being traced is unloaded, are the filters supposed to be setup again after the module is unloaded, before the module can be loaded again?
Quoted 9 lines
Quoted 3 lines
Actually I learnt that it doesn't get preserved and wrote a patch to fix that, let me know what you think, thanks. (its only lightly tested - checked that the filters are preserved, will do more testing tomorrow):
They should not be preserved, it's too non-deterministic.
I'm wondering why the setting of the ip to zero didn't keep it from showing up again. I'll have to investigate that.
I found that the entries weren't being cleared for the init functions as the records weren't available at module unload time. Posted a patch to fix this: https://lkml.org/lkml/2017/10/8/196
thanks,
- Joel
Re: [for-next][PATCH 15/16] ftrace: Add freeing algorithm to free ftrace_mod_maps
In reply to Joel Fernandes (Google)Hi Steve,
On Sat, Oct 7, 2017 at 9:52 PM, Joel Fernandes (Google) <[email protected]> wrote:
[..]
Quoted 55 lines
Quoted 14 lines
Quoted 9 lines · Steven Rostedt
Quoted 6 lines
From: "Steven Rostedt (VMware)" <[email protected]>
The
ftrace_mod_mapis a descriptor to save module init function names in case they were traced, and the trace output needs to reference the function name from the function address. But after the function is unloaded, it the maps should be freed, as the rest of the function names are as well.Quoted 4 lines
Just checking for my understanding of this patch - wouldn't this also mean that if there were any look ups of the init functions that may be needed at trace output time, then those look ups wont be possible any more after module is unloaded?
Quoted 6 lines
Yes. That's true for all functions in the module. When a module is unloaded, all references to it in kallsyms is also freed. Try it on a current kernel. Trace a function in a module, then unload that module. The trace data will just show the ip hex address of the module function after that.
I tried your core branch and I see some weirdness with the filters:
Here's the code for the module:
================================================
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/init.h>void bar(void)
{ printk(KERN_INFO "bar!\n"); }void foo(void)
{ printk(KERN_INFO "foo!\n"); bar(); }static int
__inithello_init(void){ printk(KERN_INFO "Hello world!\n"); foo(); return 0; }static void
__exithello_cleanup(void){ printk(KERN_INFO "Cleaning up module.\n"); }
module_init(hello_init);module_exit(hello_cleanup);
================================================
[..]
Quoted 3 lines
Also I wasn't able to see the call from
hello_init-> bar and bar -> foo so I'm not fully sure if that's a bug or I did something wrong. I'm happy to try out anything you suggest.
This was I think because of function inlining, using 'noinline' on the foo and bar functions in the above module makes it all work properly (tested on ftrace/core branch).
thanks,
- Joel
Steven Rostedt
maintainerPATCH 16/16 ftrace/kallsyms: Have /proc/kallsyms show saved mod init functions
In reply to Steven RostedtFrom: "Steven Rostedt (VMware)" <[email protected]>
If a module is loaded while tracing is enabled, then there's a possibility that the module init functions were traced. These functions have their name and address stored by ftrace such that it can translate the function address that is written into the buffer into a human readable function name.
As userspace tools may be doing the same, they need a way to map function names to their address as well. This is done through reading /proc/kallsyms.
--- include/linux/ftrace.h | 9 +++++++++ kernel/kallsyms.c | 38 ++++++++++++++++++++++++++++++++------ kernel/trace/ftrace.c | 40 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 81 insertions(+), 6 deletions(-)
include/linux/ftrace.h +9 -0
@@ -56,6 +56,9 @@ struct ftrace_hash;const char *ftrace_mod_address_lookup (unsigned long addr,unsigned long *size,unsigned long *off,char **modname,char *sym); +int ftrace_mod_get_kallsym (unsigned int symnum,unsigned long *value, +char *type,char *name, +char *module_name,int *exported);#else static inline const char * ftrace_mod_address_lookup(unsigned long addr, unsigned long *size,
@@ -63,6 +66,12 @@ ftrace_mod_address_lookup(unsigned long addr, unsigned long *size, {return NULL ; } +static inline int ftrace_mod_get_kallsym (unsigned int symnum,unsigned long *value, +char *type,char *name, +char *module_name,int *exported) +{ +return -1 ; +}#endif
kernel/kallsyms.c +32 -6
@@ -479,6 +479,7 @@ EXPORT_SYMBOL(__print_symbol);struct kallsym_iter {loff_t pos;loff_t pos_mod_end; +loff_t pos_ftrace_mod_end;unsigned long value;unsigned int nameoff;/* If iterating in core kernel symbols. */ char type;
@@ -501,11 +502,25 @@ static int get_ksymbol_mod(struct kallsym_iter *iter)return 1 ; } +static int get_ksymbol_ftrace_mod (struct kallsym_iter *iter) +{ +int ret =ftrace_mod_get_kallsym (iter->pos - iter->pos_mod_end, + &iter->value, &iter->type, + iter->name, iter->module_name, + &iter->exported); +if (ret <0 ) { + iter->pos_ftrace_mod_end = iter->pos; +return 0 ; + } + +return 1 ; +} +static int get_ksymbol_bpf (struct kallsym_iter *iter) { iter->module_name[0 ] ='\0' ; iter->exported =0 ; -return bpf_get_kallsym (iter->pos - iter->pos_mod_end, +return bpf_get_kallsym (iter->pos - iter->pos_ftrace_mod_end, &iter->value, &iter->type, iter->name) <0 ?0 :1 ; }
@@ -530,20 +545,31 @@ static void reset_iter(struct kallsym_iter *iter, loff_t new_pos) iter->name[0 ] ='\0' ; iter->nameoff =get_symbol_offset (new_pos); iter->pos = new_pos; -if (new_pos ==0 ) +if (new_pos ==0 ) { iter->pos_mod_end =0 ; + iter->pos_ftrace_mod_end =0 ; + } }static int update_iter_mod (struct kallsym_iter *iter,loff_t pos) { iter->pos = pos; -if (iter->pos_mod_end >0 && - iter->pos_mod_end < iter->pos) +if (iter->pos_ftrace_mod_end >0 && + iter->pos_ftrace_mod_end < iter->pos)return get_ksymbol_bpf (iter); -if (!get_ksymbol_mod (iter)) -return get_ksymbol_bpf (iter); +if (iter->pos_mod_end >0 && + iter->pos_mod_end < iter->pos) { +if (!get_ksymbol_ftrace_mod (iter)) +return get_ksymbol_bpf (iter); +return 1 ; + } + +if (!get_ksymbol_mod (iter)) { +if (!get_ksymbol_ftrace_mod (iter)) +return get_ksymbol_bpf (iter); + }return 1 ; }
kernel/trace/ftrace.c +40 -0
@@ -5689,6 +5689,7 @@ struct ftrace_mod_map {unsigned long start_addr;unsigned long end_addr;struct list_head funcs; +unsigned int num_funcs; };#ifdef CONFIG_MODULES
@@ -5940,6 +5941,8 @@ static void save_ftrace_mod_rec(struct ftrace_mod_map *mod_map, mod_func->ip = rec->ip - offset; mod_func->size = symsize; + mod_map->num_funcs++; +list_add_rcu (&mod_func->list, &mod_map->funcs); }
@@ -5956,6 +5959,7 @@ allocate_ftrace_mod_map(struct module *mod, mod_map->mod = mod; mod_map->start_addr = start; mod_map->end_addr = end; + mod_map->num_funcs =0 ;INIT_LIST_HEAD_RCU (&mod_map->funcs);
@@ -6016,6 +6020,42 @@ ftrace_mod_address_lookup(unsigned long addr, unsigned long *size,return ret; } +int ftrace_mod_get_kallsym (unsigned int symnum,unsigned long *value, +char *type,char *name, +char *module_name,int *exported) +{ +struct ftrace_mod_map *mod_map; +struct ftrace_mod_func *mod_func; + +preempt_disable (); +list_for_each_entry_rcu (mod_map, &ftrace_mod_maps, list) { + +if (symnum >= mod_map->num_funcs) { + symnum -= mod_map->num_funcs; +continue ; + } + +list_for_each_entry_rcu (mod_func, &mod_map->funcs, list) { +if (symnum >1 ) { + symnum--; +continue ; + } + + *value = mod_func->ip; + *type ='T' ; +strlcpy (name, mod_func->name,KSYM_NAME_LEN ); +strlcpy (module_name, mod_map->mod->name,MODULE_NAME_LEN ); + *exported =1 ; +preempt_enable (); +return 0 ; + } +WARN_ON (1 ); +break ; + } +preempt_enable (); +return -ERANGE ; +} +#else static void save_ftrace_mod_rec (struct ftrace_mod_map *mod_map,struct dyn_ftrace *rec) { }
Signature · 1 line
2.13.2