Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1737919

Re: [PATCH 4/4] tracing: Remove RCU work arounds from stack tracer

Show all headers | View raw

On Fri, Sep 22, 2017 at 06:15:47PM -0400, Steven Rostedt wrote:
> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> 
> While debugging some RCU issues with the stack tracer, it was discovered
> that the problem was much more than with the stack tracer itself, but with
> the saving of the stack trace, which could happen from any WARN() as well.
> The problem was fixed within kernel_text_address().
> 
> One of the bugs that was discovered was that the stack tracer called
> rcu_enter_irq() unconditionally. Paul McKenney said that could cause issues
> as well. Instead of adding logic to only call rcu_enter_irq() if RCU is not
> watching from within the stack tracer, since the core issue has been fixed
> (within save_stack_trace()), we can simply remove all the logic in the stack
> tracer that deals with RCU work arounds.

I must confess that I am having some difficulty parsing this paragraph,
especially the last sentence...

Does this capture it?

	One problem is that the stack tracer called rcu_irq_enter()
	unconditionally, which is problematic if RCU's last
	not-watching-to-watching transition was carried out by
	rcu_nmi_enter.	In that case, rcu_irq_enter() actually switches
	RCU back to the not-watching state for this CPU, which results
	in lockdep splats complaining about rcu_read_lock() being
	used on an idle (not-watched) CPU.  The first patch of this
	series addressed this problem by having rcu_irq_enter() and
	rcu_irq_exit() refrain from doing anything when rcu_nmi_enter()
	caused RCU to start watching this CPU.	The third patch in this
	series caused save_stack_trace() to invoke rcu_nmi_enter() and
	rcu_nmi_exit() as needed, so this fourth patch now removes the
	rcu_irq_enter() and rcu_irq_exit() from within the stack tracer.

One further question...  Can I now remove the rcu_irq_enter_disabled()
logic?

> Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> Cc: stable@vger.kernel.org
> Fixes: 0be964be0 ("module: Sanitize RCU usage and locking")
> Suggested-by: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

With the hard-to-parse paragraph fixed:

Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

> ---
>  kernel/trace/trace_stack.c | 15 ---------------
>  1 file changed, 15 deletions(-)
> 
> diff --git a/kernel/trace/trace_stack.c b/kernel/trace/trace_stack.c
> index a4df67cbc711..49cb41412eec 100644
> --- a/kernel/trace/trace_stack.c
> +++ b/kernel/trace/trace_stack.c
> @@ -96,23 +96,9 @@ check_stack(unsigned long ip, unsigned long *stack)
>  	if (in_nmi())
>  		return;
> 
> -	/*
> -	 * There's a slight chance that we are tracing inside the
> -	 * RCU infrastructure, and rcu_irq_enter() will not work
> -	 * as expected.
> -	 */
> -	if (unlikely(rcu_irq_enter_disabled()))
> -		return;
> -
>  	local_irq_save(flags);
>  	arch_spin_lock(&stack_trace_max_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;
> @@ -205,7 +191,6 @@ check_stack(unsigned long ip, unsigned long *stack)
>  	}
> 
>   out:
> -	rcu_irq_exit();
>  	arch_spin_unlock(&stack_trace_max_lock);
>  	local_irq_restore(flags);
>  }
> -- 
> 2.13.2
> 
> 

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH 0/4] rcu/tracing/extable: Fix stack dump when RCU is not watching Steven Rostedt <rostedt@goodmis.org> - 2017-09-23 00:20 +0200
  [PATCH 1/4] rcu: Allow for page faults in NMI handlers Steven Rostedt <rostedt@goodmis.org> - 2017-09-23 00:20 +0200
  [PATCH 4/4] tracing: Remove RCU work arounds from stack tracer Steven Rostedt <rostedt@goodmis.org> - 2017-09-23 00:20 +0200
    Re: [PATCH 4/4] tracing: Remove RCU work arounds from stack tracer "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-09-23 01:00 +0200
      Re: [PATCH 4/4] tracing: Remove RCU work arounds from stack tracer Steven Rostedt <rostedt@goodmis.org> - 2017-09-23 03:30 +0200
        Re: [PATCH 4/4] tracing: Remove RCU work arounds from stack tracer "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-09-23 08:10 +0200
          Re: [PATCH 4/4] tracing: Remove RCU work arounds from stack tracer Steven Rostedt <rostedt@goodmis.org> - 2017-09-23 13:30 +0200
            Re: [PATCH 4/4] tracing: Remove RCU work arounds from stack tracer "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-09-23 19:20 +0200
  [PATCH 3/4] extable: Enable RCU if it is not watching in kernel_text_address() Steven Rostedt <rostedt@goodmis.org> - 2017-09-23 00:20 +0200
    Re: [PATCH 3/4] extable: Enable RCU if it is not watching in  kernel_text_address() Josh Poimboeuf <jpoimboe@redhat.com> - 2017-09-23 00:40 +0200
      [PATCH 3/4 v2] extable: Enable RCU if it is not watching in  kernel_text_address() Steven Rostedt <rostedt@goodmis.org> - 2017-09-23 03:20 +0200
    Re: [PATCH 3/4] extable: Enable RCU if it is not watching in  kernel_text_address() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-09-23 00:50 +0200
      Re: [PATCH 3/4] extable: Enable RCU if it is not watching in  kernel_text_address() Steven Rostedt <rostedt@goodmis.org> - 2017-09-23 03:10 +0200

csiph-web