Module Name: src
Committed By: riastradh
Date: Thu Feb 23 02:58:28 UTC 2023
Modified Files:
src/sys/kern: kern_descrip.c
Log Message:
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
To generate a diff of this commit:
cvs rdiff -u -r1.251 -r1.252 src/sys/kern/kern_descrip.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/kern/kern_descrip.c
diff -u src/sys/kern/kern_descrip.c:1.251 src/sys/kern/kern_descrip.c:1.252
--- src/sys/kern/kern_descrip.c:1.251 Tue Jun 29 22:40:53 2021
+++ src/sys/kern/kern_descrip.c Thu Feb 23 02:58:28 2023
@@ -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 @@ fd_putfile(unsigned fd)
* CPU.
*/
#ifndef __HAVE_ATOMIC_AS_MEMBAR
- membar_exit();
+ membar_release();
#endif
/*
@@ -602,6 +602,9 @@ fd_close(unsigned fd)
* 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 @@ fd_close(unsigned fd)
} 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 @@ fd_free(void)
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.
