Re: [PATCH] rcutorture: Use __set_current_state() set task state in rcu_nocb_toggle()

On Jul 7, 2026, at 02:48, Paul E. McKenney <[email protected]> wrote:
> 
> On Thu, Jul 02, 2026 at 06:11:25PM +0800, Zqiang wrote:
>> In the rcu_nocb_toggle(), the schedule_hrtimeout() is called after the
>> state assignment with no condition check. the release-acquire pair from
>> raw_spin_unlock/lock(&cpu_base->lock), guarantee that task->__state is
>> visible to the hrtimer callback:
>> 
>>              CPU0                                                     CPU1
>> __set_current_state(TASK_INTERRUPTIBLE)
>> ->WRITE_ONCE(task->__state, TASK_INTERRUPTIBLE)
>> schedule_hrtimeout
>> ->hrtimer_sleeper_start_expires()
>>  ->raw_spin_lock_irqsave(&cpu_base->lock)
>>    ....
>>  ->raw_spin_unlock_irqrestore(&cpu_base->lock)
>> 
>> hard-irq:
>> raw_spin_lock_irqsave(&cpu_base->lock)
>> __hrtimer_run_queues
>> ->__run_hrtimer
>>  ->raw_spin_unlock_irqrestore(&cpu_base->lock)
>>  ->fn(timer)
>>    ->hrtimer_wakeup
>>      ->wake_up_process
>> ->try_to_wake_up
>>  ->READ task->__state
>> 
>> This commit therefore use the __set_current_state() to replace the
>> set_current_state() in rcu_nocb_toggle().
>> 
>> Signed-off-by: Zqiang <[email protected]>
> 
> This looks correct, and either drops an smp_mb() or converts an
> xchg() to a WRITE_ONCE(), which does decrease overhead.  Except that
> rcu_nocb_toggle() is invoked very infrequently and has high overhead
> that I would expect to lose this overhead decrease in the noise.  And it
> forces those reading the code to go figure out what is different between

Agreed.  I did end up reading the code.

> set_current_state() and __set_current_state().
> 
> So I am not convinced to take this patch.  But am I missing something here?
> 
> Thanx, Paul
> 
>> ---
>> kernel/rcu/rcutorture.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
>> index b1bab59efde5..a0e6901e0f90 100644
>> --- a/kernel/rcu/rcutorture.c
>> +++ b/kernel/rcu/rcutorture.c
>> @@ -2947,7 +2947,7 @@ static int rcu_nocb_toggle(void *arg)
>> atomic_long_inc(&n_nocb_deoffload);
>> }
>> toggle_delay = torture_random(&rand) % toggle_fuzz + toggle_interval;
>> - set_current_state(TASK_INTERRUPTIBLE);
>> + __set_current_state(TASK_INTERRUPTIBLE);
>> schedule_hrtimeout(&toggle_delay, HRTIMER_MODE_REL);
>> if (stutter_wait("rcu_nocb_toggle"))
>> sched_set_normal(current, oldnice);
>> -- 
>> 2.17.1
>> 
>