On Thu, Oct 08, 2020 at 12:49:40PM +0530, Neeraj Upadhyay wrote:
>
>
> On 10/8/2020 4:04 AM, Paul E. McKenney wrote:
> > On Sun, Oct 04, 2020 at 10:11:32PM -0400, Joel Fernandes (Google) wrote:
> > > During testing, I see it is possible that rcu_pending() returns 1 when
> > > offloaded callbacks are ready to execute thus raising the RCU softirq.
> > >
> > > However, softirq does not execute offloaded callbacks. They are executed
> > > in a
> > > kthread which is awakened independent of the softirq.
> > >
> > > This commit therefore avoids raising the softirq in the first place.
> > > That's
> > > probably a good thing considering that the purpose of callback offloading
> > > is to
> > > reduce softirq activity.
> > >
> > > Passed 30 minute tests of TREE01 through TREE09 each.
> > >
> > > On TREE08, I notice that there is atmost 150us from when the softirq was
> > > NOT raised when ready cbs were present, to when the ready callbacks were
> > > invoked by the rcuop thread. This also further confirms that there is no
> > > need to raise the softirq for ready cbs in the first place.
> > >
> > > Cc: neer...@codeaurora.org
> > > Signed-off-by: Joel Fernandes (Google) <j...@joelfernandes.org>
> >
> > Looks good, applied, thank you! I reworked things a bit based on
> > previous patches and to more precisely capture why this patch does
> > not cause additional problems. Please let me know if I messed
> > anything up.
> >
> > Thanx, Paul
> >
> > ------------------------------------------------------------------------
> >
> > commit 33847a34a2d261354a79b4a24d9d37222e8ec888
> > Author: Joel Fernandes (Google) <j...@joelfernandes.org>
> > Date: Wed Oct 7 13:50:36 2020 -0700
> >
> > rcu/tree: nocb: Avoid raising softirq for offloaded ready-to-execute
> > CBs
> > Testing showed that rcu_pending() can return 1 when offloaded callbacks
> > are ready to execute. This invokes RCU core processing, for example,
> > by raising RCU_SOFTIRQ, eventually resulting in a call to rcu_core().
> > However, rcu_core() explicitly avoids in any way manipulating offloaded
> > callbacks, which are instead handled by the rcuog and rcuoc kthreads,
> > which work independently of rcu_core().
> > One exception to this independence is that rcu_core() invokes
> > do_nocb_deferred_wakeup(), however, rcu_pending() also checks
> > rcu_nocb_need_deferred_wakeup() in order to correctly handle this case,
> > invoking rcu_core() when needed.
> > This commit therefore avoids needlessly invoking RCU core processing
> > by checking rcu_segcblist_ready_cbs() only on non-offloaded CPUs.
> > This reduces overhead, for example, by reducing softirq activity.
> > This change passed 30 minute tests of TREE01 through TREE09 each.
> > On TREE08, there is at most 150us from the time that rcu_pending()
> > chose
> > not to invoke RCU core processing to the time when the ready callbacks
> > were invoked by the rcuoc kthread. This provides further evidence that
> > there is no need to invoke rcu_core() for offloaded callbacks that are
> > ready to invoke.
> > Cc: Neeraj Upadhyay <neer...@codeaurora.org>
> > Signed-off-by: Joel Fernandes (Google) <j...@joelfernandes.org>
> > Signed-off-by: Paul E. McKenney <paul...@kernel.org>
> >
>
> Reviewed-by: Neeraj Upadhyay <neer...@codeaurora.org>
Applied, thank you!
Thanx, Paul
> Thanks
> Neeraj
>
> > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > index 85e3f29..bfd38f2 100644
> > --- a/kernel/rcu/tree.c
> > +++ b/kernel/rcu/tree.c
> > @@ -3716,7 +3716,8 @@ static int rcu_pending(int user)
> > return 1;
> > /* Does this CPU have callbacks ready to invoke? */
> > - if (rcu_segcblist_ready_cbs(&rdp->cblist))
> > + if (!rcu_segcblist_is_offloaded(&rdp->cblist) &&
> > + rcu_segcblist_ready_cbs(&rdp->cblist))
> > return 1;
> > /* Has RCU gone idle with this CPU needing another grace period? */
> >
>
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of
> the Code Aurora Forum, hosted by The Linux Foundation
