tracing: Small improvements for 4.14
Subsystems (3)
Steven Rostedt
maintainercover tracing: Small improvements for 4.14
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
for-next
Head SHA1: 2a5bfe47624bfc835aa0632a0505ba55576c98db
Steven Rostedt (VMware) (2):
tracing: Only have rmmod clear buffers that its events were active in
ftrace: Zero out ftrace hashes when a module is removed
Zev Weiss (1):
ftrace: Fix debug preempt config name in stack_tracer_{en,dis}able
----
include/linux/ftrace.h | 4 +-- include/linux/trace_events.h | 8 +++--- kernel/trace/ftrace.c | 58 +++++++++++++++++++++++++++++++++++++++++--- kernel/trace/trace.c | 3 +++ kernel/trace/trace.h | 1 + kernel/trace/trace_events.c | 15 ++++++------ 6 files changed, 71 insertions(+), 18 deletions(-)
Steven Rostedt
maintainerPATCH 1/3 ftrace: Fix debug preempt config name in stack_tracer_{en,dis}able
From: Zev Weiss <[email protected]>
stack_tracer_disable()/stack_tracer_enable() had been using the wrong name for the config symbol to enable their preempt-debugging checks -- fix with a word swap.
--- include/linux/ftrace.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
include/linux/ftrace.h +2 -2
@@ -307,7 +307,7 @@ DECLARE_PER_CPU(int, disable_stack_tracer);static inline void stack_tracer_disable (void ) {/* Preemption or interupts must be disabled */ -if (IS_ENABLED (CONFIG_PREEMPT_DEBUG )) +if (IS_ENABLED (CONFIG_DEBUG_PREEMPT ))WARN_ON_ONCE (!preempt_count () || !irqs_disabled ());this_cpu_inc (disable_stack_tracer); }
@@ -320,7 +320,7 @@ static inline void stack_tracer_disable(void)*/ static inline void stack_tracer_enable (void ) { -if (IS_ENABLED (CONFIG_PREEMPT_DEBUG )) +if (IS_ENABLED (CONFIG_DEBUG_PREEMPT ))WARN_ON_ONCE (!preempt_count () || !irqs_disabled ());this_cpu_dec (disable_stack_tracer); }
Signature · 1 line
2.13.2
Steven Rostedt
maintainerPATCH 2/3 tracing: Only have rmmod clear buffers that its events were active in
In reply to Steven RostedtFrom: "Steven Rostedt (VMware)" <[email protected]>
Currently, when a module event is enabled, when that module is removed, it clears all ring buffers. This is to prevent another module from being loaded and having one of its trace event IDs from reusing a trace event ID of the removed module. This could cause undesirable effects as the trace event of the new module would be using its own processing algorithms to process raw data of another event. To prevent this, when a module is loaded, if any of its events have been used (signified by the WAS_ENABLED event call flag, which is never cleared), all ring buffers are cleared, just in case any one of them contains event data of the removed event.
The problem is, there's no reason to clear all ring buffers if only one (or less than all of them) uses one of the events. Instead, only clear the ring buffers that recorded the events of a module that is being removed.
To do this, instead of keeping the WAS_ENABLED flag with the trace event call, move it to the per instance (per ring buffer) event file descriptor. The event file descriptor maps each event to a separate ring buffer instance. Then when the module is removed, only the ring buffers that activated one of the module's events get cleared. The rest are not touched.
--- include/linux/trace_events.h | 8 +++----- kernel/trace/trace.c | 3 +++ kernel/trace/trace.h | 1 + kernel/trace/trace_events.c | 15 +++++++-------- 4 files changed, 14 insertions(+), 13 deletions(-)
include/linux/trace_events.h +3 -5
@@ -217,7 +217,6 @@ enum {TRACE_EVENT_FL_CAP_ANY_BIT ,TRACE_EVENT_FL_NO_SET_FILTER_BIT ,TRACE_EVENT_FL_IGNORE_ENABLE_BIT , -TRACE_EVENT_FL_WAS_ENABLED_BIT ,TRACE_EVENT_FL_TRACEPOINT_BIT ,TRACE_EVENT_FL_KPROBE_BIT , TRACE_EVENT_FL_UPROBE_BIT,
@@ -229,9 +228,6 @@ enum {* CAP_ANY - Any user can enable for perf * NO_SET_FILTER - Set when filter has error and is to be ignored * IGNORE_ENABLE - For trace internal events, do not enable with debugfs file -* WAS_ENABLED - Set and stays set when an event was ever enabled -* (used for module unloading, if a module event is enabled, -* it is best to clear the buffers that used it). * TRACEPOINT - Event is a tracepoint * KPROBE - Event is a kprobe * UPROBE - Event is a uprobe
@@ -241,7 +237,6 @@ enum {TRACE_EVENT_FL_CAP_ANY = (1 <<TRACE_EVENT_FL_CAP_ANY_BIT ),TRACE_EVENT_FL_NO_SET_FILTER = (1 <<TRACE_EVENT_FL_NO_SET_FILTER_BIT ),TRACE_EVENT_FL_IGNORE_ENABLE = (1 <<TRACE_EVENT_FL_IGNORE_ENABLE_BIT ), -TRACE_EVENT_FL_WAS_ENABLED = (1 <<TRACE_EVENT_FL_WAS_ENABLED_BIT ),TRACE_EVENT_FL_TRACEPOINT = (1 <<TRACE_EVENT_FL_TRACEPOINT_BIT ),TRACE_EVENT_FL_KPROBE = (1 <<TRACE_EVENT_FL_KPROBE_BIT ), TRACE_EVENT_FL_UPROBE = (1 << TRACE_EVENT_FL_UPROBE_BIT),
@@ -306,6 +301,7 @@ enum {EVENT_FILE_FL_TRIGGER_MODE_BIT ,EVENT_FILE_FL_TRIGGER_COND_BIT ,EVENT_FILE_FL_PID_FILTER_BIT , + EVENT_FILE_FL_WAS_ENABLED_BIT, };/*
@@ -321,6 +317,7 @@ enum {* TRIGGER_MODE - When set, invoke the triggers associated with the event * TRIGGER_COND - When set, one or more triggers has an associated filter * PID_FILTER - When set, the event is filtered based on pid +* WAS_ENABLED - Set when enabled to know to clear trace on module removal */ enum {EVENT_FILE_FL_ENABLED = (1 <<EVENT_FILE_FL_ENABLED_BIT ),
@@ -333,6 +330,7 @@ enum {EVENT_FILE_FL_TRIGGER_MODE = (1 <<EVENT_FILE_FL_TRIGGER_MODE_BIT ),EVENT_FILE_FL_TRIGGER_COND = (1 <<EVENT_FILE_FL_TRIGGER_COND_BIT ),EVENT_FILE_FL_PID_FILTER = (1 <<EVENT_FILE_FL_PID_FILTER_BIT ), + EVENT_FILE_FL_WAS_ENABLED = (1 << EVENT_FILE_FL_WAS_ENABLED_BIT), }; struct trace_event_file {
kernel/trace/trace.c +3 -0
@@ -1702,6 +1702,9 @@ void tracing_reset_all_online_cpus(void)struct trace_array *tr;list_for_each_entry (tr, &ftrace_trace_arrays, list) { +if (!tr->clear_trace) +continue ; + tr->clear_trace = false;tracing_reset_online_cpus (&tr->trace_buffer);#ifdef CONFIG_TRACER_MAX_TRACE tracing_reset_online_cpus (&tr->max_buffer);
kernel/trace/trace.h +1 -0
@@ -245,6 +245,7 @@ struct trace_array {int stop_count;int clock_id;int nr_topts; +bool clear_trace;struct tracer *current_trace;unsigned int trace_flags;unsigned char trace_flags_index[TRACE_FLAGS_MAX_SIZE ];
kernel/trace/trace_events.c +7 -8
@@ -466,7 +466,7 @@ static int __ftrace_event_enable_disable(struct trace_event_file *file,set_bit (EVENT_FILE_FL_ENABLED_BIT , &file->flags);/* WAS_ENABLED gets set but never cleared. */ - call->flags |=TRACE_EVENT_FL_WAS_ENABLED ; +set_bit (EVENT_FILE_FL_WAS_ENABLED_BIT , &file->flags); }break ; }
@@ -2058,6 +2058,10 @@ static void event_remove(struct trace_event_call *call)do_for_each_event_file (tr, file) {if (file->event_call != call)continue ; + +if (file->flags &EVENT_FILE_FL_WAS_ENABLED ) + tr->clear_trace = true; +ftrace_event_enable_disable (file,0 );/* * The do_for_each_event_file() is
@@ -2396,15 +2400,11 @@ static void trace_module_add_events(struct module *mod)static void trace_module_remove_events (struct module *mod) {struct trace_event_call *call, *p; -bool clear_trace = false;down_write (&trace_event_sem);list_for_each_entry_safe (call, p, &ftrace_events, list) { -if (call->mod == mod) { -if (call->flags &TRACE_EVENT_FL_WAS_ENABLED ) - clear_trace = true; +if (call->mod == mod)__trace_remove_event_call (call); - } }up_write (&trace_event_sem);
@@ -2416,8 +2416,7 @@ static void trace_module_remove_events(struct module *mod)* over from this module may be passed to the new module events and * unexpected results may occur. */ -if (clear_trace) -tracing_reset_all_online_cpus (); +tracing_reset_all_online_cpus (); } static int trace_module_notify(struct notifier_block *self,
Signature · 1 line
2.13.2
Steven Rostedt
maintainerPATCH 3/3 ftrace: Zero out ftrace hashes when a module is removed
In reply to Steven RostedtFrom: "Steven Rostedt (VMware)" <[email protected]>
When a ftrace filter has a module function, and that module is removed, the filter still has its address as being enabled. This can cause interesting side effects. Nothing dangerous, but unwanted functions can be traced because of it.
# cd /sys/kernel/tracing # echo ':mod:snd_seq' > set_ftrace_filter # cat set_ftrace_filter snd_use_lock_sync_helper [snd_seq] check_event_type_and_length [snd_seq] snd_seq_ioctl_pversion [snd_seq] snd_seq_ioctl_client_id [snd_seq] snd_seq_ioctl_get_queue_tempo [snd_seq] update_timestamp_of_queue [snd_seq] snd_seq_ioctl_get_queue_status [snd_seq] snd_seq_set_queue_tempo [snd_seq] snd_seq_ioctl_set_queue_tempo [snd_seq] snd_seq_ioctl_get_queue_timer [snd_seq] seq_free_client1 [snd_seq] [..] # rmmod snd_seq # cat set_ftrace_filter
# modprobe kvm # cat set_ftrace_filter kvm_set_cr4 [kvm] kvm_emulate_hypercall [kvm] kvm_set_dr [kvm]
This is because removing the snd_seq module after it was being filtered, left the address of the snd_seq functions in the hash. When the kvm module was loaded, some of its functions were loaded at the same address as the snd_seq module. This would enable them to be filtered and traced.
Now we don't want to clear the hash completely. That would cause removing a module where only its functions are filtered, to cause the tracing to enable all functions, as an empty filter means to trace all functions. Instead, just set the hash ip address to zero. Then it will never match any function.
--- kernel/trace/ftrace.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 55 insertions(+), 3 deletions(-)
kernel/trace/ftrace.c +55 -3
@@ -5690,10 +5690,51 @@ static int referenced_filters(struct dyn_ftrace *rec)return cnt; } +static void +clear_mod_from_hash (struct ftrace_page *pg,struct ftrace_hash *hash) +{ +struct ftrace_func_entry *entry; +struct dyn_ftrace *rec; +int i; + +if (ftrace_hash_empty (hash)) +return ; + +for (i =0 ; i < pg->index; i++) { + rec = &pg->records[i]; + entry =__ftrace_lookup_ip (hash, rec->ip); +/* +* Do not allow this rec to match again. +* Yeah, it may waste some memory, but will be removed +* if/when the hash is modified again. +*/ +if (entry) + entry->ip =0 ; + } +} + +/* Clear any records from hashs */ +static void clear_mod_from_hashes (struct ftrace_page *pg) +{ +struct trace_array *tr; + +mutex_lock (&trace_types_lock); +list_for_each_entry (tr, &ftrace_trace_arrays, list) { +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); +mutex_unlock (&tr->ops->func_hash->regex_lock); + } +mutex_unlock (&trace_types_lock); +} +void ftrace_release_mod (struct module *mod) {struct dyn_ftrace *rec;struct ftrace_page **last_pg; +struct ftrace_page *tmp_page =NULL ;struct ftrace_page *pg;int order;
@@ -5723,14 +5764,25 @@ void ftrace_release_mod(struct module *mod) ftrace_update_tot_cnt -= pg->index; *last_pg = pg->next; - order =get_count_order (pg->size /ENTRIES_PER_PAGE ); -free_pages ((unsigned long )pg->records, order); -kfree (pg); + + pg->next = tmp_page; + tmp_page = pg; }else last_pg = &pg->next; } out_unlock:mutex_unlock (&ftrace_lock); + +for (pg = tmp_page; pg; pg = tmp_page) { + +/* Needs to be called outside of ftrace_lock */ +clear_mod_from_hashes (pg); + + order =get_count_order (pg->size /ENTRIES_PER_PAGE ); +free_pages ((unsigned long )pg->records, order); + tmp_page = pg->next; +kfree (pg); + } }void ftrace_module_enable (struct module *mod)
Signature · 1 line
2.13.2