Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit450c7de

Browse files
committed
Remove volatiles from {procarray,volatile}.c and fix memory ordering issue.
The use of volatiles in procarray.c largely originated from the timewhen postgres did not have reliable compiler and memorybarriers. That's not the case anymore, so we can do better.Several of the functions in procarray.c can be bottlenecks, andremoval of volatile yields mildly better code.The new state, with explicit memory barriers, is also morecorrect. The previous use of volatile did not actually deliversufficient guarantees on weakly ordered machines, in particular thelogic in GetNewTransactionId() does not look safe. It seems unlikelyto be a problem in practice, but worth fixing.Thomas and I independently wrote a patch for this.Reported-By: Andres Freund and Thomas MunroAuthor: Andres Freund, with cherrypicked changes from a patch by Thomas MunroDiscussion:https://postgr.es/m/20181005172955.wyjb4fzcdzqtaxjq@alap3.anarazel.dehttps://postgr.es/m/CAEepm=1nff0x=7i3YQO16jLA2qw-F9O39YmUew4oq-xcBQBs0g@mail.gmail.com
1 parent69ee2ff commit450c7de

File tree

2 files changed

+108
-99
lines changed

2 files changed

+108
-99
lines changed

‎src/backend/access/transam/varsup.c

Lines changed: 22 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -186,20 +186,23 @@ GetNewTransactionId(bool isSubXact)
186186
* latestCompletedXid is present in the ProcArray, which is essential for
187187
* correct OldestXmin tracking; see src/backend/access/transam/README.
188188
*
189-
* XXX by storing xid into MyPgXact without acquiring ProcArrayLock, we
190-
* are relying on fetch/store of an xid to be atomic, else other backends
191-
* might see a partially-set xid here. But holding both locks at once
192-
* would be a nasty concurrency hit. So for now, assume atomicity.
193-
*
194189
* Note that readers of PGXACT xid fields should be careful to fetch the
195190
* value only once, rather than assume they can read a value multiple
196-
* times and get the same answer each time.
191+
* times and get the same answer each time. Note we are assuming that
192+
* TransactionId and int fetch/store are atomic.
197193
*
198194
* The same comments apply to the subxact xid count and overflow fields.
199195
*
200-
* A solution to the atomic-store problem would be to give each PGXACT its
201-
* own spinlock used only for fetching/storing that PGXACT's xid and
202-
* related fields.
196+
* Use of a write barrier prevents dangerous code rearrangement in this
197+
* function; other backends could otherwise e.g. be examining my subxids
198+
* info concurrently, and we don't want them to see an invalid
199+
* intermediate state, such as an incremented nxids before the array entry
200+
* is filled.
201+
*
202+
* Other processes that read nxids should do so before reading xids
203+
* elements with a pg_read_barrier() in between, so that they can be sure
204+
* not to read an uninitialized array element; see
205+
* src/backend/storage/lmgr/README.barrier.
203206
*
204207
* If there's no room to fit a subtransaction XID into PGPROC, set the
205208
* cache-overflowed flag instead. This forces readers to look in
@@ -211,31 +214,20 @@ GetNewTransactionId(bool isSubXact)
211214
* window *will* include the parent XID, so they will deliver the correct
212215
* answer later on when someone does have a reason to inquire.)
213216
*/
217+
if (!isSubXact)
218+
MyPgXact->xid=xid;/* LWLockRelease acts as barrier */
219+
else
214220
{
215-
/*
216-
* Use volatile pointer to prevent code rearrangement; other backends
217-
* could be examining my subxids info concurrently, and we don't want
218-
* them to see an invalid intermediate state, such as incrementing
219-
* nxids before filling the array entry. Note we are assuming that
220-
* TransactionId and int fetch/store are atomic.
221-
*/
222-
volatilePGPROC*myproc=MyProc;
223-
volatilePGXACT*mypgxact=MyPgXact;
221+
intnxids=MyPgXact->nxids;
224222

225-
if (!isSubXact)
226-
mypgxact->xid=xid;
227-
else
223+
if (nxids<PGPROC_MAX_CACHED_SUBXIDS)
228224
{
229-
intnxids=mypgxact->nxids;
230-
231-
if (nxids<PGPROC_MAX_CACHED_SUBXIDS)
232-
{
233-
myproc->subxids.xids[nxids]=xid;
234-
mypgxact->nxids=nxids+1;
235-
}
236-
else
237-
mypgxact->overflowed= true;
225+
MyProc->subxids.xids[nxids]=xid;
226+
pg_write_barrier();
227+
MyPgXact->nxids=nxids+1;
238228
}
229+
else
230+
MyPgXact->overflowed= true;
239231
}
240232

241233
LWLockRelease(XidGenLock);

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp