All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Vince Weaver <vincent.weaver@maine.edu>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Ingo Molnar <mingo@redhat.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	"dvyukov@google.com" <dvyukov@google.com>,
	pmladek@suse.com
Subject: Re: perf: fuzzer BUG: KASAN: stack-out-of-bounds in __unwind_start
Date: Tue, 29 Nov 2016 09:36:00 -0800	[thread overview]
Message-ID: <20161129173600.GW3924@linux.vnet.ibm.com> (raw)
In-Reply-To: <20161129171725.dql7evlzqiit63a3@treble>

On Tue, Nov 29, 2016 at 11:17:25AM -0600, Josh Poimboeuf wrote:
> On Tue, Nov 29, 2016 at 08:51:52AM -0800, Paul E. McKenney wrote:
> > On Tue, Nov 29, 2016 at 09:09:17AM -0600, Josh Poimboeuf wrote:
> > > On Tue, Nov 29, 2016 at 06:07:34AM -0800, Paul E. McKenney wrote:
> > > > On Tue, Nov 29, 2016 at 10:16:50AM +0100, Peter Zijlstra wrote:
> > > > > On Mon, Nov 28, 2016 at 11:52:41PM -0600, Josh Poimboeuf wrote:
> > > > > > > We used to do that, but the resulting NMIs were problematic on some
> > > > > > > platforms.  Perhaps things have gotten better?
> > > > > > 
> > > > > > Did a little digging on git blame and found the following commit (which
> > > > > > seems to be the cause of the KASAN warning and missing stack dump):
> > > > > > 
> > > > > >   bc1dce514e9b ("rcu: Don't use NMIs to dump other CPUs' stacks")
> > > > > > 
> > > > > > I presume this commit is still needed because of the NMI printk deadlock
> > > > > > issues which were discussed at Kernel Summit.  I guess those issues need
> > > > > > to be sorted out before the above commit can be reverted.
> > > > > 
> > > > > so printk should more or less work from NMI, esp. after:
> > > > > 
> > > > >   42a0bb3f7138 ("printk/nmi: generic solution for safe printk in NMI")
> > > > 
> > > > And of course bc1dce514e9b doesn't revert cleanly, but see hand reversion
> > > > below.  Also, 42a0bb3f7138's commit log calls out MN10300 and Xtensa as
> > > > needing more work.  Has that happened?
> > > 
> > > Petr M, any idea?
> > 
> > My Not-yet-signed-off-by is due to this concern, FWIW.
> 
> I think Petr's replies have addressed that now.
> 
> > > > But I really like the fact that RCU CPU stall warnings dump only those
> > > > stacks that are likely to be involved, and the patch below goes back
> > > > to dumping everyone.  Shouldn't be that hard to fix, though...
> > > 
> > > There's a new trigger_single_cpu_backtrace() function which can be used
> > > for that.
> > 
> > Even better, thank you!  Killed an hour or so of coding, but I must
> > confess that it was a mercy killing.  ;-)
> 
> Ha :-)
> 
> > Much nicer (but completely untested) patch below.
> 
> The kernel/rcu/tree.h changes seem intended for another patch?

Indeed it was, thank you for catching this, fixed.

> Otherwise:
> 
>   Reviewed-by: Josh Poimboeuf <jpoimboe@redhat.com>
> 
> Also I think this will fix the KASAN warnings reported by Vince, so you
> might add:
> 
>   Reported-by: Vince Weaver <vincent.weaver@maine.edu>

Added both of these, thank you!

Updated (but still untested) commit below.


							Thanx, Paul

------------------------------------------------------------------------

commit d3df9bc5fb5d838b049f32a476721eadbc349553
Author: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Date:   Tue Nov 29 05:49:06 2016 -0800

    rcu: Once again use NMI-based stack traces in stall warnings
    
    This commit is for all intents and purposes a revert of bc1dce514e9b
    ("rcu: Don't use NMIs to dump other CPUs' stacks").  The reason to suppose
    that this can now safely be reverted is the presence of 42a0bb3f7138
    ("printk/nmi: generic solution for safe printk in NMI"), which is said
    to have made NMI-based stack dumps safe.
    
    However, this reversion keeps one nice property of bc1dce514e9b
    ("rcu: Don't use NMIs to dump other CPUs' stacks"), namely that
    only those CPUs blocking the grace period are dumped.  The new
    trigger_single_cpu_backtrace() is used to make this happen, as
    suggested by Josh Poimboeuf.
    
    Reported-by: Vince Weaver <vincent.weaver@maine.edu>
    Not-yet-signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
    Cc: Petr Mladek <pmladek@suse.com>
    Cc: Peter Zijlstra <peterz@infradead.org>
    Reviewed-by: Josh Poimboeuf <jpoimboe@redhat.com>

diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 91a68e4e6671..ba0e4825be9d 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -1396,7 +1396,10 @@ static void rcu_check_gp_kthread_starvation(struct rcu_state *rsp)
 }
 
 /*
- * Dump stacks of all tasks running on stalled CPUs.
+ * Dump stacks of all tasks running on stalled CPUs.  First try using
+ * NMIs, but fall back to manual remote stack tracing on architectures
+ * that don't support NMI-based stack dumps.  The NMI-triggered stack
+ * traces are more accurate because they are printed by the target CPU.
  */
 static void rcu_dump_cpu_stacks(struct rcu_state *rsp)
 {
@@ -1406,11 +1409,10 @@ static void rcu_dump_cpu_stacks(struct rcu_state *rsp)
 
 	rcu_for_each_leaf_node(rsp, rnp) {
 		raw_spin_lock_irqsave_rcu_node(rnp, flags);
-		if (rnp->qsmask != 0) {
-			for_each_leaf_node_possible_cpu(rnp, cpu)
-				if (rnp->qsmask & leaf_node_cpu_bit(rnp, cpu))
+		for_each_leaf_node_possible_cpu(rnp, cpu)
+			if (rnp->qsmask & leaf_node_cpu_bit(rnp, cpu))
+				if (!trigger_single_cpu_backtrace(cpu))
 					dump_cpu_task(cpu);
-		}
 		raw_spin_unlock_irqrestore_rcu_node(rnp, flags);
 	}
 }

  reply	other threads:[~2016-11-29 17:36 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-24 17:33 perf: fuzzer BUG: KASAN: stack-out-of-bounds in __unwind_start Vince Weaver
2016-11-28 21:54 ` Josh Poimboeuf
2016-11-29  0:40   ` Paul E. McKenney
2016-11-29  5:52     ` Josh Poimboeuf
2016-11-29  9:16       ` Peter Zijlstra
2016-11-29 14:07         ` Paul E. McKenney
2016-11-29 15:09           ` Josh Poimboeuf
2016-11-29 16:12             ` Petr Mladek
2016-11-29 18:01               ` Paul E. McKenney
2016-11-29 16:51             ` Paul E. McKenney
2016-11-29 17:17               ` Josh Poimboeuf
2016-11-29 17:36                 ` Paul E. McKenney [this message]
2016-11-30  9:29                   ` Petr Mladek
2016-11-29 10:28       ` Paul E. McKenney
2016-11-29 12:43       ` Peter Zijlstra
2016-11-29 15:10         ` Paul E. McKenney
2016-11-29 16:29           ` Petr Mladek
2016-11-29 17:10             ` Peter Zijlstra
2016-11-29 19:39               ` Paul E. McKenney
2016-11-29 19:52                 ` Peter Zijlstra
2016-11-29 20:07                   ` Paul E. McKenney
2016-11-29 20:32                     ` Paul E. McKenney
2016-11-30 19:13                       ` Josh Poimboeuf
2016-11-30 19:49                         ` Paul E. McKenney
2016-12-01  5:52                         ` Peter Zijlstra
2016-12-01 12:33                           ` Paul E. McKenney
2016-12-01 16:41                             ` Peter Zijlstra
2016-12-01 17:00                               ` Paul E. McKenney
2016-11-30 10:01               ` Petr Mladek
2016-11-30 11:06                 ` Peter Zijlstra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161129173600.GW3924@linux.vnet.ibm.com \
    --to=paulmck@linux.vnet.ibm.com \
    --cc=acme@kernel.org \
    --cc=dvyukov@google.com \
    --cc=jpoimboe@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=pmladek@suse.com \
    --cc=vincent.weaver@maine.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.