mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: linux-kernel@vger.kernel.org
Cc: Ingo Molnar <mingo@elte.hu>,
	Andrew Morton <akpm@linux-foundation.org>,
	Frederic Weisbecker <fweisbec@gmail.com>
Subject: [PATCH 05/20] ftrace: Fix dynamic selftest failure on some archs
Date: Sat, 16 Jul 2011 07:22:05 -0400	[thread overview]
Message-ID: <20110716112252.536093524@goodmis.org> (raw)
In-Reply-To: <20110716112200.096203519@goodmis.org>

[-- Attachment #1: 0005-ftrace-Fix-dynamic-selftest-failure-on-some-archs.patch --]
[-- Type: text/plain, Size: 4629 bytes --]

From: Steven Rostedt <srostedt@redhat.com>

Archs that do not implement CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST, will
fail the dynamic ftrace selftest.

The function tracer has a quick 'off' variable that will prevent
the call back functions from being called. This variable is called
function_trace_stop. In x86, this is implemented directly in the mcount
assembly, but for other archs, an intermediate function is used called
ftrace_test_stop_func().

In dynamic ftrace, the function pointer variable ftrace_trace_function is
used to update the caller code in the mcount caller. But for archs that
do not have CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST set, it only calls
ftrace_test_stop_func() instead, which in turn calls __ftrace_trace_function.

When more than one ftrace_ops is registered, the function it calls is
ftrace_ops_list_func(), which will iterate over all registered ftrace_ops
and call the callbacks that have their hash matching.

The issue happens when two ftrace_ops are registered for different functions
and one is then unregistered. The __ftrace_trace_function is then pointed
to the remaining ftrace_ops callback function directly. This mean it will
be called for all functions that were registered to trace by both ftrace_ops
that were registered.

This is not an issue for archs with CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST,
because the update of ftrace_trace_function doesn't happen until after all
functions have been updated, and then the mcount caller is updated. But
for those archs that do use the ftrace_test_stop_func(), the update is
immediate.

The dynamic selftest fails because it hits this situation, and the
ftrace_ops that it registers fails to only trace what it was suppose to
and instead traces all other functions.

The solution is to delay the setting of __ftrace_trace_function until
after all the functions have been updated according to the registered
ftrace_ops. Also, function_trace_stop is set during the update to prevent
function tracing from calling code that is caused by the function tracer
itself.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/ftrace.c |   26 ++++++++++++++++++++++++++
 1 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index a0dc0de..62e26d9 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -88,6 +88,7 @@ static struct ftrace_ops ftrace_list_end __read_mostly = {
 static struct ftrace_ops *ftrace_global_list __read_mostly = &ftrace_list_end;
 static struct ftrace_ops *ftrace_ops_list __read_mostly = &ftrace_list_end;
 ftrace_func_t ftrace_trace_function __read_mostly = ftrace_stub;
+static ftrace_func_t __ftrace_trace_function_delay __read_mostly = ftrace_stub;
 ftrace_func_t __ftrace_trace_function __read_mostly = ftrace_stub;
 ftrace_func_t ftrace_pid_function __read_mostly = ftrace_stub;
 static struct ftrace_ops global_ops;
@@ -146,9 +147,11 @@ void clear_ftrace_function(void)
 {
 	ftrace_trace_function = ftrace_stub;
 	__ftrace_trace_function = ftrace_stub;
+	__ftrace_trace_function_delay = ftrace_stub;
 	ftrace_pid_function = ftrace_stub;
 }
 
+#undef CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST
 #ifndef CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST
 /*
  * For those archs that do not test ftrace_trace_stop in their
@@ -208,7 +211,12 @@ static void update_ftrace_function(void)
 #ifdef CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST
 	ftrace_trace_function = func;
 #else
+#ifdef CONFIG_DYNAMIC_FTRACE
+	/* do not update till all functions have been modified */
+	__ftrace_trace_function_delay = func;
+#else
 	__ftrace_trace_function = func;
+#endif
 	ftrace_trace_function = ftrace_test_stop_func;
 #endif
 }
@@ -1607,6 +1615,12 @@ static int __ftrace_modify_code(void *data)
 {
 	int *command = data;
 
+	/*
+	 * Do not call function tracer while we update the code.
+	 * We are in stop machine, no worrying about races.
+	 */
+	function_trace_stop++;
+
 	if (*command & FTRACE_ENABLE_CALLS)
 		ftrace_replace_code(1);
 	else if (*command & FTRACE_DISABLE_CALLS)
@@ -1620,6 +1634,18 @@ static int __ftrace_modify_code(void *data)
 	else if (*command & FTRACE_STOP_FUNC_RET)
 		ftrace_disable_ftrace_graph_caller();
 
+#ifndef CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST
+	/*
+	 * For archs that call ftrace_test_stop_func(), we must
+	 * wait till after we update all the function callers
+	 * before we update the callback. This keeps different
+	 * ops that record different functions from corrupting
+	 * each other.
+	 */
+	__ftrace_trace_function = __ftrace_trace_function_delay;
+#endif
+	function_trace_stop--;
+
 	return 0;
 }
 
-- 
1.7.5.4



  parent reply	other threads:[~2011-07-16 11:26 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-16 11:22 [PATCH 00/20] [GIT PULL] perf/tracing: various updates Steven Rostedt
2011-07-16 11:22 ` [PATCH 01/20] tracing: Still trace filtered irq functions when irq trace is Steven Rostedt
2011-07-16 11:22 ` [PATCH 02/20] ftrace: Do not disable interrupts for modules in mcount update Steven Rostedt
2011-07-16 11:22 ` [PATCH 03/20] ftrace: Balance records when updating the hash Steven Rostedt
2011-07-16 11:22 ` [PATCH 04/20] ftrace: Update filter when tracing enabled in set_ftrace_filter() Steven Rostedt
2011-07-16 11:22 ` Steven Rostedt [this message]
2011-07-16 11:22 ` [PATCH 06/20] perf: Robustify proc and debugfs file recording Steven Rostedt
2011-07-16 11:22 ` [PATCH 07/20] tracing: Have dynamic size event stack traces Steven Rostedt
2011-07-16 11:22 ` [PATCH 08/20] perf, x86: P4 PMU - Introduce event alias feature Steven Rostedt
2011-07-16 11:22 ` [PATCH 09/20] tracing/kprobes: Rename probe_* to trace_probe_* Steven Rostedt
2011-07-16 11:22 ` [PATCH 10/20] tracing/kprobes: Merge trace probe enable/disable functions Steven Rostedt
2011-07-16 11:22 ` [PATCH 11/20] kprobes: Return -ENOENT if probe point doesnt exist Steven Rostedt
2011-07-16 11:22 ` [PATCH 12/20] tracing/kprobes: Support module init function probing Steven Rostedt
2011-07-16 11:22 ` [PATCH 13/20] tracing/kprobe: Update symbol reference when loading module Steven Rostedt
2011-07-16 11:22 ` [PATCH 14/20] perf probe: Rename DIE_FIND_CB_FOUND to DIE_FIND_CB_END Steven Rostedt
2011-07-16 11:22 ` [PATCH 15/20] perf probe: Move strtailcmp to string.c Steven Rostedt
2011-07-16 11:22 ` [PATCH 16/20] perf probe: Remove redundant dwarf functions Steven Rostedt
2011-07-16 11:22 ` [PATCH 17/20] perf-probe: Move dwarf library routines to dwarf-aux.{c, h} Steven Rostedt
2011-07-16 11:22 ` [PATCH 18/20] perf probe: Introduce debuginfo to encapsulate dwarf information Steven Rostedt
2011-07-16 11:22 ` [PATCH 19/20] perf probe: Add probed module in front of function Steven Rostedt
2011-07-16 11:22 ` [PATCH 20/20] perf probe: Support adding probes on offline kernel modules Steven Rostedt
2011-07-21  9:10 ` [PATCH 00/20] [GIT PULL] perf/tracing: various updates Ingo Molnar

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=20110716112252.536093524@goodmis.org \
    --to=rostedt@goodmis.org \
    --cc=akpm@linux-foundation.org \
    --cc=fweisbec@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    /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

all inboxes | Powered by JetHome®