Re: need_heavy_qs flag for PREEMPT=y kernels

[Joel Fernandes <joel@joelfernandes.org>]

On Thu, Aug 15, 2019 at 10:17:14AM -0700, Paul E. McKenney wrote:
> On Mon, Aug 12, 2019 at 09:02:49PM -0400, Joel Fernandes wrote:
> > On Mon, Aug 12, 2019 at 04:01:38PM -0700, Paul E. McKenney wrote:
> 
> [ . . . ]
> 
> > > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > > index 8c494a692728..ad906d6a74fb 100644
> > > --- a/kernel/rcu/tree.c
> > > +++ b/kernel/rcu/tree.c
> > > @@ -651,6 +651,12 @@ static __always_inline void rcu_nmi_exit_common(bool irq)
> > >  	 */
> > >  	if (rdp->dynticks_nmi_nesting != 1) {
> > >  		trace_rcu_dyntick(TPS("--="), rdp->dynticks_nmi_nesting, rdp->dynticks_nmi_nesting - 2, rdp->dynticks);
> > > +		if (tick_nohz_full_cpu(rdp->cpu) &&
> > > +		    rdp->dynticks_nmi_nesting == 2 &&
> > > +		    rdp->rcu_urgent_qs && !rdp->rcu_forced_tick) {
> > > +			rdp->rcu_forced_tick = true;
> > > +			tick_dep_set_cpu(rdp->cpu, TICK_DEP_MASK_RCU);
> > > +		}
> > 
> > 
> > Instead of checking dynticks_nmi_nesting == 2 in rcu_nmi_exit_common(), can
> > we do the tick_dep_set_cpu(rdp->cpu, TICK_DEP_MASK_RCU)  from
> > rcu_nmi_enter_common() ? We could add this code there, under the "if
> > (rcu_dynticks_curr_cpu_in_eqs())".
> 
> This would need to go in an "else" clause, correct?  But there would still
> want to be a check for interrupt from base level (which would admittedly
> be an equality comparison with zero) and we would also still need to check
> for rdp->rcu_urgent_qs && !rdp->rcu_forced_tick.

True, agreed. I replied to this before saying it should be
!rcu_dynticks_curr_cpu_in_eqs() in the "if" ;) But it seems I could also be
missing the check for TICK_DEP_MASK_RCU in my tree so I think we need this as
well which is below as diff. Testing it more now!

And, with this I do get many more ticks during the test. But there are
intervals where the tick is not seen. Still it is much better than before:

diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index be9707f68024..e697c7a2ce67 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -198,6 +198,10 @@ static bool check_tick_dependency(atomic_t *dep)
 		return true;
 	}
 
+	if (val & TICK_DEP_MASK_CLOCK_RCU) {
+		return true;
+	}
+
 	return false;
 }
 
> Still, an equal-zero comparison is probably going to be a bit cheaper than
> an equals-two comparison, and this is on the interrupt-entry fastpath,
> so this change is likely worth making.  Good call!!!

Thanks!!

 - Joel

[snip]