diff mbox series

rcu: Further comment and explain the state space of GP sequences

Message ID 20230119141134.686626-1-frederic@kernel.org (mailing list archive)
State Superseded
Headers show
Series rcu: Further comment and explain the state space of GP sequences | expand

Commit Message

Frederic Weisbecker Jan. 19, 2023, 2:11 p.m. UTC
The state space of the GP sequence number isn't documented and the
definitions of its special values are scattered. Try to gather some
common knowledge near the GP seq headers.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
---
 kernel/rcu/rcu.h | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

Comments

Frederic Weisbecker Jan. 19, 2023, 2:15 p.m. UTC | #1
On Thu, Jan 19, 2023 at 03:11:35PM +0100, Frederic Weisbecker wrote:
> The state space of the GP sequence number isn't documented and the
> definitions of its special values are scattered. Try to gather some
> common knowledge near the GP seq headers.
> 
> Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
> ---
>  kernel/rcu/rcu.h | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/kernel/rcu/rcu.h b/kernel/rcu/rcu.h
> index 115616ac3bfa..fb95de039596 100644
> --- a/kernel/rcu/rcu.h
> +++ b/kernel/rcu/rcu.h
> @@ -14,6 +14,39 @@
>  
>  /*
>   * Grace-period counter management.
> + *
> + * The two lowest significant bits gather the control flags.
> + * The higher bits form the RCU sequence counter.
> + *
> + * About the control flags, a common value of 0 means that no GP is in progress.
> + * A value of 1 means that a grace period has started and is in progress. When
> + * the grace period completes, the control flags are reset to 0 and the sequence
> + * counter is incremented.
> + *
> + * However some specific RCU usages make use of custom values.
> + *
> + * SRCU special control values:
> + *
> + * 	SRCU_SNP_INIT_SEQ	:	Invalid/init value set when SRCU node
> + * 							is initialized.
> + *
> + * 	SRCU_STATE_IDLE		:	No SRCU gp is in progress
> + *
> + * 	SRCU_STATE_SCAN1	:	State set by rcu_seq_start(). Indicates
> + *								we are scanning the inactive readers
> + *								index.
> + *
> + *		SRCU_STATE_SCAN2	:	State set manually via rcu_seq_set_state()
> + *								Indicates we are flipping the readers
> + *								index and then scanning the newly inactive
> + *								readers index.
> + *
> + * RCU polled GP special control value:
> + *
> + *	RCU_GET_STATE_COMPLETED :	State value indicating that a polled GP
> + *								has completed. It's an absolute value
> + *								covering both the state and the counter of
> + *								the GP sequence.
>   */
>  
>  #define RCU_SEQ_CTR_SHIFT	2
> -- 
> 2.34.1
> 

Ok perhaps this one got the tabs right:

---
From: Frederic Weisbecker <frederic@kernel.org>
Date: Thu, 19 Jan 2023 14:29:34 +0100
Subject: [PATCH v2] rcu: Further comment and explain the state space of GP
 sequences

The state space of the GP sequence number isn't documented and the
definitions of its special values are scattered. Try to gather some
common knowledge near the GP seq headers.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
---
 kernel/rcu/rcu.h | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/kernel/rcu/rcu.h b/kernel/rcu/rcu.h
index 115616ac3bfa..fb95de039596 100644
--- a/kernel/rcu/rcu.h
+++ b/kernel/rcu/rcu.h
@@ -14,6 +14,39 @@
 
 /*
  * Grace-period counter management.
+ *
+ * The two lowest significant bits gather the control flags.
+ * The higher bits form the RCU sequence counter.
+ *
+ * About the control flags, a common value of 0 means that no GP is in progress.
+ * A value of 1 means that a grace period has started and is in progress. When
+ * the grace period completes, the control flags are reset to 0 and the sequence
+ * counter is incremented.
+ *
+ * However some specific RCU usages make use of custom values.
+ *
+ * SRCU special control values:
+ *
+ * 	SRCU_SNP_INIT_SEQ	:	Invalid/init value set when SRCU node
+ * 					is initialized.
+ *
+ * 	SRCU_STATE_IDLE		:	No SRCU gp is in progress
+ *
+ *	SRCU_STATE_SCAN1	:	State set by rcu_seq_start(). Indicates
+ *					we are scanning the inactive readers
+ *					index.
+ *
+ *	SRCU_STATE_SCAN2	:	State set manually via rcu_seq_set_state()
+ *					Indicates we are flipping the readers
+ *					index and then scanning the newly inactive
+ *					readers index.
+ *
+ * RCU polled GP special control value:
+ *
+ *	RCU_GET_STATE_COMPLETED :	State value indicating that a polled GP
+ *					has completed. It's an absolute value
+ *					covering both the state and the counter of
+ *					the GP sequence.
  */
 
 #define RCU_SEQ_CTR_SHIFT	2
Joel Fernandes Jan. 19, 2023, 5:06 p.m. UTC | #2
On Thu, Jan 19, 2023 at 2:15 PM Frederic Weisbecker <frederic@kernel.org> wrote:
>
> On Thu, Jan 19, 2023 at 03:11:35PM +0100, Frederic Weisbecker wrote:
> > The state space of the GP sequence number isn't documented and the
> > definitions of its special values are scattered. Try to gather some
> > common knowledge near the GP seq headers.
> >
> > Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
> > ---
> >  kernel/rcu/rcu.h | 33 +++++++++++++++++++++++++++++++++
> >  1 file changed, 33 insertions(+)
> >
> > diff --git a/kernel/rcu/rcu.h b/kernel/rcu/rcu.h
> > index 115616ac3bfa..fb95de039596 100644
> > --- a/kernel/rcu/rcu.h
> > +++ b/kernel/rcu/rcu.h
> > @@ -14,6 +14,39 @@
> >
> >  /*
> >   * Grace-period counter management.
> > + *
> > + * The two lowest significant bits gather the control flags.
> > + * The higher bits form the RCU sequence counter.
> > + *
> > + * About the control flags, a common value of 0 means that no GP is in progress.
> > + * A value of 1 means that a grace period has started and is in progress. When
> > + * the grace period completes, the control flags are reset to 0 and the sequence
> > + * counter is incremented.
> > + *
> > + * However some specific RCU usages make use of custom values.
> > + *
> > + * SRCU special control values:
> > + *
> > + *   SRCU_SNP_INIT_SEQ       :       Invalid/init value set when SRCU node
> > + *                                                   is initialized.
> > + *
> > + *   SRCU_STATE_IDLE         :       No SRCU gp is in progress
> > + *
> > + *   SRCU_STATE_SCAN1        :       State set by rcu_seq_start(). Indicates
> > + *                                                           we are scanning the inactive readers
> > + *                                                           index.

The term "inactive reader" is confusing. The readers can very much be
active during scans. During a scan stage, there might be a reader on
any of the 2 indexes that can be right in the middle of their critical
section (and we don't know which index because they could have got
preempted, right after sampling idx). Maybe "inactive slot" is a
better term? And define "inactive slot" as the slot which is no longer
going to be sampled by new readers.

> > + *
> > + *           SRCU_STATE_SCAN2        :       State set manually via rcu_seq_set_state()
> > + *                                                           Indicates we are flipping the readers
> > + *                                                           index and then scanning the newly inactive
> > + *                                                           readers index.
> > + *
> > + * RCU polled GP special control value:
> > + *
> > + *   RCU_GET_STATE_COMPLETED :       State value indicating that a polled GP
> > + *                                                           has completed. It's an absolute value
> > + *                                                           covering both the state and the counter of
> > + *                                                           the GP sequence.

This part I still have to learn (polling) so I'll go do that. But
otherwise your patch LGTM and thanks for doing it.

 - Joel

> >   */
> >
> >  #define RCU_SEQ_CTR_SHIFT    2
> > --
> > 2.34.1
> >
>
> Ok perhaps this one got the tabs right:
>
> ---
> From: Frederic Weisbecker <frederic@kernel.org>
> Date: Thu, 19 Jan 2023 14:29:34 +0100
> Subject: [PATCH v2] rcu: Further comment and explain the state space of GP
>  sequences
>
> The state space of the GP sequence number isn't documented and the
> definitions of its special values are scattered. Try to gather some
> common knowledge near the GP seq headers.
>
> Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
> ---
>  kernel/rcu/rcu.h | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
>
> diff --git a/kernel/rcu/rcu.h b/kernel/rcu/rcu.h
> index 115616ac3bfa..fb95de039596 100644
> --- a/kernel/rcu/rcu.h
> +++ b/kernel/rcu/rcu.h
> @@ -14,6 +14,39 @@
>
>  /*
>   * Grace-period counter management.
> + *
> + * The two lowest significant bits gather the control flags.
> + * The higher bits form the RCU sequence counter.
> + *
> + * About the control flags, a common value of 0 means that no GP is in progress.
> + * A value of 1 means that a grace period has started and is in progress. When
> + * the grace period completes, the control flags are reset to 0 and the sequence
> + * counter is incremented.
> + *
> + * However some specific RCU usages make use of custom values.
> + *
> + * SRCU special control values:
> + *
> + *     SRCU_SNP_INIT_SEQ       :       Invalid/init value set when SRCU node
> + *                                     is initialized.
> + *
> + *     SRCU_STATE_IDLE         :       No SRCU gp is in progress
> + *
> + *     SRCU_STATE_SCAN1        :       State set by rcu_seq_start(). Indicates
> + *                                     we are scanning the inactive readers
> + *                                     index.
> + *
> + *     SRCU_STATE_SCAN2        :       State set manually via rcu_seq_set_state()
> + *                                     Indicates we are flipping the readers
> + *                                     index and then scanning the newly inactive
> + *                                     readers index.
> + *
> + * RCU polled GP special control value:
> + *
> + *     RCU_GET_STATE_COMPLETED :       State value indicating that a polled GP
> + *                                     has completed. It's an absolute value
> + *                                     covering both the state and the counter of
> + *                                     the GP sequence.
>   */
>
>  #define RCU_SEQ_CTR_SHIFT      2
> --
> 2.34.1
>
Joel Fernandes Jan. 20, 2023, 1:47 a.m. UTC | #3
On Thu, Jan 19, 2023 at 03:15:40PM +0100, Frederic Weisbecker wrote:
> On Thu, Jan 19, 2023 at 03:11:35PM +0100, Frederic Weisbecker wrote:
> > The state space of the GP sequence number isn't documented and the
> > definitions of its special values are scattered. Try to gather some
> > common knowledge near the GP seq headers.
> > 
> > Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
> > ---
> >  kernel/rcu/rcu.h | 33 +++++++++++++++++++++++++++++++++
> >  1 file changed, 33 insertions(+)
> > 
> > diff --git a/kernel/rcu/rcu.h b/kernel/rcu/rcu.h
> > index 115616ac3bfa..fb95de039596 100644
> > --- a/kernel/rcu/rcu.h
> > +++ b/kernel/rcu/rcu.h
> > @@ -14,6 +14,39 @@
> >  
> >  /*
> >   * Grace-period counter management.
> > + *
> > + * The two lowest significant bits gather the control flags.
> > + * The higher bits form the RCU sequence counter.
> > + *
> > + * About the control flags, a common value of 0 means that no GP is in progress.
> > + * A value of 1 means that a grace period has started and is in progress. When
> > + * the grace period completes, the control flags are reset to 0 and the sequence
> > + * counter is incremented.
> > + *
> > + * However some specific RCU usages make use of custom values.
> > + *
> > + * SRCU special control values:
> > + *
> > + * 	SRCU_SNP_INIT_SEQ	:	Invalid/init value set when SRCU node
> > + * 							is initialized.
> > + *
> > + * 	SRCU_STATE_IDLE		:	No SRCU gp is in progress
> > + *
> > + * 	SRCU_STATE_SCAN1	:	State set by rcu_seq_start(). Indicates
> > + *								we are scanning the inactive readers
> > + *								index.
> > + *
> > + *		SRCU_STATE_SCAN2	:	State set manually via rcu_seq_set_state()
> > + *								Indicates we are flipping the readers
> > + *								index and then scanning the newly inactive
> > + *								readers index.
> > + *
> > + * RCU polled GP special control value:
> > + *
> > + *	RCU_GET_STATE_COMPLETED :	State value indicating that a polled GP
> > + *								has completed. It's an absolute value
> > + *								covering both the state and the counter of
> > + *								the GP sequence.
> >   */
> >  
> >  #define RCU_SEQ_CTR_SHIFT	2
> > -- 
> > 2.34.1
> > 
> 
> Ok perhaps this one got the tabs right:
> 
> ---
> From: Frederic Weisbecker <frederic@kernel.org>
> Date: Thu, 19 Jan 2023 14:29:34 +0100
> Subject: [PATCH v2] rcu: Further comment and explain the state space of GP
>  sequences
> 
> The state space of the GP sequence number isn't documented and the
> definitions of its special values are scattered. Try to gather some
> common knowledge near the GP seq headers.
> 
> Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
> ---
>  kernel/rcu/rcu.h | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/kernel/rcu/rcu.h b/kernel/rcu/rcu.h
> index 115616ac3bfa..fb95de039596 100644
> --- a/kernel/rcu/rcu.h
> +++ b/kernel/rcu/rcu.h
> @@ -14,6 +14,39 @@
>  
>  /*
>   * Grace-period counter management.
> + *
> + * The two lowest significant bits gather the control flags.
> + * The higher bits form the RCU sequence counter.
> + *
> + * About the control flags, a common value of 0 means that no GP is in progress.
> + * A value of 1 means that a grace period has started and is in progress. When
> + * the grace period completes, the control flags are reset to 0 and the sequence
> + * counter is incremented.
> + *
> + * However some specific RCU usages make use of custom values.
> + *
> + * SRCU special control values:
> + *
> + * 	SRCU_SNP_INIT_SEQ	:	Invalid/init value set when SRCU node
> + * 					is initialized.
> + *
> + * 	SRCU_STATE_IDLE		:	No SRCU gp is in progress
> + *
> + *	SRCU_STATE_SCAN1	:	State set by rcu_seq_start(). Indicates
> + *					we are scanning the inactive readers
> + *					index.
> + *
> + *	SRCU_STATE_SCAN2	:	State set manually via rcu_seq_set_state()
> + *					Indicates we are flipping the readers
> + *					index and then scanning the newly inactive
> + *					readers index.
> + *
> + * RCU polled GP special control value:
> + *
> + *	RCU_GET_STATE_COMPLETED :	State value indicating that a polled GP
> + *					has completed. It's an absolute value
> + *					covering both the state and the counter of
> + *					the GP sequence.
>   */

Other than the minor comments in the other thread:

Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org>

thanks,

 - Joel


>  
>  #define RCU_SEQ_CTR_SHIFT	2
> -- 
> 2.34.1
>
diff mbox series

Patch

diff --git a/kernel/rcu/rcu.h b/kernel/rcu/rcu.h
index 115616ac3bfa..fb95de039596 100644
--- a/kernel/rcu/rcu.h
+++ b/kernel/rcu/rcu.h
@@ -14,6 +14,39 @@ 
 
 /*
  * Grace-period counter management.
+ *
+ * The two lowest significant bits gather the control flags.
+ * The higher bits form the RCU sequence counter.
+ *
+ * About the control flags, a common value of 0 means that no GP is in progress.
+ * A value of 1 means that a grace period has started and is in progress. When
+ * the grace period completes, the control flags are reset to 0 and the sequence
+ * counter is incremented.
+ *
+ * However some specific RCU usages make use of custom values.
+ *
+ * SRCU special control values:
+ *
+ * 	SRCU_SNP_INIT_SEQ	:	Invalid/init value set when SRCU node
+ * 							is initialized.
+ *
+ * 	SRCU_STATE_IDLE		:	No SRCU gp is in progress
+ *
+ * 	SRCU_STATE_SCAN1	:	State set by rcu_seq_start(). Indicates
+ *								we are scanning the inactive readers
+ *								index.
+ *
+ *		SRCU_STATE_SCAN2	:	State set manually via rcu_seq_set_state()
+ *								Indicates we are flipping the readers
+ *								index and then scanning the newly inactive
+ *								readers index.
+ *
+ * RCU polled GP special control value:
+ *
+ *	RCU_GET_STATE_COMPLETED :	State value indicating that a polled GP
+ *								has completed. It's an absolute value
+ *								covering both the state and the counter of
+ *								the GP sequence.
  */
 
 #define RCU_SEQ_CTR_SHIFT	2