linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC][PATCH 0/8] ftrace/rcu: Handle unsafe RCU functions and ftrace callbacks
@ 2013-08-30 13:02 Steven Rostedt
  2013-08-30 13:02 ` [RFC][PATCH 1/8] ftrace: Add hash list to save RCU unsafe functions Steven Rostedt
                   ` (7 more replies)
  0 siblings, 8 replies; 10+ messages in thread
From: Steven Rostedt @ 2013-08-30 13:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: Ingo Molnar, Andrew Morton, Peter Zijlstra, Frederic Weisbecker,
	Paul E. McKenney, Jiri Olsa, Dave Jones

Function tracing of RCU is extremely helpful in debugging, as RCU is
quite complex, especially with the new dynamic ticks code.

There are some kernel functions that are called when RCU is ignoring
the given CPU. For example, when coming out of userspace, the
"rcu_user_exit()" call is the function that tells RCU to start tracking
the given CPU again. But as the function tracer traces rcu_user_exit()
if a callback uses a rcu_read_lock() it will not do what is expected
as RCU is ignoring the given CPU and the rcu_read_lock() will not extend
the current grace period. You can see debug splats from RCU because
of this:

[53693.297516] ===============================
[53693.298109] [ INFO: suspicious RCU usage. ]
[53693.298562] 3.10.0-rc2+ #38 Not tainted
[53693.299017] -------------------------------
[53693.299474] include/linux/rcupdate.h:771 rcu_read_lock() used illegally while idle!
[53693.299959] 
other info that might help us debug this:

[53693.301420] RCU used illegally from idle CPU!  rcu_scheduler_active = 1, debug_locks = 0
[53693.302918] RCU used illegally from extended quiescent state!
[53693.303462] 1 lock held by trinity-child1/18786:
[53693.303966]  #0:  (rcu_read_lock){.+.+..}, at: [<ffffffff8113dd48>] __perf_event_overflow+0x108/0x310
[53693.304557] stack backtrace:
[53693.305608] CPU: 3 PID: 18786 Comm: trinity-child1 Not tainted 3.10.0-rc2+ #38 
[53693.306790]  0000000000000000 ffff88020767bac8 ffffffff816e2f6b ffff88020767baf8
[53693.307408]  ffffffff810b5897 ffff88021de92520 0000000000000000 ffff88020767bbf8
[53693.308035]  0000000000000000 ffff88020767bb78 ffffffff8113ded4 ffffffff8113dd48
[53693.308671] Call Trace:
[53693.309301]  [<ffffffff816e2f6b>] dump_stack+0x19/0x1b
[53693.309943]  [<ffffffff810b5897>] lockdep_rcu_suspicious+0xe7/0x120
[53693.310596]  [<ffffffff8113ded4>] __perf_event_overflow+0x294/0x310
[53693.311256]  [<ffffffff8113dd48>] ? __perf_event_overflow+0x108/0x310
[53693.311923]  [<ffffffff81309289>] ? __const_udelay+0x29/0x30
[53693.312596]  [<ffffffff81076054>] ? __rcu_read_unlock+0x54/0xa0
[53693.313275]  [<ffffffff816f4000>] ? ftrace_call+0x5/0x2f
[53693.313958]  [<ffffffff8113dfa1>] perf_swevent_overflow+0x51/0xe0
[53693.314650]  [<ffffffff8113e08f>] perf_swevent_event+0x5f/0x90
[53693.315347]  [<ffffffff8113e1c9>] perf_tp_event+0x109/0x4f0
[53693.316059]  [<ffffffff8113e36f>] ? perf_tp_event+0x2af/0x4f0
[53693.316773]  [<ffffffff81074630>] ? __rcu_read_lock+0x20/0x20
[53693.317485]  [<ffffffff8112d79f>] perf_ftrace_function_call+0xbf/0xd0
[53693.318207]  [<ffffffff8110e1e1>] ? ftrace_ops_control_func+0x181/0x210
[53693.318921]  [<ffffffff81074630>] ? __rcu_read_lock+0x20/0x20
[53693.319621]  [<ffffffff81100cae>] ? rcu_eqs_enter_common+0x5e/0x470
[53693.320330]  [<ffffffff8110e1e1>] ftrace_ops_control_func+0x181/0x210
[53693.321046]  [<ffffffff816f4000>] ftrace_call+0x5/0x2f
[53693.321763]  [<ffffffff8110e229>] ? ftrace_ops_control_func+0x1c9/0x210
[53693.322490]  [<ffffffff816f4000>] ? ftrace_call+0x5/0x2f
[53693.323221]  [<ffffffff81074635>] ? debug_lockdep_rcu_enabled+0x5/0x40
[53693.323988]  [<ffffffff81074635>] ? debug_lockdep_rcu_enabled+0x5/0x40
[53693.324728]  [<ffffffff81100cae>] ? rcu_eqs_enter_common+0x5e/0x470
[53693.325489]  [<ffffffff8110112a>] rcu_eqs_enter+0x6a/0xb0
[53693.326273]  [<ffffffff81103673>] rcu_user_enter+0x13/0x20
[53693.327025]  [<ffffffff8114541a>] user_enter+0x6a/0xd0
[53693.327783]  [<ffffffff8100f6d8>] syscall_trace_leave+0x78/0x140
[53693.328551]  [<ffffffff816f46af>] int_check_syscall_exit_work+0x34/0x3d
 
The above splat comes from tracing rcu_eqs_enter_common, and the
perf callback uses rcu_read_lock(), but at this moment, RCU is not
tracking the current CPU.

Instead of adding notrace to all these locations and lose the ability
to trace these critical RCU functions for the few users that use RCU,
this patch set creates an infrastructure to disable tracing of these
"unsafe" RCU functions by ftrace callbacks that do not explicitly tell
ftrace it does not use RCU.

A new macro is created called "FTRACE_UNSAFE_RCU()" which is used just
like EXPORT_SYMBOL(). For example:

static void rcu_eqs_enter(bool user)
{
	[...]
}
FTRACE_UNSAFE_RCU(rcu_eqs_enter);

Then the rcu_eqs_enter() function will not be traced by perf or any
other ftrace callback that does not explicitly state it does not use
RCU.

The last patch of the series is not complete. I only added a few
RCU unsafe functions to test the code. It is not a complete list.
I'll be adding another CONFIG test that adds a rcu unsafe callback
that traces everything, and will trigger the RCU warnings if something
is not covered by the FTRACE_UNSAFE_RCU(). This will help in catching
all functions that need this macro.

-- Steve

Steven Rostedt (Red Hat) (8):
      ftrace: Add hash list to save RCU unsafe functions
      ftrace: Do not set ftrace records for unsafe RCU when not allowed
      ftrace: Set ftrace internal function tracing RCU safe
      ftrace: Add test for ops against unsafe RCU functions in callback
      ftrace: Do not display non safe RCU functions in available_filter_functions
      ftrace: Add rcu_unsafe_filter_functions file
      ftrace: Add selftest to check if RCU unsafe functions are filtered properly
      rcu/ftrace: Add FTRACE_UNSAFE_RCU() to unsafe RCU functions

----
 include/asm-generic/vmlinux.lds.h     |   10 ++
 include/linux/ftrace.h                |   40 ++++++++
 kernel/rcutree.c                      |    4 +
 kernel/trace/ftrace.c                 |  165 ++++++++++++++++++++++++++++++++-
 kernel/trace/trace.h                  |    2 +
 kernel/trace/trace_selftest.c         |   94 +++++++++++++++++++
 kernel/trace/trace_selftest_dynamic.c |    7 ++
 7 files changed, 319 insertions(+), 3 deletions(-)

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [RFC][PATCH 1/8] ftrace: Add hash list to save RCU unsafe functions
  2013-08-30 13:02 [RFC][PATCH 0/8] ftrace/rcu: Handle unsafe RCU functions and ftrace callbacks Steven Rostedt
@ 2013-08-30 13:02 ` Steven Rostedt
  2013-08-30 13:53   ` Steven Rostedt
  2013-08-30 13:02 ` [RFC][PATCH 2/8] ftrace: Do not set ftrace records for unsafe RCU when not allowed Steven Rostedt
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 10+ messages in thread
From: Steven Rostedt @ 2013-08-30 13:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: Ingo Molnar, Andrew Morton, Peter Zijlstra, Frederic Weisbecker,
	Paul E. McKenney, Jiri Olsa, Dave Jones

[-- Attachment #1: 0001-ftrace-Add-hash-list-to-save-RCU-unsafe-functions.patch --]
[-- Type: text/plain, Size: 7908 bytes --]

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

Some ftrace function tracing callbacks use RCU (perf), thus if
it gets called from tracing a function outside of the RCU tracking,
like in entering or leaving NO_HZ idle/userspace, the RCU read locks
in the callback are useless.

As normal function tracing does not use RCU, and function tracing
happens to help debug RCU, we do not want to limit all callbacks
from tracing these "unsafe" RCU functions. Instead, we create an
infrastructure that will let us mark these unsafe RCU functions
and we will only allow callbacks that explicitly say they do not
use RCU to be called by these functions.

This patch adds the infrastructure to create the hash of functions
that are RCU unsafe. This is done with the FTRACE_UNSAFE_RCU()
macro. It works like EXPORT_SYMBOL() does. Simply place the macro
under the RCU unsafe function:

void func(void)
{
	[...]
}
FTRACE_UNSAFE_RCU(func);

Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 include/asm-generic/vmlinux.lds.h |   10 ++++
 include/linux/ftrace.h            |   38 ++++++++++++++
 kernel/trace/ftrace.c             |  105 +++++++++++++++++++++++++++++++++++++
 3 files changed, 153 insertions(+)

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 69732d2..fdfddd2 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -93,6 +93,15 @@
 #define MCOUNT_REC()
 #endif
 
+#ifdef CONFIG_FUNCTION_TRACER
+#define FTRACE_UNSAFE_RCU()	. = ALIGN(8);				\
+			VMLINUX_SYMBOL(__start_ftrace_unsafe_rcu) = .;	\
+			*(_ftrace_unsafe_rcu)				\
+			VMLINUX_SYMBOL(__stop_ftrace_unsafe_rcu) = .;
+#else
+#define FTRACE_UNSAFE_RCU()
+#endif
+
 #ifdef CONFIG_TRACE_BRANCH_PROFILING
 #define LIKELY_PROFILE()	VMLINUX_SYMBOL(__start_annotated_branch_profile) = .; \
 				*(_ftrace_annotated_branch)			      \
@@ -479,6 +488,7 @@
 	MEM_DISCARD(init.data)						\
 	KERNEL_CTORS()							\
 	MCOUNT_REC()							\
+	FTRACE_UNSAFE_RCU()						\
 	*(.init.rodata)							\
 	FTRACE_EVENTS()							\
 	TRACE_SYSCALLS()						\
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 9f15c00..1d17a82 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -92,6 +92,9 @@ typedef void (*ftrace_func_t)(unsigned long ip, unsigned long parent_ip,
  * STUB   - The ftrace_ops is just a place holder.
  * INITIALIZED - The ftrace_ops has already been initialized (first use time
  *            register_ftrace_function() is called, it will initialized the ops)
+ * RCU_SAFE - The callback uses no rcu type locking. This allows the
+ *            callback to be called in locations that RCU is not active.
+ *            (like going into or coming out of idle NO_HZ)
  */
 enum {
 	FTRACE_OPS_FL_ENABLED			= 1 << 0,
@@ -103,6 +106,7 @@ enum {
 	FTRACE_OPS_FL_RECURSION_SAFE		= 1 << 6,
 	FTRACE_OPS_FL_STUB			= 1 << 7,
 	FTRACE_OPS_FL_INITIALIZED		= 1 << 8,
+	FTRACE_OPS_FL_RCU_SAFE			= 1 << 9,
 };
 
 struct ftrace_ops {
@@ -219,6 +223,40 @@ static inline int ftrace_function_local_disabled(struct ftrace_ops *ops)
 extern void ftrace_stub(unsigned long a0, unsigned long a1,
 			struct ftrace_ops *op, struct pt_regs *regs);
 
+/*
+ * In order to speed up boot, save both the name and the
+ * address of the function to find to add to hashes. The
+ * list of function mcount addresses are sorted by the address,
+ * but we still need to use the name to find the actual location
+ * as the mcount call is usually not at the address of the
+ * start of the function.
+ */
+struct ftrace_func_finder {
+	const char		*name;
+	unsigned long		ip;
+};
+
+/*
+ * For functions that are called when RCU is not tracking the CPU
+ * (like for entering or leaving NO_HZ mode, and RCU then ignores
+ * the CPU), they need to be marked with FTRACE_UNSAFE_RCU().
+ * This will prevent all function tracing callbacks from calling
+ * this function unless the callback explicitly states that it
+ * doesn't use RCU with the FTRACE_OPS_FL_RCU_SAFE flag.
+ *
+ * The usage of FTRACE_UNSAFE_RCU() is the same as EXPORT_SYMBOL().
+ * Just place the macro underneath the function that is unsafe.
+ */
+#define FTRACE_UNSAFE_RCU(func) \
+	static struct ftrace_func_finder __ftrace_unsafe_rcu_##func	\
+	 __initdata = {							\
+		.name = #func,						\
+		.ip = (unsigned long)func,				\
+	};							\
+	struct ftrace_func_finder *__ftrace_unsafe_rcu_ptr_##func	\
+		  __attribute__((__section__("_ftrace_unsafe_rcu"))) =	\
+		&__ftrace_unsafe_rcu_##func
+
 #else /* !CONFIG_FUNCTION_TRACER */
 /*
  * (un)register_ftrace_function must be a macro since the ops parameter
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index a6d098c..2580cdb 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -1486,6 +1486,72 @@ ftrace_ops_test(struct ftrace_ops *ops, unsigned long ip, void *regs)
 	}
 
 
+static __init int ftrace_find_ip(const void *a, const void *b)
+{
+	const struct dyn_ftrace *key = a;
+	const struct dyn_ftrace *rec = b;
+
+	if (key->ip == rec->ip)
+		return 0;
+
+	if (key->ip < rec->ip) {
+		/* If previous is less than, then this is our func */
+		rec--;
+		if (rec->ip < key->ip)
+			return 0;
+		return -1;
+	}
+
+	/* All else, we are greater */
+	return 1;
+}
+
+/*
+ * Find the mcount caller location given the ip address of the
+ * function that contains it. As the mcount caller is usually
+ * after the mcount itself.
+ *
+ * Done for just core functions at boot.
+ */
+static __init unsigned long ftrace_mcount_from_func(unsigned long ip)
+{
+	struct ftrace_page *pg, *last_pg = NULL;
+	struct dyn_ftrace *rec;
+	struct dyn_ftrace key;
+
+	key.ip = ip;
+
+	for (pg = ftrace_pages_start; pg; last_pg = pg, pg = pg->next) {
+		if (ip >= (pg->records[pg->index - 1].ip + MCOUNT_INSN_SIZE))
+			break;
+
+		/*
+		 * Do the first record manually, as we need to check
+		 * the previous record from the previous page
+		 */
+		if (ip < pg->records[0].ip) {
+			/* First page? Then we found our record! */
+			if (!last_pg)
+				return pg->records[0].ip;
+
+			rec = &last_pg->records[last_pg->index - 1];
+			if (rec->ip < ip)
+				return pg->records[0].ip;
+
+			/* Not found */
+			return 0;
+		}
+
+		rec = bsearch(&key, pg->records + 1, pg->index - 1,
+			      sizeof(struct dyn_ftrace),
+			      ftrace_find_ip);
+		if (rec)
+			return rec->ip;
+	}
+
+	return 0;
+}
+
 static int ftrace_cmp_recs(const void *a, const void *b)
 {
 	const struct dyn_ftrace *key = a;
@@ -4211,6 +4277,43 @@ struct notifier_block ftrace_module_exit_nb = {
 	.priority = INT_MIN,	/* Run after anything that can remove kprobes */
 };
 
+extern struct ftrace_func_finder *__start_ftrace_unsafe_rcu[];
+extern struct ftrace_func_finder *__stop_ftrace_unsafe_rcu[];
+
+static struct ftrace_hash *ftrace_unsafe_rcu;
+
+static void __init create_unsafe_rcu_hash(void)
+{
+	struct ftrace_func_finder *finder;
+	struct ftrace_func_finder **p;
+	unsigned long ip;
+	char str[KSYM_SYMBOL_LEN];
+	int count;
+
+	count = __stop_ftrace_unsafe_rcu - __start_ftrace_unsafe_rcu;
+	if (!count)
+		return;
+
+	ftrace_unsafe_rcu = alloc_ftrace_hash(count);
+
+	for (p = __start_ftrace_unsafe_rcu;
+	     p < __stop_ftrace_unsafe_rcu;
+	     p++) {
+		finder = *p;
+
+		ip = ftrace_mcount_from_func(finder->ip);
+		if (WARN_ON_ONCE(!ip))
+			continue;
+
+		/* Make sure this is our ip */
+		kallsyms_lookup(ip, NULL, NULL, NULL, str);
+		if (WARN_ON_ONCE(strcmp(str, finder->name) != 0))
+			continue;
+
+		add_hash_entry(ftrace_unsafe_rcu, ip);
+	}
+}
+
 extern unsigned long __start_mcount_loc[];
 extern unsigned long __stop_mcount_loc[];
 
@@ -4250,6 +4353,8 @@ void __init ftrace_init(void)
 	if (ret)
 		pr_warning("Failed to register trace ftrace module exit notifier\n");
 
+	create_unsafe_rcu_hash();
+
 	set_ftrace_early_filters();
 
 	return;
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [RFC][PATCH 2/8] ftrace: Do not set ftrace records for unsafe RCU when not allowed
  2013-08-30 13:02 [RFC][PATCH 0/8] ftrace/rcu: Handle unsafe RCU functions and ftrace callbacks Steven Rostedt
  2013-08-30 13:02 ` [RFC][PATCH 1/8] ftrace: Add hash list to save RCU unsafe functions Steven Rostedt
@ 2013-08-30 13:02 ` Steven Rostedt
  2013-08-30 13:02 ` [RFC][PATCH 3/8] ftrace: Set ftrace internal function tracing RCU safe Steven Rostedt
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Steven Rostedt @ 2013-08-30 13:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: Ingo Molnar, Andrew Morton, Peter Zijlstra, Frederic Weisbecker,
	Paul E. McKenney, Jiri Olsa, Dave Jones

[-- Attachment #1: 0002-ftrace-Do-not-set-ftrace-records-for-unsafe-RCU-when.patch --]
[-- Type: text/plain, Size: 2614 bytes --]

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

For the ftrace_ops that use RCU read locks, and can not be called by
unsafe RCU functions (those outside of RCU tracking), have them not
update the RCU unsafe function records when they are being registered
or unregistered.

The ftrace function records store a counter of all the ftrace_ops
callbacks that are hooked to the function the record represents.
As unsafe RCU functions do not call callbacks that do not specify
that they do not use RCU, do not update those records.

Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/ftrace.c |   19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 2580cdb..dc11951 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -1168,6 +1168,12 @@ static struct ftrace_page *ftrace_new_pgs;
 static struct ftrace_page	*ftrace_pages_start;
 static struct ftrace_page	*ftrace_pages;
 
+/*
+ * Hash of functions that are not safe to be called by
+ * callbacks that use RCU read locks.
+ */
+static struct ftrace_hash *ftrace_unsafe_rcu;
+
 static bool ftrace_hash_empty(struct ftrace_hash *hash)
 {
 	return !hash || !hash->count;
@@ -1627,6 +1633,7 @@ static void __ftrace_hash_rec_update(struct ftrace_ops *ops,
 {
 	struct ftrace_hash *hash;
 	struct ftrace_hash *other_hash;
+	struct ftrace_hash *rcu_hash;
 	struct ftrace_page *pg;
 	struct dyn_ftrace *rec;
 	int count = 0;
@@ -1636,6 +1643,12 @@ static void __ftrace_hash_rec_update(struct ftrace_ops *ops,
 	if (!(ops->flags & FTRACE_OPS_FL_ENABLED))
 		return;
 
+	/* Ignore rcu unsafe functions unless ops handles them */
+	if (ops->flags & FTRACE_OPS_FL_RCU_SAFE)
+		rcu_hash = NULL;
+	else
+		rcu_hash = ftrace_unsafe_rcu;
+
 	/*
 	 * In the filter_hash case:
 	 *   If the count is zero, we update all records.
@@ -1669,6 +1682,10 @@ static void __ftrace_hash_rec_update(struct ftrace_ops *ops,
 		int in_hash = 0;
 		int match = 0;
 
+		/* Ignore all in the rcu unsafe hash */
+		if (ftrace_lookup_ip(rcu_hash, rec->ip))
+			continue;
+
 		if (all) {
 			/*
 			 * Only the filter_hash affects all records.
@@ -4280,8 +4297,6 @@ struct notifier_block ftrace_module_exit_nb = {
 extern struct ftrace_func_finder *__start_ftrace_unsafe_rcu[];
 extern struct ftrace_func_finder *__stop_ftrace_unsafe_rcu[];
 
-static struct ftrace_hash *ftrace_unsafe_rcu;
-
 static void __init create_unsafe_rcu_hash(void)
 {
 	struct ftrace_func_finder *finder;
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [RFC][PATCH 3/8] ftrace: Set ftrace internal function tracing RCU safe
  2013-08-30 13:02 [RFC][PATCH 0/8] ftrace/rcu: Handle unsafe RCU functions and ftrace callbacks Steven Rostedt
  2013-08-30 13:02 ` [RFC][PATCH 1/8] ftrace: Add hash list to save RCU unsafe functions Steven Rostedt
  2013-08-30 13:02 ` [RFC][PATCH 2/8] ftrace: Do not set ftrace records for unsafe RCU when not allowed Steven Rostedt
@ 2013-08-30 13:02 ` Steven Rostedt
  2013-08-30 13:02 ` [RFC][PATCH 4/8] ftrace: Add test for ops against unsafe RCU functions in callback Steven Rostedt
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Steven Rostedt @ 2013-08-30 13:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: Ingo Molnar, Andrew Morton, Peter Zijlstra, Frederic Weisbecker,
	Paul E. McKenney, Jiri Olsa, Dave Jones

[-- Attachment #1: 0003-ftrace-Set-ftrace-internal-function-tracing-RCU-safe.patch --]
[-- Type: text/plain, Size: 957 bytes --]

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

Since none of the internal ftrace function tracing uses RCU in
their callbacks, it is OK to set the global_ops (the one that
they all use) to RCU safe.

Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/ftrace.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index dc11951..3761663 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -1146,7 +1146,9 @@ static struct ftrace_ops global_ops = {
 	.func			= ftrace_stub,
 	.notrace_hash		= EMPTY_HASH,
 	.filter_hash		= EMPTY_HASH,
-	.flags			= FTRACE_OPS_FL_RECURSION_SAFE | FTRACE_OPS_FL_INITIALIZED,
+	.flags			= FTRACE_OPS_FL_RECURSION_SAFE |
+				  FTRACE_OPS_FL_INITIALIZED |
+				  FTRACE_OPS_FL_RCU_SAFE,
 	INIT_REGEX_LOCK(global_ops)
 };
 
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [RFC][PATCH 4/8] ftrace: Add test for ops against unsafe RCU functions in callback
  2013-08-30 13:02 [RFC][PATCH 0/8] ftrace/rcu: Handle unsafe RCU functions and ftrace callbacks Steven Rostedt
                   ` (2 preceding siblings ...)
  2013-08-30 13:02 ` [RFC][PATCH 3/8] ftrace: Set ftrace internal function tracing RCU safe Steven Rostedt
@ 2013-08-30 13:02 ` Steven Rostedt
  2013-08-30 13:02 ` [RFC][PATCH 5/8] ftrace: Do not display non safe RCU functions in available_filter_functions Steven Rostedt
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Steven Rostedt @ 2013-08-30 13:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: Ingo Molnar, Andrew Morton, Peter Zijlstra, Frederic Weisbecker,
	Paul E. McKenney, Jiri Olsa, Dave Jones

[-- Attachment #1: 0004-ftrace-Add-test-for-ops-against-unsafe-RCU-functions.patch --]
[-- Type: text/plain, Size: 1640 bytes --]

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

When more than one ftrace_ops is registered, the list function is
is used to call all registered functions. It uses the filter and
notrace hashes from the ftrace_ops to determine if the corresponding
callback should be called or not.

Currently, it does not take into account for RCU unsafe functions.
If multiple functions are registered, and an RCU safe callback is
used on a RCU unsafe function, and the RCU unsafe callback says to
trace all functions, it will end up tracing this RCU unsafe function
and still suffer the problems when using RCU when RCU tracing is
off.

Add a test to the multiple ops list function to test if the ops in
question can use an RCU unsafe function or not, and if the function
being traced happens to be an RCU unsafe function.

Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/ftrace.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 3761663..c7ef936 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -1471,7 +1471,9 @@ ftrace_ops_test(struct ftrace_ops *ops, unsigned long ip, void *regs)
 	if ((ftrace_hash_empty(filter_hash) ||
 	     ftrace_lookup_ip(filter_hash, ip)) &&
 	    (ftrace_hash_empty(notrace_hash) ||
-	     !ftrace_lookup_ip(notrace_hash, ip)))
+	     !ftrace_lookup_ip(notrace_hash, ip)) &&
+	    (ops->flags & FTRACE_OPS_FL_RCU_SAFE ||
+	     !ftrace_lookup_ip(ftrace_unsafe_rcu, ip)))
 		ret = 1;
 	else
 		ret = 0;
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [RFC][PATCH 5/8] ftrace: Do not display non safe RCU functions in available_filter_functions
  2013-08-30 13:02 [RFC][PATCH 0/8] ftrace/rcu: Handle unsafe RCU functions and ftrace callbacks Steven Rostedt
                   ` (3 preceding siblings ...)
  2013-08-30 13:02 ` [RFC][PATCH 4/8] ftrace: Add test for ops against unsafe RCU functions in callback Steven Rostedt
@ 2013-08-30 13:02 ` Steven Rostedt
  2013-08-30 13:02 ` [RFC][PATCH 6/8] ftrace: Add rcu_unsafe_filter_functions file Steven Rostedt
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Steven Rostedt @ 2013-08-30 13:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: Ingo Molnar, Andrew Morton, Peter Zijlstra, Frederic Weisbecker,
	Paul E. McKenney, Jiri Olsa, Dave Jones

[-- Attachment #1: 0005-ftrace-Do-not-display-non-safe-RCU-functions-in-avai.patch --]
[-- Type: text/plain, Size: 1775 bytes --]

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

As available_filter_functions file displays functions that are generally
available for tracing, do not show the ones that are RCU unsafe. Otherwise
it may be confusing for perf users to see these functions in this file but
not be able to trace them.

Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 include/linux/ftrace.h |    1 +
 kernel/trace/ftrace.c  |    6 +++++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 1d17a82..4709264 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -411,6 +411,7 @@ enum {
 	FTRACE_ITER_DO_HASH	= (1 << 3),
 	FTRACE_ITER_HASH	= (1 << 4),
 	FTRACE_ITER_ENABLED	= (1 << 5),
+	FTRACE_ITER_NO_UNSAFE	= (1 << 6),
 };
 
 void arch_ftrace_update_code(int command);
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index c7ef936..6550f72 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -2634,7 +2634,10 @@ t_next(struct seq_file *m, void *v, loff_t *pos)
 		     !ftrace_lookup_ip(ops->notrace_hash, rec->ip)) ||
 
 		    ((iter->flags & FTRACE_ITER_ENABLED) &&
-		     !(rec->flags & FTRACE_FL_ENABLED))) {
+		     !(rec->flags & FTRACE_FL_ENABLED)) ||
+
+		    ((iter->flags & FTRACE_ITER_NO_UNSAFE) &&
+		     ftrace_lookup_ip(ftrace_unsafe_rcu, rec->ip))) {
 
 			rec = NULL;
 			goto retry;
@@ -2762,6 +2765,7 @@ ftrace_avail_open(struct inode *inode, struct file *file)
 	iter = __seq_open_private(file, &show_ftrace_seq_ops, sizeof(*iter));
 	if (iter) {
 		iter->pg = ftrace_pages_start;
+		iter->flags = FTRACE_ITER_NO_UNSAFE;
 		iter->ops = &global_ops;
 	}
 
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [RFC][PATCH 6/8] ftrace: Add rcu_unsafe_filter_functions file
  2013-08-30 13:02 [RFC][PATCH 0/8] ftrace/rcu: Handle unsafe RCU functions and ftrace callbacks Steven Rostedt
                   ` (4 preceding siblings ...)
  2013-08-30 13:02 ` [RFC][PATCH 5/8] ftrace: Do not display non safe RCU functions in available_filter_functions Steven Rostedt
@ 2013-08-30 13:02 ` Steven Rostedt
  2013-08-30 13:02 ` [RFC][PATCH 7/8] ftrace: Add selftest to check if RCU unsafe functions are filtered properly Steven Rostedt
  2013-08-30 13:02 ` [RFC][PATCH 8/8] rcu/ftrace: Add FTRACE_UNSAFE_RCU() to unsafe RCU functions Steven Rostedt
  7 siblings, 0 replies; 10+ messages in thread
From: Steven Rostedt @ 2013-08-30 13:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: Ingo Molnar, Andrew Morton, Peter Zijlstra, Frederic Weisbecker,
	Paul E. McKenney, Jiri Olsa, Dave Jones

[-- Attachment #1: 0006-ftrace-Add-rcu_unsafe_filter_functions-file.patch --]
[-- Type: text/plain, Size: 2859 bytes --]

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

Since the RCU unsafe functions are no longer displayed by the
available_filter_functions, we still need a way to see these
functions in order to trace them. Create a new file that lists
the functions that were declared RCU unsafe.

Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 include/linux/ftrace.h |    1 +
 kernel/trace/ftrace.c  |   31 +++++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 4709264..4752764 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -412,6 +412,7 @@ enum {
 	FTRACE_ITER_HASH	= (1 << 4),
 	FTRACE_ITER_ENABLED	= (1 << 5),
 	FTRACE_ITER_NO_UNSAFE	= (1 << 6),
+	FTRACE_ITER_UNSAFE_ONLY	= (1 << 7),
 };
 
 void arch_ftrace_update_code(int command);
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 6550f72..e2dc641 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -2636,6 +2636,9 @@ t_next(struct seq_file *m, void *v, loff_t *pos)
 		    ((iter->flags & FTRACE_ITER_ENABLED) &&
 		     !(rec->flags & FTRACE_FL_ENABLED)) ||
 
+		    ((iter->flags & FTRACE_ITER_UNSAFE_ONLY) &&
+		     !ftrace_lookup_ip(ftrace_unsafe_rcu, rec->ip)) ||
+
 		    ((iter->flags & FTRACE_ITER_NO_UNSAFE) &&
 		     ftrace_lookup_ip(ftrace_unsafe_rcu, rec->ip))) {
 
@@ -2773,6 +2776,24 @@ ftrace_avail_open(struct inode *inode, struct file *file)
 }
 
 static int
+ftrace_rcu_unsafe_open(struct inode *inode, struct file *file)
+{
+	struct ftrace_iterator *iter;
+
+	if (unlikely(ftrace_disabled))
+		return -ENODEV;
+
+	iter = __seq_open_private(file, &show_ftrace_seq_ops, sizeof(*iter));
+	if (iter) {
+		iter->pg = ftrace_pages_start;
+		iter->flags = FTRACE_ITER_UNSAFE_ONLY;
+		iter->ops = &global_ops;
+	}
+
+	return iter ? 0 : -ENOMEM;
+}
+
+static int
 ftrace_enabled_open(struct inode *inode, struct file *file)
 {
 	struct ftrace_iterator *iter;
@@ -3824,6 +3845,13 @@ static const struct file_operations ftrace_avail_fops = {
 	.release = seq_release_private,
 };
 
+static const struct file_operations ftrace_rcu_unsafe_fops = {
+	.open = ftrace_rcu_unsafe_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = seq_release_private,
+};
+
 static const struct file_operations ftrace_enabled_fops = {
 	.open = ftrace_enabled_open,
 	.read = seq_read,
@@ -4060,6 +4088,9 @@ static __init int ftrace_init_dyn_debugfs(struct dentry *d_tracer)
 	trace_create_file("available_filter_functions", 0444,
 			d_tracer, NULL, &ftrace_avail_fops);
 
+	trace_create_file("rcu_unsafe_filter_functions", 0444,
+			d_tracer, NULL, &ftrace_rcu_unsafe_fops);
+
 	trace_create_file("enabled_functions", 0444,
 			d_tracer, NULL, &ftrace_enabled_fops);
 
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [RFC][PATCH 7/8] ftrace: Add selftest to check if RCU unsafe functions are filtered properly
  2013-08-30 13:02 [RFC][PATCH 0/8] ftrace/rcu: Handle unsafe RCU functions and ftrace callbacks Steven Rostedt
                   ` (5 preceding siblings ...)
  2013-08-30 13:02 ` [RFC][PATCH 6/8] ftrace: Add rcu_unsafe_filter_functions file Steven Rostedt
@ 2013-08-30 13:02 ` Steven Rostedt
  2013-08-30 13:02 ` [RFC][PATCH 8/8] rcu/ftrace: Add FTRACE_UNSAFE_RCU() to unsafe RCU functions Steven Rostedt
  7 siblings, 0 replies; 10+ messages in thread
From: Steven Rostedt @ 2013-08-30 13:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: Ingo Molnar, Andrew Morton, Peter Zijlstra, Frederic Weisbecker,
	Paul E. McKenney, Jiri Olsa, Dave Jones

[-- Attachment #1: 0007-ftrace-Add-selftest-to-check-if-RCU-unsafe-functions.patch --]
[-- Type: text/plain, Size: 4480 bytes --]

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

Add a boot time start up test that has a RCU safe ftrace_ops as well
as an unsafe one. Make sure the RCU safe ops can trace RCU unsafe
functions while the unsafe ftrace_ops can not.

Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/trace.h                  |    2 +
 kernel/trace/trace_selftest.c         |   94 +++++++++++++++++++++++++++++++++
 kernel/trace/trace_selftest_dynamic.c |    7 +++
 3 files changed, 103 insertions(+)

diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 502fed7..3578be6 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -648,6 +648,8 @@ extern unsigned long ftrace_update_tot_cnt;
 extern int DYN_FTRACE_TEST_NAME(void);
 #define DYN_FTRACE_TEST_NAME2 trace_selftest_dynamic_test_func2
 extern int DYN_FTRACE_TEST_NAME2(void);
+#define DYN_FTRACE_TEST_RCU_UNSAFE trace_selftest_dynamic_test_rcu_unsafe
+extern int DYN_FTRACE_TEST_RCU_UNSAFE(void);
 
 extern bool ring_buffer_expanded;
 extern bool tracing_selftest_disabled;
diff --git a/kernel/trace/trace_selftest.c b/kernel/trace/trace_selftest.c
index a7329b7..24b790a 100644
--- a/kernel/trace/trace_selftest.c
+++ b/kernel/trace/trace_selftest.c
@@ -185,6 +185,97 @@ static void reset_counts(void)
 	trace_selftest_test_dyn_cnt = 0;
 }
 
+static struct ftrace_ops ftrace_rcu_safe = {
+	.func		= trace_selftest_test_probe1_func,
+	.flags		= FTRACE_OPS_FL_RECURSION_SAFE | FTRACE_OPS_FL_RCU_SAFE,
+};
+
+static struct ftrace_ops ftrace_rcu_unsafe = {
+	.func		= trace_selftest_test_probe2_func,
+	.flags		= FTRACE_OPS_FL_RECURSION_SAFE,
+};
+
+static int test_rcu_safe_unsafe(void)
+{
+	int save_ftrace_enabled = ftrace_enabled;
+	char *func_name;
+	int len;
+	int ret = 0;
+
+	printk(KERN_CONT "PASSED\n");
+	pr_info("Testing ftrace rcu safe unsafe: ");
+
+	ftrace_enabled = 1;
+
+	trace_selftest_test_probe1_cnt = 0;
+	trace_selftest_test_probe2_cnt = 0;
+
+	func_name = "*" __stringify(DYN_FTRACE_TEST_RCU_UNSAFE);
+	len = strlen(func_name);
+
+	ftrace_set_filter(&ftrace_rcu_safe, func_name, len, 1);
+	ftrace_set_filter(&ftrace_rcu_unsafe, func_name, len, 1);
+
+	/* Do single registrations first */
+	register_ftrace_function(&ftrace_rcu_safe);
+
+	DYN_FTRACE_TEST_RCU_UNSAFE();
+
+	unregister_ftrace_function(&ftrace_rcu_safe);
+
+	if (trace_selftest_test_probe1_cnt != 1) {
+		printk(KERN_CONT "rcu_safe expected 1 call but had %d ",
+		       trace_selftest_test_probe1_cnt);
+		goto out;
+	}
+
+	register_ftrace_function(&ftrace_rcu_unsafe);
+
+	DYN_FTRACE_TEST_RCU_UNSAFE();
+
+	unregister_ftrace_function(&ftrace_rcu_unsafe);
+
+	if (trace_selftest_test_probe2_cnt != 0) {
+		printk(KERN_CONT "rcu_safe expected 0 calls but had %d ",
+		       trace_selftest_test_probe2_cnt);
+		goto out;
+	}
+
+	/* Run both together, which uses the list op */
+
+	trace_selftest_test_probe1_cnt = 0;
+	trace_selftest_test_probe2_cnt = 0;
+
+	register_ftrace_function(&ftrace_rcu_safe);
+	register_ftrace_function(&ftrace_rcu_unsafe);
+
+	DYN_FTRACE_TEST_RCU_UNSAFE();
+	DYN_FTRACE_TEST_RCU_UNSAFE();
+	DYN_FTRACE_TEST_RCU_UNSAFE();
+
+	unregister_ftrace_function(&ftrace_rcu_unsafe);
+	unregister_ftrace_function(&ftrace_rcu_safe);
+
+
+	if (trace_selftest_test_probe1_cnt != 3) {
+		printk(KERN_CONT "rcu_safe expected 3 calls but had %d ",
+		       trace_selftest_test_probe1_cnt);
+		goto out;
+	}
+
+	if (trace_selftest_test_probe2_cnt != 0) {
+		printk(KERN_CONT "rcu_safe expected 0 calls but had %d ",
+		       trace_selftest_test_probe2_cnt);
+		goto out;
+	}
+
+	ret = 0;
+ out:
+	ftrace_enabled = save_ftrace_enabled;
+
+	return ret;
+}
+
 static int trace_selftest_ops(int cnt)
 {
 	int save_ftrace_enabled = ftrace_enabled;
@@ -401,6 +492,9 @@ int trace_selftest_startup_dynamic_tracing(struct tracer *trace,
 	if (!ret)
 		ret = trace_selftest_ops(2);
 
+	if (!ret)
+		ret = test_rcu_safe_unsafe();
+
 	return ret;
 }
 
diff --git a/kernel/trace/trace_selftest_dynamic.c b/kernel/trace/trace_selftest_dynamic.c
index b4c475a..5eefa56 100644
--- a/kernel/trace/trace_selftest_dynamic.c
+++ b/kernel/trace/trace_selftest_dynamic.c
@@ -11,3 +11,10 @@ int DYN_FTRACE_TEST_NAME2(void)
 	/* used to call mcount */
 	return 0;
 }
+
+int trace_selftest_dynamic_test_rcu_unsafe(void)
+{
+	/* used to call mcount */
+	return 0;
+}
+FTRACE_UNSAFE_RCU(trace_selftest_dynamic_test_rcu_unsafe);
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [RFC][PATCH 8/8] rcu/ftrace: Add FTRACE_UNSAFE_RCU() to unsafe RCU functions
  2013-08-30 13:02 [RFC][PATCH 0/8] ftrace/rcu: Handle unsafe RCU functions and ftrace callbacks Steven Rostedt
                   ` (6 preceding siblings ...)
  2013-08-30 13:02 ` [RFC][PATCH 7/8] ftrace: Add selftest to check if RCU unsafe functions are filtered properly Steven Rostedt
@ 2013-08-30 13:02 ` Steven Rostedt
  7 siblings, 0 replies; 10+ messages in thread
From: Steven Rostedt @ 2013-08-30 13:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: Ingo Molnar, Andrew Morton, Peter Zijlstra, Frederic Weisbecker,
	Paul E. McKenney, Jiri Olsa, Dave Jones

[-- Attachment #1: 0008-rcu-ftrace-Add-FTRACE_UNSAFE_RCU-to-unsafe-RCU-funct.patch --]
[-- Type: text/plain, Size: 1299 bytes --]

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

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

diff --git a/kernel/rcutree.c b/kernel/rcutree.c
index 068de3a..9abf305 100644
--- a/kernel/rcutree.c
+++ b/kernel/rcutree.c
@@ -53,6 +53,7 @@
 #include <linux/delay.h>
 #include <linux/stop_machine.h>
 #include <linux/random.h>
+#include <linux/ftrace.h>
 
 #include "rcutree.h"
 #include <trace/events/rcu.h>
@@ -373,6 +374,7 @@ static void rcu_eqs_enter_common(struct rcu_dynticks *rdtp, long long oldval,
 	rcu_lockdep_assert(!lock_is_held(&rcu_sched_lock_map),
 			   "Illegal idle entry in RCU-sched read-side critical section.");
 }
+FTRACE_UNSAFE_RCU(rcu_eqs_enter_common);
 
 /*
  * Enter an RCU extended quiescent state, which can be either the
@@ -392,6 +394,7 @@ static void rcu_eqs_enter(bool user)
 		rdtp->dynticks_nesting -= DYNTICK_TASK_NEST_VALUE;
 	rcu_eqs_enter_common(rdtp, oldval, user);
 }
+FTRACE_UNSAFE_RCU(rcu_eqs_enter);
 
 /**
  * rcu_idle_enter - inform RCU that current CPU is entering idle
@@ -565,6 +568,7 @@ void rcu_user_exit(void)
 {
 	rcu_eqs_exit(1);
 }
+FTRACE_UNSAFE_RCU(rcu_user_exit);
 
 /**
  * rcu_user_exit_after_irq - inform RCU that we won't resume to userspace
-- 
1.7.10.4



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [RFC][PATCH 1/8] ftrace: Add hash list to save RCU unsafe functions
  2013-08-30 13:02 ` [RFC][PATCH 1/8] ftrace: Add hash list to save RCU unsafe functions Steven Rostedt
@ 2013-08-30 13:53   ` Steven Rostedt
  0 siblings, 0 replies; 10+ messages in thread
From: Steven Rostedt @ 2013-08-30 13:53 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, Ingo Molnar, Andrew Morton, Peter Zijlstra,
	Frederic Weisbecker, Paul E. McKenney, Jiri Olsa, Dave Jones

On Fri, 30 Aug 2013 09:02:06 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
> 
> Some ftrace function tracing callbacks use RCU (perf), thus if
> it gets called from tracing a function outside of the RCU tracking,
> like in entering or leaving NO_HZ idle/userspace, the RCU read locks
> in the callback are useless.
> 
> As normal function tracing does not use RCU, and function tracing
> happens to help debug RCU, we do not want to limit all callbacks
> from tracing these "unsafe" RCU functions. Instead, we create an
> infrastructure that will let us mark these unsafe RCU functions
> and we will only allow callbacks that explicitly say they do not
> use RCU to be called by these functions.
> 
> This patch adds the infrastructure to create the hash of functions
> that are RCU unsafe. This is done with the FTRACE_UNSAFE_RCU()
> macro. It works like EXPORT_SYMBOL() does. Simply place the macro
> under the RCU unsafe function:
> 
> void func(void)
> {
> 	[...]
> }
> FTRACE_UNSAFE_RCU(func);
> 
> Cc: Jiri Olsa <jolsa@redhat.com>
> Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

I need to add:

Reported-by: Dave Jones <davej@redhat.com>

-- Steve

> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> ---
>  include/asm-generic/vmlinux.lds.h |   10 ++++
>  include/linux/ftrace.h            |   38 ++++++++++++++
>  kernel/trace/ftrace.c             |  105 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 153 insertions(+)
> 
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index 69732d2..fdfddd2 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -93,6 +93,15 @@
>  #define MCOUNT_REC()
>  #endif
>  
> +#ifdef CONFIG_FUNCTION_TRACER
> +#define FTRACE_UNSAFE_RCU()	. = ALIGN(8);				\
> +			VMLINUX_SYMBOL(__start_ftrace_unsafe_rcu) = .;	\
> +			*(_ftrace_unsafe_rcu)				\
> +			VMLINUX_SYMBOL(__stop_ftrace_unsafe_rcu) = .;
> +#else
> +#define FTRACE_UNSAFE_RCU()
> +#endif
> +
>  #ifdef CONFIG_TRACE_BRANCH_PROFILING
>  #define LIKELY_PROFILE()	VMLINUX_SYMBOL(__start_annotated_branch_profile) = .; \
>  				*(_ftrace_annotated_branch)			      \
> @@ -479,6 +488,7 @@
>  	MEM_DISCARD(init.data)						\
>  	KERNEL_CTORS()							\
>  	MCOUNT_REC()							\
> +	FTRACE_UNSAFE_RCU()						\
>  	*(.init.rodata)							\
>  	FTRACE_EVENTS()							\
>  	TRACE_SYSCALLS()						\
> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> index 9f15c00..1d17a82 100644
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
> @@ -92,6 +92,9 @@ typedef void (*ftrace_func_t)(unsigned long ip, unsigned long parent_ip,
>   * STUB   - The ftrace_ops is just a place holder.
>   * INITIALIZED - The ftrace_ops has already been initialized (first use time
>   *            register_ftrace_function() is called, it will initialized the ops)
> + * RCU_SAFE - The callback uses no rcu type locking. This allows the
> + *            callback to be called in locations that RCU is not active.
> + *            (like going into or coming out of idle NO_HZ)
>   */
>  enum {
>  	FTRACE_OPS_FL_ENABLED			= 1 << 0,
> @@ -103,6 +106,7 @@ enum {
>  	FTRACE_OPS_FL_RECURSION_SAFE		= 1 << 6,
>  	FTRACE_OPS_FL_STUB			= 1 << 7,
>  	FTRACE_OPS_FL_INITIALIZED		= 1 << 8,
> +	FTRACE_OPS_FL_RCU_SAFE			= 1 << 9,
>  };
>  
>  struct ftrace_ops {
> @@ -219,6 +223,40 @@ static inline int ftrace_function_local_disabled(struct ftrace_ops *ops)
>  extern void ftrace_stub(unsigned long a0, unsigned long a1,
>  			struct ftrace_ops *op, struct pt_regs *regs);
>  
> +/*
> + * In order to speed up boot, save both the name and the
> + * address of the function to find to add to hashes. The
> + * list of function mcount addresses are sorted by the address,
> + * but we still need to use the name to find the actual location
> + * as the mcount call is usually not at the address of the
> + * start of the function.
> + */
> +struct ftrace_func_finder {
> +	const char		*name;
> +	unsigned long		ip;
> +};
> +
> +/*
> + * For functions that are called when RCU is not tracking the CPU
> + * (like for entering or leaving NO_HZ mode, and RCU then ignores
> + * the CPU), they need to be marked with FTRACE_UNSAFE_RCU().
> + * This will prevent all function tracing callbacks from calling
> + * this function unless the callback explicitly states that it
> + * doesn't use RCU with the FTRACE_OPS_FL_RCU_SAFE flag.
> + *
> + * The usage of FTRACE_UNSAFE_RCU() is the same as EXPORT_SYMBOL().
> + * Just place the macro underneath the function that is unsafe.
> + */
> +#define FTRACE_UNSAFE_RCU(func) \
> +	static struct ftrace_func_finder __ftrace_unsafe_rcu_##func	\
> +	 __initdata = {							\
> +		.name = #func,						\
> +		.ip = (unsigned long)func,				\
> +	};							\
> +	struct ftrace_func_finder *__ftrace_unsafe_rcu_ptr_##func	\
> +		  __attribute__((__section__("_ftrace_unsafe_rcu"))) =	\
> +		&__ftrace_unsafe_rcu_##func
> +
>  #else /* !CONFIG_FUNCTION_TRACER */
>  /*
>   * (un)register_ftrace_function must be a macro since the ops parameter
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index a6d098c..2580cdb 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -1486,6 +1486,72 @@ ftrace_ops_test(struct ftrace_ops *ops, unsigned long ip, void *regs)
>  	}
>  
>  
> +static __init int ftrace_find_ip(const void *a, const void *b)
> +{
> +	const struct dyn_ftrace *key = a;
> +	const struct dyn_ftrace *rec = b;
> +
> +	if (key->ip == rec->ip)
> +		return 0;
> +
> +	if (key->ip < rec->ip) {
> +		/* If previous is less than, then this is our func */
> +		rec--;
> +		if (rec->ip < key->ip)
> +			return 0;
> +		return -1;
> +	}
> +
> +	/* All else, we are greater */
> +	return 1;
> +}
> +
> +/*
> + * Find the mcount caller location given the ip address of the
> + * function that contains it. As the mcount caller is usually
> + * after the mcount itself.
> + *
> + * Done for just core functions at boot.
> + */
> +static __init unsigned long ftrace_mcount_from_func(unsigned long ip)
> +{
> +	struct ftrace_page *pg, *last_pg = NULL;
> +	struct dyn_ftrace *rec;
> +	struct dyn_ftrace key;
> +
> +	key.ip = ip;
> +
> +	for (pg = ftrace_pages_start; pg; last_pg = pg, pg = pg->next) {
> +		if (ip >= (pg->records[pg->index - 1].ip + MCOUNT_INSN_SIZE))
> +			break;
> +
> +		/*
> +		 * Do the first record manually, as we need to check
> +		 * the previous record from the previous page
> +		 */
> +		if (ip < pg->records[0].ip) {
> +			/* First page? Then we found our record! */
> +			if (!last_pg)
> +				return pg->records[0].ip;
> +
> +			rec = &last_pg->records[last_pg->index - 1];
> +			if (rec->ip < ip)
> +				return pg->records[0].ip;
> +
> +			/* Not found */
> +			return 0;
> +		}
> +
> +		rec = bsearch(&key, pg->records + 1, pg->index - 1,
> +			      sizeof(struct dyn_ftrace),
> +			      ftrace_find_ip);
> +		if (rec)
> +			return rec->ip;
> +	}
> +
> +	return 0;
> +}
> +
>  static int ftrace_cmp_recs(const void *a, const void *b)
>  {
>  	const struct dyn_ftrace *key = a;
> @@ -4211,6 +4277,43 @@ struct notifier_block ftrace_module_exit_nb = {
>  	.priority = INT_MIN,	/* Run after anything that can remove kprobes */
>  };
>  
> +extern struct ftrace_func_finder *__start_ftrace_unsafe_rcu[];
> +extern struct ftrace_func_finder *__stop_ftrace_unsafe_rcu[];
> +
> +static struct ftrace_hash *ftrace_unsafe_rcu;
> +
> +static void __init create_unsafe_rcu_hash(void)
> +{
> +	struct ftrace_func_finder *finder;
> +	struct ftrace_func_finder **p;
> +	unsigned long ip;
> +	char str[KSYM_SYMBOL_LEN];
> +	int count;
> +
> +	count = __stop_ftrace_unsafe_rcu - __start_ftrace_unsafe_rcu;
> +	if (!count)
> +		return;
> +
> +	ftrace_unsafe_rcu = alloc_ftrace_hash(count);
> +
> +	for (p = __start_ftrace_unsafe_rcu;
> +	     p < __stop_ftrace_unsafe_rcu;
> +	     p++) {
> +		finder = *p;
> +
> +		ip = ftrace_mcount_from_func(finder->ip);
> +		if (WARN_ON_ONCE(!ip))
> +			continue;
> +
> +		/* Make sure this is our ip */
> +		kallsyms_lookup(ip, NULL, NULL, NULL, str);
> +		if (WARN_ON_ONCE(strcmp(str, finder->name) != 0))
> +			continue;
> +
> +		add_hash_entry(ftrace_unsafe_rcu, ip);
> +	}
> +}
> +
>  extern unsigned long __start_mcount_loc[];
>  extern unsigned long __stop_mcount_loc[];
>  
> @@ -4250,6 +4353,8 @@ void __init ftrace_init(void)
>  	if (ret)
>  		pr_warning("Failed to register trace ftrace module exit notifier\n");
>  
> +	create_unsafe_rcu_hash();
> +
>  	set_ftrace_early_filters();
>  
>  	return;


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2013-08-30 13:53 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-30 13:02 [RFC][PATCH 0/8] ftrace/rcu: Handle unsafe RCU functions and ftrace callbacks Steven Rostedt
2013-08-30 13:02 ` [RFC][PATCH 1/8] ftrace: Add hash list to save RCU unsafe functions Steven Rostedt
2013-08-30 13:53   ` Steven Rostedt
2013-08-30 13:02 ` [RFC][PATCH 2/8] ftrace: Do not set ftrace records for unsafe RCU when not allowed Steven Rostedt
2013-08-30 13:02 ` [RFC][PATCH 3/8] ftrace: Set ftrace internal function tracing RCU safe Steven Rostedt
2013-08-30 13:02 ` [RFC][PATCH 4/8] ftrace: Add test for ops against unsafe RCU functions in callback Steven Rostedt
2013-08-30 13:02 ` [RFC][PATCH 5/8] ftrace: Do not display non safe RCU functions in available_filter_functions Steven Rostedt
2013-08-30 13:02 ` [RFC][PATCH 6/8] ftrace: Add rcu_unsafe_filter_functions file Steven Rostedt
2013-08-30 13:02 ` [RFC][PATCH 7/8] ftrace: Add selftest to check if RCU unsafe functions are filtered properly Steven Rostedt
2013-08-30 13:02 ` [RFC][PATCH 8/8] rcu/ftrace: Add FTRACE_UNSAFE_RCU() to unsafe RCU functions Steven Rostedt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).