[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()

Posted by Zqiang 1 month, 2 weeks ago

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

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

Posted by Breno Leitao 1 month, 2 weeks ago

On Tue, Aug 11, 2026 at 08:29:51AM +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 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>

Tested-by: Breno Leitao <leitao@debian.org>

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

Posted by Paul E. McKenney 1 month, 2 weeks ago

On Wed, Aug 12, 2026 at 02:07:13AM -0700, Breno Leitao wrote:
> On Tue, Aug 11, 2026 at 08:29:51AM +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 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>
> 
> Tested-by: Breno Leitao <leitao@debian.org>

Thank you, Breno!  I will add this to the original, which I merged this
fix into.

							Thanx, Paul

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

Posted by Frederic Weisbecker 1 month, 2 weeks ago

Le Tue, Aug 11, 2026 at 08:29:51AM +0800, Zqiang a écrit :
> 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>

Reviewed-by: Frederic Weisbecker <frederic@kernel.org>

-- 
Frederic Weisbecker
SUSE Labs

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

Posted by Paul E. McKenney 1 month, 2 weeks ago

On Tue, Aug 11, 2026 at 04:54:20PM +0200, Frederic Weisbecker wrote:
> Le Tue, Aug 11, 2026 at 08:29:51AM +0800, Zqiang a écrit :
> > 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>
> 
> Reviewed-by: Frederic Weisbecker <frederic@kernel.org>

I pulled this in for further review and testing.  My plan is to fold
it in to the commit that it fixes, of course with attribution.

Thank you both!

							Thanx, Paul