* [PATCH 0/3] Patches for recent updates
@ 2016-12-23 8:07 Akira Yokosawa
2016-12-23 8:09 ` [PATCH 1/3] advsync/memorybarriers: Use consistent quotation marks Akira Yokosawa
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Akira Yokosawa @ 2016-12-23 8:07 UTC (permalink / raw)
To: Paul E. McKenney; +Cc: perfbook, Akira Yokosawa
From cbb7797d6b307850a280248589bc09b8852e8a68 Mon Sep 17 00:00:00 2001
From: Akira Yokosawa <akiyks@gmail.com>
Date: Fri, 23 Dec 2016 16:46:06 +0900
Subject: [PATCH 0/3] Patches for recent updates
Hi Paul,
This short patch set consists of fixes to recent updates in
advsync/memorybarriers.
I'm not sure what is your preference of quotation of short variable
names and keywords. In the 1st patch, I chose the form of ~~\co{a}''.
If you prefer other one, please let me know.
Ya, I know a variety of styles are mixed-used in perfbook,
reflecting various original publications...
But at least within a (sub)section, it would be better to keep
consistency.
Thanks, Akira
--
Akira Yokosawa (3):
advsync/memorybarriers: Use consistent quotation marks
advsync/memorybarriers: Fix typo (READ_ONCE -> WRITE_ONCE)
advsync/memorybarriers: Fix trivial typo
advsync/memorybarriers.tex | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/3] advsync/memorybarriers: Use consistent quotation marks 2016-12-23 8:07 [PATCH 0/3] Patches for recent updates Akira Yokosawa @ 2016-12-23 8:09 ` Akira Yokosawa 2016-12-23 8:10 ` [PATCH 2/3] advsync/memorybarriers: Fix typo (READ_ONCE -> WRITE_ONCE) Akira Yokosawa ` (2 subsequent siblings) 3 siblings, 0 replies; 10+ messages in thread From: Akira Yokosawa @ 2016-12-23 8:09 UTC (permalink / raw) To: Paul E. McKenney; +Cc: perfbook, Akira Yokosawa From 3a094a4d803cb8b38edc292dd4233e16d8acfd9a Mon Sep 17 00:00:00 2001 From: Akira Yokosawa <akiyks@gmail.com> Date: Fri, 23 Dec 2016 16:00:05 +0900 Subject: [PATCH 1/3] advsync/memorybarriers: Use consistent quotation marks Recent update of "Data Dependency Barriers" section inherits inconsistent quotation of variable names and C language keywords in memory-barriers.txt. To make them look consistent, this commit uses double quotation marks around \co{} macro for them. It also fixes a typo of atomic64_read(). Signed-off-by: Akira Yokosawa <akiyks@gmail.com> --- advsync/memorybarriers.tex | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/advsync/memorybarriers.tex b/advsync/memorybarriers.tex index 10eafd1..8018f8c 100644 --- a/advsync/memorybarriers.tex +++ b/advsync/memorybarriers.tex @@ -1795,7 +1795,7 @@ Consider the following bit of code: This will not have the desired effect because there is no actual data dependency, but rather a control dependency that the CPU may short-circuit by attempting to predict the outcome in advance, so that other CPUs see -the load from b as having happened before the load from a. +the load from ``\co{b}'' as having happened before the load from ``\co{a}''. In such a case what's actually required is: \vspace{5pt} @@ -1830,13 +1830,13 @@ Control dependencies pair normally with other types of barriers. That said, please note that neither \co{READ_ONCE()} nor \co{WRITE_ONCE()} are optional! Without the \co{READ_ONCE()}, the compiler might combine the load -from `\co{a}' with other loads from `\co{a}'. +from ``\co{a}'' with other loads from ``\co{a}''. Without the \co{WRITE_ONCE()}, the compiler might combine the store to -`\co{b}' with other stores to `\co{b}'. +``\co{b}'' with other stores to ``\co{b}''. Either can result in highly counterintuitive effects on ordering. Worse yet, if the compiler is able to prove (say) that the value of -variable `\co{a}' is always non-zero, it would be well within its rights +variable ``\co{a}'' is always non-zero, it would be well within its rights to optimize the original example by eliminating the ``\co{if}'' statement as follows: @@ -1890,8 +1890,8 @@ optimization levels: \end{minipage} \vspace{5pt} -Now there is no conditional between the load from `\co{a}' and the store to -`\co{b}', which means that the CPU is within its rights to reorder them: +Now there is no conditional between the load from ``\co{a}'' and the store to +``\co{b}'', which means that the CPU is within its rights to reorder them: The conditional is absolutely required, and must be present in the assembly code even after all compiler optimizations have been applied. Therefore, if you need ordering in this example, you need explicit @@ -1914,10 +1914,10 @@ memory barriers, for example, a release store: \vspace{5pt} The initial \co{READ_ONCE()} is still required to prevent the compiler from -proving the value of `\co{a}'. +proving the value of ``\co{a}''. In addition, you need to be careful what you do with the local variable -`\co{q}', +``\co{q}'', otherwise the compiler might be able to guess the value and again remove the needed conditional. For example: @@ -1954,7 +1954,7 @@ transform the above code into the following: \vspace{5pt} Given this transformation, the CPU is not required to respect the ordering -between the load from variable `\co{a}' and the store to variable `\co{b}'. +between the load from variable ``\co{a}'' and the store to variable ``\co{b}''. It is tempting to add a \co{barrier()} to constrain the compiler, but this does not help. The conditional is gone, and the barrier won't bring it back. @@ -1978,7 +1978,7 @@ that \co{MAX} is greater than one, perhaps as follows: \end{minipage} \vspace{5pt} -Please note once again that the stores to `\co{b}' differ. +Please note once again that the stores to ``\co{b}'' differ. If they were identical, as noted earlier, the compiler could pull this store outside of the ``\co{if}'' statement. @@ -2131,7 +2131,7 @@ The following list of rules summarizes the lessons of this section: \co{smp_wmb()}, or, in the case of prior stores and later loads, \co{smp_mb()}. -\item If both legs of the "if" statement begin with identical stores +\item If both legs of the ``\co{if}'' statement begin with identical stores to the same variable, then those stores must be ordered, either by preceding both of them with \co{smp_mb()} or by using \co{smp_store_release()} to carry out the stores. @@ -2151,14 +2151,14 @@ The following list of rules summarizes the lessons of this section: \item Control dependencies require that the compiler avoid reordering the dependency into nonexistence. Careful use of \co{READ_ONCE()}, \co{atomic_read()}, or - \co{atomic,64_read()} can help to preserve your control + \co{atomic64_read()} can help to preserve your control dependency. \item Control dependencies apply only to the ``\co{then}'' and ``\co{else}'' of the ``\co{if}'' containing the control dependency, including any functions that these two clauses call. Control dependencies do \emph{not} apply to code following the - `\co{if}'' containing the control dependency. + ``\co{if}'' containing the control dependency. \item Control dependencies pair normally with other types of barriers. -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/3] advsync/memorybarriers: Fix typo (READ_ONCE -> WRITE_ONCE) 2016-12-23 8:07 [PATCH 0/3] Patches for recent updates Akira Yokosawa 2016-12-23 8:09 ` [PATCH 1/3] advsync/memorybarriers: Use consistent quotation marks Akira Yokosawa @ 2016-12-23 8:10 ` Akira Yokosawa 2016-12-23 8:11 ` [PATCH 3/3] advsync/memorybarriers: Fix trivial typo Akira Yokosawa 2016-12-23 18:58 ` [PATCH 0/3] Patches for recent updates Paul E. McKenney 3 siblings, 0 replies; 10+ messages in thread From: Akira Yokosawa @ 2016-12-23 8:10 UTC (permalink / raw) To: Paul E. McKenney; +Cc: perfbook, Akira Yokosawa From 9cd64784778068743b68110c830547e9dfeb5e9b Mon Sep 17 00:00:00 2001 From: Akira Yokosawa <akiyks@gmail.com> Date: Fri, 23 Dec 2016 16:14:38 +0900 Subject: [PATCH 2/3] advsync/memorybarriers: Fix typo (READ_ONCE -> WRITE_ONCE) Signed-off-by: Akira Yokosawa <akiyks@gmail.com> --- advsync/memorybarriers.tex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/advsync/memorybarriers.tex b/advsync/memorybarriers.tex index 8018f8c..e117c2c 100644 --- a/advsync/memorybarriers.tex +++ b/advsync/memorybarriers.tex @@ -1845,7 +1845,7 @@ as follows: \scriptsize \begin{verbatim} 1 q = READ_ONCE(a); - 2 READ_ONCE(b, 1); /* BUG: CPU can reorder!!! */ + 2 WRITE_ONCE(b, 1); /* BUG: CPU can reorder!!! */ \end{verbatim} \end{minipage} \vspace{5pt} -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/3] advsync/memorybarriers: Fix trivial typo 2016-12-23 8:07 [PATCH 0/3] Patches for recent updates Akira Yokosawa 2016-12-23 8:09 ` [PATCH 1/3] advsync/memorybarriers: Use consistent quotation marks Akira Yokosawa 2016-12-23 8:10 ` [PATCH 2/3] advsync/memorybarriers: Fix typo (READ_ONCE -> WRITE_ONCE) Akira Yokosawa @ 2016-12-23 8:11 ` Akira Yokosawa 2016-12-23 18:58 ` [PATCH 0/3] Patches for recent updates Paul E. McKenney 3 siblings, 0 replies; 10+ messages in thread From: Akira Yokosawa @ 2016-12-23 8:11 UTC (permalink / raw) To: Paul E. McKenney; +Cc: perfbook, Akira Yokosawa From cbb7797d6b307850a280248589bc09b8852e8a68 Mon Sep 17 00:00:00 2001 From: Akira Yokosawa <akiyks@gmail.com> Date: Fri, 23 Dec 2016 16:27:22 +0900 Subject: [PATCH 3/3] advsync/memorybarriers: Fix trivial typo Signed-off-by: Akira Yokosawa <akiyks@gmail.com> --- advsync/memorybarriers.tex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/advsync/memorybarriers.tex b/advsync/memorybarriers.tex index e117c2c..e35e04c 100644 --- a/advsync/memorybarriers.tex +++ b/advsync/memorybarriers.tex @@ -1770,7 +1770,7 @@ for a larger example. \subsubsection{Control Dependencies} \label{sec:advsync:Control Dependencies} -Control dependencies are expecially tricky because current compilers +Control dependencies are especially tricky because current compilers do not understand them. The rules and examples in this section are intended to help you prevent your compiler's ignorance from breaking your code. -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 0/3] Patches for recent updates 2016-12-23 8:07 [PATCH 0/3] Patches for recent updates Akira Yokosawa ` (2 preceding siblings ...) 2016-12-23 8:11 ` [PATCH 3/3] advsync/memorybarriers: Fix trivial typo Akira Yokosawa @ 2016-12-23 18:58 ` Paul E. McKenney 2016-12-23 23:56 ` Akira Yokosawa 3 siblings, 1 reply; 10+ messages in thread From: Paul E. McKenney @ 2016-12-23 18:58 UTC (permalink / raw) To: Akira Yokosawa; +Cc: perfbook On Fri, Dec 23, 2016 at 05:07:17PM +0900, Akira Yokosawa wrote: > >From cbb7797d6b307850a280248589bc09b8852e8a68 Mon Sep 17 00:00:00 2001 > From: Akira Yokosawa <akiyks@gmail.com> > Date: Fri, 23 Dec 2016 16:46:06 +0900 > Subject: [PATCH 0/3] Patches for recent updates > > Hi Paul, > > This short patch set consists of fixes to recent updates in > advsync/memorybarriers. > > I'm not sure what is your preference of quotation of short variable > names and keywords. In the 1st patch, I chose the form of ~~\co{a}''. > If you prefer other one, please let me know. > > Ya, I know a variety of styles are mixed-used in perfbook, > reflecting various original publications... My current rule is that an identifier can be mistaken for an English word or letter of the alphabet, then it should be in double quotation marks and under \co{}, consistent with your changes. However, something like "rcu_node" cannot reasonably be mistaken for English, so it only needs \co{}. Thoughts? > But at least within a (sub)section, it would be better to keep > consistency. Applied and pushed, thank you! Thanx, Paul > Thanks, Akira > -- > Akira Yokosawa (3): > advsync/memorybarriers: Use consistent quotation marks > advsync/memorybarriers: Fix typo (READ_ONCE -> WRITE_ONCE) > advsync/memorybarriers: Fix trivial typo > > advsync/memorybarriers.tex | 30 +++++++++++++++--------------- > 1 file changed, 15 insertions(+), 15 deletions(-) > > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/3] Patches for recent updates 2016-12-23 18:58 ` [PATCH 0/3] Patches for recent updates Paul E. McKenney @ 2016-12-23 23:56 ` Akira Yokosawa 2016-12-24 0:22 ` Paul E. McKenney 0 siblings, 1 reply; 10+ messages in thread From: Akira Yokosawa @ 2016-12-23 23:56 UTC (permalink / raw) To: paulmck; +Cc: perfbook, Akira Yokosawa On 2016/12/23 10:58:42 -0800, Paul E. McKenney wrote: > On Fri, Dec 23, 2016 at 05:07:17PM +0900, Akira Yokosawa wrote: >> >From cbb7797d6b307850a280248589bc09b8852e8a68 Mon Sep 17 00:00:00 2001 >> From: Akira Yokosawa <akiyks@gmail.com> >> Date: Fri, 23 Dec 2016 16:46:06 +0900 >> Subject: [PATCH 0/3] Patches for recent updates >> >> Hi Paul, >> >> This short patch set consists of fixes to recent updates in >> advsync/memorybarriers. >> >> I'm not sure what is your preference of quotation of short variable >> names and keywords. In the 1st patch, I chose the form of ~~\co{a}''. >> If you prefer other one, please let me know. >> >> Ya, I know a variety of styles are mixed-used in perfbook, >> reflecting various original publications... > > My current rule is that an identifier can be mistaken for an English > word or letter of the alphabet, then it should be in double quotation > marks and under \co{}, consistent with your changes. However, something > like "rcu_node" cannot reasonably be mistaken for English, so it only > needs \co{}. > > Thoughts? So, the cause of potential confusion here is the use of variable name "a", isn't it? If "a" were not there, other variable names b, c, q, x, and y would not need quotation marks, I suppose. "if", "then", and "else" still need quotation. Hmm? > >> But at least within a (sub)section, it would be better to keep >> consistency. > > Applied and pushed, thank you! > > Thanx, Paul > >> Thanks, Akira >> -- >> Akira Yokosawa (3): >> advsync/memorybarriers: Use consistent quotation marks >> advsync/memorybarriers: Fix typo (READ_ONCE -> WRITE_ONCE) >> advsync/memorybarriers: Fix trivial typo >> >> advsync/memorybarriers.tex | 30 +++++++++++++++--------------- >> 1 file changed, 15 insertions(+), 15 deletions(-) >> >> -- >> 2.7.4 >> > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/3] Patches for recent updates 2016-12-23 23:56 ` Akira Yokosawa @ 2016-12-24 0:22 ` Paul E. McKenney 2016-12-24 4:21 ` [PATCH 0/2] advsync: Use different set of variable names (was Re: [PATCH 0/3] Patches for recent updates) Akira Yokosawa 0 siblings, 1 reply; 10+ messages in thread From: Paul E. McKenney @ 2016-12-24 0:22 UTC (permalink / raw) To: Akira Yokosawa; +Cc: perfbook On Sat, Dec 24, 2016 at 08:56:27AM +0900, Akira Yokosawa wrote: > On 2016/12/23 10:58:42 -0800, Paul E. McKenney wrote: > > On Fri, Dec 23, 2016 at 05:07:17PM +0900, Akira Yokosawa wrote: > >> >From cbb7797d6b307850a280248589bc09b8852e8a68 Mon Sep 17 00:00:00 2001 > >> From: Akira Yokosawa <akiyks@gmail.com> > >> Date: Fri, 23 Dec 2016 16:46:06 +0900 > >> Subject: [PATCH 0/3] Patches for recent updates > >> > >> Hi Paul, > >> > >> This short patch set consists of fixes to recent updates in > >> advsync/memorybarriers. > >> > >> I'm not sure what is your preference of quotation of short variable > >> names and keywords. In the 1st patch, I chose the form of ~~\co{a}''. > >> If you prefer other one, please let me know. > >> > >> Ya, I know a variety of styles are mixed-used in perfbook, > >> reflecting various original publications... > > > > My current rule is that an identifier can be mistaken for an English > > word or letter of the alphabet, then it should be in double quotation > > marks and under \co{}, consistent with your changes. However, something > > like "rcu_node" cannot reasonably be mistaken for English, so it only > > needs \co{}. > > > > Thoughts? > > So, the cause of potential confusion here is the use of variable name "a", > isn't it? If "a" were not there, other variable names b, c, q, x, and y would > not need quotation marks, I suppose. > "if", "then", and "else" still need quotation. > > Hmm? Indeed, "a" is the main cause of confusion that leads me to believe that single-letter variable names need quotes. Unless the variable names are capitalized, in which case "I" is the bad boy. ;-) Thanx, Paul > >> But at least within a (sub)section, it would be better to keep > >> consistency. > > > > Applied and pushed, thank you! > > > > Thanx, Paul > > > >> Thanks, Akira > >> -- > >> Akira Yokosawa (3): > >> advsync/memorybarriers: Use consistent quotation marks > >> advsync/memorybarriers: Fix typo (READ_ONCE -> WRITE_ONCE) > >> advsync/memorybarriers: Fix trivial typo > >> > >> advsync/memorybarriers.tex | 30 +++++++++++++++--------------- > >> 1 file changed, 15 insertions(+), 15 deletions(-) > >> > >> -- > >> 2.7.4 > >> > > > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 0/2] advsync: Use different set of variable names (was Re: [PATCH 0/3] Patches for recent updates) 2016-12-24 0:22 ` Paul E. McKenney @ 2016-12-24 4:21 ` Akira Yokosawa 2016-12-24 4:24 ` [PATCH 1/2] advsync/memorybarriers: Use non-confusing variable names Akira Yokosawa 2016-12-24 4:25 ` [PATCH 2/2] advsync/memorybarriers: Fix trivial typo Akira Yokosawa 0 siblings, 2 replies; 10+ messages in thread From: Akira Yokosawa @ 2016-12-24 4:21 UTC (permalink / raw) To: Paul E. McKenney; +Cc: perfbook, Akira Yokosawa On 2016/12/23 16:22:17 -0800, Paul E. McKenney wrote: > On Sat, Dec 24, 2016 at 08:56:27AM +0900, Akira Yokosawa wrote: >> On 2016/12/23 10:58:42 -0800, Paul E. McKenney wrote: >>> On Fri, Dec 23, 2016 at 05:07:17PM +0900, Akira Yokosawa wrote: >>>> >From cbb7797d6b307850a280248589bc09b8852e8a68 Mon Sep 17 00:00:00 2001 >>>> From: Akira Yokosawa <akiyks@gmail.com> >>>> Date: Fri, 23 Dec 2016 16:46:06 +0900 >>>> Subject: [PATCH 0/3] Patches for recent updates >>>> >>>> Hi Paul, >>>> >>>> This short patch set consists of fixes to recent updates in >>>> advsync/memorybarriers. >>>> >>>> I'm not sure what is your preference of quotation of short variable >>>> names and keywords. In the 1st patch, I chose the form of ~~\co{a}''. >>>> If you prefer other one, please let me know. >>>> >>>> Ya, I know a variety of styles are mixed-used in perfbook, >>>> reflecting various original publications... >>> >>> My current rule is that an identifier can be mistaken for an English >>> word or letter of the alphabet, then it should be in double quotation >>> marks and under \co{}, consistent with your changes. However, something >>> like "rcu_node" cannot reasonably be mistaken for English, so it only >>> needs \co{}. >>> >>> Thoughts? >> >> So, the cause of potential confusion here is the use of variable name "a", >> isn't it? If "a" were not there, other variable names b, c, q, x, and y would >> not need quotation marks, I suppose. >> "if", "then", and "else" still need quotation. >> >> Hmm? > > Indeed, "a" is the main cause of confusion that leads me to believe that > single-letter variable names need quotes. Unless the variable names > are capitalized, in which case "I" is the bad boy. ;-) Well, "A" at the begging of a line is also confusing. So in this case, we have another option to use a different set of variable names and get rid of quotation marks. For example, the following patch uses x, y, and z instead of "a", "b", and "c". x and y are used in the explanation of transitivity, but they are not confusing to be used in the first half of this section. Thoughts? Thanks, Akira P.S. There was another trivial typo. 2nd patch fixes it within this context. -- Akira Yokosawa (2): advsync/memorybarriers: Use non-confusing variable names advsync/memorybarriers: Fix trivial typo advsync/memorybarriers.tex | 98 +++++++++++++++++++++++----------------------- 1 file changed, 49 insertions(+), 49 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] advsync/memorybarriers: Use non-confusing variable names 2016-12-24 4:21 ` [PATCH 0/2] advsync: Use different set of variable names (was Re: [PATCH 0/3] Patches for recent updates) Akira Yokosawa @ 2016-12-24 4:24 ` Akira Yokosawa 2016-12-24 4:25 ` [PATCH 2/2] advsync/memorybarriers: Fix trivial typo Akira Yokosawa 1 sibling, 0 replies; 10+ messages in thread From: Akira Yokosawa @ 2016-12-24 4:24 UTC (permalink / raw) To: Paul E. McKenney; +Cc: perfbook, Akira Yokosawa From 22918af5495588c6e80e6249c7c57a6e88a18c5c Mon Sep 17 00:00:00 2001 From: Akira Yokosawa <akiyks@gmail.com> Date: Sat, 24 Dec 2016 12:30:35 +0900 Subject: [PATCH 1/2] advsync/memorybarriers: Use non-confusing variable names Variable name of "a" requires quotation marks to avoid confusion. By using other single letter names, we can remove quotation marks around them. This commit uses x, y, and z instead of "a", "b", and "c". Signed-off-by: Akira Yokosawa <akiyks@gmail.com> --- advsync/memorybarriers.tex | 98 +++++++++++++++++++++++----------------------- 1 file changed, 49 insertions(+), 49 deletions(-) diff --git a/advsync/memorybarriers.tex b/advsync/memorybarriers.tex index 4ecee71..b344364 100644 --- a/advsync/memorybarriers.tex +++ b/advsync/memorybarriers.tex @@ -1783,10 +1783,10 @@ Consider the following bit of code: \begin{minipage}[t]{\columnwidth} \scriptsize \begin{verbatim} - 1 q = READ_ONCE(a); + 1 q = READ_ONCE(x); 2 if (q) { 3 <data dependency barrier> - 4 q = READ_ONCE(b); + 4 q = READ_ONCE(y); 5 } \end{verbatim} \end{minipage} @@ -1795,17 +1795,17 @@ Consider the following bit of code: This will not have the desired effect because there is no actual data dependency, but rather a control dependency that the CPU may short-circuit by attempting to predict the outcome in advance, so that other CPUs see -the load from ``\co{b}'' as having happened before the load from ``\co{a}''. +the load from \co{y} as having happened before the load from \co{x}. In such a case what's actually required is: \vspace{5pt} \begin{minipage}[t]{\columnwidth} \scriptsize \begin{verbatim} - 1 q = READ_ONCE(a); + 1 q = READ_ONCE(x); 2 if (q) { 3 <read barrier> - 4 q = READ_ONCE(b); + 4 q = READ_ONCE(y); 5 } \end{verbatim} \end{minipage} @@ -1819,9 +1819,9 @@ dependencies, as in the following example: \begin{minipage}[t]{\columnwidth} \scriptsize \begin{verbatim} - 1 q = READ_ONCE(a); + 1 q = READ_ONCE(x); 2 if (q) - 3 WRITE_ONCE(b, 1); + 3 WRITE_ONCE(y, 1); \end{verbatim} \end{minipage} \vspace{5pt} @@ -1830,13 +1830,13 @@ Control dependencies pair normally with other types of barriers. That said, please note that neither \co{READ_ONCE()} nor \co{WRITE_ONCE()} are optional! Without the \co{READ_ONCE()}, the compiler might combine the load -from ``\co{a}'' with other loads from ``\co{a}''. +from \co{x} with other loads from \co{x}. Without the \co{WRITE_ONCE()}, the compiler might combine the store to -``\co{b}'' with other stores to ``\co{b}''. +\co{y} with other stores to \co{y}. Either can result in highly counterintuitive effects on ordering. Worse yet, if the compiler is able to prove (say) that the value of -variable ``\co{a}'' is always non-zero, it would be well within its rights +variable \co{x} is always non-zero, it would be well within its rights to optimize the original example by eliminating the ``\co{if}'' statement as follows: @@ -1844,8 +1844,8 @@ as follows: \begin{minipage}[t]{\columnwidth} \scriptsize \begin{verbatim} - 1 q = READ_ONCE(a); - 2 WRITE_ONCE(b, 1); /* BUG: CPU can reorder!!! */ + 1 q = READ_ONCE(x); + 2 WRITE_ONCE(y, 1); /* BUG: CPU can reorder!!! */ \end{verbatim} \end{minipage} \vspace{5pt} @@ -1857,14 +1857,14 @@ branches of the ``\co{if}'' statement as follows: \begin{minipage}[t]{\columnwidth} \scriptsize \begin{verbatim} - 1 q = READ_ONCE(a); + 1 q = READ_ONCE(x); 2 if (q) { 3 barrier(); - 4 WRITE_ONCE(b, 1); + 4 WRITE_ONCE(y, 1); 5 do_something(); 6 } else { 7 barrier(); - 8 WRITE_ONCE(b, 1); + 8 WRITE_ONCE(y, 1); 9 do_something_else(); 10 } \end{verbatim} @@ -1878,9 +1878,9 @@ optimization levels: \begin{minipage}[t]{\columnwidth} \scriptsize \begin{verbatim} - 1 q = READ_ONCE(a); + 1 q = READ_ONCE(x); 2 barrier(); - 3 WRITE_ONCE(b, 1); /* BUG: No ordering!!! */ + 3 WRITE_ONCE(y, 1); /* BUG: No ordering!!! */ 4 if (q) { 5 do_something(); 6 } else { @@ -1890,8 +1890,8 @@ optimization levels: \end{minipage} \vspace{5pt} -Now there is no conditional between the load from ``\co{a}'' and the store to -``\co{b}'', which means that the CPU is within its rights to reorder them: +Now there is no conditional between the load from \co{x} and the store to +\co{y}, which means that the CPU is within its rights to reorder them: The conditional is absolutely required, and must be present in the assembly code even after all compiler optimizations have been applied. Therefore, if you need ordering in this example, you need explicit @@ -1901,12 +1901,12 @@ memory barriers, for example, a release store: \begin{minipage}[t]{\columnwidth} \scriptsize \begin{verbatim} - 1 q = READ_ONCE(a); + 1 q = READ_ONCE(x); 2 if (q) { - 3 smp_store_release(&b, 1); + 3 smp_store_release(&y, 1); 4 do_something(); 5 } else { - 6 smp_store_release(&b, 1); + 6 smp_store_release(&y, 1); 7 do_something_else(); 8 } \end{verbatim} @@ -1914,10 +1914,10 @@ memory barriers, for example, a release store: \vspace{5pt} The initial \co{READ_ONCE()} is still required to prevent the compiler from -proving the value of ``\co{a}''. +proving the value of \co{x}. In addition, you need to be careful what you do with the local variable -``\co{q}'', +\co{q}, otherwise the compiler might be able to guess the value and again remove the needed conditional. For example: @@ -1926,12 +1926,12 @@ For example: \begin{minipage}[t]{\columnwidth} \scriptsize \begin{verbatim} - 1 q = READ_ONCE(a); + 1 q = READ_ONCE(x); 2 if (q % MAX) { - 3 WRITE_ONCE(b, 1); + 3 WRITE_ONCE(y, 1); 4 do_something(); 5 } else { - 6 WRITE_ONCE(b, 2); + 6 WRITE_ONCE(y, 2); 7 do_something_else(); 8 } \end{verbatim} @@ -1946,15 +1946,15 @@ transform the above code into the following: \begin{minipage}[t]{\columnwidth} \scriptsize \begin{verbatim} - 1 q = READ_ONCE(a); - 2 WRITE_ONCE(b, 1); + 1 q = READ_ONCE(x); + 2 WRITE_ONCE(y, 1); 3 do_something_else(); \end{verbatim} \end{minipage} \vspace{5pt} Given this transformation, the CPU is not required to respect the ordering -between the load from variable ``\co{a}'' and the store to variable ``\co{b}''. +between the load from variable \co{x} and the store to variable \co{y}. It is tempting to add a \co{barrier()} to constrain the compiler, but this does not help. The conditional is gone, and the barrier won't bring it back. @@ -1965,20 +1965,20 @@ that \co{MAX} is greater than one, perhaps as follows: \begin{minipage}[t]{\columnwidth} \scriptsize \begin{verbatim} - 1 q = READ_ONCE(a); + 1 q = READ_ONCE(x); 2 BUILD_BUG_ON(MAX <= 1); 3 if (q % MAX) { - 4 WRITE_ONCE(b, 1); + 4 WRITE_ONCE(y, 1); 5 do_something(); 6 } else { - 7 WRITE_ONCE(b, 2); + 7 WRITE_ONCE(y, 2); 8 do_something_else(); 9 } \end{verbatim} \end{minipage} \vspace{5pt} -Please note once again that the stores to ``\co{b}'' differ. +Please note once again that the stores to \co{y} differ. If they were identical, as noted earlier, the compiler could pull this store outside of the ``\co{if}'' statement. @@ -1989,9 +1989,9 @@ Consider this example: \begin{minipage}[t]{\columnwidth} \scriptsize \begin{verbatim} - 1 q = READ_ONCE(a); + 1 q = READ_ONCE(x); 2 if (q || 1 > 0) - 3 WRITE_ONCE(b, 1); + 3 WRITE_ONCE(y, 1); \end{verbatim} \end{minipage} \vspace{5pt} @@ -2004,8 +2004,8 @@ defeating control dependency: \begin{minipage}[t]{\columnwidth} \scriptsize \begin{verbatim} - 1 q = READ_ONCE(a); - 2 WRITE_ONCE(b, 1); + 1 q = READ_ONCE(x); + 2 WRITE_ONCE(y, 1); \end{verbatim} \end{minipage} \vspace{5pt} @@ -2025,22 +2025,22 @@ not necessarily apply to code following the if-statement: \begin{minipage}[t]{\columnwidth} \scriptsize \begin{verbatim} - 1 q = READ_ONCE(a); + 1 q = READ_ONCE(x); 2 if (q) { - 3 WRITE_ONCE(b, 1); + 3 WRITE_ONCE(y, 1); 4 } else { - 5 WRITE_ONCE(b, 2); + 5 WRITE_ONCE(y, 2); 6 } - 7 WRITE_ONCE(c, 1); /* BUG: No ordering. */ + 7 WRITE_ONCE(z, 1); /* BUG: No ordering. */ \end{verbatim} \end{minipage} \vspace{5pt} It is tempting to argue that there in fact is ordering because the compiler cannot reorder volatile accesses and also cannot reorder -the writes to ``\co{b}'' with the condition. +the writes to \co{y} with the condition. Unfortunately for this line -of reasoning, the compiler might compile the two writes to ``\co{b}'' as +of reasoning, the compiler might compile the two writes to \co{y} as conditional-move instructions, as in this fanciful pseudo-assembly language: @@ -2048,18 +2048,18 @@ language: \begin{minipage}[t]{\columnwidth} \scriptsize \begin{verbatim} - 1 ld r1,a + 1 ld r1,x 2 cmp r1,$0 3 cmov,ne r4,$1 4 cmov,eq r4,$2 - 5 st r4,b - 6 st $1,c + 5 st r4,y + 6 st $1,z \end{verbatim} \end{minipage} \vspace{5pt} A weakly ordered CPU would have no dependency of any sort between the load -from ``\co{a}'' and the store to ``\co{c}''. +from \co{x} and the store to \co{z}. The control dependencies would extend only to the pair of cmov instructions and the store depending on them. In short, control dependencies apply only to the stores in the ``\co{then}'' @@ -2068,7 +2068,7 @@ invoked by those two clauses), not to code following that ``\co{if}''. Finally, control dependencies do \emph{not} provide transitivity. This is demonstrated by two related examples, with the initial values of -``\co{x}'' and ``\co{y}'' both being zero: +\co{x} and \co{y} both being zero: \vspace{5pt} \begin{minipage}[t]{\columnwidth} -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] advsync/memorybarriers: Fix trivial typo 2016-12-24 4:21 ` [PATCH 0/2] advsync: Use different set of variable names (was Re: [PATCH 0/3] Patches for recent updates) Akira Yokosawa 2016-12-24 4:24 ` [PATCH 1/2] advsync/memorybarriers: Use non-confusing variable names Akira Yokosawa @ 2016-12-24 4:25 ` Akira Yokosawa 1 sibling, 0 replies; 10+ messages in thread From: Akira Yokosawa @ 2016-12-24 4:25 UTC (permalink / raw) To: Paul E. McKenney; +Cc: perfbook, Akira Yokosawa From 0d9a7906973a76ffa56cf1251a01b3708b9eb283 Mon Sep 17 00:00:00 2001 From: Akira Yokosawa <akiyks@gmail.com> Date: Sat, 24 Dec 2016 12:34:12 +0900 Subject: [PATCH 2/2] advsync/memorybarriers: Fix trivial typo Signed-off-by: Akira Yokosawa <akiyks@gmail.com> --- advsync/memorybarriers.tex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/advsync/memorybarriers.tex b/advsync/memorybarriers.tex index b344364..b8bc611 100644 --- a/advsync/memorybarriers.tex +++ b/advsync/memorybarriers.tex @@ -1947,7 +1947,7 @@ transform the above code into the following: \scriptsize \begin{verbatim} 1 q = READ_ONCE(x); - 2 WRITE_ONCE(y, 1); + 2 WRITE_ONCE(y, 2); 3 do_something_else(); \end{verbatim} \end{minipage} -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-12-24 4:25 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-12-23 8:07 [PATCH 0/3] Patches for recent updates Akira Yokosawa 2016-12-23 8:09 ` [PATCH 1/3] advsync/memorybarriers: Use consistent quotation marks Akira Yokosawa 2016-12-23 8:10 ` [PATCH 2/3] advsync/memorybarriers: Fix typo (READ_ONCE -> WRITE_ONCE) Akira Yokosawa 2016-12-23 8:11 ` [PATCH 3/3] advsync/memorybarriers: Fix trivial typo Akira Yokosawa 2016-12-23 18:58 ` [PATCH 0/3] Patches for recent updates Paul E. McKenney 2016-12-23 23:56 ` Akira Yokosawa 2016-12-24 0:22 ` Paul E. McKenney 2016-12-24 4:21 ` [PATCH 0/2] advsync: Use different set of variable names (was Re: [PATCH 0/3] Patches for recent updates) Akira Yokosawa 2016-12-24 4:24 ` [PATCH 1/2] advsync/memorybarriers: Use non-confusing variable names Akira Yokosawa 2016-12-24 4:25 ` [PATCH 2/2] advsync/memorybarriers: Fix trivial typo Akira Yokosawa
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.