From owner-freebsd-threads@FreeBSD.ORG Wed Apr 25 06:26:32 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 3ABA0106566C for ; Wed, 25 Apr 2012 06:26:32 +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 C9E808FC17 for ; Wed, 25 Apr 2012 06:26:31 +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 q3P6QSfg046705; Wed, 25 Apr 2012 09:26:28 +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 q3P6QRbD044397; Wed, 25 Apr 2012 09:26:27 +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 q3P6QRc2044396; Wed, 25 Apr 2012 09:26:27 +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: Wed, 25 Apr 2012 09:26:27 +0300 From: Konstantin Belousov To: Fengwei yin Message-ID: <20120425062627.GI2358@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> <20120424165842.GZ2358@deviant.kiev.zoral.com.ua> <201204241800.q3OI0X89007384@higson.cam.lispworks.com> <20120424181302.GB2358@deviant.kiev.zoral.com.ua> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="REd8r87flCTRYq/l" Content-Disposition: inline In-Reply-To: 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: Wed, 25 Apr 2012 06:26:32 -0000 --REd8r87flCTRYq/l Content-Type: text/plain; charset=koi8-r Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Apr 25, 2012 at 11:25:40AM +0800, Fengwei yin wrote: > On Wed, Apr 25, 2012 at 2:13 AM, Konstantin Belousov > wrote: > > On Tue, Apr 24, 2012 at 07:00:33PM +0100, Martin Simmons wrote: > >> >>>>> On Tue, 24 Apr 2012 19:58:42 +0300, Konstantin Belousov said: > >> > > >> > + > >> > + =9A /* > >> > + =9A =9A* Lock the spinlock used to protect __sglue list walk in > >> > + =9A =9A* __sfp(). =9AThe __sfp() uses fp->_flags =3D=3D 0 test as = an > >> > + =9A =9A* indication of the unused FILE. > >> > + =9A =9A* > >> > + =9A =9A* Taking the lock prevents possible compiler or processor > >> > + =9A =9A* reordering of the writes performed before the final _flags > >> > + =9A =9A* cleanup, making sure that we are done with the FILE before > >> > + =9A =9A* it is considered available. > >> > + =9A =9A*/ > >> > + =9A STDIO_THREAD_LOCK(); > >> > =9A =9A fp->_flags =3D 0; =9A =9A =9A =9A /* Release this FILE for r= euse. */ > >> > + =9A STDIO_THREAD_UNLOCK(); > >> > =9A =9A FUNLOCKFILE(fp); > >> > =9A =9A return (r); > >> > >> Is that assumption about reordering correct? =9AI.e. is FreeBSD's spin= lock a > >> two-way barrier? > >> > >> It isn't unusual for the locking primitive to be a one-way barrier, i.= e. (from > >> Linux kernel memory-barriers.txt) "Memory operations that occur before= a LOCK > >> operation may appear to happen after it completes." =9ASee also acq an= d rel in > >> atomic(9). > > Taking the lock prevents the __sfp from iterating the list until the > > spinlock is released. Since release makes sure that all previous memory > > operations become visible, the acquire in the spinlock lock provides > > the neccesary guarentee. >=20 > IMHO, the lock to me is too heavy here. What about this patch? >=20 > NOTE: patch just show thoughts. I didn't even check build checking. Yes, it might be correct. But FreeBSD does prefer the acq/rel barriers over the rmb/wmb. Also, the lock is not that heavy right there, and the committed patch provides mostly zero overhead for non-threaded case. >=20 > diff --git a/lib/libc/stdio/fclose.c b/lib/libc/stdio/fclose.c > index f0629e8..a26f944 100644 > --- a/lib/libc/stdio/fclose.c > +++ b/lib/libc/stdio/fclose.c > @@ -38,6 +38,7 @@ __FBSDID("$FreeBSD$"); >=20 > #include "namespace.h" > #include > +#include > #include > #include > #include "un-namespace.h" > @@ -65,6 +66,7 @@ fclose(FILE *fp) > FREELB(fp); > fp->_file =3D -1; > fp->_r =3D fp->_w =3D 0; /* Mess up if reaccessed. */ > + wmb(); > fp->_flags =3D 0; /* Release this FILE for reuse. */ > FUNLOCKFILE(fp); > return (r); > diff --git a/lib/libc/stdio/findfp.c b/lib/libc/stdio/findfp.c > index 89c0536..03b2945 100644 > --- a/lib/libc/stdio/findfp.c > +++ b/lib/libc/stdio/findfp.c > @@ -129,9 +129,16 @@ __sfp() > */ > 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) > + for (fp =3D g->iobs, n =3D g->niobs; --n >=3D 0; fp++) { > + int __flags =3D fp->_flags; > + rmb(); > + /* > + * If could see __flags is zero here, we are sure > + * the cleanup in fclose is done. > + */ > + if (__flags =3D=3D 0) > goto found; > + } > } > THREAD_UNLOCK(); /* don't hold lock while malloc()ing. */ > if ((g =3D moreglue(NDYNAMIC)) =3D=3D NULL) --REd8r87flCTRYq/l Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (FreeBSD) iEYEARECAAYFAk+XmRMACgkQC3+MBN1Mb4jXWQCfTn0XopbMM1f3hqyZQOP46LZW 1jsAn2QhZGPePVNspHwvZ5oeQvSChtWA =ANAw -----END PGP SIGNATURE----- --REd8r87flCTRYq/l--