[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! */

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

[Breno Leitao]

Hello Zqiang,

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

I started seeing this on my tests, it is not trivial to decode this one,
but, I can try harder if _really_ needed.

 07:11:44  Invalid logical block size (65535)
           Invalid logical block size (256)
 07:11:45  WARNING: kernel/rcu/srcutree.c:706 at cleanup_srcu_struct+0x408/0x7e8, CPU#50: stress-ng-loop/1233713  ^[[36m[warn]^[[0m
           Modules linked in: sch_htb(E) cls_u32(E) cls_cgroup(E) cls_basic(E) act_police(E) act_mirred(E) act_bpf(E) sch_fq(E) br_netfilter(E) udp_diag(E) i
           CPU: 50 UID: 0 PID: 1233713 Comm: stress-ng-loop Kdump: loaded Tainted: G            E       7.2.0-rc5-next-20260730upstream-baseline #1 PREEMPT(f
           Tainted: [E]=UNSIGNED_MODULE
           pstate: 03401009 (nzcv daif +PAN -UAO +TCO +DIT +SSBS BTYPE=--)
           pc : cleanup_srcu_struct+0x408/0x7e8
           lr : cleanup_srcu_struct+0x3dc/0x7e8
           sp : ffff800150357b30
           x29: ffff800150357b50 x28: fffffdffbdb51c40 x27: 0000000000000000
           x26: dfff800000000000 x25: 1fffe000202daa5f x24: ffff800083bbb010
           x23: ffff800083f67210 x22: ffff0001016d52f8 x21: 00007dc98b656c40
           x20: 1ffff000107ece42 x19: 0000000000000003 x18: 1fffe006d6db3f20
           x17: 0000000000000000 x16: ffff800083f67000 x15: 0000000000000001
           x14: 1fffe006d6db6388 x13: 0000000000000000 x12: 0000000000000000
           x11: 0000000000000003 x10: fffffdffbdb51e40 x9 : dfff800000000000
           x8 : 0000000100000000 x7 : 0000000000000000 x6 : ffff80008148f060
           x5 : 0000000000000000 x4 : 0000000000000001 x3 : 0000000000000010
           x2 : ffff800150357a60 x1 : ffff80008036827c x0 : 0000000000000001
           Call trace:
            cleanup_srcu_struct+0x408/0x7e8 (P)
            blk_mq_free_tag_set+0x4b4/0x5f0
            loop_remove+0x50/0xd8 [loop]
            loop_control_ioctl+0x340/0x438 [loop]
            __arm64_sys_ioctl+0xc04/0x1310
            invoke_syscall+0x74/0x188
            do_el0_svc+0x10c/0x198
            el0_svc+0x64/0x260
            el0t_64_sync_handler+0x84/0x130
            el0t_64_sync+0x198/0x1a0
           irq event stamp: 0
           hardirqs last  enabled at (0): [<0000000000000000>] 0x0
           hardirqs last disabled at (0): [<ffff800080100734>] copy_process+0x109c/0x31e8
           softirqs last  enabled at (0): [<ffff800080100760>] copy_process+0x10c8/0x31e8
           softirqs last disabled at (0): [<0000000000000000>] 0x0
           ---[ end trace 0000000000000000 ]---