Skip to contentSkip to conversation

tracing: Fix RCU warnings in stack tracer

GIT PULL In review Steven Rostedt 2 participants 4 messages Oct 22, 2015 – Oct 29, 2015

Subsystems (2)
lore
Share & tools
lore

Steven Rostedt

author maintainer
#0 lore raw

Linus,

Running tests on other changes, the system locked up due to lots of warnings. It was caused by the stack tracer triggering a warning about using rcu_dereference() when RCU was not watching. This can happen due to the fact that the stack tracer uses the function tracer to check each function, and there's functions that may be called and traced when RCU stopped watching. Namely when a function is called just before going idle or to userspace and after RCU stopped watching that current CPU.

The first patch makes sure that RCU is watching when the stack tracer uses RCU. The second patch is to make sure that the stack tracer does not get called by functions in NMI, as it's not NMI safe.

Please pull the latest trace-fixes-v4.3-rc6 tree, which can be found at:

git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
trace-fixes-v4.3-rc6

Tag SHA1: 520ef22047724f1fae143e504b4da9737e50d32c Head SHA1: 1904be1b6bb92058c8e00063dd59df2df294e258

Steven Rostedt (Red Hat) (2):
tracing: Have stack tracer force RCU to be watching
tracing: Do not allow stack_tracer to record stack in NMI

----

 kernel/trace/trace_stack.c | 11 +++++++++++
 1 file changed, 11 insertions(+)
author maintainer
#1 lore raw

PATCH 1/2 tracing: Have stack tracer force RCU to be watching

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

The stack tracer was triggering the WARN_ON() in module.c:

static void module_assert_mutex_or_preempt(void)
{

 #ifdef CONFIG_LOCKDEP
	if (unlikely(!debug_locks))
		return;

	WARN_ON(!rcu_read_lock_sched_held() &&
		!lockdep_is_held(&module_mutex));

#endif
}

The reason is that the stack tracer traces all function calls, and some of those calls happen while exiting or entering user space and idle. Some of these functions are called after RCU had already stopped watching, as RCU does not watch userspace or idle CPUs.

If a max stack is hit, then the save_stack_trace() is called, which will check module addresses and call module_assert_mutex_or_preempt(), and then trigger the warning. Sad part is, the warning itself will also do a stack trace and tigger the same warning. That probably should be fixed.

The warning was added by 0be964be0d45 "module: Sanitize RCU usage and locking" but this bug has probably been around longer. But it's unlikely to cause much harm, but the new warning causes the system to lock up.

Cc Peter Zijlstra <[email protected]>
Cc "Paul E. McKenney" <[email protected]>
---
 kernel/trace/trace_stack.c | 7 +++++++
 1 file changed, 7 insertions(+)
kernel/trace/trace_stack.c +7 -0
index b746399ab59c..5f29402bff0f 100644
@@ -88,6 +88,12 @@ check_stack(unsigned long ip, unsigned long *stack)
 	local_irq_save(flags);
 	arch_spin_lock(&max_stack_lock);
 
+	/*
+	 * RCU may not be watching, make it see us.
+	 * The stack trace code uses rcu_sched.
+	 */
+	rcu_irq_enter();
+
 	/* In case another CPU set the tracer_frame on us */
 	if (unlikely(!frame_size))
 		this_size -= tracer_frame;
@@ -169,6 +175,7 @@ check_stack(unsigned long ip, unsigned long *stack)
 	}
 
  out:
+	rcu_irq_exit();
 	arch_spin_unlock(&max_stack_lock);
 	local_irq_restore(flags);
 }
Signature · 1 line
2.6.1
Acked-by on v1 1/2
#2 lore raw

Re: [PATCH 1/2] tracing: Have stack tracer force RCU to be watching

Quoted 25 lines · Steven Rostedt

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

The stack tracer was triggering the WARN_ON() in module.c:

static void module_assert_mutex_or_preempt(void)
{

 #ifdef CONFIG_LOCKDEP
	if (unlikely(!debug_locks))
		return;

	WARN_ON(!rcu_read_lock_sched_held() &&
		!lockdep_is_held(&module_mutex));

#endif
}

The reason is that the stack tracer traces all function calls, and some of those calls happen while exiting or entering user space and idle. Some of these functions are called after RCU had already stopped watching, as RCU does not watch userspace or idle CPUs.

If a max stack is hit, then the save_stack_trace() is called, which will check module addresses and call module_assert_mutex_or_preempt(), and then trigger the warning. Sad part is, the warning itself will also do a stack trace and tigger the same warning. That probably should be fixed.

Quoted 8 lines

The warning was added by 0be964be0d45 "module: Sanitize RCU usage and locking" but this bug has probably been around longer. But it's unlikely to cause much harm, but the new warning causes the system to lock up.

Cc Peter Zijlstra <[email protected]>
Cc "Paul E. McKenney" <[email protected]>

It does look like it will make RCU pay attention in the region of interest.

Quoted 33 lines
---
 kernel/trace/trace_stack.c | 7 +++++++
 1 file changed, 7 insertions(+)
kernel/trace/trace_stack.c +7 -0
index b746399ab59c..5f29402bff0f 100644
@@ -88,6 +88,12 @@ check_stack(unsigned long ip, unsigned long *stack)
 	local_irq_save(flags);
 	arch_spin_lock(&max_stack_lock);
 
+	/*
+	 * RCU may not be watching, make it see us.
+	 * The stack trace code uses rcu_sched.
+	 */
+	rcu_irq_enter();
+
 	/* In case another CPU set the tracer_frame on us */
 	if (unlikely(!frame_size))
 		this_size -= tracer_frame;
@@ -169,6 +175,7 @@ check_stack(unsigned long ip, unsigned long *stack)
 	}
 
  out:
+	rcu_irq_exit();
 	arch_spin_unlock(&max_stack_lock);
 	local_irq_restore(flags);
 }
Signature · 1 line
2.6.1
author maintainer
#3 lore raw

PATCH 2/2 tracing: Do not allow stack_tracer to record stack in NMI

In reply to Steven Rostedt

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

The code in stack tracer should not be executed within an NMI as it grabs spinlocks and stack tracing an NMI gives the possibility of causing a deadlock. Although this is safe on x86_64, because it does not perform stack traces when the task struct stack is not in use (interrupts and NMIs), it may be an issue for NMIs on i386 and other archs that use the same stack as the NMI.

---
 kernel/trace/trace_stack.c | 4 ++++
 1 file changed, 4 insertions(+)
kernel/trace/trace_stack.c +4 -0
index 5f29402bff0f..8abf1ba18085 100644
@@ -85,6 +85,10 @@ check_stack(unsigned long ip, unsigned long *stack)
 	if (!object_is_on_stack(stack))
 		return;
 
+	/* Can't do this from NMI context (can cause deadlocks) */
+	if (in_nmi())
+		return;
+
 	local_irq_save(flags);
 	arch_spin_lock(&max_stack_lock);
 
Signature · 1 line
2.6.1