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@xxxxxxxxx>
> >> 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@xxxxxxxxx>
> >
> > 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@xxxxxxxxx}
> >> \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
> >>
> >
>
