[PATCH 0/2] [GIT PULL][v3.0] ftrace: urgent fixes

[PATCH 0/2] [GIT PULL][v3.0] ftrace: urgent fixes

[Steven Rostedt]

Ingo,

I noticed that tip/perf/urgent wasn't pulled yet. I added it to this
pull request. The first patch is what you reported to me, the second
is a regression that Thomas stumbled upon.

Please pull the latest tip/perf/urgent-2 tree, which can be found at:

  git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
tip/perf/urgent-2

Head SHA1: f7bc8b61f65726ff98f52e286b28e294499d7a08


Steven Rostedt (2):
      ftrace: Fix warning when CONFIG_FUNCTION_TRACER is not defined
      ftrace: Fix regression where ftrace breaks when modules are loaded

----
 include/linux/ftrace.h |    4 ++--
 kernel/trace/ftrace.c  |   30 ++++++++++++++++++++++++++++--
 2 files changed, 30 insertions(+), 4 deletions(-)

[PATCH 1/2] ftrace: Fix warning when CONFIG_FUNCTION_TRACER is not defined

[Steven Rostedt]

[-- Attachment #1: 0001-ftrace-Fix-warning-when-CONFIG_FUNCTION_TRACER-is-no.patch --]
[-- Type: text/plain, Size: 877 bytes --]

From: Steven Rostedt <srostedt@redhat.com>

The struct ftrace_hash was declared within CONFIG_FUNCTION_TRACER
but was referenced outside of it.

Reported-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 include/linux/ftrace.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index ed0eb52..f0c0e8a 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -19,6 +19,8 @@
 
 #include <asm/ftrace.h>
 
+struct ftrace_hash;
+
 #ifdef CONFIG_FUNCTION_TRACER
 
 extern int ftrace_enabled;
@@ -29,8 +31,6 @@ ftrace_enable_sysctl(struct ctl_table *table, int write,
 
 typedef void (*ftrace_func_t)(unsigned long ip, unsigned long parent_ip);
 
-struct ftrace_hash;
-
 enum {
 	FTRACE_OPS_FL_ENABLED		= 1 << 0,
 	FTRACE_OPS_FL_GLOBAL		= 1 << 1,
-- 
1.7.5.4

[PATCH 2/2] ftrace: Fix regression where ftrace breaks when modules are loaded

[Steven Rostedt]

[-- Attachment #1: 0002-ftrace-Fix-regression-where-ftrace-breaks-when-modul.patch --]
[-- Type: text/plain, Size: 3936 bytes --]

From: Steven Rostedt <srostedt@redhat.com>

Enabling function tracer to trace all functions, then load a module and
then disable function tracing will cause ftrace to fail.

This can also happen by enabling function tracing on the command line:

  ftrace=function

and during boot up, modules are loaded, then you disable function tracing
with 'echo nop > current_tracer' you will trigger a bug in ftrace that
will shut itself down.

The reason is, the new ftrace code keeps ref counts of all ftrace_ops that
are registered for tracing. When one or more ftrace_ops are registered,
all the records that represent the functions that the ftrace_ops will
trace have a ref count incremented. If this ref count is not zero,
when the code modification runs, that function will be enabled for tracing.
If the ref count is zero, that function will be disabled from tracing.

To make sure the accounting was working, FTRACE_WARN_ON()s were added
to updating of the ref counts.

If the ref count hits its max (> 2^30 ftrace_ops added), or if
the ref count goes below zero, a FTRACE_WARN_ON() is triggered which
disables all modification of code.

Since it is common for ftrace_ops to trace all functions in the kernel,
instead of creating > 20,000 hash items for the ftrace_ops, the hash
count is just set to zero, and it represents that the ftrace_ops is
to trace all functions. This is where the issues arrise.

If you enable function tracing to trace all functions, and then add
a module, the modules function records do not get the ref count updated.
When the function tracer is disabled, all function records ref counts
are subtracted. Since the modules never had their ref counts incremented,
they go below zero and the FTRACE_WARN_ON() is triggered.

The solution to this is rather simple. When modules are loaded, and
their functions are added to the the ftrace pool, look to see if any
ftrace_ops are registered that trace all functions. And for those,
update the ref count for the module function records.

Reported-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/ftrace.c |   30 ++++++++++++++++++++++++++++--
 1 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 1c4c0b0..ef9271b 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -1744,10 +1744,36 @@ static cycle_t		ftrace_update_time;
 static unsigned long	ftrace_update_cnt;
 unsigned long		ftrace_update_tot_cnt;
 
+static int ops_traces_mod(struct ftrace_ops *ops)
+{
+	struct ftrace_hash *hash;
+
+	hash = ops->filter_hash;
+	return !!(!hash || !hash->count);
+}
+
 static int ftrace_update_code(struct module *mod)
 {
 	struct dyn_ftrace *p;
 	cycle_t start, stop;
+	unsigned long ref = 0;
+
+	/*
+	 * When adding a module, we need to check if tracers are
+	 * currently enabled and if they are set to trace all functions.
+	 * If they are, we need to enable the module functions as well
+	 * as update the reference counts for those function records.
+	 */
+	if (mod) {
+		struct ftrace_ops *ops;
+
+		for (ops = ftrace_ops_list;
+		     ops != &ftrace_list_end; ops = ops->next) {
+			if (ops->flags & FTRACE_OPS_FL_ENABLED &&
+			    ops_traces_mod(ops))
+				ref++;
+		}
+	}
 
 	start = ftrace_now(raw_smp_processor_id());
 	ftrace_update_cnt = 0;
@@ -1760,7 +1786,7 @@ static int ftrace_update_code(struct module *mod)
 
 		p = ftrace_new_addrs;
 		ftrace_new_addrs = p->newlist;
-		p->flags = 0L;
+		p->flags = ref;
 
 		/*
 		 * Do the initial record conversion from mcount jump
@@ -1783,7 +1809,7 @@ static int ftrace_update_code(struct module *mod)
 		 * conversion puts the module to the correct state, thus
 		 * passing the ftrace_make_call check.
 		 */
-		if (ftrace_start_up) {
+		if (ftrace_start_up && ref) {
 			int failed = __ftrace_replace_code(p, 1);
 			if (failed) {
 				ftrace_bug(failed, p->ip);
-- 
1.7.5.4

Re: [PATCH 0/2] [GIT PULL][v3.0] ftrace: urgent fixes

[Ingo Molnar]

* Steven Rostedt <rostedt@goodmis.org> wrote:

> Ingo,
> 
> I noticed that tip/perf/urgent wasn't pulled yet. I added it to this
> pull request. The first patch is what you reported to me, the second
> is a regression that Thomas stumbled upon.
> 
> Please pull the latest tip/perf/urgent-2 tree, which can be found at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
> tip/perf/urgent-2
> 
> Head SHA1: f7bc8b61f65726ff98f52e286b28e294499d7a08
> 
> 
> Steven Rostedt (2):
>       ftrace: Fix warning when CONFIG_FUNCTION_TRACER is not defined
>       ftrace: Fix regression where ftrace breaks when modules are loaded
> 
> ----
>  include/linux/ftrace.h |    4 ++--
>  kernel/trace/ftrace.c  |   30 ++++++++++++++++++++++++++++--
>  2 files changed, 30 insertions(+), 4 deletions(-)

Pulled, thanks a lot Steve!

	Ingo