Messages in this thread | | | Date | Thu, 21 Mar 2019 06:26:03 -0700 | From | "Paul E. McKenney" <> | Subject | Re: [PATCH v3] rcu: Allow to eliminate softirq processing from rcutree |
| |
On Thu, Mar 21, 2019 at 09:27:37AM +0100, Sebastian Andrzej Siewior wrote: > On 2019-03-20 16:46:01 [-0700], Paul E. McKenney wrote: > > Thank you! I reverted v2 and applied this one with the same sort of > > update. Testing is going well thus far aside from my failing to add > > the required "=0" after the rcutree.use_softirq. I will probably not > > be the only one who will run afoul of this, so I updated the commit log > > and the documentation accordingly, as shown below. > > perfect, thank you. > > > Thanx, Paul > > > > diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h > > index f46b4af96ab9..b807204ffd83 100644 > > --- a/kernel/rcu/tree_plugin.h > > +++ b/kernel/rcu/tree_plugin.h > > @@ -629,7 +609,10 @@ static void rcu_read_unlock_special(struct task_struct *t) > > /* Need to defer quiescent state until everything is enabled. */ > > if (irqs_were_disabled) { > > /* Enabling irqs does not reschedule, so... */ > > - raise_softirq_irqoff(RCU_SOFTIRQ); > > + if (!use_softirq) > > that exclamation mark needs to go :/
That might explain some of the failures in TREE01, TREE02, TREE03, and TREE09. TREE01 got a NULL pointer dereference, but in __do_softirq(). So I suspect that this was related. Ditto for TREE02, TREE03, and TREE09. These also all have CONFIG_PREEMPT=y, and are the only ones run by default that are set up this way. (Well, so do SRCU-P, TASKS01, and TASKS03, but they are torturing other forms of RCU.)
Anyway, I applied your fix above and will rerun. The failures happened within a few seconds in all cases, so a short run should cover this.
Once I get good rcutorture runs, I will ask you to run a heavy-duty run. Once that passes, I will look at your changes in more detail.
Thanx, Paul
> > + raise_softirq_irqoff(RCU_SOFTIRQ); > > + else > > + invoke_rcu_core(); > > } else { > > /* Enabling BH or preempt does reschedule, so... */ > > set_tsk_need_resched(current); > > Sebastian >
| |