svn commit: r194783 - head/lib/libc/stdtime
John Baldwin
jhb at freebsd.org
Thu Nov 19 21:55:34 UTC 2009
On Wednesday 18 November 2009 8:41:54 am John Baldwin wrote:
> 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.
Alan Cox suggested that it might be best to provide an abstraction for this
sort of thing rather than having N copies of using various memory barriers
and atomic ops, etc. This patch adds a new internal method to libc _once()
that is just like pthread_once(). It is called _once() because it has some
extra trickery to use pthread_once() for a multithreaded process and to use
an internal stub version for single-threaded processes. I also converted the
gmt_is_set stuff to use this instead. This should restore the lockless code
for the common case. Edwin, can you test this against the bug you were
seeing?
--
John Baldwin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: stdtime.patch
Type: text/x-diff
Size: 6112 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/svn-src-all/attachments/20091119/7e9d6df2/stdtime.bin
More information about the svn-src-all
mailing list