Source-Changes archive

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

CVS commit: src



Module Name:    src
Committed By:   riastradh
Date:           Sat Apr  9 23:32:53 UTC 2022

Modified Files:
        src/common/lib/libc/arch/aarch64/atomic: membar_ops.S
        src/common/lib/libc/arch/alpha/atomic: membar_ops.S
        src/common/lib/libc/arch/arm/atomic: membar_ops.S
        src/common/lib/libc/arch/hppa/atomic: membar_ops.S
        src/common/lib/libc/arch/i386/atomic: atomic.S
        src/common/lib/libc/arch/ia64/atomic: atomic.S
        src/common/lib/libc/arch/mips/atomic: membar_ops.S
        src/common/lib/libc/arch/or1k/atomic: membar_ops.S
        src/common/lib/libc/arch/powerpc/atomic: membar_ops.S
        src/common/lib/libc/arch/riscv/atomic: membar_ops.S
        src/common/lib/libc/arch/sparc/atomic: membar_ops.S
        src/common/lib/libc/arch/sparc64/atomic: membar_ops.S
        src/common/lib/libc/arch/x86_64/atomic: atomic.S
        src/common/lib/libc/atomic: membar_ops_nop.c
        src/distrib/sets/lists/comp: mi
        src/lib/libc/atomic: Makefile.inc membar_ops.3
        src/share/man/man9: atomic_loadstore.9
        src/sys/sys: atomic.h
        src/tests/lib/libc/membar: t_dekker.c t_spinlock.c

Log Message:
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.


To generate a diff of this commit:
cvs rdiff -u -r1.3 -r1.4 src/common/lib/libc/arch/aarch64/atomic/membar_ops.S
cvs rdiff -u -r1.8 -r1.9 src/common/lib/libc/arch/alpha/atomic/membar_ops.S
cvs rdiff -u -r1.9 -r1.10 src/common/lib/libc/arch/arm/atomic/membar_ops.S
cvs rdiff -u -r1.2 -r1.3 src/common/lib/libc/arch/hppa/atomic/membar_ops.S
cvs rdiff -u -r1.34 -r1.35 src/common/lib/libc/arch/i386/atomic/atomic.S
cvs rdiff -u -r1.6 -r1.7 src/common/lib/libc/arch/ia64/atomic/atomic.S
cvs rdiff -u -r1.11 -r1.12 src/common/lib/libc/arch/mips/atomic/membar_ops.S
cvs rdiff -u -r1.1 -r1.2 src/common/lib/libc/arch/or1k/atomic/membar_ops.S
cvs rdiff -u -r1.5 -r1.6 src/common/lib/libc/arch/powerpc/atomic/membar_ops.S
cvs rdiff -u -r1.3 -r1.4 src/common/lib/libc/arch/riscv/atomic/membar_ops.S
cvs rdiff -u -r1.7 -r1.8 src/common/lib/libc/arch/sparc/atomic/membar_ops.S
cvs rdiff -u -r1.8 -r1.9 src/common/lib/libc/arch/sparc64/atomic/membar_ops.S
cvs rdiff -u -r1.27 -r1.28 src/common/lib/libc/arch/x86_64/atomic/atomic.S
cvs rdiff -u -r1.7 -r1.8 src/common/lib/libc/atomic/membar_ops_nop.c
cvs rdiff -u -r1.2412 -r1.2413 src/distrib/sets/lists/comp/mi
cvs rdiff -u -r1.4 -r1.5 src/lib/libc/atomic/Makefile.inc
cvs rdiff -u -r1.9 -r1.10 src/lib/libc/atomic/membar_ops.3
cvs rdiff -u -r1.6 -r1.7 src/share/man/man9/atomic_loadstore.9
cvs rdiff -u -r1.22 -r1.23 src/sys/sys/atomic.h
cvs rdiff -u -r1.1 -r1.2 src/tests/lib/libc/membar/t_dekker.c \
    src/tests/lib/libc/membar/t_spinlock.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.




Home | Main Index | Thread Index | Old Index