On Sat, Jul 29, 2023 at 02:27:34PM +0000, Joel Fernandes (Google) wrote:
> /*
> * No-CBs GP kthreads come here to wait for additional callbacks to show up
> * or for grace periods to end.
> */
> static void nocb_gp_wait(struct rcu_data *my_rdp)
> {
> - bool bypass = false;
> int __maybe_unused cpu = my_rdp->cpu;
> unsigned long cur_gp_seq;
> unsigned long flags;
> bool gotcbs = false;
> - unsigned long j = jiffies;
> - bool lazy = false;
> bool needwait_gp = false; // This prevents actual uninitialized use.
> bool needwake;
> bool needwake_gp;
> + int defer_wake_type = RCU_NOCB_WAKE_NOT;
> struct rcu_data *rdp, *rdp_toggling = NULL;
> struct rcu_node *rnp;
> unsigned long wait_gp_seq = 0; // Suppress "use uninitialized" warning.
> @@ -712,44 +758,24 @@ static void nocb_gp_wait(struct rcu_data *my_rdp)
> * won't be ignored for long.
> */
> list_for_each_entry(rdp, &my_rdp->nocb_head_rdp, nocb_entry_rdp) {
> - long bypass_ncbs;
> - bool flush_bypass = false;
> - long lazy_ncbs;
> + int defer_wake_type_one = RCU_NOCB_WAKE_NOT;
No need to initialize it, nocb_gp_flush_wake() always returns a value, and
it will avoid mistakes in the future if nocb_gp_flush_wake() is moved and
accidentally not called.
> + bool flushed;
> + bool empty;
>
> - trace_rcu_nocb_wake(rcu_state.name, rdp->cpu, TPS("Check"));
> rcu_nocb_lock_irqsave(rdp, flags);
> - lockdep_assert_held(&rdp->nocb_lock);
> - bypass_ncbs = rcu_cblist_n_cbs(&rdp->nocb_bypass);
> - lazy_ncbs = READ_ONCE(rdp->lazy_len);
> + defer_wake_type_one = nocb_gp_flush_wake(rdp, &empty, &flushed);
>
> - if (bypass_ncbs && (lazy_ncbs == bypass_ncbs) &&
> - (time_after(j, READ_ONCE(rdp->nocb_bypass_first) + jiffies_till_flush) ||
> - bypass_ncbs > 2 * qhimark)) {
> - flush_bypass = true;
> - } else if (bypass_ncbs && (lazy_ncbs != bypass_ncbs) &&
> - (time_after(j, READ_ONCE(rdp->nocb_bypass_first) + 1) ||
> - bypass_ncbs > 2 * qhimark)) {
> - flush_bypass = true;
> - } else if (!bypass_ncbs && rcu_segcblist_empty(&rdp->cblist)) {
> - rcu_nocb_unlock_irqrestore(rdp, flags);
> - continue; /* No callbacks here, try next. */
> - }
> + // We may need to do a deferred wakeup later for bypass/lazy
> + // So note down what we learnt from the rdp.
> + defer_wake_type = max(defer_wake_type_one, defer_wake_type);
>
> - if (flush_bypass) {
> - // Bypass full or old, so flush it.
> - (void)rcu_nocb_try_flush_bypass(rdp, j);
> - bypass_ncbs = rcu_cblist_n_cbs(&rdp->nocb_bypass);
> - lazy_ncbs = READ_ONCE(rdp->lazy_len);
> + // Did we make any updates to main cblist? If not, no
> + // non-deferred wake up to do for this rdp.
> + if (!flushed && empty) {
Can you ever have (flushed && empty) ? If not you should be able to remove the
flushed parameter.
Thanks.
> + rcu_nocb_unlock_irqrestore(rdp, flags);
> + continue;
> }
>
> - if (bypass_ncbs) {
> - trace_rcu_nocb_wake(rcu_state.name, rdp->cpu,
> - bypass_ncbs == lazy_ncbs ? TPS("Lazy") : TPS("Bypass"));
> - if (bypass_ncbs == lazy_ncbs)
> - lazy = true;
> - else
> - bypass = true;
> - }
> rnp = rdp->mynode;
>
