[PATCH v2] srcu: Fix WARN_ON() for timer_delete_sync() in cleanup_srcu_struct()

[PATCH v2] srcu: Fix WARN_ON() for timer_delete_sync() in cleanup_srcu_struct()

[Zqiang]

The WARN_ON() for timer_delete_sync() means that the caller forgot to
call srcu_barrier() before cleanup. however, it still can trigger even
when srcu_barrier() was properly called.

When a SRCU grace period ends and ss_state < SRCU_SIZE_BIG, the mask=~0
causes delay_work timer be queued on every online CPU regardless of
whether it's sdp->srcu_cblist has callbacks. srcu_barrier() only wait
srcu_barrier_head's callbacks to complete, however, the srcu_barrier_head
will not be inserted into an empty sdp->srcu_cblist. after that, when
cleanup_srcu_struct() finds a pending delay_work timer on a CPU with no
callbacks, triggering a false positive.

This commit therefore add rcu_segcblist_n_cbs() to WARN_ON(), make the
warning triggers only when the timer is still pending and there are actual
outstanding callbacks.

Fixes: 05c3e88488ed ("srcu: Queue sdp->work when the delay timer is successfully deleted")
Reported-by: Breno Leitao <leitao@debian.org>
Reported-by: kernel test robot <oliver.sang@intel.com> 
Closes: https://lore.kernel.org/oe-lkp/202608060833.bce92909-lkp@intel.com/ 
Tested-by: kernel test robot <oliver.sang@intel.com>
Signed-off-by: Zqiang <qiang.zhang@linux.dev>
---
 kernel/rcu/srcutree.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
index 35fface51d50..090dddfc7463 100644
--- a/kernel/rcu/srcutree.c
+++ b/kernel/rcu/srcutree.c
@@ -753,8 +753,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)))
-- 
2.17.1

Re: [PATCH v2] srcu: Fix WARN_ON() for timer_delete_sync() in cleanup_srcu_struct()

[Breno Leitao]

On Mon, Aug 10, 2026 at 06:55:22PM +0800, Zqiang wrote:
> The WARN_ON() for timer_delete_sync() means that the caller forgot to
> call srcu_barrier() before cleanup. however, it still can trigger even
> when srcu_barrier() was properly called.
> 
> When a SRCU grace period ends and ss_state < SRCU_SIZE_BIG, the mask=~0
> causes delay_work timer be queued on every online CPU regardless of
> whether it's sdp->srcu_cblist has callbacks. srcu_barrier() only wait
> srcu_barrier_head's callbacks to complete, however, the srcu_barrier_head
> will not be inserted into an empty sdp->srcu_cblist. after that, when
> cleanup_srcu_struct() finds a pending delay_work timer on a CPU with no
> callbacks, triggering a false positive.
> 
> This commit therefore add rcu_segcblist_n_cbs() to WARN_ON(), make the
> warning triggers only when the timer is still pending and there are actual
> outstanding callbacks.
> 
> Fixes: 05c3e88488ed ("srcu: Queue sdp->work when the delay timer is successfully deleted")
> Reported-by: Breno Leitao <leitao@debian.org>
> Reported-by: kernel test robot <oliver.sang@intel.com> 
> Closes: https://lore.kernel.org/oe-lkp/202608060833.bce92909-lkp@intel.com/ 
> Tested-by: kernel test robot <oliver.sang@intel.com>
> Signed-off-by: Zqiang <qiang.zhang@linux.dev>
> ---
>  kernel/rcu/srcutree.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
> index 35fface51d50..090dddfc7463 100644
> --- a/kernel/rcu/srcutree.c
> +++ b/kernel/rcu/srcutree.c
> @@ -753,8 +753,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))

This is quite hard to read, mainly with this indentation.

You probably want to rewrite the checks , otherwise the indentation
should be something like, which is horrible (although correct?):

	if (WARN_ON(timer_delete_sync(&sdp->delay_work) &&
	            rcu_segcblist_n_cbs(&sdp->srcu_cblist)) &&
	    rcu_cpu_beenfullyonline(sdp->cpu))

Would something like this work?

	if (rcu_cpu_beenfullyonline(sdp->cpu) &&
	    WARN_ON(timer_delete_sync(&sdp->delay_work) &&
	            rcu_segcblist_n_cbs(&sdp->srcu_cblist)))

Re: [PATCH v2] srcu: Fix WARN_ON() for timer_delete_sync() in cleanup_srcu_struct()

[Zqiang]

> 
> On Mon, Aug 10, 2026 at 06:55:22PM +0800, Zqiang wrote:
> 
> > 
> > The WARN_ON() for timer_delete_sync() means that the caller forgot to
> >  call srcu_barrier() before cleanup. however, it still can trigger even
> >  when srcu_barrier() was properly called.
> >  
> >  When a SRCU grace period ends and ss_state < SRCU_SIZE_BIG, the mask=~0
> >  causes delay_work timer be queued on every online CPU regardless of
> >  whether it's sdp->srcu_cblist has callbacks. srcu_barrier() only wait
> >  srcu_barrier_head's callbacks to complete, however, the srcu_barrier_head
> >  will not be inserted into an empty sdp->srcu_cblist. after that, when
> >  cleanup_srcu_struct() finds a pending delay_work timer on a CPU with no
> >  callbacks, triggering a false positive.
> >  
> >  This commit therefore add rcu_segcblist_n_cbs() to WARN_ON(), make the
> >  warning triggers only when the timer is still pending and there are actual
> >  outstanding callbacks.
> >  
> >  Fixes: 05c3e88488ed ("srcu: Queue sdp->work when the delay timer is successfully deleted")
> >  Reported-by: Breno Leitao <leitao@debian.org>
> >  Reported-by: kernel test robot <oliver.sang@intel.com> 
> >  Closes: https://lore.kernel.org/oe-lkp/202608060833.bce92909-lkp@intel.com/ 
> >  Tested-by: kernel test robot <oliver.sang@intel.com>
> >  Signed-off-by: Zqiang <qiang.zhang@linux.dev>
> >  ---
> >  kernel/rcu/srcutree.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >  
> >  diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
> >  index 35fface51d50..090dddfc7463 100644
> >  --- a/kernel/rcu/srcutree.c
> >  +++ b/kernel/rcu/srcutree.c
> >  @@ -753,8 +753,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))
> > 
> This is quite hard to read, mainly with this indentation.
> 
> You probably want to rewrite the checks , otherwise the indentation
> should be something like, which is horrible (although correct?):
> 
>  if (WARN_ON(timer_delete_sync(&sdp->delay_work) &&
>  rcu_segcblist_n_cbs(&sdp->srcu_cblist)) &&
>  rcu_cpu_beenfullyonline(sdp->cpu))

Will update and resend (I still mark v2 because v1 has not been sent before).

Thanks
Zqiang

> 
> Would something like this work?
> 
>  if (rcu_cpu_beenfullyonline(sdp->cpu) &&
>  WARN_ON(timer_delete_sync(&sdp->delay_work) &&
>  rcu_segcblist_n_cbs(&sdp->srcu_cblist)))
>

[PATCH v2] srcu: Fix WARN_ON() for timer_delete_sync() in cleanup_srcu_struct()

[Zqiang]

The WARN_ON() for timer_delete_sync() means that the caller forgot
to call srcu_barrier() before cleanup. however, it still can trigger
even when srcu_barrier() was properly called.

When a SRCU grace period ends and find that the allocation of srcu_node
tree has been completed, but the ss_state is still less than SRCU_SIZE_BIG,
the mask=~0 causes delay_work timer be queued on every CPU which has
been online regardless of whether it's sdp->srcu_cblist has callbacks.
the srcu_barrier() only wait srcu_barrier_head's callbacks to complete,
however, the srcu_barrier_head will not be inserted into an empty
sdp->srcu_cblist. after that, when cleanup_srcu_struct() finds a pending
delay_work timer on a CPU with no callbacks, triggering a false positive.

This commit therefore add rcu_segcblist_n_cbs() to WARN_ON(), make the
warning triggers only when the timer is still pending and there are
actual outstanding callbacks.

Fixes: 05c3e88488ed ("srcu: Queue sdp->work when the delay timer is successfully deleted")
Reported-by: Breno Leitao <leitao@debian.org>
Reported-by: kernel test robot <oliver.sang@intel.com>
Closes: https://lore.kernel.org/oe-lkp/202608060833.bce92909-lkp@intel.com/
Tested-by: kernel test robot <oliver.sang@intel.com>
Signed-off-by: Zqiang <qiang.zhang@linux.dev>
---
 v1->v2:
 Update commit log and modify indentation. 

 kernel/rcu/srcutree.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
index 35fface51d50..be420ecde851 100644
--- a/kernel/rcu/srcutree.c
+++ b/kernel/rcu/srcutree.c
@@ -753,8 +753,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)))
-- 
2.48.1