On Sun, Aug 28, 2011 at 8:00 PM, Tom Lane <t...@sss.pgh.pa.us> wrote:
> Robert Haas <robertmh...@gmail.com> writes:
>> On Sun, Aug 28, 2011 at 7:19 PM, Tom Lane <t...@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 or unlock. 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 be a 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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
