public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: linux-kernel@vger.kernel.org
Cc: Ingo Molnar <mingo@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
	Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>,
	Namhyung Kim <namhyung@kernel.org>,
	"H. Peter Anvin" <hpa@zytor.com>, Oleg Nesterov <oleg@redhat.com>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	Jiri Kosina <jkosina@suse.cz>,
	Seth Jennings <sjenning@redhat.com>, Jiri Slaby <jslaby@suse.cz>,
	"H. Peter Anvin" <hpa@linux.intel.com>
Subject: [RFC][PATCH 3/3] ftrace/x86: Allow !CONFIG_PREEMPT dynamic ops to use allocated trampolines
Date: Thu, 03 Jul 2014 16:07:53 -0400	[thread overview]
Message-ID: <20140703202325.240729487@goodmis.org> (raw)
In-Reply-To: 20140703200750.648550267@goodmis.org

[-- Attachment #1: 0003-ftrace-x86-Allow-CONFIG_PREEMPT-dynamic-ops-to-use-a.patch --]
[-- Type: text/plain, Size: 3490 bytes --]

From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>

When the static ftrace_ops (like function tracer) enables tracing, and it
is the only callback that is referencing a function, a trampoline is
dynamically allocated to the function that calls the callback directly
instead of calling a loop function that iterates over all the registered
ftrace ops (if more than one ops is registered).

But when it comes to dynamically allocated ftrace_ops, where they may be
freed, on a CONFIG_PREEMPT kernel there's no way to know when it is safe
to free the trampoline. If a task was preempted while executing on the
trampoline, there's currently no way to know when it will be off that
trampoline.

But this is not true when it comes to !CONFIG_PREEMPT. The current method
of calling schedule_on_each_cpu() will force tasks off the trampoline,
becaues they can not schedule while on it (kernel preemption is not
configured). That means it is safe to free a dynamically allocated
ftrace ops trampoline when CONFIG_PREEMPT is not configured.

Cc: H. Peter Anvin <hpa@linux.intel.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 arch/x86/kernel/ftrace.c |  8 ++++++++
 kernel/trace/ftrace.c    | 18 ++++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index 50de611d44b4..b718fe6f881a 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -874,6 +874,14 @@ void *arch_ftrace_trampoline_func(struct ftrace_ops *ops, struct dyn_ftrace *rec
 	return addr_from_call((void *)ops->trampoline + offset);
 }
 
+void arch_ftrace_trampoline_free(struct ftrace_ops *ops)
+{
+	if (!ops || !(ops->flags & FTRACE_OPS_FL_ALLOC_TRAMP))
+		return;
+
+	module_free(NULL, ops->trampoline);
+	ops->trampoline = NULL;
+}
 
 #endif /* CONFIG_X86_64 */
 #endif /* CONFIG_DYNAMIC_FTRACE */
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 66cbb3b7251d..a1202c6d57d2 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -2281,6 +2281,10 @@ static void ftrace_run_update_code(int command)
 static ftrace_func_t saved_ftrace_func;
 static int ftrace_start_up;
 
+void __weak arch_ftrace_trampoline_free(struct ftrace_ops *ops)
+{
+}
+
 static void control_ops_free(struct ftrace_ops *ops)
 {
 	free_percpu(ops->disabled);
@@ -2391,6 +2395,8 @@ static int ftrace_shutdown(struct ftrace_ops *ops, int command)
 	if (ops->flags & (FTRACE_OPS_FL_DYNAMIC | FTRACE_OPS_FL_CONTROL)) {
 		schedule_on_each_cpu(ftrace_sync);
 
+		arch_ftrace_trampoline_free(ops);
+
 		if (ops->flags & FTRACE_OPS_FL_CONTROL)
 			control_ops_free(ops);
 	}
@@ -4632,9 +4638,21 @@ void __weak arch_ftrace_update_trampoline(struct ftrace_ops *ops)
 
 static void ftrace_update_trampoline(struct ftrace_ops *ops)
 {
+
+/*
+ * Currently there's no safe way to free a trampoline when the kernel
+ * is configured with PREEMPT. That is because a task could be preempted
+ * when it jumped to the trampoline, it may be preempted for a long time
+ * depending on the system load, and currently there's no way to know
+ * when it will be off the trampoline. If the trampoline is freed
+ * too early, when the task runs again, it will be executing on freed
+ * memory and crash.
+ */
+#ifdef CONFIG_PREEMPT
 	/* Currently, only non dynamic ops can have a trampoline */
 	if (ops->flags & FTRACE_OPS_FL_DYNAMIC)
 		return;
+#endif
 
 	arch_ftrace_update_trampoline(ops);
 }
-- 
2.0.0



  parent reply	other threads:[~2014-07-03 20:24 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-03 20:07 [RFC][PATCH 0/3] ftrace: Add dynamically allocated trampolines Steven Rostedt
2014-07-03 20:07 ` [RFC][PATCH 1/3] ftrace/x86: Add dynamic allocated trampoline for ftrace_ops Steven Rostedt
2014-07-04 13:32   ` Masami Hiramatsu
2014-07-04 14:25     ` Steven Rostedt
2014-07-14  2:34   ` Masami Hiramatsu
2014-07-03 20:07 ` [RFC][PATCH 2/3] ftrace/x86: Show trampoline call function in enabled_functions Steven Rostedt
2014-07-03 20:07 ` Steven Rostedt [this message]
2014-07-03 20:32 ` [RFC][PATCH 0/3] ftrace: Add dynamically allocated trampolines Steven Rostedt
2014-07-04 13:20 ` Masami Hiramatsu
2014-07-04 14:21   ` Steven Rostedt
2014-07-07 13:22     ` Jiri Kosina
2014-07-08 14:24       ` Steven Rostedt
2014-07-07 13:58 ` Jiri Kosina
2014-07-10 21:36 ` Josh Poimboeuf
2014-07-10 21:44   ` Jiri Kosina
2014-07-10 22:01     ` Josh Poimboeuf
2014-07-11  2:26     ` Masami Hiramatsu
2014-07-11 13:24       ` Jiri Kosina
2014-07-11 14:29         ` Josh Poimboeuf
2014-07-14  1:35           ` Masami Hiramatsu
2014-07-14  7:16             ` Namhyung Kim
2014-07-14  8:18               ` Masami Hiramatsu
2014-07-14 14:18                 ` Namhyung Kim
2014-07-15  1:20                   ` Masami Hiramatsu
2014-07-22 16:47 ` Oleg Nesterov
2014-07-22 19:02   ` Steven Rostedt
2014-07-23 12:08     ` Oleg Nesterov
2014-07-23 15:48       ` Steven Rostedt
2014-07-23 17:05         ` Oleg Nesterov
2014-07-23 17:20           ` Steven Rostedt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140703202325.240729487@goodmis.org \
    --to=rostedt@goodmis.org \
    --cc=akpm@linux-foundation.org \
    --cc=hpa@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=jkosina@suse.cz \
    --cc=jpoimboe@redhat.com \
    --cc=jslaby@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=masami.hiramatsu.pt@hitachi.com \
    --cc=mingo@kernel.org \
    --cc=namhyung@kernel.org \
    --cc=oleg@redhat.com \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=sjenning@redhat.com \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox