lkml.org 
[lkml]   [2017]   [Sep]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 4/4] tracing: Remove RCU work arounds from stack tracer
    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
    >
    >

    \
     
     \ /
      Last update: 2017-09-23 00:55    [W:2.422 / U:1.316 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site