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
>>
>
