Code review request: small optimization to localtime.c

Bruce Evans brde at optusnet.com.au
Mon Dec 17 08:25:06 PST 2007


On Sun, 16 Dec 2007, M. Warner Losh wrote:

> In message: <20071204085502.N83722 at tribble.ilrt.bris.ac.uk>
>            Jan Grant <jan.grant at bristol.ac.uk> writes:
> : On Mon, 3 Dec 2007, Alfred Perlstein wrote:
> :
> : [on the double-checked locking idiom]
> :
> : > Karsten, _typically_ (but not always) an "unlock" operation
> : > requires that writes prior to the unlock be globally visible.
> : >
> : > This is why it works almost everywhere.
> :
> : Perhaps, but if you use it you should probably mark the code with
> : 	/* XXX not guaranteed to be correct by POSIX */
> :
> : Double-checked locking is broken without an appropriate barrier.
> : "Correctness over speed" should surely be our watchword :-)
>
> Actually, the code I posted for review *IS* posixly correct.

No, bugs in it were pointed out in the review.

> It doesn't matter if the write posts or not.  If it doesn't post, then
> we know the guard variable will be false still and we take out the
> lock, test it see that it is true (since nothing would work well if
> the lock/unlock pairs didn't force a consistent variable after the
> lock is released).  If it is posted, we don't take the branch.

That works for the code that tests the variable, but not for the code
that sets it.

> Since these variables are initialized to zero and set exactly once to
> true, the above is true.

IIRC, the code that sets the variable does something like:

 	initialized = 1;
 	actually_do_the_initialization();

It would be little better to do:

 	actually_do_the_initialization();
 	initialized = 1;

since the code that tests the variable may see the changes in any order,
because it doesn't do any locking in cases where it sees the variiable at 1.

Either order would work if everything used locking, but the point of the
optimization is to not use any locking in most paths.

Bruce


More information about the freebsd-arch mailing list