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.
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;
}
/*
