On Wed, Jun 22, 2022 at 10:50:55PM +0000, Joel Fernandes (Google) wrote:
> @@ -414,30 +427,37 @@ static bool rcu_nocb_try_bypass(struct rcu_data *rdp, struct rcu_head *rhp,
> }
> WRITE_ONCE(rdp->nocb_nobypass_count, c);
>
> - // If there hasn't yet been all that many ->cblist enqueues
> - // this jiffy, tell the caller to enqueue onto ->cblist. But flush
> - // ->nocb_bypass first.
> - if (rdp->nocb_nobypass_count < nocb_nobypass_lim_per_jiffy) {
> + // If caller passed a non-lazy CB and there hasn't yet been all that
> + // many ->cblist enqueues this jiffy, tell the caller to enqueue it
> + // onto ->cblist. But flush ->nocb_bypass first. Also do so, if total
> + // number of CBs (lazy + non-lazy) grows too much.
> + //
> + // Note that if the bypass list has lazy CBs, and the main list is
> + // empty, and rhp happens to be non-lazy, then we end up flushing all
> + // the lazy CBs to the main list as well. That's the right thing to do,
> + // since we are kick-starting RCU GP processing anyway for the non-lazy
> + // one, we can just reuse that GP for the already queued-up lazy ones.
> + if ((rdp->nocb_nobypass_count < nocb_nobypass_lim_per_jiffy && !lazy) ||
> + (lazy && n_lazy_cbs >= qhimark)) {
> rcu_nocb_lock(rdp);
> *was_alldone = !rcu_segcblist_pend_cbs(&rdp->cblist);
> if (*was_alldone)
> trace_rcu_nocb_wake(rcu_state.name, rdp->cpu,
> - TPS("FirstQ"));
> - WARN_ON_ONCE(!rcu_nocb_flush_bypass(rdp, NULL, j));
> + lazy ? TPS("FirstLazyQ") : TPS("FirstQ"));
> + WARN_ON_ONCE(!rcu_nocb_flush_bypass(rdp, NULL, j, false));
That's outside the scope of this patchset but this makes me realize we
unconditionally try to flush the bypass from call_rcu() fastpath, and
therefore we unconditionally lock the bypass lock from call_rcu() fastpath.
It shouldn't be contended at this stage since we are holding the nocb_lock
already, and only the local CPU can hold the nocb_bypass_lock without holding
the nocb_lock. But still...
It looks safe to locklessly early check if (rcu_cblist_n_cbs(&rdp->nocb_bypass))
before doing anything. Only the local CPU can enqueue to the bypass list.
Adding that to my TODO list...
