Re: [PATCH v5 06/18] rcu: Introduce call_rcu_lazy() API implementation

[Joel Fernandes <joel@joelfernandes.org>]

Hi Frederic,

On 9/4/2022 5:01 PM, Frederic Weisbecker wrote:
> On Sat, Sep 03, 2022 at 10:00:29PM +0000, Joel Fernandes wrote:
>> On Fri, Sep 02, 2022 at 05:21:32PM +0200, Frederic Weisbecker wrote:
>> +
>> +	raw_spin_lock_irqsave(&my_rdp->nocb_gp_lock, flags);
> 
> This is locking during the whole group iteration potentially contending call_rcu(),
> idle loop, resume to userspace on all rdp in the group...
> 
> How about not overwriting timers instead and only set the RCU_NOCB_WAKE_LAZY
> timer when it is previously in RCU_NOCB_WAKE_NOT state? After all if the timer is armed,
> it's because we have regular callbacks queued and thus we don't need to wait
> before processing the lazy callbacks since we are going to start a grace period
> anyway.
> 
> Thanks.

I like your idea better. I think it will work well to resolve the race you
described.

Thanks!

 - Joel


> 
> 
>>  	list_for_each_entry_rcu(rdp, &my_rdp->nocb_head_rdp, nocb_entry_rdp, 1) {
>>  		bool needwake_state = false;
>>  		bool flush_bypass = false;
>> @@ -855,14 +870,15 @@ static void nocb_gp_wait(struct rcu_data *my_rdp)
>>  		// If bypass list only has lazy CBs. Add a deferred
>>  		// lazy wake up.
>>  		if (lazy && !bypass) {
>> -			wake_nocb_gp_defer(my_rdp, RCU_NOCB_WAKE_LAZY,
>> +			wake_nocb_gp_defer_locked(my_rdp, RCU_NOCB_WAKE_LAZY,
>>  					TPS("WakeLazyIsDeferred"));
>>  		// Otherwise add a deferred bypass wake up.
>>  		} else if (bypass) {
>> -			wake_nocb_gp_defer(my_rdp, RCU_NOCB_WAKE_BYPASS,
>> +			wake_nocb_gp_defer_locked(my_rdp, RCU_NOCB_WAKE_BYPASS,
>>  					TPS("WakeBypassIsDeferred"));
>>  		}
>>  	}
>> +	raw_spin_unlock_irqrestore(&my_rdp->nocb_gp_lock, flags);
>>  
>>  	if (rcu_nocb_poll) {
>>  		/* Polling, so trace if first poll in the series. */
>> -- 
>> 2.37.2.789.g6183377224-goog
>>