* [PATCH] srcu: Queue sdp->work when the delay timer is successfully deleted
@ 2026-07-09 10:06 Zqiang
2026-07-10 18:34 ` Paul E. McKenney
2026-07-31 17:45 ` Breno Leitao
0 siblings, 2 replies; 9+ messages in thread
From: Zqiang @ 2026-07-09 10:06 UTC (permalink / raw)
To: paulmck, frederic, neeraj.upadhyay, joelagnelf, urezki, boqun
Cc: qiang.zhang, rcu, linux-kernel
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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] srcu: Queue sdp->work when the delay timer is successfully deleted
2026-07-09 10:06 [PATCH] srcu: Queue sdp->work when the delay timer is successfully deleted Zqiang
@ 2026-07-10 18:34 ` Paul E. McKenney
2026-07-31 17:45 ` Breno Leitao
1 sibling, 0 replies; 9+ messages in thread
From: Paul E. McKenney @ 2026-07-10 18:34 UTC (permalink / raw)
To: Zqiang
Cc: frederic, neeraj.upadhyay, joelagnelf, urezki, boqun, rcu,
linux-kernel
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! */
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] srcu: Queue sdp->work when the delay timer is successfully deleted
2026-07-09 10:06 [PATCH] srcu: Queue sdp->work when the delay timer is successfully deleted Zqiang
2026-07-10 18:34 ` Paul E. McKenney
@ 2026-07-31 17:45 ` Breno Leitao
2026-08-01 5:19 ` Zqiang
1 sibling, 1 reply; 9+ messages in thread
From: Breno Leitao @ 2026-07-31 17:45 UTC (permalink / raw)
To: Zqiang
Cc: paulmck, frederic, neeraj.upadhyay, joelagnelf, urezki, boqun,
rcu, linux-kernel
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 ]---
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] srcu: Queue sdp->work when the delay timer is successfully deleted
2026-07-31 17:45 ` Breno Leitao
@ 2026-08-01 5:19 ` Zqiang
2026-08-03 10:49 ` Breno Leitao
0 siblings, 1 reply; 9+ messages in thread
From: Zqiang @ 2026-08-01 5:19 UTC (permalink / raw)
To: Breno Leitao
Cc: paulmck, frederic, neeraj.upadhyay, joelagnelf, urezki, boqun,
rcu, linux-kernel
>
> 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.
>
The scenario I can think of is that we missed the call to srcu_barrier()
before cleanup_srcu_struct():
loop_add()
->blk_mq_alloc_tag_set
init_srcu_struct(&set->tags_srcu)
blk_mq_alloc_set_map_and_rqs() {
->__blk_mq_alloc_rq_maps()
->__blk_mq_alloc_map_and_rqs() return error
goto out_unwind: __blk_mq_free_map_and_rqs()
->blk_mq_free_rq_map()
->blk_mq_free_tags()
->call_srcu(&set->tags_srcu, &tags->rcu_head, blk_mq_free_tags_callback);
} return error
goto out_free_mq_map:
....
cleanup_srcu_struct(&set->tags_srcu)
-> trigger WARN_ON(timer_delete_sync(&sdp->delay_work)
Can you try the following patch?
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 2c850330a32b..6eaea75d0363 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -4925,6 +4925,7 @@ int blk_mq_alloc_tag_set(struct blk_mq_tag_set *set)
kfree(set->tags);
set->tags = NULL;
out_cleanup_tags_srcu:
+ srcu_barrier(&set->tags_srcu);
cleanup_srcu_struct(&set->tags_srcu);
out_cleanup_srcu:
if (set->flags & BLK_MQ_F_BLOCKING)
Thanks
Zqiang
>
> 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 ]---
>
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] srcu: Queue sdp->work when the delay timer is successfully deleted
2026-08-01 5:19 ` Zqiang
@ 2026-08-03 10:49 ` Breno Leitao
2026-08-03 14:11 ` Zqiang
0 siblings, 1 reply; 9+ messages in thread
From: Breno Leitao @ 2026-08-03 10:49 UTC (permalink / raw)
To: Zqiang
Cc: paulmck, frederic, neeraj.upadhyay, joelagnelf, urezki, boqun,
rcu, linux-kernel
On Sat, Aug 01, 2026 at 05:19:25AM +0000, Zqiang wrote:
> >
> > 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.
> >
>
> The scenario I can think of is that we missed the call to srcu_barrier()
> before cleanup_srcu_struct():
>
> loop_add()
> ->blk_mq_alloc_tag_set
> init_srcu_struct(&set->tags_srcu)
>
> blk_mq_alloc_set_map_and_rqs() {
> ->__blk_mq_alloc_rq_maps()
> ->__blk_mq_alloc_map_and_rqs() return error
> goto out_unwind: __blk_mq_free_map_and_rqs()
> ->blk_mq_free_rq_map()
> ->blk_mq_free_tags()
> ->call_srcu(&set->tags_srcu, &tags->rcu_head, blk_mq_free_tags_callback);
> } return error
>
> goto out_free_mq_map:
> ....
> cleanup_srcu_struct(&set->tags_srcu)
> -> trigger WARN_ON(timer_delete_sync(&sdp->delay_work)
>
> Can you try the following patch?
I tried it, and it does not help -- the WARN still fires at the same rate. I think the analysis points at the wrong call site.
Setup: linux-next-20260731 (arm64), 32 vCPU VM, HZ=1000, PROVE_LOCKING
and DEBUG_OBJECTS_TIMERS enabled, reproducer stress-ng --loop 32
--timeout 60s.
baseline 3 x WARN srcutree.c:706 in 60s
+ your blk-mq patch 4 x WARN srcutree.c:706 in 60s
(3 vs 4 is just jitter on a one-jiffy race, not a regression.)
The reason it cannot help is that the splat comes from
blk_mq_free_tag_set(), not from the blk_mq_alloc_tag_set() error path
your patch touches. All four splats in the patched run have the same
trace:
cleanup_srcu_struct+0x274/0x450 (P)
blk_mq_free_tag_set+0x1a4/0x1e0
loop_remove+0x2c/0x78
loop_control_ioctl+0x248/0x2a0
__arm64_sys_ioctl+0x9c0/0xb00
I also put a pr_warn() at out_cleanup_tags_srcu: to be sure -- it fired
zero times over the whole run, so loop_add() never takes that error path
in this workload.
Why do you wangt to have this
WARN_ON(timer_delete_sync(&sdp->delay_work)) ?
timer_delete_sync() != 0 means "a timer was armed", not "callbacks are
pending".
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] srcu: Queue sdp->work when the delay timer is successfully deleted
2026-08-03 10:49 ` Breno Leitao
@ 2026-08-03 14:11 ` Zqiang
2026-08-03 14:37 ` Zqiang
0 siblings, 1 reply; 9+ messages in thread
From: Zqiang @ 2026-08-03 14:11 UTC (permalink / raw)
To: Breno Leitao
Cc: paulmck, frederic, neeraj.upadhyay, joelagnelf, urezki, boqun,
rcu, linux-kernel
>
> On Sat, Aug 01, 2026 at 05:19:25AM +0000, Zqiang wrote:
>
> >
> > 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.
> >
> >
> > The scenario I can think of is that we missed the call to srcu_barrier()
> > before cleanup_srcu_struct():
> >
> > loop_add()
> > ->blk_mq_alloc_tag_set
> > init_srcu_struct(&set->tags_srcu)
> >
> > blk_mq_alloc_set_map_and_rqs() {
> > ->__blk_mq_alloc_rq_maps()
> > ->__blk_mq_alloc_map_and_rqs() return error
> > goto out_unwind: __blk_mq_free_map_and_rqs()
> > ->blk_mq_free_rq_map()
> > ->blk_mq_free_tags()
> > ->call_srcu(&set->tags_srcu, &tags->rcu_head, blk_mq_free_tags_callback);
> > } return error
> >
> > goto out_free_mq_map:
> > ....
> > cleanup_srcu_struct(&set->tags_srcu)
> > -> trigger WARN_ON(timer_delete_sync(&sdp->delay_work)
> >
> > Can you try the following patch?
> >
> I tried it, and it does not help -- the WARN still fires at the same rate. I think the analysis points at the wrong call site.
>
> Setup: linux-next-20260731 (arm64), 32 vCPU VM, HZ=1000, PROVE_LOCKING
> and DEBUG_OBJECTS_TIMERS enabled, reproducer stress-ng --loop 32
> --timeout 60s.
>
Thanks for provide testing methods, I will also testing it.
> baseline 3 x WARN srcutree.c:706 in 60s
> + your blk-mq patch 4 x WARN srcutree.c:706 in 60s
>
> (3 vs 4 is just jitter on a one-jiffy race, not a regression.)
>
> The reason it cannot help is that the splat comes from
> blk_mq_free_tag_set(), not from the blk_mq_alloc_tag_set() error path
> your patch touches. All four splats in the patched run have the same
> trace:
>
> cleanup_srcu_struct+0x274/0x450 (P)
> blk_mq_free_tag_set+0x1a4/0x1e0
> loop_remove+0x2c/0x78
> loop_control_ioctl+0x248/0x2a0
> __arm64_sys_ioctl+0x9c0/0xb00
This may trigger a new srcu grace period again during the window period between
srcu-barrier() and cleanup_srcu_struct().
>
> I also put a pr_warn() at out_cleanup_tags_srcu: to be sure -- it fired
> zero times over the whole run, so loop_add() never takes that error path
> in this workload.
>
> Why do you wangt to have this
> WARN_ON(timer_delete_sync(&sdp->delay_work)) ?
>
> timer_delete_sync() != 0 "a timer was armed", not "callbacks are
> pending".
There are only two types of return values for timer_delete_sync(),
return 0 or 1, the timer_delete_sync() != 0 means that this timer
was pending and has been deactivated, right?
Thanks
Zqiang
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] srcu: Queue sdp->work when the delay timer is successfully deleted
2026-08-03 14:11 ` Zqiang
@ 2026-08-03 14:37 ` Zqiang
2026-08-03 23:40 ` Zqiang
0 siblings, 1 reply; 9+ messages in thread
From: Zqiang @ 2026-08-03 14:37 UTC (permalink / raw)
To: Breno Leitao
Cc: paulmck, frederic, neeraj.upadhyay, joelagnelf, urezki, boqun,
rcu, linux-kernel
>
> >
> > On Sat, Aug 01, 2026 at 05:19:25AM +0000, Zqiang wrote:
> >
> >
> > 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.
> >
> >
> > The scenario I can think of is that we missed the call to srcu_barrier()
> > before cleanup_srcu_struct():
> >
> > loop_add()
> > ->blk_mq_alloc_tag_set
> > init_srcu_struct(&set->tags_srcu)
> >
> > blk_mq_alloc_set_map_and_rqs() {
> > ->__blk_mq_alloc_rq_maps()
> > ->__blk_mq_alloc_map_and_rqs() return error
> > goto out_unwind: __blk_mq_free_map_and_rqs()
> > ->blk_mq_free_rq_map()
> > ->blk_mq_free_tags()
> > ->call_srcu(&set->tags_srcu, &tags->rcu_head, blk_mq_free_tags_callback);
> > } return error
> >
> > goto out_free_mq_map:
> > ....
> > cleanup_srcu_struct(&set->tags_srcu)
> > -> trigger WARN_ON(timer_delete_sync(&sdp->delay_work)
> >
> > Can you try the following patch?
> >
> > I tried it, and it does not help -- the WARN still fires at the same rate. I think the analysis points at the wrong call site.
> >
> > Setup: linux-next-20260731 (arm64), 32 vCPU VM, HZ=1000, PROVE_LOCKING
> > and DEBUG_OBJECTS_TIMERS enabled, reproducer stress-ng --loop 32
> > --timeout 60s.
> >
> Thanks for provide testing methods, I will also testing it.
>
> >
> > baseline 3 x WARN srcutree.c:706 in 60s
> > + your blk-mq patch 4 x WARN srcutree.c:706 in 60s
> >
> > (3 vs 4 is just jitter on a one-jiffy race, not a regression.)
> >
> > The reason it cannot help is that the splat comes from
> > blk_mq_free_tag_set(), not from the blk_mq_alloc_tag_set() error path
> > your patch touches. All four splats in the patched run have the same
> > trace:
> >
> > cleanup_srcu_struct+0x274/0x450 (P)
> > blk_mq_free_tag_set+0x1a4/0x1e0
> > loop_remove+0x2c/0x78
> > loop_control_ioctl+0x248/0x2a0
> > __arm64_sys_ioctl+0x9c0/0xb00
> >
> This may trigger a new srcu grace period again during the window period between
> srcu-barrier() and cleanup_srcu_struct().
>
> >
> > I also put a pr_warn() at out_cleanup_tags_srcu: to be sure -- it fired
> > zero times over the whole run, so loop_add() never takes that error path
> > in this workload.
> >
> > Why do you wangt to have this
> > WARN_ON(timer_delete_sync(&sdp->delay_work)) ?
> >
> > timer_delete_sync() != 0 "a timer was armed", not "callbacks are
> > pending".
> >
> There are only two types of return values for timer_delete_sync(),
> return 0 or 1, the timer_delete_sync() != 0 means that this timer
> was pending and has been deactivated, right?
How about this?
diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
index b9fe57ff9100..62da31ec2bd0 100644
--- a/kernel/rcu/srcutree.c
+++ b/kernel/rcu/srcutree.c
@@ -751,7 +751,8 @@ void cleanup_srcu_struct(struct srcu_struct *ssp)
// Call srcu_barrier() before this cleanup_srcu_struct()
// to avoid triggering this WARN_ON().
- if (WARN_ON(timer_delete_sync(&sdp->delay_work)) &&
+ if (WARN_ON(rcu_segcblist_n_cbs(&sdp->srcu_cblist) &&
+ 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);
Thanks
Zqiang
>
> Thanks
> Zqiang
>
> >
> >
> >
>
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] srcu: Queue sdp->work when the delay timer is successfully deleted
2026-08-03 14:37 ` Zqiang
@ 2026-08-03 23:40 ` Zqiang
2026-08-04 8:04 ` Breno Leitao
0 siblings, 1 reply; 9+ messages in thread
From: Zqiang @ 2026-08-03 23:40 UTC (permalink / raw)
To: Breno Leitao
Cc: paulmck, frederic, neeraj.upadhyay, joelagnelf, urezki, boqun,
rcu, linux-kernel
>
> >
> > On Sat, Aug 01, 2026 at 05:19:25AM +0000, Zqiang wrote:
> >
> >
> > 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.
> >
> >
> > The scenario I can think of is that we missed the call to srcu_barrier()
> > before cleanup_srcu_struct():
> >
> > loop_add()
> > ->blk_mq_alloc_tag_set
> > init_srcu_struct(&set->tags_srcu)
> >
> > blk_mq_alloc_set_map_and_rqs() {
> > ->__blk_mq_alloc_rq_maps()
> > ->__blk_mq_alloc_map_and_rqs() return error
> > goto out_unwind: __blk_mq_free_map_and_rqs()
> > ->blk_mq_free_rq_map()
> > ->blk_mq_free_tags()
> > ->call_srcu(&set->tags_srcu, &tags->rcu_head, blk_mq_free_tags_callback);
> > } return error
> >
> > goto out_free_mq_map:
> > ....
> > cleanup_srcu_struct(&set->tags_srcu)
> > -> trigger WARN_ON(timer_delete_sync(&sdp->delay_work)
> >
> > Can you try the following patch?
> >
> > I tried it, and it does not help -- the WARN still fires at the same rate. I think the analysis points at the wrong call site.
> >
> > Setup: linux-next-20260731 (arm64), 32 vCPU VM, HZ=1000, PROVE_LOCKING
> > and DEBUG_OBJECTS_TIMERS enabled, reproducer stress-ng --loop 32
> > --timeout 60s.
> >
> > Thanks for provide testing methods, I will also testing it.
> >
> >
> > baseline 3 x WARN srcutree.c:706 in 60s
> > + your blk-mq patch 4 x WARN srcutree.c:706 in 60s
> >
> > (3 vs 4 is just jitter on a one-jiffy race, not a regression.)
> >
> > The reason it cannot help is that the splat comes from
> > blk_mq_free_tag_set(), not from the blk_mq_alloc_tag_set() error path
> > your patch touches. All four splats in the patched run have the same
> > trace:
> >
> > cleanup_srcu_struct+0x274/0x450 (P)
> > blk_mq_free_tag_set+0x1a4/0x1e0
> > loop_remove+0x2c/0x78
> > loop_control_ioctl+0x248/0x2a0
> > __arm64_sys_ioctl+0x9c0/0xb00
> >
> > This may trigger a new srcu grace period again during the window period between
> > srcu-barrier() and cleanup_srcu_struct().
> >
> >
> > I also put a pr_warn() at out_cleanup_tags_srcu: to be sure -- it fired
> > zero times over the whole run, so loop_add() never takes that error path
> > in this workload.
> >
> > Why do you wangt to have this
> > WARN_ON(timer_delete_sync(&sdp->delay_work)) ?
> >
> > timer_delete_sync() != 0 "a timer was armed", not "callbacks are
> > pending".
> >
> > There are only two types of return values for timer_delete_sync(),
> > return 0 or 1, the timer_delete_sync() != 0 means that this timer
> > was pending and has been deactivated, right?
> >
> How about this?
>
> diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
> index b9fe57ff9100..62da31ec2bd0 100644
> --- a/kernel/rcu/srcutree.c
> +++ b/kernel/rcu/srcutree.c
> @@ -751,7 +751,8 @@ void cleanup_srcu_struct(struct srcu_struct *ssp)
>
> // Call srcu_barrier() before this cleanup_srcu_struct()
> // to avoid triggering this WARN_ON().
> - if (WARN_ON(timer_delete_sync(&sdp->delay_work)) &&
> + if (WARN_ON(rcu_segcblist_n_cbs(&sdp->srcu_cblist) &&
> + 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);
Please ignore this change, this is my mistake.
There is a scenario where the 'rcu_segcblist_n_cbs(&sdp->srcu_cblist) == 0'
srcu_barrier() cannot intercept.
however, for sup->srcu_size_state being between SRCU_SIZE_WAIT_BARRIER and
SRCU_SIZE_BIG, we will queue sdp->delay_work for all CPUs belonging to the
leaf SRCU node, regardless of whether there is a callback on the current
CPU's sdp->srcu_cblist in srcu_gp_end().
so the timer_delete_sync(&sdp->delay_work) should be called unconditionally.
to ensure that the timer callback has ended or the timer which in pending
status has been successfully deleted.
diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
index b9fe57ff9100..d13c12f61150 100644
--- a/kernel/rcu/srcutree.c
+++ b/kernel/rcu/srcutree.c
@@ -751,8 +751,9 @@ void cleanup_srcu_struct(struct srcu_struct *ssp)
// 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))
+ if (WARN_ON(timer_delete_sync(&sdp->delay_work) &&
+ rcu_segcblist_n_cbs(&sdp->srcu_cblist)) &&
+ 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)))
Thanks
Zqiang
>
> Thanks
> Zqiang
>
> >
> > Thanks
> > Zqiang
> >
>
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] srcu: Queue sdp->work when the delay timer is successfully deleted
2026-08-03 23:40 ` Zqiang
@ 2026-08-04 8:04 ` Breno Leitao
0 siblings, 0 replies; 9+ messages in thread
From: Breno Leitao @ 2026-08-04 8:04 UTC (permalink / raw)
To: Zqiang
Cc: paulmck, frederic, neeraj.upadhyay, joelagnelf, urezki, boqun,
rcu, linux-kernel
On Mon, Aug 03, 2026 at 11:40:14PM +0000, Zqiang wrote:
> diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
> index b9fe57ff9100..d13c12f61150 100644
> --- a/kernel/rcu/srcutree.c
> +++ b/kernel/rcu/srcutree.c
> @@ -751,8 +751,9 @@ void cleanup_srcu_struct(struct srcu_struct *ssp)
>
> // 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))
> + if (WARN_ON(timer_delete_sync(&sdp->delay_work) &&
> + rcu_segcblist_n_cbs(&sdp->srcu_cblist)) &&
> + rcu_cpu_beenfullyonline(sdp->cpu))
Isn't the WARN_ON() the same? thus it will hit again?
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-08-04 8:04 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-09 10:06 [PATCH] srcu: Queue sdp->work when the delay timer is successfully deleted Zqiang
2026-07-10 18:34 ` Paul E. McKenney
2026-07-31 17:45 ` Breno Leitao
2026-08-01 5:19 ` Zqiang
2026-08-03 10:49 ` Breno Leitao
2026-08-03 14:11 ` Zqiang
2026-08-03 14:37 ` Zqiang
2026-08-03 23:40 ` Zqiang
2026-08-04 8:04 ` Breno Leitao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox