Enforce proper memory ordering in refcnt_rele() and refcnt_finalize()

Make refcnt_rele() and refcnt_finalize() order memory operations so that
preceding loads and stores happen before 1->0 transition. Also ensure
that loads and stores that depend on the transition really begin only
after the transition has occurred. Otherwise the object destructor might
not see the object's latest state.

OK bluhm@

diff --git a/share/man/man9/refcnt_init.9 b/share/man/man9/refcnt_init.9
index cd19b15..22bf643 100644 (file)
--- a/share/man/man9/refcnt_init.9
+++ b/share/man/man9/refcnt_init.9
@@ -1,4 +1,4 @@
-.\"    $OpenBSD: refcnt_init.9,v 1.2 2022/03/16 14:13:01 visa Exp $
+.\"    $OpenBSD: refcnt_init.9,v 1.3 2022/04/30 14:44:04 visa Exp $
 .\"
 .\" Copyright (c) 2015 David Gwynne <dlg@openbsd.org>
 .\"
@@ -14,7 +14,7 @@
 .\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
 .\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 .\"
-.Dd $Mdocdate: March 16 2022 $
+.Dd $Mdocdate: April 30 2022 $
 .Dt REFCNT_INIT 9
 .Os
 .Sh NAME
@@ -74,6 +74,17 @@ There may only be one caller to
 per refcnt
 .Fa r .
 .Pp
+.Fn refcnt_rele ,
+.Fn refcnt_rele_wake
+and
+.Fn refcnt_finalize
+order prior memory loads and stores before the release of the reference.
+The functions enforce control dependency so that after the final reference
+has been released, subsequent loads and stores happen after the release.
+These ensure that concurrent accesses cease before the object's destructor
+runs and that the destructor sees all updates done during the lifetime
+of the object.
+.Pp
 .Fn refcnt_shared
 tests if the object has multiple references.
 .Pp

diff --git a/sys/kern/kern_synch.c b/sys/kern/kern_synch.c
index 243b820..218fb9d 100644 (file)
--- a/sys/kern/kern_synch.c
+++ b/sys/kern/kern_synch.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: kern_synch.c,v 1.185 2022/03/18 15:32:06 bluhm Exp $  */
+/*     $OpenBSD: kern_synch.c,v 1.186 2022/04/30 14:44:04 visa Exp $   */
 /*     $NetBSD: kern_synch.c,v 1.37 1996/04/22 01:38:37 christos Exp $ */
 
 /*
@@ -822,9 +822,14 @@ refcnt_rele(struct refcnt *r)
 {
        u_int refs;
 
+       membar_exit_before_atomic();
        refs = atomic_dec_int_nv(&r->r_refs);
        KASSERT(refs != ~0);
-       return (refs == 0);
+       if (refs == 0) {
+               membar_enter_after_atomic();
+               return (1);
+       }
+       return (0);
 }
 
 void
@@ -840,6 +845,7 @@ refcnt_finalize(struct refcnt *r, const char *wmesg)
        struct sleep_state sls;
        u_int refs;
 
+       membar_exit_before_atomic();
        refs = atomic_dec_int_nv(&r->r_refs);
        KASSERT(refs != ~0);
        while (refs) {
@@ -847,6 +853,8 @@ refcnt_finalize(struct refcnt *r, const char *wmesg)
                refs = atomic_load_int(&r->r_refs);
                sleep_finish(&sls, refs);
        }
+       /* Order subsequent loads and stores after refs == 0 load. */
+       membar_sync();
 }
 
 int