linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Joel Fernandes <joel@joelfernandes.org>
To: "Paul E. McKenney" <paulmck@linux.ibm.com>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	Steven Rostedt <rostedt@goodmis.org>, rcu <rcu@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Josh Triplett <josh@joshtriplett.org>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	Lai Jiangshan <jiangshanlai@gmail.com>
Subject: Re: [RFC] Deadlock via recursive wakeup via RCU with threadirqs
Date: Thu, 27 Jun 2019 14:51:03 -0400	[thread overview]
Message-ID: <20190627185103.GA8956@google.com> (raw)
In-Reply-To: <20190627182722.GA216610@google.com>

On Thu, Jun 27, 2019 at 02:27:22PM -0400, Joel Fernandes wrote:
> On Thu, Jun 27, 2019 at 11:11:12AM -0700, Paul E. McKenney wrote:
> > On Thu, Jun 27, 2019 at 01:46:27PM -0400, Joel Fernandes wrote:
> > > On Thu, Jun 27, 2019 at 1:43 PM Joel Fernandes <joel@joelfernandes.org> wrote:
> > > >
> > > > On Thu, Jun 27, 2019 at 11:40 AM Sebastian Andrzej Siewior
> > > > <bigeasy@linutronix.de> wrote:
> > > > >
> > > > > On 2019-06-27 11:37:10 [-0400], Joel Fernandes wrote:
> > > > > > Sebastian it would be nice if possible to trace where the
> > > > > > t->rcu_read_unlock_special is set for this scenario of calling
> > > > > > rcu_read_unlock_special, to give a clear idea about whether it was
> > > > > > really because of an IPI. I guess we could also add additional RCU
> > > > > > debug fields to task_struct (just for debugging) to see where there
> > > > > > unlock_special is set.
> > > > > >
> > > > > > Is there a test to reproduce this, or do I just boot an intel x86_64
> > > > > > machine with "threadirqs" and run into it?
> > > > >
> > > > > Do you want to send me a patch or should I send you my kvm image which
> > > > > triggers the bug on boot?
> > > >
> > > > I could reproduce this as well just booting Linus tree with threadirqs
> > > > command line and running rcutorture. In 15 seconds or so it locks
> > > > up... gdb backtrace shows the recursive lock:
> > > 
> > > Sorry that got badly wrapped, so I pasted it here:
> > > https://hastebin.com/ajivofomik.shell
> > 
> > Which rcutorture scenario would that be?  TREE03 is thus far refusing
> > to fail for me when run this way:
> > 
> > $ tools/testing/selftests/rcutorture/bin/kvm.sh --cpus 8 --duration 5 --trust-make --configs "TREE03" --bootargs "threadirqs"
> 
> I built x86_64_defconfig with CONFIG_PREEMPT enabled, then I ran it with
> following boot params:
> rcutorture.shutdown_secs=60 rcutorture.n_barrier_cbs=4 rcutree.kthread_prio=2
> 
> and also "threadirqs"
> 
> This was not a TREE config, but just my simple RCU test using qemu.

Ah, it seems that the issue is reproducible in Linus tree only (which matches
the initial diff Sebastian posted). It cannot be reproduced with your /dev
branch. So perhaps the in_irq() check indeed works.

Looking further, in_irq() does also set the HARDIRQ_MASK in the preempt_count
courtesy of:
#define __irq_enter()                                   \
        do {                                            \
                account_irq_enter_time(current);        \
                preempt_count_add(HARDIRQ_OFFSET);      \
                trace_hardirq_enter();

I dumped the stack at this point as well even with "threadirqs" just to
double confirm that is the case.

So probably, the in_irq() check is sufficient. However I am still a bit
nervous about this issue manifesting in other paths of the scheduler
that don't execute from an interrupt handler, but still would have RCU
reader sections with spinlocks held - I am not sure if this is possible
though but it does make me nervous.

Thanks!

> 
> 
> I will try this diff and let you know.
> 
> > If it had failed, I would have tried the patch shown below.  I know that
> > Sebastian has some concerns about the bug happening anyway, but we have
> > to start somewhere!  ;-)
> > 
> > ------------------------------------------------------------------------
> > 
> > diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
> > index 82c925df1d92..be7bafc2c0a0 100644
> > --- a/kernel/rcu/tree_plugin.h
> > +++ b/kernel/rcu/tree_plugin.h
> > @@ -624,25 +624,16 @@ static void rcu_read_unlock_special(struct task_struct *t)
> >  		      (rdp->grpmask & rnp->expmask) ||
> >  		      tick_nohz_full_cpu(rdp->cpu);
> >  		// Need to defer quiescent state until everything is enabled.
> > -		if ((exp || in_irq()) && irqs_were_disabled && use_softirq &&
> > -		    (in_irq() || !t->rcu_read_unlock_special.b.deferred_qs)) {
> > -			// Using softirq, safe to awaken, and we get
> > -			// no help from enabling irqs, unlike bh/preempt.
> > -			raise_softirq_irqoff(RCU_SOFTIRQ);
> > -		} else {
> > -			// Enabling BH or preempt does reschedule, so...
> > -			// Also if no expediting or NO_HZ_FULL, slow is OK.
> > -			set_tsk_need_resched(current);
> > -			set_preempt_need_resched();
> > -			if (IS_ENABLED(CONFIG_IRQ_WORK) && irqs_were_disabled &&
> > -			    !rdp->defer_qs_iw_pending && exp) {
> > -				// Get scheduler to re-evaluate and call hooks.
> > -				// If !IRQ_WORK, FQS scan will eventually IPI.
> > -				init_irq_work(&rdp->defer_qs_iw,
> > -					      rcu_preempt_deferred_qs_handler);
> > -				rdp->defer_qs_iw_pending = true;
> > -				irq_work_queue_on(&rdp->defer_qs_iw, rdp->cpu);
> > -			}
> > +		set_tsk_need_resched(current);
> > +		set_preempt_need_resched();
> > +		if (IS_ENABLED(CONFIG_IRQ_WORK) && irqs_were_disabled &&
> > +		    !rdp->defer_qs_iw_pending && exp) {
> > +			// Get scheduler to re-evaluate and call hooks.
> > +			// If !IRQ_WORK, FQS scan will eventually IPI.
> > +			init_irq_work(&rdp->defer_qs_iw,
> > +				      rcu_preempt_deferred_qs_handler);
> > +			rdp->defer_qs_iw_pending = true;
> > +			irq_work_queue_on(&rdp->defer_qs_iw, rdp->cpu);
> 
> Nice to see the code here got simplified ;-)
> 

  reply	other threads:[~2019-06-27 18:51 UTC|newest]

Thread overview: 78+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-26 13:54 [RFC] Deadlock via recursive wakeup via RCU with threadirqs Sebastian Andrzej Siewior
2019-06-26 16:25 ` Paul E. McKenney
2019-06-27  7:47   ` Sebastian Andrzej Siewior
2019-06-27 15:52     ` Paul E. McKenney
2019-06-27 14:24   ` Joel Fernandes
2019-06-27 14:34     ` Steven Rostedt
2019-06-27 15:30       ` Joel Fernandes
2019-06-27 15:37         ` Joel Fernandes
2019-06-27 15:40           ` Sebastian Andrzej Siewior
2019-06-27 15:42             ` Joel Fernandes
2019-06-27 17:43             ` Joel Fernandes
2019-06-27 17:46               ` Joel Fernandes
2019-06-27 18:11                 ` Paul E. McKenney
2019-06-27 18:27                   ` Joel Fernandes
2019-06-27 18:51                     ` Joel Fernandes [this message]
2019-06-27 19:14                       ` Paul E. McKenney
2019-06-27 19:15                     ` Paul E. McKenney
2019-06-27 18:30                   ` Paul E. McKenney
2019-06-27 20:45                     ` Paul E. McKenney
2019-06-27 15:55         ` Paul E. McKenney
2019-06-27 16:47           ` Joel Fernandes
2019-06-27 17:38             ` Paul E. McKenney
2019-06-27 18:16               ` Joel Fernandes
2019-06-27 18:41                 ` Paul E. McKenney
2019-06-27 20:17                   ` Scott Wood
2019-06-27 20:36                     ` Paul E. McKenney
2019-06-28  7:31                       ` Byungchul Park
2019-06-28  7:43                         ` Byungchul Park
2019-06-28  8:14                           ` Byungchul Park
2019-06-28  8:24                             ` Byungchul Park
2019-06-28 12:24                               ` Paul E. McKenney
2019-06-28  9:10                           ` Byungchul Park
2019-06-28  9:28                             ` Byungchul Park
2019-06-28 12:21                           ` Paul E. McKenney
2019-06-28 10:40                         ` Byungchul Park
2019-06-28 12:27                           ` Paul E. McKenney
2019-06-28 15:44                           ` Steven Rostedt
2019-06-29 15:12                             ` Andrea Parri
2019-06-29 16:55                               ` Paul E. McKenney
2019-06-29 18:09                                 ` Andrea Parri
2019-06-29 18:21                                   ` Andrea Parri
2019-06-29 19:15                                   ` Paul E. McKenney
2019-06-29 19:35                                     ` Andrea Parri
2019-06-30 23:55                             ` Byungchul Park
2019-06-28 14:15                       ` Peter Zijlstra
2019-06-28 15:54                         ` Paul E. McKenney
2019-06-28 16:04                           ` Peter Zijlstra
2019-06-28 17:20                             ` Paul E. McKenney
2019-07-01  9:42                               ` Peter Zijlstra
2019-07-01 10:24                                 ` Sebastian Andrzej Siewior
2019-07-01 12:23                                   ` Paul E. McKenney
2019-07-01 14:00                                     ` Peter Zijlstra
2019-07-01 16:01                                       ` Paul E. McKenney
2019-06-28 20:01                         ` Scott Wood
2019-07-01  9:45                           ` Peter Zijlstra
2019-06-28 13:54                   ` Peter Zijlstra
2019-06-28 15:30                     ` Paul E. McKenney
2019-06-28 18:40                       ` Sebastian Andrzej Siewior
2019-06-28 18:52                         ` Paul E. McKenney
2019-06-28 19:24                           ` Joel Fernandes
2019-06-28 20:04                             ` Paul E. McKenney
2019-06-28 21:40                               ` Joel Fernandes
2019-06-28 22:25                                 ` Paul E. McKenney
2019-06-28 23:12                                   ` Joel Fernandes
2019-06-29  0:06                                     ` Paul E. McKenney
2019-06-28 16:40                   ` Joel Fernandes
2019-06-28 16:45                     ` Joel Fernandes
2019-06-28 17:30                       ` Paul E. McKenney
2019-06-28 17:41                         ` Paul E. McKenney
2019-06-28 17:45                         ` Sebastian Andrzej Siewior
2019-06-28 18:07                           ` Joel Fernandes
2019-06-28 18:20                             ` Sebastian Andrzej Siewior
2019-07-01  2:08                               ` Joel Fernandes
2019-06-28 18:22                           ` Paul E. McKenney
2019-06-28 19:29                             ` Joel Fernandes
2019-06-28 20:06                               ` Paul E. McKenney
2019-06-28 18:05                         ` Joel Fernandes
2019-06-28 18:23                           ` Paul E. McKenney

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=20190627185103.GA8956@google.com \
    --to=joel@joelfernandes.org \
    --cc=bigeasy@linutronix.de \
    --cc=jiangshanlai@gmail.com \
    --cc=josh@joshtriplett.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mingo@redhat.com \
    --cc=paulmck@linux.ibm.com \
    --cc=peterz@infradead.org \
    --cc=rcu@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).