On Wed, Sep 07, 2022 at 12:06:26AM +0000, Joel Fernandes wrote:
> > > @@ -326,13 +372,20 @@ static bool rcu_nocb_do_flush_bypass(struct rcu_data *rdp, struct rcu_head *rhp,
> > > * Note that this function always returns true if rhp is NULL.
> > > */
> > > static bool rcu_nocb_flush_bypass(struct rcu_data *rdp, struct rcu_head *rhp,
> > > - unsigned long j)
> > > + unsigned long j, unsigned long flush_flags)
> > > {
> > > + bool ret;
> > > +
> > > if (!rcu_rdp_is_offloaded(rdp))
> > > return true;
> > > rcu_lockdep_assert_cblist_protected(rdp);
> > > rcu_nocb_bypass_lock(rdp);
> > > - return rcu_nocb_do_flush_bypass(rdp, rhp, j);
> > > + ret = rcu_nocb_do_flush_bypass(rdp, rhp, j, flush_flags);
> > > +
> > > + if (flush_flags & FLUSH_BP_WAKE)
> > > + wake_nocb_gp(rdp, true);
> >
> > Why the true above?
> >
> > Also should we check if the wake up is really necessary (otherwise it means we
> > force a wake up for all rdp's from rcu_barrier())?
> >
> > was_alldone = rcu_segcblist_pend_cbs(&rdp->cblist);
> > ret = rcu_nocb_do_flush_bypass(rdp, rhp, j, flush_flags);
> > if (was_alldone && rcu_segcblist_pend_cbs(&rdp->cblist))
> > wake_nocb_gp(rdp, false);
>
> You mean something like the following right? Though I'm thinking if its
> better to call wake_nocb_gp() from tree.c in entrain() and let that handle
> the wake. That way, we can get rid of the extra FLUSH_BP flags as well and
> let the flush callers deal with the wakeups..
Ah yes that could make sense if only one caller cares.
>
> Anyway, for testing this should be good...
>
> ---8<-----------------------
>
> diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h
> index bd8f39ee2cd0..e3344c262672 100644
> --- a/kernel/rcu/tree_nocb.h
> +++ b/kernel/rcu/tree_nocb.h
> @@ -382,15 +382,19 @@ static bool rcu_nocb_flush_bypass(struct rcu_data *rdp, struct rcu_head *rhp,
> unsigned long j, unsigned long flush_flags)
> {
> bool ret;
> + bool was_alldone;
>
> if (!rcu_rdp_is_offloaded(rdp))
> return true;
> rcu_lockdep_assert_cblist_protected(rdp);
> rcu_nocb_bypass_lock(rdp);
> + if (flush_flags & FLUSH_BP_WAKE)
> + was_alldone = !rcu_segcblist_pend_cbs(&rdp->cblist);
> +
You can check that outside bypass lock (but you still need nocb_lock).
> ret = rcu_nocb_do_flush_bypass(rdp, rhp, j, flush_flags);
>
> - if (flush_flags & FLUSH_BP_WAKE)
> - wake_nocb_gp(rdp, true);
> + if (flush_flags & FLUSH_BP_WAKE && was_alldone)
> + wake_nocb_gp(rdp, false);
That doesn't check if the bypass list was empty.
Thanks.
