[src/trunk]: src/sys/kern kern_descrip.c: Fix membars around reference count ...

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

details:   https://anonhg.NetBSD.org/src/rev/1c7054a17a0c
branches:  trunk
changeset: 373652:1c7054a17a0c
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Thu Feb 23 02:58:28 2023 +0000

description:
kern_descrip.c: Fix membars around reference count decrement.

In general, the `last one out hit the lights' style of reference
counting (as opposed to the `whoever's destroying must wait for
pending users to finish' style) requires memory barriers like so:

        ... usage of resources associated with object ...
        membar_release();
        if (atomic_dec_uint_nv(&obj->refcnt) != 0)
                return;
        membar_acquire();
        ... freeing of resources associated with object ...

This way, all usage happens-before all freeing.  This fixes several
errors:

- fd_close failed to ensure whatever its caller did would
  happen-before the freeing, in the case where another thread is
  concurrently trying to close the fd (ff->ff_file == NULL).

  Fix: Add membar_release before atomic_dec_uint(&ff->ff_refcnt) in
  that branch.

- fd_close failed to ensure all loads its caller had issued will have
  happened-before the freeing, in the case where the fd is still in
  use by another thread (fdp->fd_refcnt > 1 and ff->ff_refcnt-- > 0).

  Fix: Change membar_producer to membar_release before
  atomic_dec_uint(&ff->ff_refcnt).

- fd_close failed to ensure that any usage of fp by other callers
  would happen-before any freeing it does.

  Fix: Add membar_acquire after atomic_dec_uint_nv(&ff->ff_refcnt).

- fd_free failed to ensure that any usage of fdp by other callers
  would happen-before any freeing it does.

  Fix: Add membar_acquire after atomic_dec_uint_nv(&fdp->fd_refcnt).

While here, change membar_exit -> membar_release.  No semantic
change, just updating away from the legacy API.

XXX pullup-8
XXX pullup-9
XXX pullup-10

diffstat:

 sys/kern/kern_descrip.c |  19 ++++++++++++++-----
 1 files changed, 14 insertions(+), 5 deletions(-)

diffs (66 lines):

diff -r e96d8521a524 -r 1c7054a17a0c sys/kern/kern_descrip.c
--- a/sys/kern/kern_descrip.c   Thu Feb 23 02:57:17 2023 +0000
+++ b/sys/kern/kern_descrip.c   Thu Feb 23 02:58:28 2023 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: kern_descrip.c,v 1.251 2021/06/29 22:40:53 dholland Exp $      */
+/*     $NetBSD: kern_descrip.c,v 1.252 2023/02/23 02:58:28 riastradh Exp $     */
 
 /*-
  * Copyright (c) 2008, 2009 The NetBSD Foundation, Inc.
@@ -70,7 +70,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: kern_descrip.c,v 1.251 2021/06/29 22:40:53 dholland Exp $");
+__KERNEL_RCSID(0, "$NetBSD: kern_descrip.c,v 1.252 2023/02/23 02:58:28 riastradh Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -451,7 +451,7 @@
         * CPU.
         */
 #ifndef __HAVE_ATOMIC_AS_MEMBAR
-       membar_exit();
+       membar_release();
 #endif
 
        /*
@@ -602,6 +602,9 @@
                 * waiting for other users of the file to drain.  Release
                 * our reference, and wake up the closer.
                 */
+#ifndef __HAVE_ATOMIC_AS_MEMBAR
+               membar_release();
+#endif
                atomic_dec_uint(&ff->ff_refcnt);
                cv_broadcast(&ff->ff_closing);
                mutex_exit(&fdp->fd_lock);
@@ -637,9 +640,12 @@
        } else {
                /* Multi threaded. */
 #ifndef __HAVE_ATOMIC_AS_MEMBAR
-               membar_producer();
+               membar_release();
 #endif
                refcnt = atomic_dec_uint_nv(&ff->ff_refcnt);
+#ifndef __HAVE_ATOMIC_AS_MEMBAR
+               membar_acquire();
+#endif
        }
        if (__predict_false(refcnt != 0)) {
                /*
@@ -1532,10 +1538,13 @@
        KASSERT(fdp->fd_dtbuiltin.dt_link == NULL);
 
 #ifndef __HAVE_ATOMIC_AS_MEMBAR
-       membar_exit();
+       membar_release();
 #endif
        if (atomic_dec_uint_nv(&fdp->fd_refcnt) > 0)
                return;
+#ifndef __HAVE_ATOMIC_AS_MEMBAR
+       membar_acquire();
+#endif
 
        /*
         * Close any files that the process holds open.