On Thu, Nov 17, 2022 at 1:26 AM Paul E. McKenney <paulmck@xxxxxxxxxx> wrote:
>
> On Wed, Nov 16, 2022 at 02:34:37PM +0100, Frederic Weisbecker wrote:
> > On Wed, Nov 16, 2022 at 09:52:44AM +0800, Pingfan Liu wrote:
> > > A seq contains two fields: counter and state.
> > >
> > > SRCU_SNP_INIT_SEQ is used as invalid initial value for srcu_node GP
> > > sequence numbers. Hence srcu_invl_snp_seq() should compare both fields
> > > of a seq.
> > >
> > > Signed-off-by: Pingfan Liu <kernelfans@xxxxxxxxx>
> > > Cc: Lai Jiangshan <jiangshanlai@xxxxxxxxx>
> > > Cc: "Paul E. McKenney" <paulmck@xxxxxxxxxx>
> > > Cc: Frederic Weisbecker <frederic@xxxxxxxxxx>
> > > Cc: Josh Triplett <josh@xxxxxxxxxxxxxxxx>
> > > Cc: Steven Rostedt <rostedt@xxxxxxxxxxx>
> > > Cc: Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx>
> > > To: rcu@xxxxxxxxxxxxxxx
> > > ---
> > > kernel/rcu/srcutree.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
> > > index 1c304fec89c0..725c82bb0a6a 100644
> > > --- a/kernel/rcu/srcutree.c
> > > +++ b/kernel/rcu/srcutree.c
> > > @@ -154,7 +154,7 @@ static void init_srcu_struct_data(struct srcu_struct *ssp)
> > > */
> > > static inline bool srcu_invl_snp_seq(unsigned long s)
> > > {
> > > - return rcu_seq_state(s) == SRCU_SNP_INIT_SEQ;
> > > + return s == SRCU_SNP_INIT_SEQ;
> >
> > Doesn't hurt and makes it less confusing as it doesn't suggest anymore
> > there _might_ be a gp number behind.
> >
> > Reviewed-by: Frederic Weisbecker <frederic@xxxxxxxxxx>
>
> Good catch, thank you both!
>
> My first reaction was that this patch was wrong, so I recorded the
> results of my investigation in the commit log. Please check below in
> case I messed something up.
>
It looks more clear now. Thanks for improving the log.
Thanks,
Pingfan
> Thanx, Paul
>
> ------------------------------------------------------------------------
>
> commit 08d9c4f609afaf5c3032320d50f2bae4acdca314
> Author: Pingfan Liu <kernelfans@xxxxxxxxx>
> Date: Wed Nov 16 09:52:44 2022 +0800
>
> srcu: Fix the comparision in srcu_invl_snp_seq()
>
> A grace-period sequence number contains two fields: counter and
> state. SRCU_SNP_INIT_SEQ provides a guaranteed invalid value for
> grace-period sequence numbers in newly allocated srcu_node structures'
> ->srcu_have_cbs[] and ->srcu_gp_seq_needed_exp fields. The point of the
> comparison in srcu_invl_snp_seq() is not to detect invalid grace-period
> sequence numbers in general, but rather to detect a newly allocated
> srcu_node structure whose ->srcu_have_cbs[] and ->srcu_gp_seq_needed_exp
> fields need to be brought into line with the srcu_struct structure's
> ->srcu_gp_seq field.
>
> This commit therefore causes srcu_invl_snp_seq() to compare both fields
> of the specified grace-period sequence number.
>
> Signed-off-by: Pingfan Liu <kernelfans@xxxxxxxxx>
> Cc: Lai Jiangshan <jiangshanlai@xxxxxxxxx>
> Cc: "Paul E. McKenney" <paulmck@xxxxxxxxxx>
> Cc: Josh Triplett <josh@xxxxxxxxxxxxxxxx>
> Cc: Steven Rostedt <rostedt@xxxxxxxxxxx>
> Cc: Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx>
> Cc: <rcu@xxxxxxxxxxxxxxx>
> Reviewed-by: Frederic Weisbecker <frederic@xxxxxxxxxx>
> Signed-off-by: Paul E. McKenney <paulmck@xxxxxxxxxx>
>
> diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
> index 16953784a0bdf..6af0312005801 100644
> --- a/kernel/rcu/srcutree.c
> +++ b/kernel/rcu/srcutree.c
> @@ -154,7 +154,7 @@ static void init_srcu_struct_data(struct srcu_struct *ssp)
> */
> static inline bool srcu_invl_snp_seq(unsigned long s)
> {
> - return rcu_seq_state(s) == SRCU_SNP_INIT_SEQ;
> + return s == SRCU_SNP_INIT_SEQ;
> }
>
> /*
