Re: Code review request: small optimization to localtime.c

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

In message: <20071204085502.N83722@xxxxxxxxxxxxxxxxxxxxxxx>
Jan Grant <jan.grant@xxxxxxxxxxxxx> 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
_______________________________________________
freebsd-arch@xxxxxxxxxxx mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "freebsd-arch-unsubscribe@xxxxxxxxxxx"

Relevant Pages

  • Re: using clustered index to optimize inserts ...
    ... I will try to explain locking in terms of Sybase docs... ... Allpages Locking: Allpages locking locks both data pages and index ... the data page is locked with an exclusive lock. ... Clustered Index: The datarows will be arranged as per the clustered ...
    (comp.databases.sybase)
  • Re: CSingleLock - known behaviour?
    ... It is better to design code that doesn't require locking. ... If you don't need the resource, don't lock it. ... magnitude less efficient, than locking once. ...
    (microsoft.public.vc.mfc)
  • Re: Strange multi-user timing phenomenon
    ... table record with pessimistic locking and both edit the same record. ... the recordset will be sufficient to lock out other users. ... update the recordset (if you had to edit data) and then close it. ...
    (microsoft.public.access.formscoding)
  • Re: Strange multi-user timing phenomenon
    ... I tried your locking method using a perssimistic recordset and it appears to ... user places his lock and grabs the same record. ... I would try opening a fixed single-row recordset with pessimistic locking, ...
    (microsoft.public.access.formscoding)
  • Re: CMultiLock example
    ... Seriously, though, locking is *mandatory* if two or more threads are accessing non-scalar ... lists ever be modified while the threads are running. ... without a lock. ... You must prevent that one piece of data is modified from multiple ...
    (microsoft.public.vc.mfc)