[PATCH v2] srcutree: Add WARN_ON_ONCE() for atomic SRCU in srcu_gp_end()

[PATCH v2] srcutree: Add WARN_ON_ONCE() for atomic SRCU in srcu_gp_end()

[Kunwu Chan]

Atomic SRCU remain in the small size state, so add a
WARN_ON_ONCE() before the transition check to catch any future
code path that might violate this invariant.

Signed-off-by: Kunwu Chan <kunwu.chan@gmail.com>
---
Changes since v1:
- Drop the !is_atomic guard from v1 and add WARN_ON_ONCE() instead,
  per Paul McKenney's suggestion.
- v1: https://lore.kernel.org/rcu/20260907075829.2073224-8-kunwu.chan@linux.dev/
---
 kernel/rcu/srcutree.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
index 84c022ec8e09..68441c32cebf 100644
--- a/kernel/rcu/srcutree.c
+++ b/kernel/rcu/srcutree.c
@@ -1074,6 +1074,8 @@ static void srcu_gp_end(struct srcu_struct *ssp, bool is_atomic)
 	}
 
 	/* Transition to big if needed. */
+	WARN_ON_ONCE(ssp->srcu_reader_flavor == SRCU_READ_FLAVOR_ATOMIC &&
+			ss_state != SRCU_SIZE_SMALL);
 	if (ss_state != SRCU_SIZE_SMALL && ss_state != SRCU_SIZE_BIG) {
 		if (ss_state == SRCU_SIZE_ALLOC)
 			init_srcu_struct_nodes(ssp, GFP_KERNEL);
-- 
2.43.0

Re: [PATCH v2] srcutree: Add WARN_ON_ONCE() for atomic SRCU in srcu_gp_end()

[Paul E. McKenney]

On Thu, Sep 10, 2026 at 04:46:29PM +0800, Kunwu Chan wrote:
> Atomic SRCU remain in the small size state, so add a
> WARN_ON_ONCE() before the transition check to catch any future
> code path that might violate this invariant.
> 
> Signed-off-by: Kunwu Chan <kunwu.chan@gmail.com>
> ---
> Changes since v1:
> - Drop the !is_atomic guard from v1 and add WARN_ON_ONCE() instead,
>   per Paul McKenney's suggestion.
> - v1: https://lore.kernel.org/rcu/20260907075829.2073224-8-kunwu.chan@linux.dev/
> ---
>  kernel/rcu/srcutree.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
> index 84c022ec8e09..68441c32cebf 100644
> --- a/kernel/rcu/srcutree.c
> +++ b/kernel/rcu/srcutree.c
> @@ -1074,6 +1074,8 @@ static void srcu_gp_end(struct srcu_struct *ssp, bool is_atomic)
>  	}
>  
>  	/* Transition to big if needed. */
> +	WARN_ON_ONCE(ssp->srcu_reader_flavor == SRCU_READ_FLAVOR_ATOMIC &&
> +			ss_state != SRCU_SIZE_SMALL);

Also much better, but we also need to avoid doing the transition in this
case, because doing that transition breaks atomic SRCU.  On way to make
this happen is something like this:

	if (ssp->srcu_reader_flavor == SRCU_READ_FLAVOR_ATOMIC && ss_state != SRCU_SIZE_SMALL) {
		WARN_ON_ONCE(1);
	} else if (ss_state != SRCU_SIZE_SMALL && ss_state != SRCU_SIZE_BIG) {
		if (ss_state == SRCU_SIZE_ALLOC)
			init_srcu_struct_nodes(ssp, GFP_KERNEL);

Other approaches might take advantage of the fact that WARN_ON_ONCE()
returns the value of its argument, allowing WARN_ON_ONCE() to be used
as a condition in an "if" statement.

							Thanx, Paul

>  	if (ss_state != SRCU_SIZE_SMALL && ss_state != SRCU_SIZE_BIG) {
>  		if (ss_state == SRCU_SIZE_ALLOC)
>  			init_srcu_struct_nodes(ssp, GFP_KERNEL);
> -- 
> 2.43.0
> 

Re: [PATCH v2] srcutree: Add WARN_ON_ONCE() for atomic SRCU in srcu_gp_end()

[KunWu Chan]

On Fri, Sep 11, 2026 at 12:58 AM Paul E. McKenney <paulmck@kernel.org> wrote:
>
> On Thu, Sep 10, 2026 at 04:46:29PM +0800, Kunwu Chan wrote:
> > Atomic SRCU remain in the small size state, so add a
> > WARN_ON_ONCE() before the transition check to catch any future
> > code path that might violate this invariant.
> >
> > Signed-off-by: Kunwu Chan <kunwu.chan@gmail.com>
> > ---
> > Changes since v1:
> > - Drop the !is_atomic guard from v1 and add WARN_ON_ONCE() instead,
> >   per Paul McKenney's suggestion.
> > - v1: https://lore.kernel.org/rcu/20260907075829.2073224-8-kunwu.chan@linux.dev/
> > ---
> >  kernel/rcu/srcutree.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
> > index 84c022ec8e09..68441c32cebf 100644
> > --- a/kernel/rcu/srcutree.c
> > +++ b/kernel/rcu/srcutree.c
> > @@ -1074,6 +1074,8 @@ static void srcu_gp_end(struct srcu_struct *ssp, bool is_atomic)
> >       }
> >
> >       /* Transition to big if needed. */
> > +     WARN_ON_ONCE(ssp->srcu_reader_flavor == SRCU_READ_FLAVOR_ATOMIC &&
> > +                     ss_state != SRCU_SIZE_SMALL);
>
> Also much better, but we also need to avoid doing the transition in this
> case, because doing that transition breaks atomic SRCU.  On way to make
> this happen is something like this:
>
>         if (ssp->srcu_reader_flavor == SRCU_READ_FLAVOR_ATOMIC && ss_state != SRCU_SIZE_SMALL) {
>                 WARN_ON_ONCE(1);
>         } else if (ss_state != SRCU_SIZE_SMALL && ss_state != SRCU_SIZE_BIG) {
>                 if (ss_state == SRCU_SIZE_ALLOC)
>                         init_srcu_struct_nodes(ssp, GFP_KERNEL);
>

Thanks, Paul. Good point. The original !is_atomic guard in v1
already prevented the transition for atomic SRCU. I was mainly
thinking of the WARN as a diagnostic, since this state should not
occur with the current code. I agree that retaining the protection
against the transition is safer if the invariant is ever violated.

> Other approaches might take advantage of the fact that WARN_ON_ONCE()
> returns the value of its argument, allowing WARN_ON_ONCE() to be used
> as a condition in an "if" statement.
>

I prefer the first approach. Using !WARN_ON_ONCE() directly in the
existing condition would make it rather long and less readable,
while the explicit if/else keeps the control flow clear.

Thanks,
Kunwu

>                                                         Thanx, Paul
>
> >       if (ss_state != SRCU_SIZE_SMALL && ss_state != SRCU_SIZE_BIG) {
> >               if (ss_state == SRCU_SIZE_ALLOC)
> >                       init_srcu_struct_nodes(ssp, GFP_KERNEL);
> > --
> > 2.43.0
> >

Re: [PATCH v2] srcutree: Add WARN_ON_ONCE() for atomic SRCU in srcu_gp_end()

[Paul E. McKenney]

On Fri, Sep 11, 2026 at 09:50:09AM +0800, KunWu Chan wrote:
> On Fri, Sep 11, 2026 at 12:58 AM Paul E. McKenney <paulmck@kernel.org> wrote:
> >
> > On Thu, Sep 10, 2026 at 04:46:29PM +0800, Kunwu Chan wrote:
> > > Atomic SRCU remain in the small size state, so add a
> > > WARN_ON_ONCE() before the transition check to catch any future
> > > code path that might violate this invariant.
> > >
> > > Signed-off-by: Kunwu Chan <kunwu.chan@gmail.com>
> > > ---
> > > Changes since v1:
> > > - Drop the !is_atomic guard from v1 and add WARN_ON_ONCE() instead,
> > >   per Paul McKenney's suggestion.
> > > - v1: https://lore.kernel.org/rcu/20260907075829.2073224-8-kunwu.chan@linux.dev/
> > > ---
> > >  kernel/rcu/srcutree.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
> > > index 84c022ec8e09..68441c32cebf 100644
> > > --- a/kernel/rcu/srcutree.c
> > > +++ b/kernel/rcu/srcutree.c
> > > @@ -1074,6 +1074,8 @@ static void srcu_gp_end(struct srcu_struct *ssp, bool is_atomic)
> > >       }
> > >
> > >       /* Transition to big if needed. */
> > > +     WARN_ON_ONCE(ssp->srcu_reader_flavor == SRCU_READ_FLAVOR_ATOMIC &&
> > > +                     ss_state != SRCU_SIZE_SMALL);
> >
> > Also much better, but we also need to avoid doing the transition in this
> > case, because doing that transition breaks atomic SRCU.  On way to make
> > this happen is something like this:
> >
> >         if (ssp->srcu_reader_flavor == SRCU_READ_FLAVOR_ATOMIC && ss_state != SRCU_SIZE_SMALL) {
> >                 WARN_ON_ONCE(1);
> >         } else if (ss_state != SRCU_SIZE_SMALL && ss_state != SRCU_SIZE_BIG) {
> >                 if (ss_state == SRCU_SIZE_ALLOC)
> >                         init_srcu_struct_nodes(ssp, GFP_KERNEL);
> >
> 
> Thanks, Paul. Good point. The original !is_atomic guard in v1
> already prevented the transition for atomic SRCU. I was mainly
> thinking of the WARN as a diagnostic, since this state should not
> occur with the current code. I agree that retaining the protection
> against the transition is safer if the invariant is ever violated.
> 
> > Other approaches might take advantage of the fact that WARN_ON_ONCE()
> > returns the value of its argument, allowing WARN_ON_ONCE() to be used
> > as a condition in an "if" statement.
> >
> 
> I prefer the first approach. Using !WARN_ON_ONCE() directly in the
> existing condition would make it rather long and less readable,
> while the explicit if/else keeps the control flow clear.

Very good, looking forward to seeing what you come up with.

							Thanx, Paul

> Thanks,
> Kunwu
> 
> >                                                         Thanx, Paul
> >
> > >       if (ss_state != SRCU_SIZE_SMALL && ss_state != SRCU_SIZE_BIG) {
> > >               if (ss_state == SRCU_SIZE_ALLOC)
> > >                       init_srcu_struct_nodes(ssp, GFP_KERNEL);
> > > --
> > > 2.43.0
> > >