lockfree-getbuffer.patch
application/octet-stream
Filename: lockfree-getbuffer.patch
Type: application/octet-stream
Part: 0
Patch
Same data as JSON:
GET /api/v1/attachments/:id/patch
the parsed metadata as JSON — format, series position, per-file stats; never the diff bytes.
API reference →
Format: unified
| File | + | − |
|---|---|---|
| src/backend/storage/buffer/freelist.c | 45 | 36 |
| src/include/storage/atomic.h | 63 | 0 |
diff --git a/src/backend/storage/buffer/freelist.c b/src/backend/storage/buffer/freelist.c
index 76a4bec..86843ae 100644
--- a/src/backend/storage/buffer/freelist.c
+++ b/src/backend/storage/buffer/freelist.c
@@ -15,10 +15,10 @@
*/
#include "postgres.h"
+#include "storage/atomic.h"
#include "storage/buf_internals.h"
#include "storage/bufmgr.h"
-
/*
* The shared freelist control information.
*/
@@ -28,12 +28,6 @@ typedef struct
int nextVictimBuffer;
int firstFreeBuffer; /* Head of list of unused buffers */
- int lastFreeBuffer; /* Tail of list of unused buffers */
-
- /*
- * NOTE: lastFreeBuffer is undefined when firstFreeBuffer is -1 (that is,
- * when the list is empty)
- */
/*
* Statistics. These counters should be wide enough that they can't
@@ -114,7 +108,9 @@ StrategyGetBuffer(BufferAccessStrategy strategy, bool *lock_held)
volatile BufferDesc *buf;
Latch *bgwriterLatch;
int trycounter;
+ int victim;
+ *lock_held = false;
/*
* If given a strategy object, see whether it can select a buffer. We
* assume strategy objects don't need the BufFreelistLock.
@@ -124,36 +120,27 @@ StrategyGetBuffer(BufferAccessStrategy strategy, bool *lock_held)
buf = GetBufferFromRing(strategy);
if (buf != NULL)
{
- *lock_held = false;
return buf;
}
}
- /* Nope, so lock the freelist */
- *lock_held = true;
- LWLockAcquire(BufFreelistLock, LW_EXCLUSIVE);
/*
* We count buffer allocation requests so that the bgwriter can estimate
* the rate of buffer consumption. Note that buffers recycled by a
* strategy object are intentionally not counted here.
*/
- StrategyControl->numBufferAllocs++;
+ pg_atomic_inc(StrategyControl->numBufferAllocs);
/*
- * If bgwriterLatch is set, we need to waken the bgwriter, but we should
- * not do so while holding BufFreelistLock; so release and re-grab. This
- * is annoyingly tedious, but it happens at most once per bgwriter cycle,
- * so the performance hit is minimal.
+ * If bgwriterLatch is set, we need to waken the bgwriter. Bgwriter needs
+ * to see our allocation to not go to sleep again so we emit a write
+ * barrier. Because this happens at most once per bgwriter cycle, the
+ * performance hit is minimal.
*/
- bgwriterLatch = StrategyControl->bgwriterLatch;
+ pg_atomic_fetch_and_set(bgwriterLatch, StrategyControl->bgwriterLatch, NULL);
if (bgwriterLatch)
- {
- StrategyControl->bgwriterLatch = NULL;
- LWLockRelease(BufFreelistLock);
SetLatch(bgwriterLatch);
- LWLockAcquire(BufFreelistLock, LW_EXCLUSIVE);
- }
/*
* Try to get a buffer from the freelist. Note that the freeNext fields
@@ -161,6 +148,16 @@ StrategyGetBuffer(BufferAccessStrategy strategy, bool *lock_held)
* individual buffer spinlocks, so it's OK to manipulate them without
* holding the spinlock.
*/
+ if (StrategyControl->firstFreeBuffer >= 0)
+ {
+ /*
+ * There might be something in the freelist, so lock and check
+ * again */
+ *lock_held = true;
+ LWLockAcquire(BufFreelistLock, LW_EXCLUSIVE);
+ /*
+ * XXX Need to avoid reusing value fetched before lock here,
+ * volatile access required? Output from GCC 4.6 says no. */
while (StrategyControl->firstFreeBuffer >= 0)
{
buf = &BufferDescriptors[StrategyControl->firstFreeBuffer];
@@ -187,17 +184,33 @@ StrategyGetBuffer(BufferAccessStrategy strategy, bool *lock_held)
UnlockBufHdr(buf);
}
+ LWLockRelease(BufFreelistLock);
+ *lock_held = false;
+ }
+
/* Nothing on the freelist, so run the "clock sweep" algorithm */
trycounter = NBuffers;
for (;;)
{
- buf = &BufferDescriptors[StrategyControl->nextVictimBuffer];
-
- if (++StrategyControl->nextVictimBuffer >= NBuffers)
+ pg_atomic_fetch_and_add(victim, StrategyControl->nextVictimBuffer, 1);
+ /*
+ * If we just incremented the value past the wrapping point
+ * emit a correcting subtraction and account for the pass.
+ */
+ if (victim == NBuffers-1)
{
- StrategyControl->nextVictimBuffer = 0;
- StrategyControl->completePasses++;
+ pg_atomic_sub(StrategyControl->nextVictimBuffer, NBuffers);
+ pg_atomic_inc(StrategyControl->completePasses);
}
+ /*
+ * Do the modulus operation asynchronously in case the value had
+ * overflowed. */
+ while (victim >= NBuffers)
+ victim -= NBuffers;
+
+ Assert(victim < NBuffers && victim >= 0);
+
+ buf = &BufferDescriptors[victim];
/*
* If the buffer is pinned or has a nonzero usage_count, we cannot use
@@ -253,8 +266,6 @@ StrategyFreeBuffer(volatile BufferDesc *buf)
if (buf->freeNext == FREENEXT_NOT_IN_LIST)
{
buf->freeNext = StrategyControl->firstFreeBuffer;
- if (buf->freeNext < 0)
- StrategyControl->lastFreeBuffer = buf->buf_id;
StrategyControl->firstFreeBuffer = buf->buf_id;
}
@@ -277,16 +288,17 @@ StrategySyncStart(uint32 *complete_passes, uint32 *num_buf_alloc)
{
int result;
- LWLockAcquire(BufFreelistLock, LW_EXCLUSIVE);
result = StrategyControl->nextVictimBuffer;
+ while (result >= NBuffers)
+ result -= NBuffers;
+
if (complete_passes)
*complete_passes = StrategyControl->completePasses;
if (num_buf_alloc)
{
- *num_buf_alloc = StrategyControl->numBufferAllocs;
- StrategyControl->numBufferAllocs = 0;
+ pg_atomic_fetch_and_set(*num_buf_alloc,
+ StrategyControl->numBufferAllocs, 0);
}
- LWLockRelease(BufFreelistLock);
return result;
}
@@ -306,9 +318,7 @@ StrategyNotifyBgWriter(Latch *bgwriterLatch)
* atomic to StrategyGetBuffer. The bgwriter should call this rather
* infrequently, so there's no performance penalty from being safe.
*/
- LWLockAcquire(BufFreelistLock, LW_EXCLUSIVE);
StrategyControl->bgwriterLatch = bgwriterLatch;
- LWLockRelease(BufFreelistLock);
}
@@ -378,7 +388,6 @@ StrategyInitialize(bool init)
* assume it was previously set up by InitBufferPool().
*/
StrategyControl->firstFreeBuffer = 0;
- StrategyControl->lastFreeBuffer = NBuffers - 1;
/* Initialize the clock sweep pointer */
StrategyControl->nextVictimBuffer = 0;
diff --git a/src/include/storage/atomic.h b/src/include/storage/atomic.h
new file mode 100644
index 0000000..359d940
--- /dev/null
+++ b/src/include/storage/atomic.h
@@ -0,0 +1,63 @@
+/*-------------------------------------------------------------------------
+ *
+ * atomic.h
+ * Atomic memory operations.
+ *
+ * Portions Copyright (c) 1996-2012, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * src/include/storage/atomic.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef ATOMIC_H
+#define ATOMIC_H
+
+#include "storage/barrier.h"
+
+#if defined(__GNUC__)
+
+#define pg_atomic_fetch_and_add(dst, src, value) \
+ do { dst = __sync_fetch_and_add(&(src), (value)); } while(0)
+
+#define pg_atomic_sub(var, value) \
+ do { __sync_fetch_and_sub(&(var), (value)); } while(0)
+
+#define pg_atomic_inc(var) do { __sync_fetch_and_add(&(var), 1); } while(0)
+
+#define pg_atomic_fetch_and_set(dst, src, value) \
+ do { dst = __sync_lock_test_and_set(&(src), (value)); } while (0)
+
+#endif
+
+#if !defined(pg_atomic_fetch_and_add)
+#define pg_atomic_fetch_and_add(dst, src, value) \
+ do { S_LOCK(&dummy_spinlock); \
+ dst = src; \
+ src += value; \
+ S_UNLOCK(&dummy_spinlock); } while (0)
+#endif
+
+#if !defined(pg_atomic_sub)
+#define pg_atomic_sub(var, value) \
+ do { S_LOCK(&dummy_spinlock); \
+ var -= value; \
+ S_UNLOCK(&dummy_spinlock); } while (0)
+#endif
+
+#if !defined(pg_atomic_inc)
+#define pg_atomic_inc(var) \
+ do { S_LOCK(&dummy_spinlock); \
+ var++; \
+ S_UNLOCK(&dummy_spinlock); } while (0)
+#endif
+
+#if !defined(pg_atomic_fetch_and_set)
+#define pg_atomic_fetch_and_set(dst, src, value) \
+ do { S_LOCK(&dummy_spinlock); \
+ dst = src; \
+ src = value; \
+ S_UNLOCK(&dummy_spinlock); } while (0)
+#endif
+
+#endif /* ATOMIC_H */