| From rostedt@goodmis.org Mon Feb 10 16:45:29 2014 |
| From: Steven Rostedt <rostedt@goodmis.org> |
| Date: Fri, 7 Feb 2014 14:42:01 -0500 |
| Subject: ftrace: Fix synchronization location disabling and freeing ftrace_ops |
| To: Luis Henriques <luis.henriques@canonical.com> |
| Cc: gregkh@linuxfoundation.org, stable@vger.kernel.org, stable-commits@vger.kernel.org |
| Message-ID: <20140207144201.38d64ed8@gandalf.local.home> |
| |
| From: Steven Rostedt <rostedt@goodmis.org> |
| |
| commit a4c35ed241129dd142be4cadb1e5a474a56d5464 upstream. |
| |
| The synchronization needed after ftrace_ops are unregistered must happen |
| after the callback is disabled from becing called by functions. |
| |
| The current location happens after the function is being removed from the |
| internal lists, but not after the function callbacks were disabled, leaving |
| the functions susceptible of being called after their callbacks are freed. |
| |
| This affects perf and any externel users of function tracing (LTTng and |
| SystemTap). |
| |
| Fixes: cdbe61bfe704 "ftrace: Allow dynamically allocated function tracers" |
| Signed-off-by: Steven Rostedt <rostedt@goodmis.org> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| kernel/trace/ftrace.c | 50 ++++++++++++++++++++++++++++++++------------------ |
| 1 file changed, 32 insertions(+), 18 deletions(-) |
| |
| --- a/kernel/trace/ftrace.c |
| +++ b/kernel/trace/ftrace.c |
| @@ -490,16 +490,6 @@ static int __unregister_ftrace_function( |
| } else if (ops->flags & FTRACE_OPS_FL_CONTROL) { |
| ret = remove_ftrace_list_ops(&ftrace_control_list, |
| &control_ops, ops); |
| - if (!ret) { |
| - /* |
| - * The ftrace_ops is now removed from the list, |
| - * so there'll be no new users. We must ensure |
| - * all current users are done before we free |
| - * the control data. |
| - */ |
| - synchronize_sched(); |
| - control_ops_free(ops); |
| - } |
| } else |
| ret = remove_ftrace_ops(&ftrace_ops_list, ops); |
| |
| @@ -509,13 +499,6 @@ static int __unregister_ftrace_function( |
| if (ftrace_enabled) |
| update_ftrace_function(); |
| |
| - /* |
| - * Dynamic ops may be freed, we must make sure that all |
| - * callers are done before leaving this function. |
| - */ |
| - if (ops->flags & FTRACE_OPS_FL_DYNAMIC) |
| - synchronize_sched(); |
| - |
| return 0; |
| } |
| |
| @@ -2184,10 +2167,41 @@ static int ftrace_shutdown(struct ftrace |
| command |= FTRACE_UPDATE_TRACE_FUNC; |
| } |
| |
| - if (!command || !ftrace_enabled) |
| + if (!command || !ftrace_enabled) { |
| + /* |
| + * If these are control ops, they still need their |
| + * per_cpu field freed. Since, function tracing is |
| + * not currently active, we can just free them |
| + * without synchronizing all CPUs. |
| + */ |
| + if (ops->flags & FTRACE_OPS_FL_CONTROL) |
| + control_ops_free(ops); |
| return 0; |
| + } |
| |
| ftrace_run_update_code(command); |
| + |
| + /* |
| + * Dynamic ops may be freed, we must make sure that all |
| + * callers are done before leaving this function. |
| + * The same goes for freeing the per_cpu data of the control |
| + * ops. |
| + * |
| + * Again, normal synchronize_sched() is not good enough. |
| + * We need to do a hard force of sched synchronization. |
| + * This is because we use preempt_disable() to do RCU, but |
| + * the function tracers can be called where RCU is not watching |
| + * (like before user_exit()). We can not rely on the RCU |
| + * infrastructure to do the synchronization, thus we must do it |
| + * ourselves. |
| + */ |
| + if (ops->flags & (FTRACE_OPS_FL_DYNAMIC | FTRACE_OPS_FL_CONTROL)) { |
| + schedule_on_each_cpu(ftrace_sync); |
| + |
| + if (ops->flags & FTRACE_OPS_FL_CONTROL) |
| + control_ops_free(ops); |
| + } |
| + |
| return 0; |
| } |
| |