membar_enter semantics

[Taylor R Campbell <riastradh%NetBSD.org@localhost>]

membar_enter is currently documented to be a store-before-load/store
barrier: https://man.netbsd.org/NetBSD-9.2/membar_ops.3

     membar_enter()
           Any store preceding membar_enter() will happen before all memory
           operations following it.

Prompted by the use of membar_enter in sys_select.c that appeared to
be intended as a load-before-load/store barrier, a.k.a. acquire
barrier (like atomic_load_acquire -- often much cheaper than, and more
useful than, a store-before-load/store barrier), I started reviewing
the various definitions and uses of membar_enter.

It looks like:

- Every definition of membar_enter implies load-before-load/store
  semantics -- and one of the definitions, powerpc's, _does not_ imply
  store-before-load/store semantics.

  Exception: riscv membar_enter is  fence w,rw  ...but I'm not sure
  the code we have in riscv matters at the moment -- as far as I know
  a working userland hasn't gone out in any release yet.

- Every use of membar_enter appears to assume load-before-load/store
  semantics, or occurs immediately after an atomic r/m/w operation,
  where the difference between the two barrier types is immaterial
  because it's an atomic load-and-store.

  Exception: There's _one_ use of membar_enter as store-before-
  load/store...except it's not necessary and can be safely deleted
  anyway.

The names membar_enter/exit came from Solaris.  Solaris's man page
(https://docs.oracle.com/cd/E86824_01/html/E54779/membar-enter-9f.html)
says:

     The membar_enter() function is a generic memory barrier used
     during lock entry.  It is placed after the memory operation that
     acquires the lock to guarantee that the lock protects its data.
     No stores from after the memory barrier will reach visibility and
     no loads from after the barrier will be resolved before the lock
     acquisition reaches global visibility.

I can't find anything about store-before-load/store in this wording --
I suspect our man page's wording (which was `Any store preceding
membar_enter() will reach global visibility before all loads and
stores following it' before I changed it to use `happens before' in
netbsd-9, <https://man.netbsd.org/NetBSD-8.2/membar_ops.3>) arose from
a misunderstanding in parroting Solaris.

So I propose to change the membar_enter documentation to match the
definitions and usage (and change the riscv definition), making it
instead:

     membar_enter()
           Any load preceding membar_enter() will happen before all memory
           operations following it.

This will also let us delete the obnoxious text I added to
atomic_loadstore(9) warning about membar_enter semantics.

Thoughts?


* Definitions

The following definitions provide load-before-load/store:

aarch64: alias for membar_sync
alpha: alias for membar_sync
arm: alias for membar_sync
hppa: same instructions as membar_sync
i386: lfence or LOCK ADDL -- load-before-load/store or full barrier
ia64: same instructions as membar_sync
mips: alias for membar_sync
or1k: alias for membar_sync
powerpc: isync -- load-before-load/store, BUT NOT store-before-load/store (!)
sparc: alias for membar_sync
sparc64: alias for membar_sync (not sure membar #LoadLoad is right, though!)
x86_64: lfence or LOCK ADDQ -- load-before-load/store or full barrier


The following definitions provide store-before-load/store but not
load-before-load/store:

riscv: fence w,rw

But I'm not sure our riscv code works at all, and it has certainly
never gone out in a release, so I'm not sure this one matters.


* Uses

These uses appear to be clearly intended as load-before-load/store:

https://nxr.netbsd.org/xref/src/sys/kern/sys_select.c?r=1.57#656
https://nxr.netbsd.org/xref/src/lib/libpthread/pthread_rwlock.c?r=1.42#232 [1]
https://nxr.netbsd.org/xref/src/lib/libpthread/pthread_rwlock.c?r=1.42#356 [1]
https://nxr.netbsd.org/xref/src/sys/external/bsd/ena-com/ena_com.c?r=1.1#508 [2]
https://nxr.netbsd.org/xref/src/sys/kern/sys_select.c?r=1.57#656
https://nxr.netbsd.org/xref/src/sys/kern/vfs_vnode.c?r=1.128#274


These uses appear to be intended as store-before-load/store:

https://nxr.netbsd.org/xref/src/sys/kern/subr_copy.c?r=1.14#447
=> However, this is not necessary because ipi_trigger_broacdast
   already serves as a release operation.  So we can just delete this
   membar_enter.


These uses I just can't figure out and I suspect they're wrong (should
probably transpose the order with the lwproc_curlwpop calls, at which
point most likely load-before-load/store is appropriate -- if any
barriers are needed at all here, which remains unclear):

https://nxr.netbsd.org/xref/src/sys/rump/librump/rumpkern/lwproc.c?r=1.51#382


[1] These also looks like they're misusing PTHREAD__ATOMIC_IS_MEMBAR,
because there's no atomic r/m/w between the the pthread__park and the
conditional membar_enter.  Side note: looks like pthread_rwlock_unlock
needs a membar_exit or atomic_store_release for thread->pt_rwlocked.

[2] In ena_com.c, rmb is defined as an alias for membar_enter at
<https://nxr.netbsd.org/xref/src/sys/external/bsd/ena-com/ena_plat.h?r=1.7#381>.