From bc0440c410db7197d03eb6ec59703e1135e5009c Mon Sep 17 00:00:00 2001 From: Alexandre Felipe Date: Sun, 6 Sep 2026 01:20:08 +0100 Subject: [PATCH 6/9] atomics in buf_table This use atomic operations on the linked lists, and detect when a buffer is recycled under a scan. Inserts are atomic (CAS), deletion is atomic, but after deletion a scan might hold a reference to an item not in the list anymore. This is why scans have to check bucket of the entries, and deletion must not be concurrent. Hash code is compared, so if the number of shared buffers is e.g. 8GB, this would be about 1000x less likely to call BufferTagsEqual (but it comes at a cost of course). Since we had to add bucket, it is not 8-aligned, and even before it was not a power of always subject to entries using two cache lines. This version has a 32-byte entry, so each entry stays in one cache line (assuming a power of two, and at least 32 bytes). --- src/backend/storage/buffer/buf_table.c | 278 +++++++++++++++---------- 1 file changed, 169 insertions(+), 109 deletions(-) diff --git a/src/backend/storage/buffer/buf_table.c b/src/backend/storage/buffer/buf_table.c index 7836d0dd664..23c14e6c688 100644 --- a/src/backend/storage/buffer/buf_table.c +++ b/src/backend/storage/buffer/buf_table.c @@ -8,27 +8,36 @@ * of two shared-memory arrays: * * buckets[num_buckets] - one chain head per hash bucket - * entries[NBuffers] - one entry per buffer, indexed by buf_id + * entries[NBuffers] - one entry per buffer, indexed by buf_id * * Each buffer slot i permanently owns entry slot i, so no freelist is needed: * bufmgr always removes a buffer's old mapping (BufTableDelete, called from * InvalidateVictimBuffer) before inserting a new tag for that same buf_id (see - * GetVictimBuffer / BufferAlloc in bufmgr.c). Empty entry slots are marked by - * tag.blockNum == P_NEW; chains are linked by int index and terminated by - * BUF_TABLE_CHAIN_END. + * GetVictimBuffer / BufferAlloc in bufmgr.c). Chains are linked by int index + * and terminated by P_NEW. * * num_buckets is a power of two and a multiple of NUM_BUFFER_PARTITIONS, so the * bucket index (hashcode % num_buckets) shares its low bits with the partition * index (hashcode % NUM_BUFFER_PARTITIONS). Every tag that maps to a given - * bucket therefore maps to a single partition, and the caller's BufMappingLock - * fully serializes each chain -- the same guarantee the dynahash table relied - * on. + * bucket therefore maps to a single partition. * - * Note: the routines in this file do no locking of their own. The caller - * must hold a suitable lock on the appropriate BufMappingLock, as specified - * in the comments. We can't do the locking inside these functions because - * in most cases the caller needs to adjust the buffer header contents - * before the lock is released (see notes in README). + * BufTableInsert and BufTableDelete must be called with the tag's + * BufMappingPartitionLock held exclusively, so a writer is the only mutator of + * its bucket and both are plain list manipulations. + * + * BufTableLookup holds no lock. Each entry caches its hashcode, and deletion + * complements it before unlinking; complementing always changes the bucket + * bits, so an unlinked or recycled entry no longer claims membership of this + * bucket and a scanner that reaches one knows its link is untrustworthy and + * starts over. + * + * Either outcome is acceptable when a lookup runs concurrently with an insert + * or delete, and the result is a hint in any case: the buffer can be evicted as + * soon as we return, so the caller must pin it and recheck its tag (as + * ReadRecentBuffer does). BufTableInsert is authoritative. + * + * Note: the entry arrays are never initialized. An entry is only ever read + * while linked into a chain, and it is fully written before being linked. * * * Portions Copyright (c) 1996-2026, PostgreSQL Global Development Group @@ -44,34 +53,34 @@ #include "common/hashfn.h" #include "miscadmin.h" +#include "port/atomics.h" #include "port/pg_bitutils.h" #include "storage/buf_internals.h" #include "storage/bufmgr.h" #include "storage/shmem.h" #include "storage/subsystems.h" -#define BUF_TABLE_CHAIN_END (-1) - -/* bucket for buffer lookup hashtable */ -typedef struct -{ - int head; /* head of hash chain, or BUF_TABLE_CHAIN_END */ -} BufferLookupBucket; - /* entry for buffer lookup hashtable */ typedef struct { - BufferTag tag; /* Tag of a disk page, or P_NEW if empty */ - int next; /* next entry in hash chain */ -} BufferLookupEnt; + BufferTag tag; /* Tag of a disk page */ + uint32 hashcode; /* tag's hash code, complemented if unlinked */ + uint32 bucket; /* which is this node inserted */ + pg_atomic_uint32 next; /* next entry in hash chain, or P_NEW */ +} pg_attribute_aligned(32) BufferLookupEnt; /* bucket and entry arrays for buffer lookup hashtable (in shared memory) */ -static BufferLookupBucket *buckets; +static pg_atomic_uint32 *buckets; static BufferLookupEnt *entries; +StaticAssertDecl( + sizeof(BufferLookupEnt) == 32, + "BufferLookupEnt must be 32-bytes" +); + /* number of hash buckets; power of two and multiple of NUM_BUFFER_PARTITIONS */ static int num_buckets; - +static int bucket_mask; static void BufTableShmemRequest(void *arg); static void BufTableShmemInit(void *arg); static void BufTableShmemAttach(void *arg); @@ -102,10 +111,11 @@ void BufTableShmemRequest(void *arg) { num_buckets = BufTableNumBuckets(); + bucket_mask = num_buckets - 1; Assert(num_buckets % NUM_BUFFER_PARTITIONS == 0); ShmemRequestStruct(.name = "Shared Buffer Lookup Buckets", - .size = (Size) num_buckets * sizeof(BufferLookupBucket), + .size = (Size) num_buckets * sizeof(pg_atomic_uint32), .ptr = (void **) &buckets, ); @@ -119,23 +129,16 @@ BufTableShmemRequest(void *arg) * Initialize the shared buffer lookup table. Called once during shared-memory * initialization (in the postmaster, or in a standalone backend). * - * Shared memory is zeroed, but zero is a valid buf_id and block 0 is a valid - * block number, so we must explicitly mark every bucket empty - * (BUF_TABLE_CHAIN_END) and every entry empty (tag.blockNum == P_NEW). + * Shared memory is zeroed, but zero is a valid buf_id, so we must explicitly + * mark every bucket empty. */ void BufTableShmemInit(void *arg) { num_buckets = BufTableNumBuckets(); - + bucket_mask = num_buckets - 1; for (int i = 0; i < num_buckets; i++) - buckets[i].head = BUF_TABLE_CHAIN_END; - - for (int i = 0; i < NBuffers; i++) - { - entries[i].tag.blockNum = P_NEW; - entries[i].next = BUF_TABLE_CHAIN_END; - } + pg_atomic_init_u32(&buckets[i], P_NEW); } /* @@ -148,6 +151,7 @@ void BufTableShmemAttach(void *arg) { num_buckets = BufTableNumBuckets(); + bucket_mask = num_buckets - 1; } /* @@ -165,24 +169,104 @@ BufTableHashCode(BufferTag *tagPtr) return tag_hash(tagPtr, sizeof(BufferTag)); } -/* - * BufTableLookup - * Lookup the given BufferTag; return buffer ID, or -1 if not found +static inline uint32 +pg_atomic_fetch_u32(volatile pg_atomic_uint32 *ptr) +{ +#if defined(HAVE_GCC__ATOMIC_INT32_CAS) + return __atomic_load_n(&ptr->value, __ATOMIC_ACQUIRE); +#else + return pg_atomic_fetch_add_u32_impl(ptr, 0); /* fallback RMW */ +#endif +} + +/* BufTableScan + * Helper for lookup and insertion + * + * Scan table following links optimistically. At some point + * it might hold reference to a link from a node already deleted + * that link is valid until the node is recycled. Once the node + * is recycled it can be either (1) inserted on a different bucket + * in which case we detect by comparing the bucket of the last + * visited entry; (2) inserted on the same bucket, in which case + * it will be before all the nodes that previously succeded it; * - * Caller must hold at least share lock on BufMappingLock for tag's partition + * what about constructing a chain (id, bucket) as + * (1,a) -> (2,b) -> (3,a). + * (1,a) -> (2,b) requires 2 being after 1 on bucket a, then removed + * and inserted on bucket b before (3,a). + * but in order to have however if 3 was on bucket b then it should + * be (3,b), so the only possibility is that 3 was on bucket a, at + * the moment the bucket was inserted. + * */ -int -BufTableLookup(BufferTag *tagPtr, uint32 hashcode) +static pg_always_inline uint32 +BufTableScan(BufferTag *tagPtr, uint32 hashcode, int buf_id) { - int id = buckets[hashcode % num_buckets].head; - while (id != BUF_TABLE_CHAIN_END) + uint32 bucket; + pg_atomic_uint32 *head; + uint32 id; + uint32 prev; + uint32 next; + uint32 attempts; + + bucket = hashcode & bucket_mask; + head = &buckets[bucket]; + attempts = 0; +retry: + if(++attempts > 1000) goto die; + prev = P_NEW; + id = pg_atomic_fetch_u32(head); + while(id != P_NEW) { + /* this will get a fresh version of entry cache line */ + next = pg_atomic_fetch_u32(&entries[id].next); if (BufferTagsEqual(&entries[id].tag, tagPtr)) return id; - id = entries[id].next; + if(entries[id].bucket != bucket) + { + if(prev != P_NEW && entries[prev].bucket == bucket) + { + /* step back for a while and try the same link again */ + id = prev; + SPIN_DELAY(); + continue; + } + else + goto retry; + }else{ + prev = id; + id = next; + } } - return -1; + if(buf_id == P_NEW) return P_NEW; + + /* Prepare the entry */ + entries[buf_id].tag = *tagPtr; + entries[buf_id].hashcode = hashcode; + entries[buf_id].bucket = bucket; + id = pg_atomic_read_u32(head); + + /* atttach the entry to the chain */ + do { + pg_atomic_write_u32(&entries[buf_id].next, id); + pg_write_barrier(); + } while(!pg_atomic_compare_exchange_u32(head, &id, buf_id)); + return P_NEW; +die: + elog(ERROR, "corrupted chain, chain starting in bucket %d ended in bucket %d", + bucket, entries[prev].bucket); +} +/* + * BufTableLookup + * Lookup the given BufferTag; return buffer ID, or -1 if not found + * + * Takes no lock; see the file header for what the caller owes us. + */ +int +BufTableLookup(BufferTag *tagPtr, uint32 hashcode) +{ + return BufTableScan(tagPtr, hashcode, P_NEW); } /* @@ -190,82 +274,58 @@ BufTableLookup(BufferTag *tagPtr, uint32 hashcode) * Insert a hashtable entry for given tag and buffer ID, * unless an entry already exists for that tag * - * Returns -1 on successful insertion. If a conflicting entry exists - * already, returns the buffer ID in that entry. - * - * Caller must hold exclusive lock on BufMappingLock for tag's partition + * Shared data integrity is guaranteed, the operation is atomic + * without intermediate states. + * Returns -1 on successful insertion, or the id, if already + * present. */ int BufTableInsert(BufferTag *tagPtr, uint32 hashcode, int buf_id) { - int bucket_id = hashcode % num_buckets; - int head = buckets[bucket_id].head; - int id = head; - - Assert(buf_id >= 0 && buf_id < NBuffers); - Assert(tagPtr->blockNum != P_NEW); /* invalid tag */ - - /* If the tag is already in the chain, surface the existing buf_id. */ - while (id != BUF_TABLE_CHAIN_END) - { - if (BufferTagsEqual(&entries[id].tag, tagPtr)) - return id; - id = entries[id].next; - } - - /* - * Not present. entry[buf_id] must be empty: bufmgr always deletes a - * buffer's old mapping before inserting a new tag for that buf_id. - */ - Assert(entries[buf_id].tag.blockNum == P_NEW); - - /* - * Link entry[buf_id] at the chain head, keeping the prior head as its - * successor. (Use the saved `head`, not `id`, which the loop above has - * advanced to BUF_TABLE_CHAIN_END.) - */ - entries[buf_id].tag = *tagPtr; - entries[buf_id].next = head; - buckets[bucket_id].head = buf_id; - - return -1; + return BufTableScan(tagPtr, hashcode, buf_id); } /* * BufTableDelete - * Delete the hashtable entry for given tag (which must exist) + * Delete the hashtable entry for given buffer (which must exist) * - * Caller must hold exclusive lock on BufMappingLock for tag's partition + * This function operates atomically, however when deleting a node + * N it assumes both prev(N) and next(N) to remain in the list until + * the deletion of N is completed. To satisfy this condition we need + * the caller must prevent concurrent calls to BufTableDelete on the + * same bucket. e.g. holding an exclusive lock. */ void -BufTableDelete(BufferTag *tagPtr, uint32 hashcode) +BufTableDelete(BufferTag *tag, uint32 hashcode) { - int bucket_id = hashcode % num_buckets; - int prev = BUF_TABLE_CHAIN_END; - int id = buckets[bucket_id].head; - - while (id != BUF_TABLE_CHAIN_END) + uint32 bucket = hashcode & bucket_mask; + pg_atomic_uint32 *link; + uint32 id; + Assert(LWLockHeldByMeInMode(BufMappingPartitionLock(hashcode), + LW_EXCLUSIVE)); +retry: + for (link = &buckets[bucket]; + (id = pg_atomic_read_u32(link)) != P_NEW; + link = &entries[id].next) { - if (BufferTagsEqual(&entries[id].tag, tagPtr)) + pg_read_barrier(); + if (entries[id].hashcode == hashcode && + BufferTagsEqual(&entries[id].tag, tag)) { - /* unlink from the chain */ - if (prev == BUF_TABLE_CHAIN_END) - buckets[bucket_id].head = entries[id].next; - else - entries[prev].next = entries[id].next; - /* mark the entry empty */ - entries[id].tag.blockNum = P_NEW; - entries[id].next = BUF_TABLE_CHAIN_END; - return; + /* XXX: memory barriers?? we are already making a decision + * assuming currency of .tag, so just continue with that */ + uint32 next = pg_atomic_read_u32(&entries[id].next); + if(pg_atomic_compare_exchange_u32(link, &id, next)) + return; + /* + * the link changed, this entry can't be recycled yet + * so the only possibility is that it was linked from + * bucket head, and a concurrent insertion changed it. + * restart from scratch, it should not be very far from + * the start. + */ + goto retry; } - prev = id; - id = entries[id].next; } - - /* - * Entry not in table. Callers never double-delete (deletion is gated by - * BM_TAG_VALID on the buffer header), so this indicates corruption. - */ - Assert(false); - elog(ERROR, "shared buffer hash table corrupted"); + elog(ERROR, "tag not in shared buffer mapping table"); } -- 2.53.0