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

Commit3e485fe

Browse files
anarazelAlexander Korotkov
authored and
Alexander Korotkov
committed
Avoid atomic operation in MarkLocalBufferDirty().
The recent patch to make Pin/UnpinBuffer lockfree in the hotpath (4835458), accidentally used pg_atomic_fetch_or_u32() inMarkLocalBufferDirty(). Other code operating on local buffers wascareful to only use pg_atomic_read/write_u32 which just read/write frommemory; to avoid unnecessary overhead.On its own that'd just make MarkLocalBufferDirty() slightly lessefficient, but in addition InitLocalBuffers() doesn't callpg_atomic_init_u32() - thus the spinlock fallback for the atomicoperations isn't initialized. That in turn caused, as reported by Tom,buildfarm animal gaur to fail. As those errors are actually usefulagainst this type of error, continue to omit - intentionally this time -initialization of the atomic variable.In addition, add an explicit note about only using pg_atomic_read/writeon local buffers's state to BufferDesc's description.Reported-By: Tom LaneDiscussion: 1881.1460431476@sss.pgh.pa.us
1 parent5a24efd commit3e485fe

File tree

2 files changed

+23
-1
lines changed

2 files changed

+23
-1
lines changed

‎src/backend/storage/buffer/localbuf.c

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -295,10 +295,14 @@ MarkLocalBufferDirty(Buffer buffer)
295295

296296
bufHdr=GetLocalBufferDescriptor(bufid);
297297

298-
buf_state=pg_atomic_fetch_or_u32(&bufHdr->state,BM_DIRTY);
298+
buf_state=pg_atomic_read_u32(&bufHdr->state);
299299

300300
if (!(buf_state&BM_DIRTY))
301301
pgBufferUsage.local_blks_dirtied++;
302+
303+
buf_state |=BM_DIRTY;
304+
305+
pg_atomic_write_u32(&bufHdr->state,buf_state);
302306
}
303307

304308
/*
@@ -432,6 +436,13 @@ InitLocalBuffers(void)
432436
* is -1.)
433437
*/
434438
buf->buf_id=-i-2;
439+
440+
/*
441+
* Intentionally do not initialize the buffer's atomic variable
442+
* (besides zeroing the underlying memory above). That way we get
443+
* errors on platforms without atomics, if somebody (re-)introduces
444+
* atomic operations for local buffers.
445+
*/
435446
}
436447

437448
/* Create the lookup hash table */

‎src/include/storage/buf_internals.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,8 +165,19 @@ typedef struct buftag
165165
* wait_backend_pid and setting flag bit BM_PIN_COUNT_WAITER. At present,
166166
* there can be only one such waiter per buffer.
167167
*
168+
<<<<<<< HEAD
168169
* We use this same struct for local buffer headers, but the lock fields
169170
* are not used and not all of the flag bits are useful either.
171+
=======
172+
* We use this same struct for local buffer headers, but the locks are not
173+
* used and not all of the flag bits are useful either. To avoid unnecessary
174+
* overhead, manipulations of the state field should be done without actual
175+
* atomic operations (i.e. only pg_atomic_read/write).
176+
*
177+
* Be careful to avoid increasing the size of the struct when adding or
178+
* reordering members. Keeping it below 64 bytes (the most common CPU
179+
* cache line size) is fairly important for performance.
180+
>>>>>>> 6b93fcd... Avoid atomic operation in MarkLocalBufferDirty().
170181
*/
171182
typedefstructBufferDesc
172183
{

0 commit comments

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