Re: [RFC PATCH RT 4/4] rcutorture: Avoid problematic critical section nesting

From: Scott Wood <swood@redhat.com>
To: paulmck@linux.ibm.com
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	Peter Zijlstra <peterz@infradead.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Juri Lelli <juri.lelli@redhat.com>,
	Clark Williams <williams@redhat.com>,
	linux-rt-users@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH RT 4/4] rcutorture: Avoid problematic critical section nesting
Date: Thu, 20 Jun 2019 16:43:53 -0500	[thread overview]
Message-ID: <70a4c99c4f8f310b017755a3d0aa08eb68f0e48b.camel@redhat.com> (raw)
In-Reply-To: <20190620211826.GX26519@linux.ibm.com>

On Thu, 2019-06-20 at 14:18 -0700, Paul E. McKenney wrote:
> On Tue, Jun 18, 2019 at 08:19:08PM -0500, Scott Wood wrote:
> > rcutorture was generating some nesting scenarios that are not
> > reasonable.  Constrain the state selection to avoid them.
> > 
> > Example #1:
> > 
> > 1. preempt_disable()
> > 2. local_bh_disable()
> > 3. preempt_enable()
> > 4. local_bh_enable()
> > 
> > On PREEMPT_RT, BH disabling takes a local lock only when called in
> > non-atomic context.  Thus, atomic context must be retained until after
> > BH
> > is re-enabled.  Likewise, if BH is initially disabled in non-atomic
> > context, it cannot be re-enabled in atomic context.
> > 
> > Example #2:
> > 
> > 1. rcu_read_lock()
> > 2. local_irq_disable()
> > 3. rcu_read_unlock()
> > 4. local_irq_enable()
> > 
> > If the thread is preempted between steps 1 and 2,
> > rcu_read_unlock_special.b.blocked will be set, but it won't be
> > acted on in step 3 because IRQs are disabled.  Thus, reporting of the
> > quiescent state will be delayed beyond the local_irq_enable().
> > 
> > Example #3:
> > 
> > 1. preempt_disable()
> > 2. local_irq_disable()
> > 3. preempt_enable()
> > 4. local_irq_enable()
> > 
> > If need_resched is set between steps 1 and 2, then the reschedule
> > in step 3 will not happen.
> > 
> > Signed-off-by: Scott Wood <swood@redhat.com>
> 
> OK for -rt, but as long as people can code those sequences without getting
> their wrists slapped, RCU needs to deal with it.  So I cannot accept
> this in mainline at the current time.  Yes, I will know when it is safe
> to accept it when rcutorture's virtual wrist gets slapped in mainline.
> Why did you ask?  ;-)

Hence the "TODO: Document restrictions and add debug checks for invalid
sequences". :-)

> But I have to ask...  With this elaboration, is it time to make this a
> data-driven state machine?  Or is the complexity not yet to the point
> where that would constitute a simplification?

Perhaps... Making the entire sequence visible to the constraint enforcement
would also allow relaxing some of the constraints that currently have to
assume the worst regarding what happened before the most recent state.

-Scott