From 498ebb239fe62f45df6f9f38476148eb480f08ff Mon Sep 17 00:00:00 2001 From: Michal 'vorner' Vaner Date: Sat, 20 Oct 2018 22:03:37 +0200 Subject: [PATCH 1/3] A less misleading intro to atomic::Ordering This goes into more detail, but without suggesting misleading things. It also tries to point out several footguns about atomic orderings. Fixes #55196. --- src/libcore/sync/atomic.rs | 46 ++++++++++++++++++++++++++++++++++---- 1 file changed, 42 insertions(+), 4 deletions(-) diff --git a/src/libcore/sync/atomic.rs b/src/libcore/sync/atomic.rs index f130dbfb0e3df..41dbc2c615e8f 100644 --- a/src/libcore/sync/atomic.rs +++ b/src/libcore/sync/atomic.rs @@ -170,10 +170,48 @@ unsafe impl Sync for AtomicPtr {} /// Atomic memory orderings /// /// Memory orderings limit the ways that both the compiler and CPU may reorder -/// instructions around atomic operations. At its most restrictive, -/// "sequentially consistent" atomics allow neither reads nor writes -/// to be moved either before or after the atomic operation; on the other end -/// "relaxed" atomics allow all reorderings. +/// instructions and optimize around atomic operations. +/// +/// An operation on an atomic variable can do three things: +/// +/// * Make the atomic itself safe to use from multiple threads. This happens always, even with the +/// lowest `Relaxed` ordering and an atomic variable always forms a "sane" time line of its own +/// operations. Note that `Relaxed` doesn't provide any guarantees at all about how different +/// threads see the relative orders of operations on *different* atomics (which is not something +/// human brain interprets as remotely "sane"). +/// * Additionally, synchronize other (even non-atomic) memory. A store with `Release` ordering +/// forms a synchronization (unidirectional) edge with any thread that does a load with `Acquire` +/// *on the written value*. Conceptually, imagine the threads being independent entities, the +/// value being tagged with a snapshot of all the memory on release and acquire waiting for all +/// that bulk of data to arrive (this is of course not what actually happens in the hardware, but +/// about the least broken intuitive understanding of the model). +/// * Finally, a `SeqCst` operation participates in a globally consistent time line (in addition to +/// being `AcqRel`). Two `SeqCst` operations have a well defined relation which one happened +/// earlier and which one later. This allows to synchronize memory without the need to "meet" on +/// the common value or even the same atomic variable ‒ a `SeqCst` load acquires from any +/// previous `SeqCst` store. +/// +/// # Common traps for the unaware +/// +/// The rules of the memory model have some very unintuitive consequences. +/// +/// * Load-only operations never "publish". Even `load(Ordering::SeqCst)` has only acquire +/// semantics, reads and writes of other memory can be reordered after that operation. +/// * Similarly, store-only operations allow reordering before them. +/// * Some operations (`compare_and_swap`) may fail. In such case they don't write any value and +/// are load-only and therefore don't have the "release" semantics (again, even if they are marked +/// as `SeqCst`). +/// * The release-acquire synchronization of other memory happens only on the one specific value, +/// not on the atomic variable. If one value released by thread `A` is overwritten by another +/// one by thread `B` (even when computed from the first one), an acquire load in `C` seeing the +/// second value acquires only the data from `B` (if `B` released). To make this work properly, +/// either use `SeqCst` for everything or chain the synchronization. `B` must acquire from `A` +/// and then release again, even if `B` is not interested in the other memory from `A`. +/// +/// # More details +/// +/// This is only an intuitive introduction. If doing anything even slightly complex, be sure to +/// study the exact definitions and prove your algorithm correct. /// /// Rust's memory orderings are [the same as /// LLVM's](https://llvm.org/docs/LangRef.html#memory-model-for-concurrent-operations). From f0777b34fd6055d44887edda9526a09ce16951ef Mon Sep 17 00:00:00 2001 From: Michal 'vorner' Vaner Date: Fri, 2 Nov 2018 19:48:59 +0100 Subject: [PATCH 2/3] fixup! A less misleading intro to atomic::Ordering --- src/libcore/sync/atomic.rs | 35 +++++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/src/libcore/sync/atomic.rs b/src/libcore/sync/atomic.rs index 41dbc2c615e8f..6fd07bb358946 100644 --- a/src/libcore/sync/atomic.rs +++ b/src/libcore/sync/atomic.rs @@ -175,37 +175,37 @@ unsafe impl Sync for AtomicPtr {} /// An operation on an atomic variable can do three things: /// /// * Make the atomic itself safe to use from multiple threads. This happens always, even with the -/// lowest `Relaxed` ordering and an atomic variable always forms a "sane" time line of its own -/// operations. Note that `Relaxed` doesn't provide any guarantees at all about how different +/// lowest [`Relaxed`] ordering and an atomic variable always forms a "sane" time line of its own +/// operations. Note that [`Relaxed`] doesn't provide any guarantees at all about how different /// threads see the relative orders of operations on *different* atomics (which is not something /// human brain interprets as remotely "sane"). -/// * Additionally, synchronize other (even non-atomic) memory. A store with `Release` ordering -/// forms a synchronization (unidirectional) edge with any thread that does a load with `Acquire` +/// * Additionally, synchronize other (even non-atomic) memory. A store with [`Release`] ordering +/// forms a synchronization (unidirectional) edge with any thread that does a load with [`Acquire`] /// *on the written value*. Conceptually, imagine the threads being independent entities, the /// value being tagged with a snapshot of all the memory on release and acquire waiting for all /// that bulk of data to arrive (this is of course not what actually happens in the hardware, but /// about the least broken intuitive understanding of the model). -/// * Finally, a `SeqCst` operation participates in a globally consistent time line (in addition to -/// being `AcqRel`). Two `SeqCst` operations have a well defined relation which one happened +/// * Finally, a [`SeqCst`] operation participates in a globally consistent time line (in addition to +/// being [`AcqRel`]). Two [`SeqCst`] operations have a well defined relation which one happened /// earlier and which one later. This allows to synchronize memory without the need to "meet" on -/// the common value or even the same atomic variable ‒ a `SeqCst` load acquires from any -/// previous `SeqCst` store. +/// the common value or even the same atomic variable ‒ a [`SeqCst`] load acquires from any +/// previous [`SeqCst`] store. /// /// # Common traps for the unaware /// /// The rules of the memory model have some very unintuitive consequences. /// -/// * Load-only operations never "publish". Even `load(Ordering::SeqCst)` has only acquire -/// semantics, reads and writes of other memory can be reordered after that operation. +/// * Load-only operations never "publish". Even [`load(Ordering::SeqCst)`][AtomicUsize::load] has +/// only acquire semantics, reads and writes of other memory can be reordered after that operation. /// * Similarly, store-only operations allow reordering before them. -/// * Some operations (`compare_and_swap`) may fail. In such case they don't write any value and -/// are load-only and therefore don't have the "release" semantics (again, even if they are marked -/// as `SeqCst`). +/// * Some operations ([`compare_and_swap`]) may fail. In such case +/// they don't write any value and are load-only and therefore don't have the "release" semantics +/// (again, even if they are marked as [`SeqCst`]). /// * The release-acquire synchronization of other memory happens only on the one specific value, /// not on the atomic variable. If one value released by thread `A` is overwritten by another /// one by thread `B` (even when computed from the first one), an acquire load in `C` seeing the /// second value acquires only the data from `B` (if `B` released). To make this work properly, -/// either use `SeqCst` for everything or chain the synchronization. `B` must acquire from `A` +/// either use [`SeqCst`] for everything or chain the synchronization. `B` must acquire from `A` /// and then release again, even if `B` is not interested in the other memory from `A`. /// /// # More details @@ -218,6 +218,13 @@ unsafe impl Sync for AtomicPtr {} /// /// For more information see the [nomicon]. /// +/// [`Relaxed`]: #variant.Relaxed +/// [`Release`]: #variant.Release +/// [`Acquire`]: #variant.Acquire +/// [`AcqRel`]: #variant.AcqRel +/// [`SeqCst`]: #variant.SeqCst +/// [AtomicUsize::load]: struct.AtomicUsize.html#method.load +/// [`compare_and_swap`]: struct.AtomicUsize.html#method.compare_and_swap /// [nomicon]: ../../../nomicon/atomics.html #[stable(feature = "rust1", since = "1.0.0")] #[derive(Copy, Clone, Debug)] From 4398a9f668d4e0afaac57ea38ed5a2910fc70615 Mon Sep 17 00:00:00 2001 From: Michal 'vorner' Vaner Date: Sat, 3 Nov 2018 04:19:18 +0100 Subject: [PATCH 3/3] fixup! A less misleading intro to atomic::Ordering --- src/libcore/sync/atomic.rs | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/libcore/sync/atomic.rs b/src/libcore/sync/atomic.rs index 6fd07bb358946..2d688e1b4e513 100644 --- a/src/libcore/sync/atomic.rs +++ b/src/libcore/sync/atomic.rs @@ -180,23 +180,24 @@ unsafe impl Sync for AtomicPtr {} /// threads see the relative orders of operations on *different* atomics (which is not something /// human brain interprets as remotely "sane"). /// * Additionally, synchronize other (even non-atomic) memory. A store with [`Release`] ordering -/// forms a synchronization (unidirectional) edge with any thread that does a load with [`Acquire`] -/// *on the written value*. Conceptually, imagine the threads being independent entities, the -/// value being tagged with a snapshot of all the memory on release and acquire waiting for all -/// that bulk of data to arrive (this is of course not what actually happens in the hardware, but -/// about the least broken intuitive understanding of the model). -/// * Finally, a [`SeqCst`] operation participates in a globally consistent time line (in addition to -/// being [`AcqRel`]). Two [`SeqCst`] operations have a well defined relation which one happened -/// earlier and which one later. This allows to synchronize memory without the need to "meet" on -/// the common value or even the same atomic variable ‒ a [`SeqCst`] load acquires from any -/// previous [`SeqCst`] store. +/// forms a synchronization (unidirectional) edge with any thread that does a load with +/// [`Acquire`] *on the written value*. Conceptually, imagine the threads being independent +/// entities, the value being tagged with a snapshot of all the memory on release and acquire +/// waiting for all that bulk of data to arrive (this is of course not what actually happens in +/// the hardware, but about the least broken intuitive understanding of the model). +/// * Finally, a [`SeqCst`] operation participates in a globally consistent time line (in addition +/// to being [`AcqRel`]). Two [`SeqCst`] operations have a well defined relation which one +/// happened earlier and which one later. This allows to synchronize memory without the need to +/// "meet" on the common value or even the same atomic variable ‒ a [`SeqCst`] load acquires from +/// any previous [`SeqCst`] store. /// /// # Common traps for the unaware /// /// The rules of the memory model have some very unintuitive consequences. /// /// * Load-only operations never "publish". Even [`load(Ordering::SeqCst)`][AtomicUsize::load] has -/// only acquire semantics, reads and writes of other memory can be reordered after that operation. +/// only acquire semantics, reads and writes of other memory can be reordered after that +/// operation. /// * Similarly, store-only operations allow reordering before them. /// * Some operations ([`compare_and_swap`]) may fail. In such case /// they don't write any value and are load-only and therefore don't have the "release" semantics