[PATCH] srcu: Queue sdp->work when the delay timer is successfully deleted

[PATCH] srcu: Queue sdp->work when the delay timer is successfully deleted

[Zqiang]

In the cleanup_srcu_struct(), when iterating over per-cpu's srcu_data,
the timer_delete_sync(&sdp->delay_work) is called to cancel the delay
timer before flush_work(&sdp->work).

However, if the timer_delete_sync() returns 1 means that it successfully
deleted an pending timer before it had a chance to fire, also means that
the sdp->work cannot be queued, the subsequent flush_work(&sdp->work)
will returns immediately without waiting for anything, this causes SRCU
callbacks to not be processed.

Fix this by checking the return value of timer_delete_sync(), if it
returns 1, explicitly queue sdp->work so that the following flush_work()
can correctly wait for the work to complete.

Signed-off-by: Zqiang <qiang.zhang@linux.dev>
---
 kernel/rcu/srcutree.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
index 7c2f7cc131f7..02c322b7c6f1 100644
--- a/kernel/rcu/srcutree.c
+++ b/kernel/rcu/srcutree.c
@@ -725,7 +725,11 @@ void cleanup_srcu_struct(struct srcu_struct *ssp)
 	for_each_possible_cpu(cpu) {
 		struct srcu_data *sdp = per_cpu_ptr(ssp->sda, cpu);
 
-		timer_delete_sync(&sdp->delay_work);
+		//In most scenarios, calling srcu_barrier before cleanup
+		//will not trigger WARN_ON().
+		if (WARN_ON(timer_delete_sync(&sdp->delay_work)) &&
+					rcu_cpu_beenfullyonline(sdp->cpu))
+			queue_work_on(sdp->cpu, rcu_gp_wq, &sdp->work);
 		flush_work(&sdp->work);
 		if (WARN_ON(rcu_segcblist_n_cbs(&sdp->srcu_cblist)))
 			return; /* Forgot srcu_barrier(), so just leak it! */
-- 
2.17.1

Re: [PATCH] srcu: Queue sdp->work when the delay timer is successfully deleted

[Paul E. McKenney]

On Thu, Jul 09, 2026 at 06:06:02PM +0800, Zqiang wrote:
> In the cleanup_srcu_struct(), when iterating over per-cpu's srcu_data,
> the timer_delete_sync(&sdp->delay_work) is called to cancel the delay
> timer before flush_work(&sdp->work).
> 
> However, if the timer_delete_sync() returns 1 means that it successfully
> deleted an pending timer before it had a chance to fire, also means that
> the sdp->work cannot be queued, the subsequent flush_work(&sdp->work)
> will returns immediately without waiting for anything, this causes SRCU
> callbacks to not be processed.
> 
> Fix this by checking the return value of timer_delete_sync(), if it
> returns 1, explicitly queue sdp->work so that the following flush_work()
> can correctly wait for the work to complete.
> 
> Signed-off-by: Zqiang <qiang.zhang@linux.dev>

Good catch!  I updated the commit log and comment as shown below.  Does
that work for you?

						Thanx, Paul

> ---
>  kernel/rcu/srcutree.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
> index 7c2f7cc131f7..02c322b7c6f1 100644
> --- a/kernel/rcu/srcutree.c
> +++ b/kernel/rcu/srcutree.c
> @@ -725,7 +725,11 @@ void cleanup_srcu_struct(struct srcu_struct *ssp)
>  	for_each_possible_cpu(cpu) {
>  		struct srcu_data *sdp = per_cpu_ptr(ssp->sda, cpu);
>  
> -		timer_delete_sync(&sdp->delay_work);
> +		//In most scenarios, calling srcu_barrier before cleanup
> +		//will not trigger WARN_ON().
> +		if (WARN_ON(timer_delete_sync(&sdp->delay_work)) &&
> +					rcu_cpu_beenfullyonline(sdp->cpu))
> +			queue_work_on(sdp->cpu, rcu_gp_wq, &sdp->work);
>  		flush_work(&sdp->work);
>  		if (WARN_ON(rcu_segcblist_n_cbs(&sdp->srcu_cblist)))
>  			return; /* Forgot srcu_barrier(), so just leak it! */
> -- 
> 2.17.1

------------------------------------------------------------------------

commit d0b7ba8ff706ad1e3a51ffa46ee79b60ff0203f3
Author: Zqiang <qiang.zhang@linux.dev>
Date:   Thu Jul 9 18:06:02 2026 +0800

    srcu: Queue sdp->work when the delay timer is successfully deleted
    
    In the cleanup_srcu_struct() function, when iterating over per-cpu's
    srcu_data, timer_delete_sync(&sdp->delay_work) is called to cancel the
    delayed work before doing flush_work(&sdp->work).
    
    However, suppose that timer_delete_sync() returns 1, which means that it
    successfully deleted an pending timer before it had a chance to fire.
    But this also means that the sdp->work will not be queued, so that the
    subsequent flush_work(&sdp->work) will returns immediately without waiting
    for anything.  Taken together, all of this means that any recently queued
    SRCU callbacks to not be invoked, which can result in memory leaks,
    hangs, or worse.
    
    Fix this by checking the return value of timer_delete_sync(), if it
    returns 1, explicitly queue sdp->work so that the callbacks will be
    invoked and the following flush_work() will correctly wait for all of
    those callbacks to finish executing.
    
    Signed-off-by: Zqiang <qiang.zhang@linux.dev>
    Signed-off-by: Paul E. McKenney <paulmck@kernel.org>

diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
index 4a00e90e17fc07..8b4e7783bdc963 100644
--- a/kernel/rcu/srcutree.c
+++ b/kernel/rcu/srcutree.c
@@ -701,7 +701,11 @@ void cleanup_srcu_struct(struct srcu_struct *ssp)
 	for_each_possible_cpu(cpu) {
 		struct srcu_data *sdp = per_cpu_ptr(ssp->sda, cpu);
 
-		timer_delete_sync(&sdp->delay_work);
+		// Call srcu_barrier() before this cleanup_srcu_struct()
+		// to avoid triggering this WARN_ON().
+		if (WARN_ON(timer_delete_sync(&sdp->delay_work)) &&
+					rcu_cpu_beenfullyonline(sdp->cpu))
+			queue_work_on(sdp->cpu, rcu_gp_wq, &sdp->work);
 		flush_work(&sdp->work);
 		if (WARN_ON(rcu_segcblist_n_cbs(&sdp->srcu_cblist)))
 			return; /* Forgot srcu_barrier(), so just leak it! */