| Message ID | 20221116015244.10539-2-kernelfans@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 08d9c4f609afaf5c3032320d50f2bae4acdca314 |
| Headers | show |
| Series | [1/2] srcu: Fix a misspelling in note | expand |
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@gmail.com> > Cc: Lai Jiangshan <jiangshanlai@gmail.com> > Cc: "Paul E. McKenney" <paulmck@kernel.org> > Cc: Frederic Weisbecker <frederic@kernel.org> > Cc: Josh Triplett <josh@joshtriplett.org> > Cc: Steven Rostedt <rostedt@goodmis.org> > Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> > To: rcu@vger.kernel.org > --- > 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@kernel.org> > } > > /* > -- > 2.31.1 >
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@gmail.com> > > Cc: Lai Jiangshan <jiangshanlai@gmail.com> > > Cc: "Paul E. McKenney" <paulmck@kernel.org> > > Cc: Frederic Weisbecker <frederic@kernel.org> > > Cc: Josh Triplett <josh@joshtriplett.org> > > Cc: Steven Rostedt <rostedt@goodmis.org> > > Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> > > To: rcu@vger.kernel.org > > --- > > 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@kernel.org> 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@gmail.com> 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@gmail.com> Cc: Lai Jiangshan <jiangshanlai@gmail.com> Cc: "Paul E. McKenney" <paulmck@kernel.org> Cc: Josh Triplett <josh@joshtriplett.org> Cc: Steven Rostedt <rostedt@goodmis.org> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> Cc: <rcu@vger.kernel.org> Reviewed-by: Frederic Weisbecker <frederic@kernel.org> Signed-off-by: Paul E. McKenney <paulmck@kernel.org> 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; } /*
On Thu, Nov 17, 2022 at 1:26 AM Paul E. McKenney <paulmck@kernel.org> 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@gmail.com> > > > Cc: Lai Jiangshan <jiangshanlai@gmail.com> > > > Cc: "Paul E. McKenney" <paulmck@kernel.org> > > > Cc: Frederic Weisbecker <frederic@kernel.org> > > > Cc: Josh Triplett <josh@joshtriplett.org> > > > Cc: Steven Rostedt <rostedt@goodmis.org> > > > Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> > > > To: rcu@vger.kernel.org > > > --- > > > 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@kernel.org> > > 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@gmail.com> > 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@gmail.com> > Cc: Lai Jiangshan <jiangshanlai@gmail.com> > Cc: "Paul E. McKenney" <paulmck@kernel.org> > Cc: Josh Triplett <josh@joshtriplett.org> > Cc: Steven Rostedt <rostedt@goodmis.org> > Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> > Cc: <rcu@vger.kernel.org> > Reviewed-by: Frederic Weisbecker <frederic@kernel.org> > Signed-off-by: Paul E. McKenney <paulmck@kernel.org> > > 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; > } > > /*
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; } /*
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@gmail.com> Cc: Lai Jiangshan <jiangshanlai@gmail.com> Cc: "Paul E. McKenney" <paulmck@kernel.org> Cc: Frederic Weisbecker <frederic@kernel.org> Cc: Josh Triplett <josh@joshtriplett.org> Cc: Steven Rostedt <rostedt@goodmis.org> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> To: rcu@vger.kernel.org --- kernel/rcu/srcutree.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)