[for-next][PATCH 0/4] ftrace: Fix the tramp count accounting

[for-next][PATCH 0/4] ftrace: Fix the tramp count accounting

[Steven Rostedt]

Thanks to Oleg Nesterov for pointing out some of the bugs.


  git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
for-next

Head SHA1: dc6f03f26f570104a2bb03f9d1deb588026d7c75


Steven Rostedt (Red Hat) (4):
      ftrace: Rename ftrace_ops field from trampolines to nr_trampolines
      ring-buffer: Use rb_page_size() instead of open coded head_page size
      ftrace: Fix trampoline hash update check on rec->flags
      ftrace: Add warning if tramp hash does not match nr_trampolines

----
 include/linux/ftrace.h     |  2 +-
 kernel/trace/ftrace.c      | 21 ++++++++++++++++-----
 kernel/trace/ring_buffer.c |  2 +-
 3 files changed, 18 insertions(+), 7 deletions(-)

[for-next][PATCH 1/4] ftrace: Rename ftrace_ops field from trampolines to nr_trampolines

[Steven Rostedt]

[-- Attachment #1: 0001-ftrace-Rename-ftrace_ops-field-from-trampolines-to-n.patch --]
[-- Type: text/plain, Size: 2433 bytes --]

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

Having two fields within the same struct that is off by one character
can be confusing and error prone. Rename the counter "trampolines"
to "nr_trampolines" to explicitly show it is a counter and not to
be confused by the "trampoline" field.

Suggested-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 include/linux/ftrace.h |  2 +-
 kernel/trace/ftrace.c  | 10 +++++-----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 7a5b7b97e539..6bb5e3f2a3b4 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -120,7 +120,7 @@ struct ftrace_ops {
 	void				*private;
 	int __percpu			*disabled;
 #ifdef CONFIG_DYNAMIC_FTRACE
-	int				trampolines;
+	int				nr_trampolines;
 	struct ftrace_hash		*notrace_hash;
 	struct ftrace_hash		*filter_hash;
 	struct ftrace_hash		*tramp_hash;
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 762806026561..eda69c9f78d0 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -1513,7 +1513,7 @@ static void ftrace_remove_tramp(struct ftrace_ops *ops,
 	 * The tramp_hash entry will be removed at time
 	 * of update.
 	 */
-	ops->trampolines--;
+	ops->nr_trampolines--;
 	rec->flags &= ~FTRACE_FL_TRAMP;
 }
 
@@ -1522,7 +1522,7 @@ static void ftrace_clear_tramps(struct dyn_ftrace *rec)
 	struct ftrace_ops *op;
 
 	do_for_each_ftrace_op(op, ftrace_ops_list) {
-		if (op->trampolines)
+		if (op->nr_trampolines)
 			ftrace_remove_tramp(op, rec);
 	} while_for_each_ftrace_op(op);
 }
@@ -1617,7 +1617,7 @@ static void __ftrace_hash_rec_update(struct ftrace_ops *ops,
 			 */
 			if (ftrace_rec_count(rec) == 1 && ops->trampoline) {
 				rec->flags |= FTRACE_FL_TRAMP;
-				ops->trampolines++;
+				ops->nr_trampolines++;
 			} else {
 				/*
 				 * If we are adding another function callback
@@ -2185,7 +2185,7 @@ static int ftrace_save_ops_tramp_hash(struct ftrace_ops *ops)
 	int size, bits;
 	int ret;
 
-	size = ops->trampolines;
+	size = ops->nr_trampolines;
 	bits = 0;
 	/*
 	 * Make the hash size about 1/2 the # found
@@ -2239,7 +2239,7 @@ static int ftrace_save_tramp_hashes(void)
 		free_ftrace_hash(op->tramp_hash);
 		op->tramp_hash = NULL;
 
-		if (op->trampolines) {
+		if (op->nr_trampolines) {
 			ret = ftrace_save_ops_tramp_hash(op);
 			if (ret)
 				return ret;
-- 
2.0.1

[for-next][PATCH 2/4] ring-buffer: Use rb_page_size() instead of open coded head_page size

[Steven Rostedt]

[-- Attachment #1: 0002-ring-buffer-Use-rb_page_size-instead-of-open-coded-h.patch --]
[-- Type: text/plain, Size: 840 bytes --]

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

There's a helper function to get a ring buffer page size (the number
of bytes of data recorded on the page), called rb_page_size().
Use that instead of open coding it.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/ring_buffer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 35825a87d6a3..d8c267ec5cca 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -3763,7 +3763,7 @@ rb_iter_peek(struct ring_buffer_iter *iter, u64 *ts)
 	if (rb_per_cpu_empty(cpu_buffer))
 		return NULL;
 
-	if (iter->head >= local_read(&iter->head_page->page->commit)) {
+	if (iter->head >= rb_page_size(iter->head_page)) {
 		rb_inc_iter(iter);
 		goto again;
 	}
-- 
2.0.1

[for-next][PATCH 3/4] ftrace: Fix trampoline hash update check on rec->flags

[Steven Rostedt]

[-- Attachment #1: 0003-ftrace-Fix-trampoline-hash-update-check-on-rec-flags.patch --]
[-- Type: text/plain, Size: 3102 bytes --]

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

In the loop of ftrace_save_ops_tramp_hash(), it adds all the recs
to the ops hash if the rec has only one callback attached and the
ops is connected to the rec. It gives a nasty warning and shuts down
ftrace if the rec doesn't have a trampoline set for it. But this
can happen with the following scenario:

  # cd /sys/kernel/debug/tracing
  # echo schedule do_IRQ > set_ftrace_filter
  # mkdir instances/foo
  # echo schedule > instances/foo/set_ftrace_filter
  # echo function_graph > current_function
  # echo function > instances/foo/current_function
  # echo nop > instances/foo/current_function

The above would then trigger the following warning and disable
ftrace:

 ------------[ cut here ]------------
 WARNING: CPU: 0 PID: 3145 at kernel/trace/ftrace.c:2212 ftrace_run_update_code+0xe4/0x15b()
 Modules linked in: ipt_MASQUERADE sunrpc ip6t_REJECT nf_conntrack_ipv6 nf_defrag_ip [...]
 CPU: 1 PID: 3145 Comm: bash Not tainted 3.16.0-rc3-test+ #136
 Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./To be filled by O.E.M., BIOS SDBLI944.86P 05/08/2007
  0000000000000000 ffffffff81808a88 ffffffff81502130 0000000000000000
  ffffffff81040ca1 ffff880077c08000 ffffffff810bd286 0000000000000001
  ffffffff81a56830 ffff88007a041be0 ffff88007a872d60 00000000000001be
 Call Trace:
  [<ffffffff81502130>] ? dump_stack+0x4a/0x75
  [<ffffffff81040ca1>] ? warn_slowpath_common+0x7e/0x97
  [<ffffffff810bd286>] ? ftrace_run_update_code+0xe4/0x15b
  [<ffffffff810bd286>] ? ftrace_run_update_code+0xe4/0x15b
  [<ffffffff810bda1a>] ? ftrace_shutdown+0x11c/0x16b
  [<ffffffff810bda87>] ? unregister_ftrace_function+0x1e/0x38
  [<ffffffff810cc7e1>] ? function_trace_reset+0x1a/0x28
  [<ffffffff810c924f>] ? tracing_set_tracer+0xc1/0x276
  [<ffffffff810c9477>] ? tracing_set_trace_write+0x73/0x91
  [<ffffffff81132383>] ? __sb_start_write+0x9a/0xcc
  [<ffffffff8120478f>] ? security_file_permission+0x1b/0x31
  [<ffffffff81130e49>] ? vfs_write+0xac/0x11c
  [<ffffffff8113115d>] ? SyS_write+0x60/0x8e
  [<ffffffff81508112>] ? system_call_fastpath+0x16/0x1b
 ---[ end trace 938c4415cbc7dc96 ]---
 ------------[ cut here ]------------

Link: http://lkml.kernel.org/r/20140723120805.GB21376@redhat.com

Reported-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/ftrace.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index eda69c9f78d0..6ef1989c2b2e 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -2208,6 +2208,14 @@ static int ftrace_save_ops_tramp_hash(struct ftrace_ops *ops)
 		if (ftrace_rec_count(rec) == 1 &&
 		    ftrace_ops_test(ops, rec->ip, rec)) {
 
+			/*
+			 * If another ops adds to a rec, the rec will
+			 * lose its trampoline and never get it back
+			 * until all ops are off of it.
+			 */
+			if (!(rec->flags & FTRACE_FL_TRAMP))
+				continue;
+
 			/* This record had better have a trampoline */
 			if (FTRACE_WARN_ON(!(rec->flags & FTRACE_FL_TRAMP_EN)))
 				return -1;
-- 
2.0.1

[for-next][PATCH 4/4] ftrace: Add warning if tramp hash does not match nr_trampolines

[Steven Rostedt]

[-- Attachment #1: 0004-ftrace-Add-warning-if-tramp-hash-does-not-match-nr_t.patch --]
[-- Type: text/plain, Size: 833 bytes --]

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

After adding all the records to the tramp_hash, add a check that makes
sure that the number of records added matches the number of records
expected to match and do a WARN_ON and disable ftrace if they do
not match.

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

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 6ef1989c2b2e..979bd8cb4349 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -2226,6 +2226,9 @@ static int ftrace_save_ops_tramp_hash(struct ftrace_ops *ops)
 		}
 	} while_for_each_ftrace_rec();
 
+	/* The number of recs in the hash must match nr_trampolines */
+	FTRACE_WARN_ON(ops->tramp_hash->count != ops->nr_trampolines);
+
 	return 0;
 }
 
-- 
2.0.1