From owner-freebsd-threads@FreeBSD.ORG Tue Apr 24 16:58:54 2012 Return-Path: Delivered-To: freebsd-threads@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 5BFE11065675; Tue, 24 Apr 2012 16:58:54 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from mail.zoral.com.ua (mx0.zoral.com.ua [91.193.166.200]) by mx1.freebsd.org (Postfix) with ESMTP id EB3D78FC0A; Tue, 24 Apr 2012 16:58:53 +0000 (UTC) Received: from skuns.kiev.zoral.com.ua (localhost [127.0.0.1]) by mail.zoral.com.ua (8.14.2/8.14.2) with ESMTP id q3OGwh5C070035; Tue, 24 Apr 2012 19:58:43 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: from deviant.kiev.zoral.com.ua (kostik@localhost [127.0.0.1]) by deviant.kiev.zoral.com.ua (8.14.5/8.14.5) with ESMTP id q3OGwgrY039057; Tue, 24 Apr 2012 19:58:42 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: (from kostik@localhost) by deviant.kiev.zoral.com.ua (8.14.5/8.14.5/Submit) id q3OGwgeY039056; Tue, 24 Apr 2012 19:58:42 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: deviant.kiev.zoral.com.ua: kostik set sender to kostikbel@gmail.com using -f Date: Tue, 24 Apr 2012 19:58:42 +0300 From: Konstantin Belousov To: John Baldwin Message-ID: <20120424165842.GZ2358@deviant.kiev.zoral.com.ua> References: <20120423084120.GD76983@zxy.spb.ru> <201204241343.q3ODhe2C032683@higson.cam.lispworks.com> <20120424140348.GY2358@deviant.kiev.zoral.com.ua> <201204241110.14017.jhb@freebsd.org> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="tKmWtG5s+ZnuAuMT" Content-Disposition: inline In-Reply-To: <201204241110.14017.jhb@freebsd.org> User-Agent: Mutt/1.4.2.3i X-Virus-Scanned: clamav-milter 0.95.2 at skuns.kiev.zoral.com.ua X-Virus-Status: Clean X-Spam-Status: No, score=-4.0 required=5.0 tests=ALL_TRUSTED,AWL,BAYES_00 autolearn=ham version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on skuns.kiev.zoral.com.ua Cc: jack.ren@intel.com, freebsd-threads@freebsd.org Subject: Re: About the memory barrier in BSD libc X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 24 Apr 2012 16:58:54 -0000 --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 #include #include "un-namespace.h" +#include #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--