public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v5 3/3] bufmgr: Use atomic sub for unpinning buffers 3+ messages / 1 participants [nested] [flat]
* [PATCH v5 3/3] bufmgr: Use atomic sub for unpinning buffers @ 2025-09-22 18:44 Andres Freund <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Andres Freund @ 2025-09-22 18:44 UTC (permalink / raw) The prior commit made it legal to modify BufferDesc.state while the buffer header spinlock is held. This allows us to replace the CAS loop inUnpinBufferNoOwner() with an atomic sub. This improves scalability significantly. See the prior commits for more background. Reviewed-by: Matthias van de Meent <[email protected]> Discussion: https://postgr.es/m/fvfmkr5kk4nyex56ejgxj3uzi63isfxovp2biecb4bspbjrze7@az2pljabhnff --- src/backend/storage/buffer/bufmgr.c | 29 +++-------------------------- 1 file changed, 3 insertions(+), 26 deletions(-) diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 42e1f50c7c8..3528a355eff 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -3272,7 +3272,6 @@ UnpinBufferNoOwner(BufferDesc *buf) ref->refcount--; if (ref->refcount == 0) { - uint32 buf_state; uint32 old_buf_state; /* @@ -3290,33 +3289,11 @@ UnpinBufferNoOwner(BufferDesc *buf) */ Assert(!LWLockHeldByMe(BufferDescriptorGetContentLock(buf))); - /* - * Decrement the shared reference count. - * - * Since buffer spinlock holder can update status using just write, - * it's not safe to use atomic decrement here; thus use a CAS loop. - * - * TODO: The above requirement does not hold anymore, in a future - * commit this will be rewritten to release the pin in a single atomic - * operation. - */ - old_buf_state = pg_atomic_read_u32(&buf->state); - for (;;) - { - if (old_buf_state & BM_LOCKED) - old_buf_state = WaitBufHdrUnlocked(buf); - - buf_state = old_buf_state; - - buf_state -= BUF_REFCOUNT_ONE; - - if (pg_atomic_compare_exchange_u32(&buf->state, &old_buf_state, - buf_state)) - break; - } + /* decrement the shared reference count */ + old_buf_state = pg_atomic_fetch_sub_u32(&buf->state, BUF_REFCOUNT_ONE); /* Support LockBufferForCleanup() */ - if (buf_state & BM_PIN_COUNT_WAITER) + if (old_buf_state & BM_PIN_COUNT_WAITER) WakePinCountWaiter(buf); ForgetPrivateRefCountEntry(ref); -- 2.48.1.76.g4e746b1a31.dirty --552mi4kotjb5rbyk-- ^ permalink raw reply [nested|flat] 3+ messages in thread
* [PATCH v4 4/6] bufmgr: Use atomic sub for unpinning buffers @ 2025-09-22 18:44 Andres Freund <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Andres Freund @ 2025-09-22 18:44 UTC (permalink / raw) The prior commit made it legal to modify BufferDesc.state while the buffer header spinlock is held. This allows us to replace the CAS loop inUnpinBufferNoOwner() with an atomic sub. This improves scalability significantly. See the prior commits for more background. Reviewed-by: Robert Haas <[email protected]> Discussion: https://postgr.es/m/fvfmkr5kk4nyex56ejgxj3uzi63isfxovp2biecb4bspbjrze7@az2pljabhnff --- src/backend/storage/buffer/bufmgr.c | 29 +++-------------------------- 1 file changed, 3 insertions(+), 26 deletions(-) diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 0ce4a736b58..594a8e5d512 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -3247,7 +3247,6 @@ UnpinBufferNoOwner(BufferDesc *buf) ref->refcount--; if (ref->refcount == 0) { - uint32 buf_state; uint32 old_buf_state; /* @@ -3262,33 +3261,11 @@ UnpinBufferNoOwner(BufferDesc *buf) /* I'd better not still hold the buffer content lock */ Assert(!LWLockHeldByMe(BufferDescriptorGetContentLock(buf))); - /* - * Decrement the shared reference count. - * - * Since buffer spinlock holder can update status using just write, - * it's not safe to use atomic decrement here; thus use a CAS loop. - * - * TODO: The above requirement does not hold anymore, in a future - * commit this will be rewritten to release the pin in a single atomic - * operation. - */ - old_buf_state = pg_atomic_read_u32(&buf->state); - for (;;) - { - if (old_buf_state & BM_LOCKED) - old_buf_state = WaitBufHdrUnlocked(buf); - - buf_state = old_buf_state; - - buf_state -= BUF_REFCOUNT_ONE; - - if (pg_atomic_compare_exchange_u32(&buf->state, &old_buf_state, - buf_state)) - break; - } + /* decrement the shared reference count */ + old_buf_state = pg_atomic_fetch_sub_u32(&buf->state, BUF_REFCOUNT_ONE); /* Support LockBufferForCleanup() */ - if (buf_state & BM_PIN_COUNT_WAITER) + if (old_buf_state & BM_PIN_COUNT_WAITER) WakePinCountWaiter(buf); ForgetPrivateRefCountEntry(ref); -- 2.48.1.76.g4e746b1a31.dirty --al7hvfxknuxztweg Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v4-0005-bufmgr-fewer-calls-to-BufferDescriptorGetContentL.patch" ^ permalink raw reply [nested|flat] 3+ messages in thread
* [PATCH v5 3/3] bufmgr: Use atomic sub for unpinning buffers @ 2025-09-22 18:44 Andres Freund <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Andres Freund @ 2025-09-22 18:44 UTC (permalink / raw) The prior commit made it legal to modify BufferDesc.state while the buffer header spinlock is held. This allows us to replace the CAS loop inUnpinBufferNoOwner() with an atomic sub. This improves scalability significantly. See the prior commits for more background. Reviewed-by: Matthias van de Meent <[email protected]> Discussion: https://postgr.es/m/fvfmkr5kk4nyex56ejgxj3uzi63isfxovp2biecb4bspbjrze7@az2pljabhnff --- src/backend/storage/buffer/bufmgr.c | 29 +++-------------------------- 1 file changed, 3 insertions(+), 26 deletions(-) diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 42e1f50c7c8..3528a355eff 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -3272,7 +3272,6 @@ UnpinBufferNoOwner(BufferDesc *buf) ref->refcount--; if (ref->refcount == 0) { - uint32 buf_state; uint32 old_buf_state; /* @@ -3290,33 +3289,11 @@ UnpinBufferNoOwner(BufferDesc *buf) */ Assert(!LWLockHeldByMe(BufferDescriptorGetContentLock(buf))); - /* - * Decrement the shared reference count. - * - * Since buffer spinlock holder can update status using just write, - * it's not safe to use atomic decrement here; thus use a CAS loop. - * - * TODO: The above requirement does not hold anymore, in a future - * commit this will be rewritten to release the pin in a single atomic - * operation. - */ - old_buf_state = pg_atomic_read_u32(&buf->state); - for (;;) - { - if (old_buf_state & BM_LOCKED) - old_buf_state = WaitBufHdrUnlocked(buf); - - buf_state = old_buf_state; - - buf_state -= BUF_REFCOUNT_ONE; - - if (pg_atomic_compare_exchange_u32(&buf->state, &old_buf_state, - buf_state)) - break; - } + /* decrement the shared reference count */ + old_buf_state = pg_atomic_fetch_sub_u32(&buf->state, BUF_REFCOUNT_ONE); /* Support LockBufferForCleanup() */ - if (buf_state & BM_PIN_COUNT_WAITER) + if (old_buf_state & BM_PIN_COUNT_WAITER) WakePinCountWaiter(buf); ForgetPrivateRefCountEntry(ref); -- 2.48.1.76.g4e746b1a31.dirty --552mi4kotjb5rbyk-- ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2025-09-22 18:44 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2025-09-22 18:44 [PATCH v5 3/3] bufmgr: Use atomic sub for unpinning buffers Andres Freund <[email protected]> 2025-09-22 18:44 [PATCH v4 4/6] bufmgr: Use atomic sub for unpinning buffers Andres Freund <[email protected]> 2025-09-22 18:44 [PATCH v5 3/3] bufmgr: Use atomic sub for unpinning buffers Andres Freund <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox