The documentation for smp_read_barrier_depends() does not mention the architectures
for which it is an optimization, for example ARM and PPC.  As a result, it is not
clear to the reader why one would use it.  Relegate Alpha to a footnote together
with other architectures where it is equivalent to smp_rmb().

Suggested-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 docs/devel/atomics.rst | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/docs/devel/atomics.rst b/docs/devel/atomics.rst
index 2157d3312956..8f6273c9283e 100644
--- a/docs/devel/atomics.rst
+++ b/docs/devel/atomics.rst
@@ -201,10 +201,9 @@ They come in six kinds:
   retrieves the address to which the second load will be directed),
   the processor will guarantee that the first LOAD will appear to happen
   before the second with respect to the other components of the system.
-  However, this is not always true---for example, it was not true on
-  Alpha processors.  Whenever this kind of access happens to shared
-  memory (that is not protected by a lock), a read barrier is needed,
-  and ``smp_read_barrier_depends()`` can be used instead of ``smp_rmb()``.
+  Therefore, unlike ``smp_rmb()`` or ``qatomic_load_acquire()``,
+  ``smp_read_barrier_depends()`` can be just a compiler barrier on
+  weakly-ordered architectures such as ARM or PPC[#]_.
 
   Note that the first load really has to have a _data_ dependency and not
   a control dependency.  If the address for the second load is dependent
@@ -212,6 +211,10 @@ They come in six kinds:
   than actually loading the address itself, then it's a _control_
   dependency and a full read barrier or better is required.
 
+.. [#] The DEC Alpha is an exception, because ``smp_read_barrier_depends()``
+   needs a processor barrier.  On strongly-ordered architectures such
+   as x86 or s390, ``smp_rmb()`` and ``qatomic_load_acquire()`` can
+   also be a compiler barriers.
 
 Memory barriers and ``qatomic_load_acquire``/``qatomic_store_release`` are
 mostly used when a data structure has one thread that is always a writer
-- 
2.39.2

On Fri, Apr 7, 2023, 09:41 Paolo Bonzini <pbonzini@redhat.com> wrote:

> The documentation for smp_read_barrier_depends() does not mention the
> architectures
> for which it is an optimization, for example ARM and PPC.  As a result, it
> is not
> clear to the reader why one would use it.  Relegate Alpha to a footnote
> together
> with other architectures where it is equivalent to smp_rmb().
>
> Suggested-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  docs/devel/atomics.rst | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>

Modulo the typo that Peter mentioned:

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

diff --git a/docs/devel/atomics.rst b/docs/devel/atomics.rst
> index 2157d3312956..8f6273c9283e 100644
> --- a/docs/devel/atomics.rst
> +++ b/docs/devel/atomics.rst
> @@ -201,10 +201,9 @@ They come in six kinds:
>    retrieves the address to which the second load will be directed),
>    the processor will guarantee that the first LOAD will appear to happen
>    before the second with respect to the other components of the system.
> -  However, this is not always true---for example, it was not true on
> -  Alpha processors.  Whenever this kind of access happens to shared
> -  memory (that is not protected by a lock), a read barrier is needed,
> -  and ``smp_read_barrier_depends()`` can be used instead of ``smp_rmb()``.
> +  Therefore, unlike ``smp_rmb()`` or ``qatomic_load_acquire()``,
> +  ``smp_read_barrier_depends()`` can be just a compiler barrier on
> +  weakly-ordered architectures such as ARM or PPC[#]_.
>
>    Note that the first load really has to have a _data_ dependency and not
>    a control dependency.  If the address for the second load is dependent
> @@ -212,6 +211,10 @@ They come in six kinds:
>    than actually loading the address itself, then it's a _control_
>    dependency and a full read barrier or better is required.
>
> +.. [#] The DEC Alpha is an exception, because
> ``smp_read_barrier_depends()``
> +   needs a processor barrier.  On strongly-ordered architectures such
> +   as x86 or s390, ``smp_rmb()`` and ``qatomic_load_acquire()`` can
> +   also be a compiler barriers.
>
>  Memory barriers and ``qatomic_load_acquire``/``qatomic_store_release`` are
>  mostly used when a data structure has one thread that is always a writer
> --
> 2.39.2
>
>
>

On Fri, 7 Apr 2023 at 14:41, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> The documentation for smp_read_barrier_depends() does not mention the architectures
> for which it is an optimization, for example ARM and PPC.  As a result, it is not
> clear to the reader why one would use it.  Relegate Alpha to a footnote together
> with other architectures where it is equivalent to smp_rmb().
>
> Suggested-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  docs/devel/atomics.rst | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/docs/devel/atomics.rst b/docs/devel/atomics.rst
> index 2157d3312956..8f6273c9283e 100644
> --- a/docs/devel/atomics.rst
> +++ b/docs/devel/atomics.rst
> @@ -201,10 +201,9 @@ They come in six kinds:
>    retrieves the address to which the second load will be directed),
>    the processor will guarantee that the first LOAD will appear to happen
>    before the second with respect to the other components of the system.
> -  However, this is not always true---for example, it was not true on
> -  Alpha processors.  Whenever this kind of access happens to shared
> -  memory (that is not protected by a lock), a read barrier is needed,
> -  and ``smp_read_barrier_depends()`` can be used instead of ``smp_rmb()``.
> +  Therefore, unlike ``smp_rmb()`` or ``qatomic_load_acquire()``,
> +  ``smp_read_barrier_depends()`` can be just a compiler barrier on
> +  weakly-ordered architectures such as ARM or PPC[#]_.

If you want to be super-picky, the preferred capitalization
these days is "Arm" :-)

>    Note that the first load really has to have a _data_ dependency and not
>    a control dependency.  If the address for the second load is dependent
> @@ -212,6 +211,10 @@ They come in six kinds:
>    than actually loading the address itself, then it's a _control_
>    dependency and a full read barrier or better is required.
>
> +.. [#] The DEC Alpha is an exception, because ``smp_read_barrier_depends()``
> +   needs a processor barrier.  On strongly-ordered architectures such
> +   as x86 or s390, ``smp_rmb()`` and ``qatomic_load_acquire()`` can
> +   also be a compiler barriers.

s/a //;

>
>  Memory barriers and ``qatomic_load_acquire``/``qatomic_store_release`` are
>  mostly used when a data structure has one thread that is always a writer
> --
> 2.39.2

thanks
-- PMM