Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/tests/lib/libc/membar Introduce membar_acquire/release. Dep...



details:   https://anonhg.NetBSD.org/src/rev/7be4679c82e5
branches:  trunk
changeset: 365144:7be4679c82e5
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Sat Apr 09 23:32:51 2022 +0000

description:
Introduce membar_acquire/release.  Deprecate membar_enter/exit.

The names membar_enter/exit were unclear, and the documentation of
membar_enter has disagreed with the implementations on sparc,
powerpc, and even x86(!) for the entire time it has been in NetBSD.

The terms `acquire' and `release' are ubiquitous in the literature
today, and have been adopted in the C and C++ standards to mean
load-before-load/store and load/store-before-store, respectively,
which are exactly the orderings required by acquiring and releasing a
mutex, as well as other useful applications like decrementing a
reference count and then freeing the underlying object if it went to
zero.

Originally I proposed changing one word in the documentation for
membar_enter to make it load-before-load/store instead of
store-before-load/store, i.e., to make it an acquire barrier.  I
proposed this on the grounds that

(a) all implementations guarantee load-before-load/store,
(b) some implementations fail to guarantee store-before-load/store,
and
(c) all uses in-tree assume load-before-load/store.

I verified parts (a) and (b) (except, for (a), powerpc didn't even
guarantee load-before-load/store -- isync isn't necessarily enough;
need lwsync in general -- but it _almost_ did, and it certainly didn't
guarantee store-before-load/store).

Part (c) might not be correct, however: under the mistaken assumption
that atomic-r/m/w then membar-w/rw is equivalent to atomic-r/m/w then
membar-r/rw, I only audited the cases of membar_enter that _aren't_
immediately after an atomic-r/m/w.  All of those cases assume
load-before-load/store.  But my assumption was wrong -- there are
cases of atomic-r/m/w then membar-w/rw that would be broken by
changing to atomic-r/m/w then membar-r/rw:

https://mail-index.netbsd.org/tech-kern/2022/03/29/msg028044.html

Furthermore, the name membar_enter has been adopted in other places
like OpenBSD where it actually does follow the documentation and
guarantee store-before-load/store, even if that order is not useful.
So the name membar_enter currently lives in a bad place where it
means either of two things -- r/rw or w/rw.

With this change, we deprecate membar_enter/exit, introduce
membar_acquire/release as better names for the useful pair (r/rw and
rw/w), and make sure the implementation of membar_enter guarantees
both what was documented _and_ what was implemented, making it an
alias for membar_sync.

While here, rework all of the membar_* definitions and aliases.  The
new logic follows a rule to make it easier to audit:

        membar_X is defined as an alias for membar_Y iff membar_X is
        guaranteed by membar_Y.

The `no stronger than' relation is (the transitive closure of):

- membar_consumer (r/r) is guaranteed by membar_acquire (r/rw)
- membar_producer (w/w) is guaranteed by membar_release (rw/w)
- membar_acquire (r/rw) is guaranteed by membar_sync (rw/rw)
- membar_release (rw/w) is guaranteed by membar_sync (rw/rw)

And, for the deprecated membars:

- membar_enter (whether r/rw, w/rw, or rw/rw) is guaranteed by
  membar_sync (rw/rw)
- membar_exit (rw/w) is guaranteed by membar_release (rw/w)

(membar_exit is identical to membar_release, but the name is
deprecated.)

Finally, while here, annotate some of the instructions with their
semantics.  For powerpc, leave an essay with citations on the
unfortunate but -- as far as I can tell -- necessary decision to use
lwsync, not isync, for membar_acquire and membar_consumer.

Also add membar(3) and atomic(3) man page links.

diffstat:

 common/lib/libc/arch/aarch64/atomic/membar_ops.S |   16 +-
 common/lib/libc/arch/alpha/atomic/membar_ops.S   |   27 ++-
 common/lib/libc/arch/arm/atomic/membar_ops.S     |   10 +-
 common/lib/libc/arch/hppa/atomic/membar_ops.S    |   40 ++---
 common/lib/libc/arch/i386/atomic/atomic.S        |   24 ++-
 common/lib/libc/arch/ia64/atomic/atomic.S        |   14 +-
 common/lib/libc/arch/mips/atomic/membar_ops.S    |   30 +++-
 common/lib/libc/arch/or1k/atomic/membar_ops.S    |   19 +-
 common/lib/libc/arch/powerpc/atomic/membar_ops.S |  121 ++++++++++++++--
 common/lib/libc/arch/riscv/atomic/membar_ops.S   |   14 +-
 common/lib/libc/arch/sparc/atomic/membar_ops.S   |   28 ++-
 common/lib/libc/arch/sparc64/atomic/membar_ops.S |   26 ++-
 common/lib/libc/arch/x86_64/atomic/atomic.S      |   24 ++-
 common/lib/libc/atomic/membar_ops_nop.c          |    8 +-
 distrib/sets/lists/comp/mi                       |   14 +-
 lib/libc/atomic/Makefile.inc                     |    6 +-
 lib/libc/atomic/membar_ops.3                     |  163 ++++++++++++----------
 share/man/man9/atomic_loadstore.9                |   38 ++--
 sys/sys/atomic.h                                 |   12 +-
 tests/lib/libc/membar/t_dekker.c                 |    8 +-
 tests/lib/libc/membar/t_spinlock.c               |    8 +-
 21 files changed, 427 insertions(+), 223 deletions(-)

diffs (truncated from 1338 to 300 lines):

diff -r e2d74bef7407 -r 7be4679c82e5 common/lib/libc/arch/aarch64/atomic/membar_ops.S
--- a/common/lib/libc/arch/aarch64/atomic/membar_ops.S  Sat Apr 09 22:53:53 2022 +0000
+++ b/common/lib/libc/arch/aarch64/atomic/membar_ops.S  Sat Apr 09 23:32:51 2022 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: membar_ops.S,v 1.3 2022/04/09 12:07:37 riastradh Exp $ */
+/* $NetBSD: membar_ops.S,v 1.4 2022/04/09 23:32:51 riastradh Exp $ */
 
 /*-
  * Copyright (c) 2014 The NetBSD Foundation, Inc.
@@ -32,19 +32,21 @@
 #include "atomic_op_asm.h"
 
 ENTRY_NP(_membar_producer)
-       dmb     ishst
+       dmb     ishst           /* store-before-store */
        ret
 END(_membar_producer)
 ATOMIC_OP_ALIAS(membar_producer,_membar_producer)
 ATOMIC_OP_ALIAS(membar_write,_membar_producer)
 STRONG_ALIAS(_membar_write,_membar_producer)
 
-ENTRY_NP(_membar_consumer)
-       dmb     ishld
+ENTRY_NP(_membar_acquire)
+       dmb     ishld           /* load-before-load/store */
        ret
-END(_membar_consumer)
-ATOMIC_OP_ALIAS(membar_consumer,_membar_consumer)
+END(_membar_acquire)
+ATOMIC_OP_ALIAS(membar_acquire,_membar_acquire)
+ATOMIC_OP_ALIAS(membar_consumer,_membar_acquire)
 ATOMIC_OP_ALIAS(membar_read,_membar_consumer)
+STRONG_ALIAS(_membar_consumer,_membar_acquire)
 STRONG_ALIAS(_membar_read,_membar_consumer)
 
 ENTRY_NP(_membar_sync)
@@ -52,8 +54,10 @@
        ret
 END(_membar_sync)
 ATOMIC_OP_ALIAS(membar_sync,_membar_sync)
+ATOMIC_OP_ALIAS(membar_release,_membar_sync)
 ATOMIC_OP_ALIAS(membar_enter,_membar_sync)
 ATOMIC_OP_ALIAS(membar_exit,_membar_sync)
 STRONG_ALIAS(__sync_synchronize,_membar_sync)
+STRONG_ALIAS(_membar_release,_membar_sync)
 STRONG_ALIAS(_membar_enter,_membar_sync)
 STRONG_ALIAS(_membar_exit,_membar_sync)
diff -r e2d74bef7407 -r 7be4679c82e5 common/lib/libc/arch/alpha/atomic/membar_ops.S
--- a/common/lib/libc/arch/alpha/atomic/membar_ops.S    Sat Apr 09 22:53:53 2022 +0000
+++ b/common/lib/libc/arch/alpha/atomic/membar_ops.S    Sat Apr 09 23:32:51 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: membar_ops.S,v 1.8 2022/04/06 22:47:55 riastradh Exp $ */
+/*     $NetBSD: membar_ops.S,v 1.9 2022/04/09 23:32:51 riastradh Exp $ */
 
 /*-
  * Copyright (c) 2006, 2007 The NetBSD Foundation, Inc.
@@ -42,45 +42,50 @@
 LEAF(_membar_producer, 0)
        RET
        nop
-       END(_membar_producer)
+END(_membar_producer)
 EXPORT(_membar_producer_end)
 
 LEAF(_membar_sync, 0)
        RET
        nop
-       END(_membar_sync)
+END(_membar_sync)
 EXPORT(_membar_sync_end)
 
 LEAF(_membar_producer_mp, 0)
-       wmb
+       wmb             /* store-before-store */
        RET
-       END(_membar_producer_mp)
+END(_membar_producer_mp)
 EXPORT(_membar_producer_mp_end)
 
 LEAF(_membar_sync_mp, 0)
-       mb
+       mb              /* load/store-before-load/store */
        RET
-       END(_membar_sync_mp)
+END(_membar_sync_mp)
 EXPORT(_membar_sync_mp_end)
 
 #else  /* _KERNEL */
 
 LEAF(_membar_producer, 0)
-       mb
+       mb              /* load/store-before-load/store */
        RET
-       END(_membar_producer)
+END(_membar_producer)
 EXPORT(_membar_producer_end)
 
 LEAF(_membar_sync, 0)
-       mb
+       mb              /* load/store-before-load/store */
        RET
-       END(_membar_sync)
+END(_membar_sync)
 EXPORT(_membar_sync_end)
 
 #endif /* _KERNEL */
 
 ATOMIC_OP_ALIAS(membar_producer,_membar_producer)
 ATOMIC_OP_ALIAS(membar_sync,_membar_sync)
+
+ATOMIC_OP_ALIAS(membar_acquire,_membar_sync)
+STRONG_ALIAS(_membar_acquire,_membar_sync)
+ATOMIC_OP_ALIAS(membar_release,_membar_sync)
+STRONG_ALIAS(_membar_release,_membar_sync)
 ATOMIC_OP_ALIAS(membar_enter,_membar_sync)
 STRONG_ALIAS(_membar_enter,_membar_sync)
 ATOMIC_OP_ALIAS(membar_exit,_membar_sync)
diff -r e2d74bef7407 -r 7be4679c82e5 common/lib/libc/arch/arm/atomic/membar_ops.S
--- a/common/lib/libc/arch/arm/atomic/membar_ops.S      Sat Apr 09 22:53:53 2022 +0000
+++ b/common/lib/libc/arch/arm/atomic/membar_ops.S      Sat Apr 09 23:32:51 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: membar_ops.S,v 1.9 2021/07/28 07:32:20 skrll Exp $     */
+/*     $NetBSD: membar_ops.S,v 1.10 2022/04/09 23:32:51 riastradh Exp $        */
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
  * All rights reserved.
@@ -33,7 +33,7 @@
 #if defined(_ARM_ARCH_6)
 
 ENTRY_NP(_membar_producer)
-       DMBST
+       DMBST           /* store-before-store */
        RET
 END(_membar_producer)
 ATOMIC_OP_ALIAS(membar_producer,_membar_producer)
@@ -41,15 +41,19 @@
 STRONG_ALIAS(_membar_write,_membar_producer)
 
 ENTRY_NP(_membar_sync)
-       DMB
+       DMB             /* load/store-before-load/store */
        RET
 END(_membar_sync)
 ATOMIC_OP_ALIAS(membar_sync,_membar_sync)
+ATOMIC_OP_ALIAS(membar_acquire,_membar_sync)
+ATOMIC_OP_ALIAS(membar_release,_membar_sync)
 ATOMIC_OP_ALIAS(membar_enter,_membar_sync)
 ATOMIC_OP_ALIAS(membar_exit,_membar_sync)
 ATOMIC_OP_ALIAS(membar_consumer,_membar_sync)
 ATOMIC_OP_ALIAS(membar_read,_membar_sync)
 STRONG_ALIAS(__sync_synchronize,_membar_sync)
+STRONG_ALIAS(_membar_acquire,_membar_sync)
+STRONG_ALIAS(_membar_release,_membar_sync)
 STRONG_ALIAS(_membar_enter,_membar_sync)
 STRONG_ALIAS(_membar_exit,_membar_sync)
 STRONG_ALIAS(_membar_consumer,_membar_sync)
diff -r e2d74bef7407 -r 7be4679c82e5 common/lib/libc/arch/hppa/atomic/membar_ops.S
--- a/common/lib/libc/arch/hppa/atomic/membar_ops.S     Sat Apr 09 22:53:53 2022 +0000
+++ b/common/lib/libc/arch/hppa/atomic/membar_ops.S     Sat Apr 09 23:32:51 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: membar_ops.S,v 1.2 2022/04/06 22:47:56 riastradh Exp $ */
+/*     $NetBSD: membar_ops.S,v 1.3 2022/04/09 23:32:51 riastradh Exp $ */
 
 /*-
  * Copyright (c) 2007 The NetBSD Foundation, Inc.
@@ -31,11 +31,11 @@
 
 #include "atomic_op_asm.h"
 
-RCSID("$NetBSD: membar_ops.S,v 1.2 2022/04/06 22:47:56 riastradh Exp $")
+RCSID("$NetBSD: membar_ops.S,v 1.3 2022/04/09 23:32:51 riastradh Exp $")
 
        .text
 
-LEAF_ENTRY(_membar_consumer)
+LEAF_ENTRY(_membar_sync)
        sync
        nop
        nop
@@ -44,24 +44,18 @@
        nop
        bv      %r0(%rp)
         nop
-EXIT(_membar_consumer)
+EXIT(_membar_sync)
+ATOMIC_OP_ALIAS(membar_sync,_membar_sync)
 
-LEAF_ENTRY(_membar_producer)
-       sync
-       nop
-       nop
-       nop
-       nop
-       nop
-       bv      %r0(%rp)
-        nop
-EXIT(_membar_producer)
-
-ATOMIC_OP_ALIAS(membar_producer,_membar_producer)
-ATOMIC_OP_ALIAS(membar_consumer,_membar_consumer)
-ATOMIC_OP_ALIAS(membar_enter,_membar_consumer)
-STRONG_ALIAS(_membar_enter,_membar_consumer)
-ATOMIC_OP_ALIAS(membar_exit,_membar_producer)
-STRONG_ALIAS(_membar_exit,_membar_producer)
-ATOMIC_OP_ALIAS(membar_sync,_membar_producer)
-STRONG_ALIAS(_membar_sync,_membar_producer)
+ATOMIC_OP_ALIAS(membar_producer,_membar_sync)
+STRONG_ALIAS(_membar_producer,_membar_sync)
+ATOMIC_OP_ALIAS(membar_consumer,_membar_sync)
+STRONG_ALIAS(_membar_consumer,_membar_sync)
+ATOMIC_OP_ALIAS(membar_acquire,_membar_sync)
+STRONG_ALIAS(_membar_acquire,_membar_sync)
+ATOMIC_OP_ALIAS(membar_release,_membar_sync)
+STRONG_ALIAS(_membar_release,_membar_sync)
+ATOMIC_OP_ALIAS(membar_enter,_membar_sync)
+STRONG_ALIAS(_membar_enter,_membar_sync)
+ATOMIC_OP_ALIAS(membar_exit,_membar_sync)
+STRONG_ALIAS(_membar_exit,_membar_sync)
diff -r e2d74bef7407 -r 7be4679c82e5 common/lib/libc/arch/i386/atomic/atomic.S
--- a/common/lib/libc/arch/i386/atomic/atomic.S Sat Apr 09 22:53:53 2022 +0000
+++ b/common/lib/libc/arch/i386/atomic/atomic.S Sat Apr 09 23:32:51 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: atomic.S,v 1.34 2022/04/09 22:53:36 riastradh Exp $    */
+/*     $NetBSD: atomic.S,v 1.35 2022/04/09 23:32:51 riastradh Exp $    */
 
 /*-
  * Copyright (c) 2007 The NetBSD Foundation, Inc.
@@ -178,23 +178,23 @@
        ret
 END(_atomic_cas_32_ni)
 
-ENTRY(_membar_consumer)
+ENTRY(_membar_acquire)
        /*
         * Every load from normal memory is a load-acquire on x86, so
         * there is never any need for explicit barriers to order
         * load-before-anything.
         */
        ret
-END(_membar_consumer)
+END(_membar_acquire)
 
-ENTRY(_membar_producer)
+ENTRY(_membar_release)
        /*
         * Every store to normal memory is a store-release on x86, so
         * there is never any need for explicit barriers to order
         * anything-before-store.
         */
        ret
-END(_membar_producer)
+END(_membar_release)
 
 ENTRY(_membar_sync)
        /*
@@ -340,10 +340,14 @@
 ALIAS(__sync_val_compare_and_swap_8,_atomic_cas_64)
 #endif /* __HAVE_ATOMIC64_OPS || _KERNEL */
 
-ALIAS(membar_consumer,_membar_consumer)
-ALIAS(membar_producer,_membar_producer)
+ALIAS(membar_acquire,_membar_acquire)
+ALIAS(membar_release,_membar_release)
+ALIAS(membar_sync,_membar_sync)
+
+ALIAS(membar_consumer,_membar_acquire)
+ALIAS(membar_producer,_membar_release)
 ALIAS(membar_enter,_membar_sync)
-ALIAS(membar_exit,_membar_producer)
+ALIAS(membar_exit,_membar_release)
 ALIAS(membar_sync,_membar_sync)
 
 STRONG_ALIAS(_atomic_add_int,_atomic_add_32)
@@ -398,8 +402,10 @@
 STRONG_ALIAS(_atomic_cas_ulong_ni,_atomic_cas_32_ni)
 STRONG_ALIAS(_atomic_cas_ptr_ni,_atomic_cas_32_ni)
 
+STRONG_ALIAS(_membar_consumer,_membar_acquire)
+STRONG_ALIAS(_membar_producer,_membar_release)
 STRONG_ALIAS(_membar_enter,_membar_sync)
-STRONG_ALIAS(_membar_exit,_membar_producer)
+STRONG_ALIAS(_membar_exit,_membar_release)
 
 #ifdef _HARDKERNEL
        .section .rodata
diff -r e2d74bef7407 -r 7be4679c82e5 common/lib/libc/arch/ia64/atomic/atomic.S
--- a/common/lib/libc/arch/ia64/atomic/atomic.S Sat Apr 09 22:53:53 2022 +0000
+++ b/common/lib/libc/arch/ia64/atomic/atomic.S Sat Apr 09 23:32:51 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: atomic.S,v 1.6 2022/04/06 22:47:56 riastradh Exp $     */
+/*     $NetBSD: atomic.S,v 1.7 2022/04/09 23:32:51 riastradh Exp $     */
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -117,6 +117,16 @@
        br.ret.sptk     rp
 END(_membar_producer)
 
+ENTRY(_membar_acquire,0)
+       mf



Home | Main Index | Thread Index | Old Index