About the memory barrier in BSD libc
Konstantin Belousov
kostikbel at gmail.com
Tue Apr 24 16:58:54 UTC 2012
On Tue, Apr 24, 2012 at 11:10:13AM -0400, John Baldwin wrote:
> Could you move the extern and THREAD_LOCK/UNLOCK macros into the stdio
> private header file?
This requires more glue because spinlock.h is separate from libc_private.h.
I also added a comment trying to explain why assignment is braced with lock.
diff --git a/lib/libc/include/libc_private.h b/lib/libc/include/libc_private.h
index d4b24a7..2182f46 100644
--- a/lib/libc/include/libc_private.h
+++ b/lib/libc/include/libc_private.h
@@ -81,6 +81,19 @@ void _rtld_error(const char *fmt, ...);
#define FLOCKFILE(fp) if (__isthreaded) _FLOCKFILE(fp)
#define FUNLOCKFILE(fp) if (__isthreaded) _funlockfile(fp)
+struct _spinlock;
+extern struct _spinlock __stdio_thread_lock;
+#define STDIO_THREAD_LOCK() \
+do { \
+ if (__isthreaded) \
+ _SPINLOCK(&__stdio_thread_lock); \
+} while (0)
+#define STDIO_THREAD_UNLOCK() \
+do { \
+ if (__isthreaded) \
+ _SPINUNLOCK(&__stdio_thread_lock); \
+} while (0)
+
/*
* Indexes into the pthread jump table.
*
diff --git a/lib/libc/stdio/fclose.c b/lib/libc/stdio/fclose.c
index f0629e8..3957b6a 100644
--- a/lib/libc/stdio/fclose.c
+++ b/lib/libc/stdio/fclose.c
@@ -41,6 +41,7 @@ __FBSDID("$FreeBSD$");
#include <stdio.h>
#include <stdlib.h>
#include "un-namespace.h"
+#include <spinlock.h>
#include "libc_private.h"
#include "local.h"
@@ -65,7 +66,20 @@ fclose(FILE *fp)
FREELB(fp);
fp->_file = -1;
fp->_r = fp->_w = 0; /* Mess up if reaccessed. */
+
+ /*
+ * Lock the spinlock used to protect __sglue list walk in
+ * __sfp(). The __sfp() uses fp->_flags == 0 test as an
+ * indication of the unused FILE.
+ *
+ * Taking the lock prevents possible compiler or processor
+ * reordering of the writes performed before the final _flags
+ * cleanup, making sure that we are done with the FILE before
+ * it is considered available.
+ */
+ STDIO_THREAD_LOCK();
fp->_flags = 0; /* Release this FILE for reuse. */
+ STDIO_THREAD_UNLOCK();
FUNLOCKFILE(fp);
return (r);
}
diff --git a/lib/libc/stdio/findfp.c b/lib/libc/stdio/findfp.c
index 89c0536..6d0b673 100644
--- a/lib/libc/stdio/findfp.c
+++ b/lib/libc/stdio/findfp.c
@@ -82,9 +82,7 @@ static struct glue *lastglue = &uglue;
static struct glue * moreglue(int);
-static spinlock_t thread_lock = _SPINLOCK_INITIALIZER;
-#define THREAD_LOCK() if (__isthreaded) _SPINLOCK(&thread_lock)
-#define THREAD_UNLOCK() if (__isthreaded) _SPINUNLOCK(&thread_lock)
+spinlock_t __stdio_thread_lock = _SPINLOCK_INITIALIZER;
#if NOT_YET
#define SET_GLUE_PTR(ptr, val) atomic_set_rel_ptr(&(ptr), (uintptr_t)(val))
@@ -127,22 +125,22 @@ __sfp()
/*
* The list must be locked because a FILE may be updated.
*/
- THREAD_LOCK();
+ STDIO_THREAD_LOCK();
for (g = &__sglue; g != NULL; g = g->next) {
for (fp = g->iobs, n = g->niobs; --n >= 0; fp++)
if (fp->_flags == 0)
goto found;
}
- THREAD_UNLOCK(); /* don't hold lock while malloc()ing. */
+ STDIO_THREAD_UNLOCK(); /* don't hold lock while malloc()ing. */
if ((g = moreglue(NDYNAMIC)) == NULL)
return (NULL);
- THREAD_LOCK(); /* reacquire the lock */
+ STDIO_THREAD_LOCK(); /* reacquire the lock */
SET_GLUE_PTR(lastglue->next, g); /* atomically append glue to list */
lastglue = g; /* not atomic; only accessed when locked */
fp = g->iobs;
found:
fp->_flags = 1; /* reserve this slot; caller sets real flags */
- THREAD_UNLOCK();
+ STDIO_THREAD_UNLOCK();
fp->_p = NULL; /* no current pointer */
fp->_w = 0; /* nothing to read or write */
fp->_r = 0;
@@ -183,10 +181,10 @@ f_prealloc(void)
for (g = &__sglue; (n -= g->niobs) > 0 && g->next; g = g->next)
/* void */;
if ((n > 0) && ((g = moreglue(n)) != NULL)) {
- THREAD_LOCK();
+ STDIO_THREAD_LOCK();
SET_GLUE_PTR(lastglue->next, g);
lastglue = g;
- THREAD_UNLOCK();
+ STDIO_THREAD_UNLOCK();
}
}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 196 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-threads/attachments/20120424/a02df4d3/attachment.pgp
More information about the freebsd-threads
mailing list