From: | Andres Freund <andres(at)anarazel(dot)de> |
---|---|
To: | Daniel Wood <hexexpert(at)comcast(dot)net> |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Skylake-S warning |
Date: | 2018-11-10 05:01:27 |
Message-ID: | 20181110050127.vzttrewetnylps35@alap3.anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
On 2018-10-05 10:29:55 -0700, Andres Freund wrote:
> - remove the volatiles from GetSnapshotData(). As we've, for quite a
> while now, made sure both lwlocks and spinlocks are proper barriers
> they're not needed.
Attached is a patch that removes all the volatiles from procarray.c and
varsup.c. I'd started to just remove them from GetSnapshotData(), but
that doesn't seem particularly consistent.
I actually think there's a bit of a correctness problem with the
previous state - the logic in GetNewTransactionId() relies on volatile
guaranteeing memory ordering, which it doesn't do:
* 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;
}
I've replaced that with a write barrier / read barrier. I strongly
suspect this isn't a problem on the write side in practice (due to the
dependent read), but the read side looks much less clear to me. I think
explicitly using barriers is much saner these days.
Comments?
> - reduce the largely redundant flag tests. With the previous change done
> the compiler should be able to do so, but there's no reason to not
> start from somewhere sane. I'm kinda wondering about backpatching
> this part.
Done.
Greetings,
Andres Freund
Attachment | Content-Type | Size |
---|---|---|
0001-Remove-volatiles-from-procarray-volatile-.c.patch | text/x-diff | 20.7 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | David Rowley | 2018-11-10 09:29:09 | Re: Performance improvements of INSERTs to a partitioned table |
Previous Message | Amit Kapila | 2018-11-10 03:57:43 | Re: Undo logs |