Re: [PATCH -next 4/8] rcu/nocb: Add warning if no rcuog wake up attempt happened during overload

[Joel Fernandes <joelagnelf@nvidia.com>]

On 1/9/2026 9:36 AM, Frederic Weisbecker wrote:
> Le Thu, Jan 08, 2026 at 10:49:30PM -0500, Joel Fernandes a écrit :
>>>> @@ -688,6 +690,7 @@ static void nocb_gp_wait(struct rcu_data *my_rdp)
>>>>  		     bypass_ncbs > 2 * qhimark)) {
>>>>  			flush_bypass = true;
>>>>  		} else if (!bypass_ncbs && rcu_segcblist_empty(&rdp->cblist)) {
>>>> +			rdp->nocb_gp_wake_attempt = false;
>>>
>>> This is when nocb_cb_wait() is done with callbacks but nocb_gp_wait() is done
>>> with them sooner, when the grace period is done for all pending callbacks.
>>>
>>> Something like this would perhaps be more accurate:
>>>
>>> diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h
>>> index e6cd56603cad..52010cbeaa76 100644
>>> --- a/kernel/rcu/tree_nocb.h
>>> +++ b/kernel/rcu/tree_nocb.h
>>> @@ -746,6 +746,8 @@ static void nocb_gp_wait(struct rcu_data *my_rdp)
>>>  			needwait_gp = true;
>>>  			trace_rcu_nocb_wake(rcu_state.name, rdp->cpu,
>>>  					    TPS("NeedWaitGP"));
>>> +		} else if (!rcu_cblist_n_cbs(&rdp->nocb_bypass)) {
>>> +			rdp->nocb_gp_wake_attempt = false;
>>>  		}
>>
>> Hmm, I am trying to understand why this suggestion is better than what I
>> already have. It is one extra line and adds another conditional.
>>
>> Also shouldn't it be:
>>
>>   } else if (!rcu_cblist_n_cbs(&rdp->nocb_bypass) &&
>>              rcu_segcblist_empty(&rdp->cblist)) {
>>       rdp->nocb_gp_wake_attempt = false;
>>   }
>>
>>   ?
> 
> This else already means that rcu_segcblist_nextgp() returned false because there
> is no pending callbacks. rcu_segcblist_empty() is different because it also test
> done callbacks.
> 
>>
>> My goal was to mark wake_attempt as false when ALL callbacks on the rdp were
>> drained. IOW, the GP thread is done with the rdp.
> 
> So nocb_gp_wait (the rcuog kthreads) handle the pending callbacks, advancing
> them throughout grace periods until they are moved to the done state.
> 
> But once they are moved as done, the callbacks are the responsibility of
> nocb_cb_wait() (the rcuo kthreads) and nocb_gp_wait() stops paying attention
> to that rdp if there are no more pending callbacks.
> 
> So with your initial patch, you still have rdp->nocb_gp_wake_attempt == true
> even when there are only done callbacks. But without an appropriate wake-up
> after subsequent enqueue, nocb_gp_wait() may ignore new callbacks, event though
> rdp->nocb_gp_wake_attempt == true suggests otherwise.

Ah, got it! I was clubbing the acting of waking up rcuog to that of both the
rcuog and rcuop/s threads. Your suggestion, instead, is more accurate so I will
do it that way instead. Thanks for the explanations!

 - Joel