Re: spinlocks on HP-UX

On Sun, Aug 28, 2011 at 8:00 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Sun, Aug 28, 2011 at 7:19 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> (IOW, +1 for inventing a second macro to use in the delay loop only.)
>
>> Beautiful.  Got a naming preference for that second macro?  I
>> suggested TAS_SPIN() because it's what you use when you spin, as
>> opposed to what you use in the uncontended case, but I'm not attached
>> to that.
>
> I had been thinking TAS_CONTENDED, but on reflection there's not a
> strong argument for that over TAS_SPIN.  Do what you will.

OK, done.  I think while we're tidying up here we ought to do
something about this comment:
*      ANOTHER CAUTION: be sure that TAS(), TAS_SPIN(), and
S_UNLOCK() represent*      sequence points, ie, loads and stores of other values must not be moved*      across a lock
orunlock.  In most cases it suffices to make 
the operation*      be done through a "volatile" pointer.

IIUC, this is basically total nonsense.  volatile only constrains the
optimizer, not the CPU; and only with regard to the ordering of one
volatile access vs. another, NOT the ordering of volatile accesses
with any non-volatile accesses that may be floating around.  So its
practical utility for synchronization purposes seems to be nearly
zero.  I think what this should say is that we expect these operations
to act as a full memory fence.  Note that in some other worlds (e.g.
Linux) a spinlock acquisition or release is only required to act as a
half-fence; that is, other loads and stores are allowed to bleed into
the critical section but not out.  However, I think we have been
assuming full-fence behavior.  In either case, claiming that the use
of a volatile pointer is all that's needed here strikes me as pretty
misleading.

In the department of loose ends, there are a bunch of other things
that maybe need cleanup here: (1) the gcc/intel compiler cases on
ia64, (2) PA-RISC, (3) ARM, (4) PowerPC... and we should also perhaps
reconsider the 32-bit x86 case.  Right now TAS() says:
       /*        * Use a non-locking test before asserting the bus lock.  Note that the        * extra test appears to
bea small loss on some x86 platforms 
and a small        * win on others; it's by no means clear that we should keep it.        */

I can't get too excited about spending a lot of effort optimizing
32-bit PostgreSQL on any architecture at this point, but if someone
else is, we might want to check whether it makes more sense to do the
non-locking test only in TAS_SPIN().

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company