Date: Tue, 24 Apr 2012 19:58:42 +0300 From: Konstantin Belousov <kostikbel@gmail.com> To: John Baldwin <jhb@freebsd.org> Cc: jack.ren@intel.com, freebsd-threads@freebsd.org Subject: Re: About the memory barrier in BSD libc Message-ID: <20120424165842.GZ2358@deviant.kiev.zoral.com.ua> In-Reply-To: <201204241110.14017.jhb@freebsd.org> References: <20120423084120.GD76983@zxy.spb.ru> <201204241343.q3ODhe2C032683@higson.cam.lispworks.com> <20120424140348.GY2358@deviant.kiev.zoral.com.ua> <201204241110.14017.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--tKmWtG5s+ZnuAuMT
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
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_privat=
e.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)
=20
+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"
=20
@@ -65,7 +66,20 @@ fclose(FILE *fp)
FREELB(fp);
fp->_file =3D -1;
fp->_r =3D fp->_w =3D 0; /* Mess up if reaccessed. */
+
+ /*
+ * Lock the spinlock used to protect __sglue list walk in
+ * __sfp(). The __sfp() uses fp->_flags =3D=3D 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 =3D 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 =3D &uglue;
=20
static struct glue * moreglue(int);
=20
-static spinlock_t thread_lock =3D _SPINLOCK_INITIALIZER;
-#define THREAD_LOCK() if (__isthreaded) _SPINLOCK(&thread_lock)
-#define THREAD_UNLOCK() if (__isthreaded) _SPINUNLOCK(&thread_lock)
+spinlock_t __stdio_thread_lock =3D _SPINLOCK_INITIALIZER;
=20
#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 =3D &__sglue; g !=3D NULL; g =3D g->next) {
for (fp =3D g->iobs, n =3D g->niobs; --n >=3D 0; fp++)
if (fp->_flags =3D=3D 0)
goto found;
}
- THREAD_UNLOCK(); /* don't hold lock while malloc()ing. */
+ STDIO_THREAD_UNLOCK(); /* don't hold lock while malloc()ing. */
if ((g =3D moreglue(NDYNAMIC)) =3D=3D 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 =3D g; /* not atomic; only accessed when locked */
fp =3D g->iobs;
found:
fp->_flags =3D 1; /* reserve this slot; caller sets real flags */
- THREAD_UNLOCK();
+ STDIO_THREAD_UNLOCK();
fp->_p =3D NULL; /* no current pointer */
fp->_w =3D 0; /* nothing to read or write */
fp->_r =3D 0;
@@ -183,10 +181,10 @@ f_prealloc(void)
for (g =3D &__sglue; (n -=3D g->niobs) > 0 && g->next; g =3D g->next)
/* void */;
if ((n > 0) && ((g =3D moreglue(n)) !=3D NULL)) {
- THREAD_LOCK();
+ STDIO_THREAD_LOCK();
SET_GLUE_PTR(lastglue->next, g);
lastglue =3D g;
- THREAD_UNLOCK();
+ STDIO_THREAD_UNLOCK();
}
}
=20
--tKmWtG5s+ZnuAuMT
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (FreeBSD)
iEYEARECAAYFAk+W28EACgkQC3+MBN1Mb4gMfgCfaX0Wy+vGyU9mCRfi4p3D+WVa
Hw8AoL4tyq96n3qE7t1iJ1a3g6UkP9PT
=HMbW
-----END PGP SIGNATURE-----
--tKmWtG5s+ZnuAuMT--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120424165842.GZ2358>