From owner-freebsd-threads@FreeBSD.ORG Wed Apr 25 03:25:42 2012 Return-Path: Delivered-To: freebsd-threads@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 009EC106566C for ; Wed, 25 Apr 2012 03:25:41 +0000 (UTC) (envelope-from yfw.bsd@gmail.com) Received: from mail-ob0-f182.google.com (mail-ob0-f182.google.com [209.85.214.182]) by mx1.freebsd.org (Postfix) with ESMTP id B38638FC21 for ; Wed, 25 Apr 2012 03:25:41 +0000 (UTC) Received: by obbuo13 with SMTP id uo13so2378052obb.13 for ; Tue, 24 Apr 2012 20:25:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; bh=GOLP4V6A/TYfb3BSmbejQGlYmAOvIj70bye12k6pHc4=; b=mWtiBcC9X2CcCkyMi1jOoQrlDhSeDJuNGREnzCW7JV+IJbAsHGfRlZisNBA37dP+7z aXgDsBQfzV062wIVbZfpbQrrLePxDjip1+a0GDWfnmU+xJQc3i5oSdwQNpv2T9sFySfd ktzfAOSWv+Dkcec3e+3gEBNsk6R8BmwxJ2OLim5siaZGltu+3Q6PNb7QjCFb88Qb+JUZ hseJO3kz+S5gKN+cr+IqqZfFyZvD1GfhQizFoa6K+sg6MQcIsvu72M+K3uCPJKOhK1Tb 2YO9nT9I0TWfnmBEB1g5XzEFlMiZUGs/tnItovpa90pfjqUod7XN5+IGbp99N2Zk+uTa FtVA== MIME-Version: 1.0 Received: by 10.182.232.38 with SMTP id tl6mr1431864obc.16.1335324341113; Tue, 24 Apr 2012 20:25:41 -0700 (PDT) Received: by 10.60.125.135 with HTTP; Tue, 24 Apr 2012 20:25:40 -0700 (PDT) In-Reply-To: <20120424181302.GB2358@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> Date: Wed, 25 Apr 2012 11:25:40 +0800 Message-ID: From: Fengwei yin To: Konstantin Belousov Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable 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 03:25:42 -0000 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: >> > >> > + >> > + =A0 /* >> > + =A0 =A0* Lock the spinlock used to protect __sglue list walk in >> > + =A0 =A0* __sfp(). =A0The __sfp() uses fp->_flags =3D=3D 0 test as an >> > + =A0 =A0* indication of the unused FILE. >> > + =A0 =A0* >> > + =A0 =A0* Taking the lock prevents possible compiler or processor >> > + =A0 =A0* reordering of the writes performed before the final _flags >> > + =A0 =A0* cleanup, making sure that we are done with the FILE before >> > + =A0 =A0* it is considered available. >> > + =A0 =A0*/ >> > + =A0 STDIO_THREAD_LOCK(); >> > =A0 =A0 fp->_flags =3D 0; =A0 =A0 =A0 =A0 /* Release this FILE for reu= se. */ >> > + =A0 STDIO_THREAD_UNLOCK(); >> > =A0 =A0 FUNLOCKFILE(fp); >> > =A0 =A0 return (r); >> >> Is that assumption about reordering correct? =A0I.e. is FreeBSD's spinlo= ck 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." =A0See also acq and = 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. IMHO, the lock to me is too heavy here. What about this patch? NOTE: patch just show thoughts. I didn't even check build checking. 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$"); #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)