svn commit: r194783 - head/lib/libc/stdtime

[John Baldwin]

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