Linux RCU subsystem development
 help / color / mirror / Atom feed
* [PATCH] rcutorture: Use __set_current_state() set task state in rcu_nocb_toggle()
@ 2026-07-02 10:11 Zqiang
  2026-07-06 18:48 ` Paul E. McKenney
  0 siblings, 1 reply; 2+ messages in thread
From: Zqiang @ 2026-07-02 10:11 UTC (permalink / raw)
  To: paulmck, frederic, neeraj.upadhyay, joelagnelf, urezki, boqun
  Cc: qiang.zhang, rcu, linux-kernel

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


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] rcutorture: Use __set_current_state() set task state in rcu_nocb_toggle()
  2026-07-02 10:11 [PATCH] rcutorture: Use __set_current_state() set task state in rcu_nocb_toggle() Zqiang
@ 2026-07-06 18:48 ` Paul E. McKenney
  0 siblings, 0 replies; 2+ messages in thread
From: Paul E. McKenney @ 2026-07-06 18:48 UTC (permalink / raw)
  To: Zqiang
  Cc: frederic, neeraj.upadhyay, joelagnelf, urezki, boqun, rcu,
	linux-kernel

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
> 

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-07-06 18:48 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-02 10:11 [PATCH] rcutorture: Use __set_current_state() set task state in rcu_nocb_toggle() Zqiang
2026-07-06 18:48 ` Paul E. McKenney

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox