Re: [HACKERS] A misconception about the meaning of 'volatile' in GetNewTransactionId?

[Tom Lane]

Thomas Munro  writes:
> I was reading xact.c and noticed this block:
> ...
> Isn't this insufficient on non-TSO systems like POWER and Arm?

Yeah, I think you're right.  That code probably predates our support
for memory barriers, so "volatile" was the best we could do at the
time --- but as you say, it doesn't fix hardware-level rearrangements.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] A misconception about the meaning of 'volatile' in GetNewTransactionId?

[Thomas Munro]

Hi hackers,

I was reading xact.c and noticed this block:

/*
 * Use volatile pointer to prevent code rearrangement;
other backends
 * could be examining my subxids info concurrently,
and we don't want
 * them to see an invalid intermediate state, such as
incrementing
 * nxids before filling the array entry.  Note we are
assuming that
 * TransactionId and int fetch/store are atomic.
 */
volatile PGPROC *myproc = MyProc;
volatile PGXACT *mypgxact = MyPgXact;

if (!isSubXact)
mypgxact->xid = xid;
else
{
int nxids = mypgxact->nxids;

if (nxids < PGPROC_MAX_CACHED_SUBXIDS)
{
myproc->subxids.xids[nxids] = xid;
mypgxact->nxids = nxids + 1;

Isn't this insufficient on non-TSO systems like POWER and Arm?  It
uses volatile qualifiers as a compiler barrier, which is probably
enough for x86 and Sparc in TSO mode, but doesn't include a memory
barrier to prevent hardware reordering.

I think the thing to do here would be to forget about volatile, stick
pg_write_barrier() between those two writes, and stick
pg_read_barrier() between the reads in any code that might read nxids
and then scan xids concurrently, such as TransactionIdIsInProgress().

This is almost exactly the example from the section "Avoiding Memory
Order Bugs" in src/backend/storage/lmgr/README.barrier.

Thoughts?

-- 
Thomas Munro
http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers