[PATCH] count: Employ new scheme for snippet of count_stat_eventual.c

[PATCH] count: Employ new scheme for snippet of count_stat_eventual.c

[Akira Yokosawa]

From 16fb7e39700268cd498f88c16b34170e3b91fd24 Mon Sep 17 00:00:00 2001
From: Akira Yokosawa <akiyks@gmail.com>
Date: Thu, 6 Oct 2018 07:20:06 +0900
Subject: [PATCH] count: Employ new scheme for snippet of count_stat_eventual.c

Also omit READ_ONCE()s which access variables owned and modified
only by the owning thread.

Read part of stopflag's increment in eventual() doesn't need
READ_ONCE() because once the "if" statement stands, stopflag
won't be modified by any other thread.

Also modify code around pthread_create() in count_init() so that
the "if" statement won't be too wide for a code snippet in two-
column layout.

Signed-off-by: Akira Yokosawa <akiyks@gmail.com>
---
Paul,

I might be missing something in omitting READ_ONCE() of stopflag's
increment in eventual().
If this were kernel code, we wouldn't be able to make sure
that eventual() is the only updater after the particular point in
execution.

Thoughts?

        Thanks, Akira
--

 CodeSamples/count/count_stat_eventual.c | 38 +++++++++-------
 count/count.tex                         | 81 +++++++--------------------------
 2 files changed, 37 insertions(+), 82 deletions(-)

diff --git a/CodeSamples/count/count_stat_eventual.c b/CodeSamples/count/count_stat_eventual.c
index 324bc24..fa8972f 100644
--- a/CodeSamples/count/count_stat_eventual.c
+++ b/CodeSamples/count/count_stat_eventual.c
@@ -21,22 +21,24 @@
 
 #include "../api.h"
 
-DEFINE_PER_THREAD(unsigned long, counter);
-unsigned long global_count;
-int stopflag;
+//\begin{snippet}[labelbase=ln:count:count_stat_eventual:whole,commandchars=\$\[\]]
+DEFINE_PER_THREAD(unsigned long, counter);		//\lnlbl{per_thr_cnt}
+unsigned long global_count;				//\lnlbl{glb_cnt}
+int stopflag;						//\lnlbl{stopflag}
 
-void inc_count(void)
+static __inline__ void inc_count(void)			//\lnlbl{inc:b}
 {
-	WRITE_ONCE(__get_thread_var(counter),
-		   READ_ONCE(__get_thread_var(counter)) + 1);
-}
+	unsigned long *p_counter = &__get_thread_var(counter);
 
-__inline__ unsigned long read_count(void)
+	WRITE_ONCE(*p_counter, *p_counter + 1);
+}							//\lnlbl{inc:e}
+
+static __inline__ unsigned long read_count(void)	//\lnlbl{read:b}
 {
 	return READ_ONCE(global_count);
-}
+}							//\lnlbl{read:e}
 
-void *eventual(void *arg)
+void *eventual(void *arg)				//\lnlbl{eventual:b}
 {
 	int t;
 	unsigned long sum;
@@ -49,29 +51,31 @@ void *eventual(void *arg)
 		poll(NULL, 0, 1);
 		if (READ_ONCE(stopflag)) {
 			smp_mb();
-			WRITE_ONCE(stopflag, READ_ONCE(stopflag) + 1);
+			WRITE_ONCE(stopflag, stopflag + 1);
 		}
 	}
 	return NULL;
-}
+}							//\lnlbl{eventual:e}
 
-void count_init(void)
+void count_init(void)					//\lnlbl{init:b}
 {
 	int en;
 	thread_id_t tid;
 
-	if ((en = pthread_create(&tid, NULL, eventual, NULL)) != 0) {
+	en = pthread_create(&tid, NULL, eventual, NULL);
+	if (en != 0) {
 		fprintf(stderr, "pthread_create: %s\n", strerror(en));
 		exit(EXIT_FAILURE);
 	}
-}
+}							//\lnlbl{init:e}
 
-void count_cleanup(void)
+void count_cleanup(void)				//\lnlbl{cleanup:b}
 {
 	WRITE_ONCE(stopflag, 1);
 	while (READ_ONCE(stopflag) < 3)
 		poll(NULL, 0, 1);
 	smp_mb();
-}
+}							//\lnlbl{cleanup:e}
+//\end{snippet}
 
 #include "counttorture.h"
diff --git a/count/count.tex b/count/count.tex
index c36e7d8..aea7b93 100644
--- a/count/count.tex
+++ b/count/count.tex
@@ -785,94 +785,45 @@ converge on the true value---hence this approach qualifies as
 eventually consistent.
 
 \begin{listing}[tbp]
-{ \scriptsize
-\begin{verbbox}
- 1 DEFINE_PER_THREAD(unsigned long, counter);
- 2 unsigned long global_count;
- 3 int stopflag;
- 4
- 5 void inc_count(void)
- 6 {
- 7   WRITE_ONCE(__get_thread_var(counter),
- 8              READ_ONCE(__get_thread_var(counter)) + 1);
- 9 }
-10
-11 unsigned long read_count(void)
-12 {
-13   return READ_ONCE(global_count);
-14 }
-15
-16 void *eventual(void *arg)
-17 {
-18   int t;
-19   unsigned long sum;
-20
-21   while (READ_ONCE(stopflag) < 3) {
-22     sum = 0;
-23     for_each_thread(t)
-24       sum += READ_ONCE(per_thread(counter, t));
-25     WRITE_ONCE(global_count, sum);
-26     poll(NULL, 0, 1);
-27     if (READ_ONCE(stopflag)) {
-28       smp_mb();
-29       WRITE_ONCE(stopflag, READ_ONCE(stopflag) + 1);
-30     }
-31   }
-32   return NULL;
-33 }
-34
-35 void count_init(void)
-36 {
-37   int en;
-38   thread_id_t tid;
-39
-40   if ((en = pthread_create(&tid, NULL, eventual, NULL)) != 0) {
-41     fprintf(stderr, "pthread_create: %s\n", strerror(en));
-42     exit(-1);
-43   }
-44 }
-45
-46 void count_cleanup(void)
-47 {
-48   WRITE_ONCE(stopflag, 1);
-49   while (READ_ONCE(stopflag) < 3)
-50     poll(NULL, 0, 1);
-51   smp_mb();
-52 }
-\end{verbbox}
-}
-\centering
-\theverbbox
+\input{CodeSamples/count/count_stat_eventual@whole.fcv}
 \caption{Array-Based Per-Thread Eventually Consistent Counters}
 \label{lst:count:Array-Based Per-Thread Eventually Consistent Counters}
 \end{listing}
 
+\begin{lineref}[ln:count:count_stat_eventual:whole]
 The implementation is shown in
 Listing~\ref{lst:count:Array-Based Per-Thread Eventually Consistent Counters}
 (\path{count_stat_eventual.c}).
-Lines~1-2 show the per-thread variable and the global variable that
-track the counter's value, and line three shows \co{stopflag}
+Lines~\lnref{per_thr_cnt}-\lnref{glb_cnt}
+show the per-thread variable and the global variable that
+track the counter's value, and line~\lnref{stopflag} shows \co{stopflag}
 which is used to coordinate termination (for the case where we want
 to terminate the program with an accurate counter value).
-The \co{inc_count()} function shown on lines~5-9 is similar to its
+The \co{inc_count()} function shown on
+lines~\lnref{inc:b}-\lnref{inc:e} is similar to its
 counterpart in
 Listing~\ref{lst:count:Array-Based Per-Thread Statistical Counters}.
-The \co{read_count()} function shown on lines~11-14 simply returns the
+The \co{read_count()} function shown on
+lines~\lnref{read:b}-\lnref{read:e} simply returns the
 value of the \co{global_count} variable.
 
-However, the \co{count_init()} function on lines~35-44
-creates the \co{eventual()} thread shown on lines~16-33, which
+However, the \co{count_init()} function on
+lines~\lnref{init:b}-\lnref{init:e}
+creates the \co{eventual()} thread shown on
+lines~\lnref{eventual:b}-\lnref{eventual:e}, which
 cycles through all the threads,
 summing the per-thread local \co{counter} and storing the
 sum to the \co{global_count} variable.
 The \co{eventual()} thread waits an arbitrarily chosen one millisecond
 between passes.
-The \co{count_cleanup()} function on lines~46-52 coordinates termination.
+The \co{count_cleanup()} function on
+lines~\lnref{cleanup:b}-\lnref{cleanup:e} coordinates termination.
 
 This approach gives extremely fast counter read-out while still
 supporting linear counter-update performance.
 However, this excellent read-side performance and update-side scalability
 comes at the cost of the additional thread running \co{eventual()}.
+\end{lineref}
 
 \QuickQuiz{}
 	Why doesn't \co{inc_count()} in
-- 
2.7.4

Re: [PATCH] count: Employ new scheme for snippet of count_stat_eventual.c

[Paul E. McKenney]

On Sat, Oct 06, 2018 at 07:35:42AM +0900, Akira Yokosawa wrote:
> >From 16fb7e39700268cd498f88c16b34170e3b91fd24 Mon Sep 17 00:00:00 2001
> From: Akira Yokosawa <akiyks@gmail.com>
> Date: Thu, 6 Oct 2018 07:20:06 +0900
> Subject: [PATCH] count: Employ new scheme for snippet of count_stat_eventual.c
> 
> Also omit READ_ONCE()s which access variables owned and modified
> only by the owning thread.
> 
> Read part of stopflag's increment in eventual() doesn't need
> READ_ONCE() because once the "if" statement stands, stopflag
> won't be modified by any other thread.
> 
> Also modify code around pthread_create() in count_init() so that
> the "if" statement won't be too wide for a code snippet in two-
> column layout.
> 
> Signed-off-by: Akira Yokosawa <akiyks@gmail.com>

Queued and pushed, thank you!

> ---
> Paul,
> 
> I might be missing something in omitting READ_ONCE() of stopflag's
> increment in eventual().
> If this were kernel code, we wouldn't be able to make sure
> that eventual() is the only updater after the particular point in
> execution.
> 
> Thoughts?

True enough!  If a given entity is the only thing that updates a
given variable, then that entity (but only that entity) need not use
READ_ONCE().

Hmmm... This applies to Figure 5.1 also, right?

							Thanx, Paul

>         Thanks, Akira
> --
> 
>  CodeSamples/count/count_stat_eventual.c | 38 +++++++++-------
>  count/count.tex                         | 81 +++++++--------------------------
>  2 files changed, 37 insertions(+), 82 deletions(-)
> 
> diff --git a/CodeSamples/count/count_stat_eventual.c b/CodeSamples/count/count_stat_eventual.c
> index 324bc24..fa8972f 100644
> --- a/CodeSamples/count/count_stat_eventual.c
> +++ b/CodeSamples/count/count_stat_eventual.c
> @@ -21,22 +21,24 @@
>  
>  #include "../api.h"
>  
> -DEFINE_PER_THREAD(unsigned long, counter);
> -unsigned long global_count;
> -int stopflag;
> +//\begin{snippet}[labelbase=ln:count:count_stat_eventual:whole,commandchars=\$\[\]]
> +DEFINE_PER_THREAD(unsigned long, counter);		//\lnlbl{per_thr_cnt}
> +unsigned long global_count;				//\lnlbl{glb_cnt}
> +int stopflag;						//\lnlbl{stopflag}
>  
> -void inc_count(void)
> +static __inline__ void inc_count(void)			//\lnlbl{inc:b}
>  {
> -	WRITE_ONCE(__get_thread_var(counter),
> -		   READ_ONCE(__get_thread_var(counter)) + 1);
> -}
> +	unsigned long *p_counter = &__get_thread_var(counter);
>  
> -__inline__ unsigned long read_count(void)
> +	WRITE_ONCE(*p_counter, *p_counter + 1);
> +}							//\lnlbl{inc:e}
> +
> +static __inline__ unsigned long read_count(void)	//\lnlbl{read:b}
>  {
>  	return READ_ONCE(global_count);
> -}
> +}							//\lnlbl{read:e}
>  
> -void *eventual(void *arg)
> +void *eventual(void *arg)				//\lnlbl{eventual:b}
>  {
>  	int t;
>  	unsigned long sum;
> @@ -49,29 +51,31 @@ void *eventual(void *arg)
>  		poll(NULL, 0, 1);
>  		if (READ_ONCE(stopflag)) {
>  			smp_mb();
> -			WRITE_ONCE(stopflag, READ_ONCE(stopflag) + 1);
> +			WRITE_ONCE(stopflag, stopflag + 1);
>  		}
>  	}
>  	return NULL;
> -}
> +}							//\lnlbl{eventual:e}
>  
> -void count_init(void)
> +void count_init(void)					//\lnlbl{init:b}
>  {
>  	int en;
>  	thread_id_t tid;
>  
> -	if ((en = pthread_create(&tid, NULL, eventual, NULL)) != 0) {
> +	en = pthread_create(&tid, NULL, eventual, NULL);
> +	if (en != 0) {
>  		fprintf(stderr, "pthread_create: %s\n", strerror(en));
>  		exit(EXIT_FAILURE);
>  	}
> -}
> +}							//\lnlbl{init:e}
>  
> -void count_cleanup(void)
> +void count_cleanup(void)				//\lnlbl{cleanup:b}
>  {
>  	WRITE_ONCE(stopflag, 1);
>  	while (READ_ONCE(stopflag) < 3)
>  		poll(NULL, 0, 1);
>  	smp_mb();
> -}
> +}							//\lnlbl{cleanup:e}
> +//\end{snippet}
>  
>  #include "counttorture.h"
> diff --git a/count/count.tex b/count/count.tex
> index c36e7d8..aea7b93 100644
> --- a/count/count.tex
> +++ b/count/count.tex
> @@ -785,94 +785,45 @@ converge on the true value---hence this approach qualifies as
>  eventually consistent.
>  
>  \begin{listing}[tbp]
> -{ \scriptsize
> -\begin{verbbox}
> - 1 DEFINE_PER_THREAD(unsigned long, counter);
> - 2 unsigned long global_count;
> - 3 int stopflag;
> - 4
> - 5 void inc_count(void)
> - 6 {
> - 7   WRITE_ONCE(__get_thread_var(counter),
> - 8              READ_ONCE(__get_thread_var(counter)) + 1);
> - 9 }
> -10
> -11 unsigned long read_count(void)
> -12 {
> -13   return READ_ONCE(global_count);
> -14 }
> -15
> -16 void *eventual(void *arg)
> -17 {
> -18   int t;
> -19   unsigned long sum;
> -20
> -21   while (READ_ONCE(stopflag) < 3) {
> -22     sum = 0;
> -23     for_each_thread(t)
> -24       sum += READ_ONCE(per_thread(counter, t));
> -25     WRITE_ONCE(global_count, sum);
> -26     poll(NULL, 0, 1);
> -27     if (READ_ONCE(stopflag)) {
> -28       smp_mb();
> -29       WRITE_ONCE(stopflag, READ_ONCE(stopflag) + 1);
> -30     }
> -31   }
> -32   return NULL;
> -33 }
> -34
> -35 void count_init(void)
> -36 {
> -37   int en;
> -38   thread_id_t tid;
> -39
> -40   if ((en = pthread_create(&tid, NULL, eventual, NULL)) != 0) {
> -41     fprintf(stderr, "pthread_create: %s\n", strerror(en));
> -42     exit(-1);
> -43   }
> -44 }
> -45
> -46 void count_cleanup(void)
> -47 {
> -48   WRITE_ONCE(stopflag, 1);
> -49   while (READ_ONCE(stopflag) < 3)
> -50     poll(NULL, 0, 1);
> -51   smp_mb();
> -52 }
> -\end{verbbox}
> -}
> -\centering
> -\theverbbox
> +\input{CodeSamples/count/count_stat_eventual@whole.fcv}
>  \caption{Array-Based Per-Thread Eventually Consistent Counters}
>  \label{lst:count:Array-Based Per-Thread Eventually Consistent Counters}
>  \end{listing}
>  
> +\begin{lineref}[ln:count:count_stat_eventual:whole]
>  The implementation is shown in
>  Listing~\ref{lst:count:Array-Based Per-Thread Eventually Consistent Counters}
>  (\path{count_stat_eventual.c}).
> -Lines~1-2 show the per-thread variable and the global variable that
> -track the counter's value, and line three shows \co{stopflag}
> +Lines~\lnref{per_thr_cnt}-\lnref{glb_cnt}
> +show the per-thread variable and the global variable that
> +track the counter's value, and line~\lnref{stopflag} shows \co{stopflag}
>  which is used to coordinate termination (for the case where we want
>  to terminate the program with an accurate counter value).
> -The \co{inc_count()} function shown on lines~5-9 is similar to its
> +The \co{inc_count()} function shown on
> +lines~\lnref{inc:b}-\lnref{inc:e} is similar to its
>  counterpart in
>  Listing~\ref{lst:count:Array-Based Per-Thread Statistical Counters}.
> -The \co{read_count()} function shown on lines~11-14 simply returns the
> +The \co{read_count()} function shown on
> +lines~\lnref{read:b}-\lnref{read:e} simply returns the
>  value of the \co{global_count} variable.
>  
> -However, the \co{count_init()} function on lines~35-44
> -creates the \co{eventual()} thread shown on lines~16-33, which
> +However, the \co{count_init()} function on
> +lines~\lnref{init:b}-\lnref{init:e}
> +creates the \co{eventual()} thread shown on
> +lines~\lnref{eventual:b}-\lnref{eventual:e}, which
>  cycles through all the threads,
>  summing the per-thread local \co{counter} and storing the
>  sum to the \co{global_count} variable.
>  The \co{eventual()} thread waits an arbitrarily chosen one millisecond
>  between passes.
> -The \co{count_cleanup()} function on lines~46-52 coordinates termination.
> +The \co{count_cleanup()} function on
> +lines~\lnref{cleanup:b}-\lnref{cleanup:e} coordinates termination.
>  
>  This approach gives extremely fast counter read-out while still
>  supporting linear counter-update performance.
>  However, this excellent read-side performance and update-side scalability
>  comes at the cost of the additional thread running \co{eventual()}.
> +\end{lineref}
>  
>  \QuickQuiz{}
>  	Why doesn't \co{inc_count()} in
> -- 
> 2.7.4
> 

Re: [PATCH] count: Employ new scheme for snippet of count_stat_eventual.c

[Akira Yokosawa]

On 2018/10/06 13:16:06 -0700, Paul E. McKenney wrote:
> On Sat, Oct 06, 2018 at 07:35:42AM +0900, Akira Yokosawa wrote:
>> >From 16fb7e39700268cd498f88c16b34170e3b91fd24 Mon Sep 17 00:00:00 2001
>> From: Akira Yokosawa <akiyks@gmail.com>
>> Date: Thu, 6 Oct 2018 07:20:06 +0900
>> Subject: [PATCH] count: Employ new scheme for snippet of count_stat_eventual.c
>>
>> Also omit READ_ONCE()s which access variables owned and modified
>> only by the owning thread.
>>
>> Read part of stopflag's increment in eventual() doesn't need
>> READ_ONCE() because once the "if" statement stands, stopflag
>> won't be modified by any other thread.
>>
>> Also modify code around pthread_create() in count_init() so that
>> the "if" statement won't be too wide for a code snippet in two-
>> column layout.
>>
>> Signed-off-by: Akira Yokosawa <akiyks@gmail.com>
> 
> Queued and pushed, thank you!
> 
>> ---
>> Paul,
>>
>> I might be missing something in omitting READ_ONCE() of stopflag's
>> increment in eventual().
>> If this were kernel code, we wouldn't be able to make sure
>> that eventual() is the only updater after the particular point in
>> execution.
>>
>> Thoughts?
> 
> True enough!  If a given entity is the only thing that updates a
> given variable, then that entity (but only that entity) need not use
> READ_ONCE().

Now, I'm beginning to feel somewhat confident to tell the necessity
of READ_ONCE()/WRITE_ONCE(). ;-)

> 
> Hmmm... This applies to Figure 5.1 also, right?

You mean Listing 5.1?

I don't think so. inc_count() and read_count() can be called from
multiple threads and any updater thread modifies "counter". 

Listing 5.1 is not supposed to be "correct", but I think the rule of
where to use READ_ONCE()/WRITE_ONCE() still applies to it.
READ_ONCE() in read_count() avoids load tearing, doesn't it?

        Thanks, Akira

> 
> 							Thanx, Paul
> 
>>         Thanks, Akira
>> --
>>
>>  CodeSamples/count/count_stat_eventual.c | 38 +++++++++-------
>>  count/count.tex                         | 81 +++++++--------------------------
>>  2 files changed, 37 insertions(+), 82 deletions(-)
>>
>> diff --git a/CodeSamples/count/count_stat_eventual.c b/CodeSamples/count/count_stat_eventual.c
>> index 324bc24..fa8972f 100644
>> --- a/CodeSamples/count/count_stat_eventual.c
>> +++ b/CodeSamples/count/count_stat_eventual.c
>> @@ -21,22 +21,24 @@
>>  
>>  #include "../api.h"
>>  
>> -DEFINE_PER_THREAD(unsigned long, counter);
>> -unsigned long global_count;
>> -int stopflag;
>> +//\begin{snippet}[labelbase=ln:count:count_stat_eventual:whole,commandchars=\$\[\]]
>> +DEFINE_PER_THREAD(unsigned long, counter);		//\lnlbl{per_thr_cnt}
>> +unsigned long global_count;				//\lnlbl{glb_cnt}
>> +int stopflag;						//\lnlbl{stopflag}
>>  
>> -void inc_count(void)
>> +static __inline__ void inc_count(void)			//\lnlbl{inc:b}
>>  {
>> -	WRITE_ONCE(__get_thread_var(counter),
>> -		   READ_ONCE(__get_thread_var(counter)) + 1);
>> -}
>> +	unsigned long *p_counter = &__get_thread_var(counter);
>>  
>> -__inline__ unsigned long read_count(void)
>> +	WRITE_ONCE(*p_counter, *p_counter + 1);
>> +}							//\lnlbl{inc:e}
>> +
>> +static __inline__ unsigned long read_count(void)	//\lnlbl{read:b}
>>  {
>>  	return READ_ONCE(global_count);
>> -}
>> +}							//\lnlbl{read:e}
>>  
>> -void *eventual(void *arg)
>> +void *eventual(void *arg)				//\lnlbl{eventual:b}
>>  {
>>  	int t;
>>  	unsigned long sum;
>> @@ -49,29 +51,31 @@ void *eventual(void *arg)
>>  		poll(NULL, 0, 1);
>>  		if (READ_ONCE(stopflag)) {
>>  			smp_mb();
>> -			WRITE_ONCE(stopflag, READ_ONCE(stopflag) + 1);
>> +			WRITE_ONCE(stopflag, stopflag + 1);
>>  		}
>>  	}
>>  	return NULL;
>> -}
>> +}							//\lnlbl{eventual:e}
>>  
>> -void count_init(void)
>> +void count_init(void)					//\lnlbl{init:b}
>>  {
>>  	int en;
>>  	thread_id_t tid;
>>  
>> -	if ((en = pthread_create(&tid, NULL, eventual, NULL)) != 0) {
>> +	en = pthread_create(&tid, NULL, eventual, NULL);
>> +	if (en != 0) {
>>  		fprintf(stderr, "pthread_create: %s\n", strerror(en));
>>  		exit(EXIT_FAILURE);
>>  	}
>> -}
>> +}							//\lnlbl{init:e}
>>  
>> -void count_cleanup(void)
>> +void count_cleanup(void)				//\lnlbl{cleanup:b}
>>  {
>>  	WRITE_ONCE(stopflag, 1);
>>  	while (READ_ONCE(stopflag) < 3)
>>  		poll(NULL, 0, 1);
>>  	smp_mb();
>> -}
>> +}							//\lnlbl{cleanup:e}
>> +//\end{snippet}
>>  
>>  #include "counttorture.h"
>> diff --git a/count/count.tex b/count/count.tex
>> index c36e7d8..aea7b93 100644
>> --- a/count/count.tex
>> +++ b/count/count.tex
>> @@ -785,94 +785,45 @@ converge on the true value---hence this approach qualifies as
>>  eventually consistent.
>>  
>>  \begin{listing}[tbp]
>> -{ \scriptsize
>> -\begin{verbbox}
>> - 1 DEFINE_PER_THREAD(unsigned long, counter);
>> - 2 unsigned long global_count;
>> - 3 int stopflag;
>> - 4
>> - 5 void inc_count(void)
>> - 6 {
>> - 7   WRITE_ONCE(__get_thread_var(counter),
>> - 8              READ_ONCE(__get_thread_var(counter)) + 1);
>> - 9 }
>> -10
>> -11 unsigned long read_count(void)
>> -12 {
>> -13   return READ_ONCE(global_count);
>> -14 }
>> -15
>> -16 void *eventual(void *arg)
>> -17 {
>> -18   int t;
>> -19   unsigned long sum;
>> -20
>> -21   while (READ_ONCE(stopflag) < 3) {
>> -22     sum = 0;
>> -23     for_each_thread(t)
>> -24       sum += READ_ONCE(per_thread(counter, t));
>> -25     WRITE_ONCE(global_count, sum);
>> -26     poll(NULL, 0, 1);
>> -27     if (READ_ONCE(stopflag)) {
>> -28       smp_mb();
>> -29       WRITE_ONCE(stopflag, READ_ONCE(stopflag) + 1);
>> -30     }
>> -31   }
>> -32   return NULL;
>> -33 }
>> -34
>> -35 void count_init(void)
>> -36 {
>> -37   int en;
>> -38   thread_id_t tid;
>> -39
>> -40   if ((en = pthread_create(&tid, NULL, eventual, NULL)) != 0) {
>> -41     fprintf(stderr, "pthread_create: %s\n", strerror(en));
>> -42     exit(-1);
>> -43   }
>> -44 }
>> -45
>> -46 void count_cleanup(void)
>> -47 {
>> -48   WRITE_ONCE(stopflag, 1);
>> -49   while (READ_ONCE(stopflag) < 3)
>> -50     poll(NULL, 0, 1);
>> -51   smp_mb();
>> -52 }
>> -\end{verbbox}
>> -}
>> -\centering
>> -\theverbbox
>> +\input{CodeSamples/count/count_stat_eventual@whole.fcv}
>>  \caption{Array-Based Per-Thread Eventually Consistent Counters}
>>  \label{lst:count:Array-Based Per-Thread Eventually Consistent Counters}
>>  \end{listing}
>>  
>> +\begin{lineref}[ln:count:count_stat_eventual:whole]
>>  The implementation is shown in
>>  Listing~\ref{lst:count:Array-Based Per-Thread Eventually Consistent Counters}
>>  (\path{count_stat_eventual.c}).
>> -Lines~1-2 show the per-thread variable and the global variable that
>> -track the counter's value, and line three shows \co{stopflag}
>> +Lines~\lnref{per_thr_cnt}-\lnref{glb_cnt}
>> +show the per-thread variable and the global variable that
>> +track the counter's value, and line~\lnref{stopflag} shows \co{stopflag}
>>  which is used to coordinate termination (for the case where we want
>>  to terminate the program with an accurate counter value).
>> -The \co{inc_count()} function shown on lines~5-9 is similar to its
>> +The \co{inc_count()} function shown on
>> +lines~\lnref{inc:b}-\lnref{inc:e} is similar to its
>>  counterpart in
>>  Listing~\ref{lst:count:Array-Based Per-Thread Statistical Counters}.
>> -The \co{read_count()} function shown on lines~11-14 simply returns the
>> +The \co{read_count()} function shown on
>> +lines~\lnref{read:b}-\lnref{read:e} simply returns the
>>  value of the \co{global_count} variable.
>>  
>> -However, the \co{count_init()} function on lines~35-44
>> -creates the \co{eventual()} thread shown on lines~16-33, which
>> +However, the \co{count_init()} function on
>> +lines~\lnref{init:b}-\lnref{init:e}
>> +creates the \co{eventual()} thread shown on
>> +lines~\lnref{eventual:b}-\lnref{eventual:e}, which
>>  cycles through all the threads,
>>  summing the per-thread local \co{counter} and storing the
>>  sum to the \co{global_count} variable.
>>  The \co{eventual()} thread waits an arbitrarily chosen one millisecond
>>  between passes.
>> -The \co{count_cleanup()} function on lines~46-52 coordinates termination.
>> +The \co{count_cleanup()} function on
>> +lines~\lnref{cleanup:b}-\lnref{cleanup:e} coordinates termination.
>>  
>>  This approach gives extremely fast counter read-out while still
>>  supporting linear counter-update performance.
>>  However, this excellent read-side performance and update-side scalability
>>  comes at the cost of the additional thread running \co{eventual()}.
>> +\end{lineref}
>>  
>>  \QuickQuiz{}
>>  	Why doesn't \co{inc_count()} in
>> -- 
>> 2.7.4
>>
> 

Re: [PATCH] count: Employ new scheme for snippet of count_stat_eventual.c

[Paul E. McKenney]

On Sun, Oct 07, 2018 at 07:22:50AM +0900, Akira Yokosawa wrote:
> On 2018/10/06 13:16:06 -0700, Paul E. McKenney wrote:
> > On Sat, Oct 06, 2018 at 07:35:42AM +0900, Akira Yokosawa wrote:
> >> >From 16fb7e39700268cd498f88c16b34170e3b91fd24 Mon Sep 17 00:00:00 2001
> >> From: Akira Yokosawa <akiyks@gmail.com>
> >> Date: Thu, 6 Oct 2018 07:20:06 +0900
> >> Subject: [PATCH] count: Employ new scheme for snippet of count_stat_eventual.c
> >>
> >> Also omit READ_ONCE()s which access variables owned and modified
> >> only by the owning thread.
> >>
> >> Read part of stopflag's increment in eventual() doesn't need
> >> READ_ONCE() because once the "if" statement stands, stopflag
> >> won't be modified by any other thread.
> >>
> >> Also modify code around pthread_create() in count_init() so that
> >> the "if" statement won't be too wide for a code snippet in two-
> >> column layout.
> >>
> >> Signed-off-by: Akira Yokosawa <akiyks@gmail.com>
> > 
> > Queued and pushed, thank you!
> > 
> >> ---
> >> Paul,
> >>
> >> I might be missing something in omitting READ_ONCE() of stopflag's
> >> increment in eventual().
> >> If this were kernel code, we wouldn't be able to make sure
> >> that eventual() is the only updater after the particular point in
> >> execution.
> >>
> >> Thoughts?
> > 
> > True enough!  If a given entity is the only thing that updates a
> > given variable, then that entity (but only that entity) need not use
> > READ_ONCE().
> 
> Now, I'm beginning to feel somewhat confident to tell the necessity
> of READ_ONCE()/WRITE_ONCE(). ;-)
> 
> > 
> > Hmmm... This applies to Figure 5.1 also, right?
> 
> You mean Listing 5.1?
> 
> I don't think so. inc_count() and read_count() can be called from
> multiple threads and any updater thread modifies "counter". 
> 
> Listing 5.1 is not supposed to be "correct", but I think the rule of
> where to use READ_ONCE()/WRITE_ONCE() still applies to it.
> READ_ONCE() in read_count() avoids load tearing, doesn't it?

You are quite correct!

							Thanx, Paul

>         Thanks, Akira
> 
> > 
> > 							Thanx, Paul
> > 
> >>         Thanks, Akira
> >> --
> >>
> >>  CodeSamples/count/count_stat_eventual.c | 38 +++++++++-------
> >>  count/count.tex                         | 81 +++++++--------------------------
> >>  2 files changed, 37 insertions(+), 82 deletions(-)
> >>
> >> diff --git a/CodeSamples/count/count_stat_eventual.c b/CodeSamples/count/count_stat_eventual.c
> >> index 324bc24..fa8972f 100644
> >> --- a/CodeSamples/count/count_stat_eventual.c
> >> +++ b/CodeSamples/count/count_stat_eventual.c
> >> @@ -21,22 +21,24 @@
> >>  
> >>  #include "../api.h"
> >>  
> >> -DEFINE_PER_THREAD(unsigned long, counter);
> >> -unsigned long global_count;
> >> -int stopflag;
> >> +//\begin{snippet}[labelbase=ln:count:count_stat_eventual:whole,commandchars=\$\[\]]
> >> +DEFINE_PER_THREAD(unsigned long, counter);		//\lnlbl{per_thr_cnt}
> >> +unsigned long global_count;				//\lnlbl{glb_cnt}
> >> +int stopflag;						//\lnlbl{stopflag}
> >>  
> >> -void inc_count(void)
> >> +static __inline__ void inc_count(void)			//\lnlbl{inc:b}
> >>  {
> >> -	WRITE_ONCE(__get_thread_var(counter),
> >> -		   READ_ONCE(__get_thread_var(counter)) + 1);
> >> -}
> >> +	unsigned long *p_counter = &__get_thread_var(counter);
> >>  
> >> -__inline__ unsigned long read_count(void)
> >> +	WRITE_ONCE(*p_counter, *p_counter + 1);
> >> +}							//\lnlbl{inc:e}
> >> +
> >> +static __inline__ unsigned long read_count(void)	//\lnlbl{read:b}
> >>  {
> >>  	return READ_ONCE(global_count);
> >> -}
> >> +}							//\lnlbl{read:e}
> >>  
> >> -void *eventual(void *arg)
> >> +void *eventual(void *arg)				//\lnlbl{eventual:b}
> >>  {
> >>  	int t;
> >>  	unsigned long sum;
> >> @@ -49,29 +51,31 @@ void *eventual(void *arg)
> >>  		poll(NULL, 0, 1);
> >>  		if (READ_ONCE(stopflag)) {
> >>  			smp_mb();
> >> -			WRITE_ONCE(stopflag, READ_ONCE(stopflag) + 1);
> >> +			WRITE_ONCE(stopflag, stopflag + 1);
> >>  		}
> >>  	}
> >>  	return NULL;
> >> -}
> >> +}							//\lnlbl{eventual:e}
> >>  
> >> -void count_init(void)
> >> +void count_init(void)					//\lnlbl{init:b}
> >>  {
> >>  	int en;
> >>  	thread_id_t tid;
> >>  
> >> -	if ((en = pthread_create(&tid, NULL, eventual, NULL)) != 0) {
> >> +	en = pthread_create(&tid, NULL, eventual, NULL);
> >> +	if (en != 0) {
> >>  		fprintf(stderr, "pthread_create: %s\n", strerror(en));
> >>  		exit(EXIT_FAILURE);
> >>  	}
> >> -}
> >> +}							//\lnlbl{init:e}
> >>  
> >> -void count_cleanup(void)
> >> +void count_cleanup(void)				//\lnlbl{cleanup:b}
> >>  {
> >>  	WRITE_ONCE(stopflag, 1);
> >>  	while (READ_ONCE(stopflag) < 3)
> >>  		poll(NULL, 0, 1);
> >>  	smp_mb();
> >> -}
> >> +}							//\lnlbl{cleanup:e}
> >> +//\end{snippet}
> >>  
> >>  #include "counttorture.h"
> >> diff --git a/count/count.tex b/count/count.tex
> >> index c36e7d8..aea7b93 100644
> >> --- a/count/count.tex
> >> +++ b/count/count.tex
> >> @@ -785,94 +785,45 @@ converge on the true value---hence this approach qualifies as
> >>  eventually consistent.
> >>  
> >>  \begin{listing}[tbp]
> >> -{ \scriptsize
> >> -\begin{verbbox}
> >> - 1 DEFINE_PER_THREAD(unsigned long, counter);
> >> - 2 unsigned long global_count;
> >> - 3 int stopflag;
> >> - 4
> >> - 5 void inc_count(void)
> >> - 6 {
> >> - 7   WRITE_ONCE(__get_thread_var(counter),
> >> - 8              READ_ONCE(__get_thread_var(counter)) + 1);
> >> - 9 }
> >> -10
> >> -11 unsigned long read_count(void)
> >> -12 {
> >> -13   return READ_ONCE(global_count);
> >> -14 }
> >> -15
> >> -16 void *eventual(void *arg)
> >> -17 {
> >> -18   int t;
> >> -19   unsigned long sum;
> >> -20
> >> -21   while (READ_ONCE(stopflag) < 3) {
> >> -22     sum = 0;
> >> -23     for_each_thread(t)
> >> -24       sum += READ_ONCE(per_thread(counter, t));
> >> -25     WRITE_ONCE(global_count, sum);
> >> -26     poll(NULL, 0, 1);
> >> -27     if (READ_ONCE(stopflag)) {
> >> -28       smp_mb();
> >> -29       WRITE_ONCE(stopflag, READ_ONCE(stopflag) + 1);
> >> -30     }
> >> -31   }
> >> -32   return NULL;
> >> -33 }
> >> -34
> >> -35 void count_init(void)
> >> -36 {
> >> -37   int en;
> >> -38   thread_id_t tid;
> >> -39
> >> -40   if ((en = pthread_create(&tid, NULL, eventual, NULL)) != 0) {
> >> -41     fprintf(stderr, "pthread_create: %s\n", strerror(en));
> >> -42     exit(-1);
> >> -43   }
> >> -44 }
> >> -45
> >> -46 void count_cleanup(void)
> >> -47 {
> >> -48   WRITE_ONCE(stopflag, 1);
> >> -49   while (READ_ONCE(stopflag) < 3)
> >> -50     poll(NULL, 0, 1);
> >> -51   smp_mb();
> >> -52 }
> >> -\end{verbbox}
> >> -}
> >> -\centering
> >> -\theverbbox
> >> +\input{CodeSamples/count/count_stat_eventual@whole.fcv}
> >>  \caption{Array-Based Per-Thread Eventually Consistent Counters}
> >>  \label{lst:count:Array-Based Per-Thread Eventually Consistent Counters}
> >>  \end{listing}
> >>  
> >> +\begin{lineref}[ln:count:count_stat_eventual:whole]
> >>  The implementation is shown in
> >>  Listing~\ref{lst:count:Array-Based Per-Thread Eventually Consistent Counters}
> >>  (\path{count_stat_eventual.c}).
> >> -Lines~1-2 show the per-thread variable and the global variable that
> >> -track the counter's value, and line three shows \co{stopflag}
> >> +Lines~\lnref{per_thr_cnt}-\lnref{glb_cnt}
> >> +show the per-thread variable and the global variable that
> >> +track the counter's value, and line~\lnref{stopflag} shows \co{stopflag}
> >>  which is used to coordinate termination (for the case where we want
> >>  to terminate the program with an accurate counter value).
> >> -The \co{inc_count()} function shown on lines~5-9 is similar to its
> >> +The \co{inc_count()} function shown on
> >> +lines~\lnref{inc:b}-\lnref{inc:e} is similar to its
> >>  counterpart in
> >>  Listing~\ref{lst:count:Array-Based Per-Thread Statistical Counters}.
> >> -The \co{read_count()} function shown on lines~11-14 simply returns the
> >> +The \co{read_count()} function shown on
> >> +lines~\lnref{read:b}-\lnref{read:e} simply returns the
> >>  value of the \co{global_count} variable.
> >>  
> >> -However, the \co{count_init()} function on lines~35-44
> >> -creates the \co{eventual()} thread shown on lines~16-33, which
> >> +However, the \co{count_init()} function on
> >> +lines~\lnref{init:b}-\lnref{init:e}
> >> +creates the \co{eventual()} thread shown on
> >> +lines~\lnref{eventual:b}-\lnref{eventual:e}, which
> >>  cycles through all the threads,
> >>  summing the per-thread local \co{counter} and storing the
> >>  sum to the \co{global_count} variable.
> >>  The \co{eventual()} thread waits an arbitrarily chosen one millisecond
> >>  between passes.
> >> -The \co{count_cleanup()} function on lines~46-52 coordinates termination.
> >> +The \co{count_cleanup()} function on
> >> +lines~\lnref{cleanup:b}-\lnref{cleanup:e} coordinates termination.
> >>  
> >>  This approach gives extremely fast counter read-out while still
> >>  supporting linear counter-update performance.
> >>  However, this excellent read-side performance and update-side scalability
> >>  comes at the cost of the additional thread running \co{eventual()}.
> >> +\end{lineref}
> >>  
> >>  \QuickQuiz{}
> >>  	Why doesn't \co{inc_count()} in
> >> -- 
> >> 2.7.4
> >>
> > 
>