On Tue, Dec 22, 2015 at 02:21:31PM +0800, Jianyu Zhan wrote:
> Hi, Paul,
>
> I noticed that in the control dependency section in
> memory-barrier.txt, you mistakenly made an inconsistent
> description:
>
> On the description part:
>
> 641 It is tempting to try to enforce ordering on identical stores on both
> 642 branches of the "if" statement as follows:
> 643
> 644 q = READ_ONCE(a);
> 645 if (q) {
> 646 barrier();
> 647 WRITE_ONCE(b, p);
> 648 do_something();
> 649 } else {
> 650 barrier();
> 651 WRITE_ONCE(b, p);
> 652 do_something_else();
> 653 }
> 654
> 655 Unfortunately, current compilers will transform this as follows at high
> 656 optimization levels:
> 657
> 658 q = READ_ONCE(a);
> 659 barrier();
> 660 WRITE_ONCE(b, p); /* BUG: No ordering vs. load from a!!! */
> 661 if (q) {
> 662 /* WRITE_ONCE(b, p); -- moved up, BUG!!! */
> 663 do_something();
> 664 } else {
> 665 /* WRITE_ONCE(b, p); -- moved up, BUG!!! */
> 666 do_something_else();
> 667 }
> 668
>
> This part is incorporated in commit 2456d2a617de ("memory-barriers: Fix
> description of 2-legged-if-based control dependencies") on 2014-08-13.
>
> However, on the summary part:
>
> 803 (*) If both legs of the "if" statement begin with identical stores
> 804 to the same variable, a barrier() statement is required at the
> 805 beginning of each leg of the "if" statement.
>
> This part is incorporated in commit 9b2b3bf53124
> ("Documentation/memory-barriers.txt:
> Need barriers() for some control dependencies"), on 2014-02-12.
>
> I think you missed fixing the summary part?
It looks like you are quite correct, good catch! Does the patch below
fix this?
Thanx, Paul
------------------------------------------------------------------------
commit add179813efa2ba8a4afd29828d3335cf346d2a8
Author: Paul E. McKenney <paul...@linux.vnet.ibm.com>
Date: Tue Dec 29 16:23:18 2015 -0800
documentation: Fix control dependency and identical stores
The summary of the "CONTROL DEPENDENCIES" section incorrectly states that
barrier() may be used to prevent compiler reordering when more than one
leg of the control-dependent "if" statement start with identical stores.
This is incorrect at high optimization levels. This commit therefore
updates the summary to match the detailed description.
Reported by: Jianyu Zhan <nasa4...@gmail.com>
Signed-off-by: Paul E. McKenney <paul...@linux.vnet.ibm.com>
diff --git a/Documentation/memory-barriers.txt
b/Documentation/memory-barriers.txt
index 85304ebd187c..50190368400c 100644
--- a/Documentation/memory-barriers.txt
+++ b/Documentation/memory-barriers.txt
@@ -800,9 +800,13 @@ In summary:
use smp_rmb(), smp_wmb(), or, in the case of prior stores and
later loads, smp_mb().
- (*) If both legs of the "if" statement begin with identical stores
- to the same variable, a barrier() statement is required at the
- beginning of each leg of the "if" statement.
+ (*) If both legs of the "if" statement begin with identical stores to
+ the same variable, then those stores must be ordered, either by
+ preceding both of them with smp_mb() or by using smp_store_release()
+ to carry out the stores. Please note that it is -not- sufficient
+ to use barrier() at beginning of each leg of the "if" statement,
+ as optimizing compilers do not necessarily respect barrier()
+ in this case.
(*) Control dependencies require at least one run-time conditional
between the prior load and the subsequent store, and this
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
