On Thu, Jun 20, 2019 at 06:08:19PM -0500, Scott Wood wrote:

On Thu, 2019-06-20 at 15:25 -0700, Paul E. McKenney wrote:

quoted

On Thu, Jun 20, 2019 at 04:59:30PM -0500, Scott Wood wrote:

quoted

On Thu, 2019-06-20 at 14:10 -0700, Paul E. McKenney wrote:

quoted

On Tue, Jun 18, 2019 at 08:19:07PM -0500, Scott Wood wrote:

quoted

[Note: Just before posting this I noticed that the invoke_rcu_core
stuff
 is part of the latest RCU pull request, and it has a patch that
 addresses this in a more complicated way that appears to deal with
the
 bare irq-disabled sequence as well.

Far easier to deal with it than to debug the lack of it.  ;-)

quoted

 Assuming we need/want to support such sequences, is the
 invoke_rcu_core() call actually going to result in scheduling any
 sooner?  resched_curr() just does the same setting of need_resched
 when it's the same cpu.
]

Yes, invoke_rcu_core() can in some cases invoke the scheduler sooner.
Setting the CPU-local bits might not have effect until the next
interrupt.

Maybe I'm missing something, but I don't see how (in the non-use_softirq
case).  It just calls wake_up_process(), which in resched_curr() will
set
need_resched but not do an IPI-to-self.

The common non-rt case will be use_softirq.  Or are you referring
specifically to this block of code in current -rcu?

		} else if (exp && irqs_were_disabled && !use_softirq &&
			   !t->rcu_read_unlock_special.b.deferred_qs) {
			// Safe to awaken and we get no help from enabling
			// irqs, unlike bh/preempt.
			invoke_rcu_core();

Yes, that one.  If that block is removed the else path should be sufficient,
now that an IPI-to-self has been added.

I will give it a try and let you know what happens.

Also, shouldn't the IPI-to-self be conditioned on irqs_were_disabled? 
Besides that being the problem the IPI was meant to address, if irqs are
enabled the IPI is likely to happen before preempt is re-enabled and thus it
won't accomplish anything.

Plus if preempt is disabled, the later preempt_enable() will check
(ditto for local_bh_enable()).  Unless the preempt_enable() is instead
a preempt_enable_no_resched(), of course.  :-/

							Thanx, Paul