On Thu, Aug 15, 2019 at 04:04:32PM -0400, Joel Fernandes wrote:
> 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;
> }
That one is not in my tree, either. Frederic, should I add this to
your patch? For that matter, may I add your Signed-off-by as well?
Your original is in my -rcu tree at:
0cb41806c799 ("EXP nohz: Add TICK_DEP_BIT_RCU")
I am testing Joel's suggested addition now.
Thanx, Paul
> > 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]
