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

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

[Zqiang]

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 <qiang.zhang@linux.dev>
---
 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

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

[Paul E. McKenney]

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 <qiang.zhang@linux.dev>

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

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

[Zqiang]

> 
> 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 <qiang.zhang@linux.dev>
> > 
> 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
> set_current_state() and __set_current_state().
> 
> So I am not convinced to take this patch. But am I missing something here?

Nothing was missed, only from memory ordering perspective,
using set_current_state() is unnecessary. 
furthermore, in RCU tasks, we used the following code:

__set_current_state(TASK_IDLE);  // in this we use __set_current_state()
schedule_hrtimeout_range(&exp, jiffies_to_nsecs(HZ / 2), HRTIMER_MODE_REL_HARD);

Or perhaps we could also add a comment something like this(kernel/time/sleep_timeout.c):

/*
 * __set_current_state() can be used in schedule_timeout_*() functions, because
 * schedule_timeout() calls schedule() unconditionally.
 */


Thanks
Zqiang


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

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

[Alan Huang]

On Jul 7, 2026, at 02:48, Paul E. McKenney <paulmck@kernel.org> 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 <qiang.zhang@linux.dev>
> 
> 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
>> 
> 

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

[Paul E. McKenney]

On Tue, Jul 07, 2026 at 01:15:10PM +0000, Zqiang 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 <qiang.zhang@linux.dev>
> > > 
> > 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
> > set_current_state() and __set_current_state().
> > 
> > So I am not convinced to take this patch. But am I missing something here?
> 
> Nothing was missed, only from memory ordering perspective,
> using set_current_state() is unnecessary. 
> furthermore, in RCU tasks, we used the following code:
> 
> __set_current_state(TASK_IDLE);  // in this we use __set_current_state()
> schedule_hrtimeout_range(&exp, jiffies_to_nsecs(HZ / 2), HRTIMER_MODE_REL_HARD);
> 
> Or perhaps we could also add a comment something like this(kernel/time/sleep_timeout.c):
> 
> /*
>  * __set_current_state() can be used in schedule_timeout_*() functions, because
>  * schedule_timeout() calls schedule() unconditionally.
>  */

Would it make sense for a variant of schedule_hrtimeout_range() to take an
TASK_* argument and then use __set_current_state() internally?  That would
get you the performance increase across all calls, but avoid bothering
the typical user with the difference between __set_current_state() vs.
set_current_state().

							Thanx, Paul

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

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

[Zqiang]

> 
> On Tue, Jul 07, 2026 at 01:15:10PM +0000, Zqiang 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 <qiang.zhang@linux.dev>
> >  > 
> >  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
> >  set_current_state() and __set_current_state().
> >  
> >  So I am not convinced to take this patch. But am I missing something here?
> >  
> >  Nothing was missed, only from memory ordering perspective,
> >  using set_current_state() is unnecessary. 
> >  furthermore, in RCU tasks, we used the following code:
> >  
> >  __set_current_state(TASK_IDLE); // in this we use __set_current_state()
> >  schedule_hrtimeout_range(&exp, jiffies_to_nsecs(HZ / 2), HRTIMER_MODE_REL_HARD);
> >  
> >  Or perhaps we could also add a comment something like this(kernel/time/sleep_timeout.c):
> >  
> >  /*
> >  * __set_current_state() can be used in schedule_timeout_*() functions, because
> >  * schedule_timeout() calls schedule() unconditionally.
> >  */
> > 
> Would it make sense for a variant of schedule_hrtimeout_range() to take an
> TASK_* argument and then use __set_current_state() internally? That would
> get you the performance increase across all calls, but avoid bothering
> the typical user with the difference between __set_current_state() vs.
> set_current_state().

This is a good idea.

Thanks
Zqiang

> 
>  Thanx, Paul
> 
> > 
> > Thanks
> >  Zqiang
> >  
> >  
> >  
> >  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
> >  >
> >
>