[src/trunk]: src/sys/kern mutex(9): Omit needless membar_consumer.

[riastradh <riastradh%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/3c2c09175c13
branches:  trunk
changeset: 374542:3c2c09175c13
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Mon May 01 12:17:56 2023 +0000

description:
mutex(9): Omit needless membar_consumer.

In practical terms, this is not necessary because MUTEX_SET_WAITERS
already issues MUTEX_MEMBAR_ENTER, which on all architectures is a
sequential consistency barrier, i.e., read/write-before-read/write,
subsuming membar_consumer.

In theoretical terms, MUTEX_MEMBAR_ENTER might imply only
write-before-read/write, so one might imagine that the
read-before-read ordering of membar_consumer _could_ be necessary.
However, the memory operations that are significant here are:

1. load owner := mtx->mtx_owner
2. store mtx->mtx_owner := owner | MUTEX_BIT_WAITERS
3. load owner->l_cpu->ci_curlwp to test if equal to owner

(1) is program-before (2) and at the same memory location,
mtx->mtx_owner, so (1) happens-before (2).

And (2) is separated in program order by MUTEX_MEMBAR_ENTER from (3),
so (2) happens-before (3).

So even if the membar_consumer were intended to guarantee that (1)
happens-before (3), it's not necessary, because we can already prove
it from MUTEX_MEMBAR_ENTER.

But actually, we don't really need (1) happens-before (3), exactly;
what we really need is (2) happens-before (3), since this is a little
manifestation of Dekker's algorithm between cpu_switchto and
mutex_exit, where each CPU sets one flag and must ensure it is
visible to the other CPUs before testing the other flag -- one flag
here is the MUTEX_BIT_WAITERS bit, and the other `flag' here is the
condition owner->l_cpu->ci_curlwp == owner; the corresponding logic,
in cpu_switchto, is:

1'. store owner->l_cpu->ci_curlwp := owner
2'. load mtx->mtx_owner to test if MUTEX_BIT_WAITERS set

diffstat:

 sys/kern/kern_mutex.c |  5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diffs (26 lines):

diff -r ba5c8e84d5d4 -r 3c2c09175c13 sys/kern/kern_mutex.c
--- a/sys/kern/kern_mutex.c     Mon May 01 11:57:53 2023 +0000
+++ b/sys/kern/kern_mutex.c     Mon May 01 12:17:56 2023 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: kern_mutex.c,v 1.105 2023/04/12 06:35:40 riastradh Exp $       */
+/*     $NetBSD: kern_mutex.c,v 1.106 2023/05/01 12:17:56 riastradh Exp $       */
 
 /*-
  * Copyright (c) 2002, 2006, 2007, 2008, 2019 The NetBSD Foundation, Inc.
@@ -40,7 +40,7 @@
 #define        __MUTEX_PRIVATE
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: kern_mutex.c,v 1.105 2023/04/12 06:35:40 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: kern_mutex.c,v 1.106 2023/05/01 12:17:56 riastradh Exp $");
 
 #include <sys/param.h>
 #include <sys/atomic.h>
@@ -673,7 +673,6 @@ mutex_vector_enter(kmutex_t *mtx)
                 * If the waiters bit is not set it's unsafe to go asleep,
                 * as we might never be awoken.
                 */
-               membar_consumer();
                if (mutex_oncpu(owner)) {
                        turnstile_exit(mtx);
                        owner = mtx->mtx_owner;