Re: [PATCH RT v3 5/5] rcutorture: Avoid problematic critical section nesting on RT

Sebastian Andrzej Siewior
2019-09-17T10:07:33Z

On 2019-09-16 11:55:57 [-0500], Scott Wood wrote:
> On Thu, 2019-09-12 at 18:17 -0400, Joel Fernandes wrote:
> > On Wed, Sep 11, 2019 at 05:57:29PM +0100, 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 I understand correctly, these examples are not unrealistic in the real
> > world unless RCU is used in the scheduler.
>
> I hope you mean "not realistic", at least when it comes to explicit
> preempt/irq disabling rather than spinlock variants that don't disable
> preempt/irqs on PREEMPT_RT.

We have:
- local_irq_disable() (+save)
- spin_lock()
- local_bh_disable()
- preempt_disable()

On non-RT you can (but should not) use the counter part of the function
in random order like:
local_bh_disable();
local_irq_disable();
local_bh_enable();
local_irq_enable();

The non-RT will survive this. On RT the counterpart functions have to be
used in reverse order:
local_bh_disable();
local_irq_disable();
local_irq_enable();
local_bh_enable();

or the kernel will fall apart.

Since you _can_ use it in random order Paul wants to test that the
random use of those function does not break RCU in any way. Since they
can not be used on RT in random order it has been agreed that we keep
the test for !RT but disable it on RT.

> -Scott

Sebastian