Date: Wed, 18 Nov 2009 08:41:54 -0500 From: John Baldwin <jhb@freebsd.org> To: Jilles Tjoelker <jilles@stack.nl> Cc: svn-src-head@freebsd.org, Dmitry Pryanishnikov <lynx.ripe@gmail.com>, svn-src-all@freebsd.org, src-committers@freebsd.org, Edwin Groothuis <edwin@freebsd.org> Subject: Re: svn commit: r194783 - head/lib/libc/stdtime Message-ID: <200911180841.55183.jhb@freebsd.org> In-Reply-To: <20091117182501.GA70742@stack.nl> References: <4B01E548.7040708@gmail.com> <20091117182501.GA70742@stack.nl>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tuesday 17 November 2009 1:25:01 pm Jilles Tjoelker wrote: > On Tue, Nov 17, 2009 at 01:50:32AM +0200, Dmitry Pryanishnikov wrote: > > > Author: edwin > > > Date: Tue Jun 23 22:28:44 2009 > > > New Revision: 194783 > > > URL: http://svn.freebsd.org/changeset/base/194783 > > > > Log: > > > Remove duplicate if-statement on gmt_is_set in gmtsub(). > > > > MFC after: 1 week > > > > Modified: > > > head/lib/libc/stdtime/localtime.c > > > This change looks like a (small?) pessimization to me: before it, > > _MUTEX_LOCK/_MUTEX_UNLOCK pair would be skipped for the case gmt_is_set > > == TRUE (all invocations except the first one), now it won't. I'm not > > sure whether this is critical here though... > > It is certainly less efficient, but the old code was (most likely) > wrong. It used an idiom known as "double checked locking", which is > incorrect in most memory models. The problem is that the store to > gmt_is_set may become visible without stores to other memory (gmtptr and > what it points to) becoming visible. That is easily fixed with a memory barrier. Just use atomic_store_rel() to set gmt_is_set at the end of the setup phase. -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200911180841.55183.jhb>