Re: [PATCH RFC tip/core/rcu 1/2] rcu: Defer reporting RCU-preempt quiescent states when disabled

From: Joel Fernandes
Date: Sun Jul 01 2018 - 20:37:41 EST


On Sun, Jul 01, 2018 at 03:25:01PM -0700, Paul E. McKenney wrote:
[...]
> > > @@ -602,6 +589,66 @@ static void rcu_read_unlock_special(struct task_struct *t)
> > > }
> > > }
> > >
> > > +/*
> > > + * Is a deferred quiescent-state pending, and are we also not in
> > > + * an RCU read-side critical section? It is the caller's responsibility
> > > + * to ensure it is otherwise safe to report any deferred quiescent
> > > + * states. The reason for this is that it is safe to report a
> > > + * quiescent state during context switch even though preemption
> > > + * is disabled. This function cannot be expected to understand these
> > > + * nuances, so the caller must handle them.
> > > + */
> > > +static bool rcu_preempt_need_deferred_qs(struct task_struct *t)
> > > +{
> > > + return (this_cpu_ptr(&rcu_preempt_data)->deferred_qs ||
> > > + READ_ONCE(t->rcu_read_unlock_special.s)) &&
> > > + !t->rcu_read_lock_nesting;
> > > +}
> > > +
> > > +/*
> > > + * Report a deferred quiescent state if needed and safe to do so.
> > > + * As with rcu_preempt_need_deferred_qs(), "safe" involves only
> > > + * not being in an RCU read-side critical section. The caller must
> > > + * evaluate safety in terms of interrupt, softirq, and preemption
> > > + * disabling.
> > > + */
> > > +static void rcu_preempt_deferred_qs(struct task_struct *t)
> > > +{
> > > + unsigned long flags;
> > > +
> > > + if (!rcu_preempt_need_deferred_qs(t))
> > > + return;
> > > + local_irq_save(flags);
> > > + rcu_preempt_deferred_qs_irqrestore(t, flags);
> > > +}
> > > +
> > > +/*
> > > + * Handle special cases during rcu_read_unlock(), such as needing to
> > > + * notify RCU core processing or task having blocked during the RCU
> > > + * read-side critical section.
> > > + */
> > > +static void rcu_read_unlock_special(struct task_struct *t)
> > > +{
> > > + unsigned long flags;
> > > + bool preempt_bh_were_disabled = !!(preempt_count() & ~HARDIRQ_MASK);
> >
> > Would it be better to just test for those bits just to be safe the higher
> > order bits don't bleed in, such as PREEMPT_NEED_RESCHED, something like the
> > following based on the 'dev' branch?
>
> Good point! My plan is to merge it into the original commit with
> attribution. Please let me know if you have objections.
>

Sure! That sounds good to me.

thanks!

- Joel