Skip to contentSkip to conversation

ftrace/rcu: Handle unsafe RCU functions and ftrace callbacks

RFC PATCH Superseded Steven Rostedt 1 participant 10 messages Aug 30, 2013

Subsystems (3)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Steven Rostedt

author maintainer
#0 lore raw

cover ftrace/rcu: Handle unsafe RCU functions and ftrace callbacks

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(-)
author maintainer
#1 lore raw

RFC PATCH 1/8 ftrace: Add hash list to save RCU unsafe functions

From: "Steven Rostedt (Red Hat)" <[email protected]>

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 <[email protected]>
Cc Paul E. McKenney <[email protected]>
---
 include/asm-generic/vmlinux.lds.h |   10 ++++
 include/linux/ftrace.h            |   38 ++++++++++++++
 kernel/trace/ftrace.c             |  105 +++++++++++++++++++++++++++++++++++++
 3 files changed, 153 insertions(+)
include/asm-generic/vmlinux.lds.h +10 -0
index 69732d2..fdfddd2 100644
@@ -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()						\
include/linux/ftrace.h +38 -0
index 9f15c00..1d17a82 100644
@@ -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
kernel/trace/ftrace.c +105 -0

123 lines not on this page · raw message

Signature · 1 line
1.7.10.4
author maintainer
#2 lore raw

Re: [RFC][PATCH 1/8] ftrace: Add hash list to save RCU unsafe functions

Quoted 19 lines · On Fri, 30 Aug 2013 09:02:06 -0400 Steven Rostedt <[email protected]> wrote:

From: "Steven Rostedt (Red Hat)" <[email protected]>

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:

Quoted 8 lines

void func(void)

{
	[...]
}

FTRACE_UNSAFE_RCU(func);

Cc Jiri Olsa <[email protected]>
Cc Paul E. McKenney <[email protected]>

I need to add:

-- Steve

Quoted 228 lines
---
 include/asm-generic/vmlinux.lds.h |   10 ++++
 include/linux/ftrace.h            |   38 ++++++++++++++
 kernel/trace/ftrace.c             |  105 +++++++++++++++++++++++++++++++++++++
 3 files changed, 153 insertions(+)
include/asm-generic/vmlinux.lds.h +10 -0
index 69732d2..fdfddd2 100644
@@ -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()						\
include/linux/ftrace.h +38 -0
index 9f15c00..1d17a82 100644
@@ -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
kernel/trace/ftrace.c +105 -0
index a6d098c..2580cdb 100644
@@ -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;
author maintainer
#3 lore raw

RFC PATCH 2/8 ftrace: Do not set ftrace records for unsafe RCU when not allowed

In reply to Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>

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 <[email protected]>
Cc Paul E. McKenney <[email protected]>
---
 kernel/trace/ftrace.c |   19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)
kernel/trace/ftrace.c +17 -2
index 2580cdb..dc11951 100644
@@ -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;
Signature · 1 line
1.7.10.4
author maintainer
#4 lore raw

RFC PATCH 3/8 ftrace: Set ftrace internal function tracing RCU safe

In reply to Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>

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 <[email protected]>
Cc Paul E. McKenney <[email protected]>
---
 kernel/trace/ftrace.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
kernel/trace/ftrace.c +3 -1
index dc11951..3761663 100644
@@ -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)
 };
 
Signature · 1 line
1.7.10.4
author maintainer
#5 lore raw

RFC PATCH 4/8 ftrace: Add test for ops against unsafe RCU functions in callback

In reply to Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>

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 <[email protected]>
Cc Paul E. McKenney <[email protected]>
---
 kernel/trace/ftrace.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
kernel/trace/ftrace.c +3 -1
index 3761663..c7ef936 100644
@@ -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;
Signature · 1 line
1.7.10.4
author maintainer
#6 lore raw

RFC PATCH 5/8 ftrace: Do not display non safe RCU functions in available_filter_functions

In reply to Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>

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 <[email protected]>
Cc Paul E. McKenney <[email protected]>
---
 include/linux/ftrace.h |    1 +
 kernel/trace/ftrace.c  |    6 +++++-
 2 files changed, 6 insertions(+), 1 deletion(-)
include/linux/ftrace.h +1 -0
index 1d17a82..4709264 100644
@@ -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);
kernel/trace/ftrace.c +5 -1
index c7ef936..6550f72 100644
@@ -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;
 	}
 
Signature · 1 line
1.7.10.4
author maintainer
#7 lore raw

RFC PATCH 6/8 ftrace: Add rcu_unsafe_filter_functions file

In reply to Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>

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 <[email protected]>
Cc Paul E. McKenney <[email protected]>
---
 include/linux/ftrace.h |    1 +
 kernel/trace/ftrace.c  |   31 +++++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+)
include/linux/ftrace.h +1 -0
index 4709264..4752764 100644
@@ -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);
kernel/trace/ftrace.c +31 -0
index 6550f72..e2dc641 100644
@@ -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);
 
Signature · 1 line
1.7.10.4
author maintainer
#8 lore raw

RFC PATCH 7/8 ftrace: Add selftest to check if RCU unsafe functions are filtered properly

In reply to Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>

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 <[email protected]>
Cc Paul E. McKenney <[email protected]>
---
 kernel/trace/trace.h                  |    2 +
 kernel/trace/trace_selftest.c         |   94 +++++++++++++++++++++++++++++++++
 kernel/trace/trace_selftest_dynamic.c |    7 +++
 3 files changed, 103 insertions(+)
kernel/trace/trace.h +2 -0
index 502fed7..3578be6 100644
@@ -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;
kernel/trace/trace_selftest.c +94 -0

106 lines not on this page · raw message

kernel/trace/trace_selftest_dynamic.c +7 -0
index b4c475a..5eefa56 100644
@@ -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);
Signature · 1 line
1.7.10.4
author maintainer
#9 lore raw

RFC PATCH 8/8 rcu/ftrace: Add FTRACE_UNSAFE_RCU() to unsafe RCU functions

In reply to Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>

---
 kernel/rcutree.c |    4 ++++
 1 file changed, 4 insertions(+)
kernel/rcutree.c +4 -0
index 068de3a..9abf305 100644
@@ -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
Signature · 1 line
1.7.10.4