Re: [PATCH 7/9] srcu: Detect duplicate callback enqueue at registration

[Paul E. McKenney]

On Mon, Oct 26, 2026 at 10:35:02AM +0800, KunWu Chan wrote:
> Thanks Paul for the detailed review.
> 
> On Sat, Oct 24, 2026 at 3:38 AM Paul E. McKenney <paulmck@xxxxxxxxxx> wrote:
> >
> > On Fri, Oct 23, 2026 at 12:30:57PM -0700, Paul E. McKenney wrote:
> > > On Fri, Oct 23, 2026 at 06:18:39PM +0800, Kunwu Chan wrote:
> > > > Move the duplicate-callback check from srcu_do_enqueue() to
> > > > __call_srcu() entry so that detected duplicate registrations
> > > > are rejected before they reach the deferred-list enqueue path.
> > > > Report the callback address, function symbol, and object
> > > > information, rate-limited to the first three occurrences.
> > > >
> > > > Signed-off-by: Kunwu Chan <chentao@xxxxxxxxxx>
> > >
> > > Getting the stackof the call_srcu() once double-enqueue is detected
> > > would be a very good thing!
> 
> I'm also working on the rcutorture support and will send it out separately.

Looking forward to it!

							Thanx, Paul

> > > One question below.
> > >
> > > > ---
> > > >  kernel/rcu/srcutree.c | 20 ++++++++++++++------
> > > >  1 file changed, 14 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
> > > > index 1993ddbdb828..3838d9fdf532 100644
> > > > --- a/kernel/rcu/srcutree.c
> > > > +++ b/kernel/rcu/srcutree.c
> > > > @@ -27,6 +27,7 @@
> > > >  #include <linux/smp.h>
> > > >  #include <linux/delay.h>
> > > >  #include <linux/module.h>
> > > > +#include <linux/mm.h>
> > > >  #include <linux/slab.h>
> > > >  #include <linux/srcu.h>
> > > >
> > > > @@ -1511,12 +1512,6 @@ static unsigned long srcu_gp_start_if_needed(struct srcu_struct *ssp,
> > > >  static void srcu_do_enqueue(struct srcu_struct *ssp, struct rcu_head *rhp,
> > > >                         rcu_callback_t func, bool do_norm)
> > > >  {
> > > > -   if (debug_rcu_head_queue(rhp)) {
> > > > -           /* Probable double call_srcu(), so leak the callback. */
> > > > -           WRITE_ONCE(rhp->func, srcu_leak_callback);
> > > > -           WARN_ONCE(1, "call_srcu(): Leaked duplicate callback\n");
> > > > -           return;
> > > > -   }
> > > >     rhp->func = func;
> > > >     (void)srcu_gp_start_if_needed(ssp, rhp, do_norm);
> > > >  }
> > > > @@ -1529,6 +1524,8 @@ static void srcu_do_enqueue(struct srcu_struct *ssp, struct rcu_head *rhp,
> > > >  static void __call_srcu(struct srcu_struct *ssp, struct rcu_head *rhp,
> > > >                     rcu_callback_t func, bool do_norm)
> > > >  {
> > > > +   static atomic_t doublefrees;
> > > > +
> > > >     /* Misaligned rcu_head! */
> > > >     WARN_ON_ONCE((unsigned long)rhp & (sizeof(void *) - 1));
> > > >
> > > > @@ -1539,6 +1536,16 @@ static void __call_srcu(struct srcu_struct *ssp, struct rcu_head *rhp,
> > > >     if (WARN_ON_ONCE(ssp->srcu_reader_flavor == SRCU_READ_FLAVOR_ATOMIC))
> > > >             return; // Leak the callback rather than corrupt SRCU state.
> > > >
> > > > +   if (debug_rcu_head_queue(rhp)) {
> > > > +           if (atomic_inc_return(&doublefrees) < 4) {
> > >
> > > Why this hard-coded 4?  Why not 3 or 5?  Should this be adjustable via
> > > Kconfig or kernel boot parameter?  Or, alternatively, should the below
> > > pr_err() instead be pr_err_ratelimited()?
> 
> Regarding the hard-coded limit, I followed the existing code in tree.c when
> choosing 4. Thanks for pointing this out. I'll switch to pr_err_ratelimited()
> in v2 and make the same change in tree.c as well, so the two paths are
> consistent.
> > >
> > > > +                   pr_err("%s(): Duplicate call_srcu() CB %p->%pS()\n",
> > > > +                          __func__, rhp, rhp->func);
> > > > +                   mem_dump_obj(rhp);
> > >
> > > Does any of this automatically dump out the stack that was recorded
> > > in earlier patches in this series?
> >
> 
> I'll also add dump_stack() to all three duplicate-enqueue patches
> to capture the current stack when a duplicate is detected.
> 
> > As in shouldn't there be a kasan_print_aux_stacks() in there somewhere?
> > I did not see an call from mem_dump_obj(), but I could easily have
> > missed it.
> >
> 
> As for kasan_print_aux_stacks(), as mentioned in my reply to patch 1,
> it is called by the existing KASAN report path, which prints the auxiliary
> stack recorded by patches 1-3. It does not need to be called directly
> from these patches.
> 
> Thanks,
> Kunwu
> 
> >                                                         Thanx, Paul
> >
> > > > +           }
> > > > +           WRITE_ONCE(rhp->func, srcu_leak_callback);
> > > > +           return;
> > > > +   }
> > > > +
> > > >     /* Record the caller: the irq_work's stack says nothing about it. */
> > > >     kasan_record_aux_stack(rhp);
> > > >
> > > > @@ -1554,6 +1561,7 @@ static void __call_srcu(struct srcu_struct *ssp, struct rcu_head *rhp,
> > > >             if (READ_ONCE(sndp->draining) && !in_nmi()) {
> > > >                     WARN_ONCE(IS_ENABLED(CONFIG_PROVE_RCU),
> > > >                               "call_srcu() re-entered during callback drain; leaking callback\n");
> > > > +                   debug_rcu_head_unqueue(rhp);
> > > >                     return;
> > > >             }
> > > >             sdp = this_cpu_ptr(ssp->sda);
> > > > --
> > > > 2.43.0
> > > >