diff --git a/contrib/bloom/blutils.c b/contrib/bloom/blutils.c index a6d9f09f31..d935ed8fbd 100644 --- a/contrib/bloom/blutils.c +++ b/contrib/bloom/blutils.c @@ -353,7 +353,6 @@ Buffer BloomNewBuffer(Relation index) { Buffer buffer; - bool needLock; /* First, try to get a page from FSM */ for (;;) @@ -387,15 +386,8 @@ BloomNewBuffer(Relation index) } /* Must extend the file */ - needLock = !RELATION_IS_LOCAL(index); - if (needLock) - LockRelationForExtension(index, ExclusiveLock); - - buffer = ReadBuffer(index, P_NEW); - LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE); - - if (needLock) - UnlockRelationForExtension(index, ExclusiveLock); + buffer = ExtendBufferedRel(EB_REL(index), MAIN_FORKNUM, NULL, + EB_LOCK_FIRST); return buffer; } diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c index 53e4721a54..41bf950a4a 100644 --- a/src/backend/access/brin/brin.c +++ b/src/backend/access/brin/brin.c @@ -837,9 +837,9 @@ brinbuild(Relation heap, Relation index, IndexInfo *indexInfo) * whole relation will be rolled back. */ - meta = ReadBuffer(index, P_NEW); + meta = ExtendBufferedRel(EB_REL(index), MAIN_FORKNUM, NULL, + EB_LOCK_FIRST | EB_SKIP_EXTENSION_LOCK); Assert(BufferGetBlockNumber(meta) == BRIN_METAPAGE_BLKNO); - LockBuffer(meta, BUFFER_LOCK_EXCLUSIVE); brin_metapage_init(BufferGetPage(meta), BrinGetPagesPerRange(index), BRIN_CURRENT_VERSION); @@ -904,9 +904,8 @@ brinbuildempty(Relation index) Buffer metabuf; /* An empty BRIN index has a metapage only. */ - metabuf = - ReadBufferExtended(index, INIT_FORKNUM, P_NEW, RBM_NORMAL, NULL); - LockBuffer(metabuf, BUFFER_LOCK_EXCLUSIVE); + metabuf = ExtendBufferedRel(EB_REL(index), INIT_FORKNUM, NULL, + EB_LOCK_FIRST | EB_SKIP_EXTENSION_LOCK); /* Initialize and xlog metabuffer. */ START_CRIT_SECTION(); diff --git a/src/backend/access/brin/brin_pageops.c b/src/backend/access/brin/brin_pageops.c index ad5a89bd05..b578d25954 100644 --- a/src/backend/access/brin/brin_pageops.c +++ b/src/backend/access/brin/brin_pageops.c @@ -730,6 +730,10 @@ brin_getinsertbuffer(Relation irel, Buffer oldbuf, Size itemsz, * There's not enough free space in any existing index page, * according to the FSM: extend the relation to obtain a shiny new * page. + * + * XXX: It's likely possible to use RBM_ZERO_AND_LOCK here, + * which'd avoid the need to hold the extension lock during buffer + * reclaim. */ if (!RELATION_IS_LOCAL(irel)) { diff --git a/src/backend/access/brin/brin_revmap.c b/src/backend/access/brin/brin_revmap.c index 7fc5226bf7..f4271ba31c 100644 --- a/src/backend/access/brin/brin_revmap.c +++ b/src/backend/access/brin/brin_revmap.c @@ -538,7 +538,6 @@ revmap_physical_extend(BrinRevmap *revmap) BlockNumber mapBlk; BlockNumber nblocks; Relation irel = revmap->rm_irel; - bool needLock = !RELATION_IS_LOCAL(irel); /* * Lock the metapage. This locks out concurrent extensions of the revmap, @@ -570,10 +569,8 @@ revmap_physical_extend(BrinRevmap *revmap) } else { - if (needLock) - LockRelationForExtension(irel, ExclusiveLock); - - buf = ReadBuffer(irel, P_NEW); + buf = ExtendBufferedRel(EB_REL(irel), MAIN_FORKNUM, NULL, + EB_LOCK_FIRST); if (BufferGetBlockNumber(buf) != mapBlk) { /* @@ -582,17 +579,11 @@ revmap_physical_extend(BrinRevmap *revmap) * up and have caller start over. We will have to evacuate that * page from under whoever is using it. */ - if (needLock) - UnlockRelationForExtension(irel, ExclusiveLock); LockBuffer(revmap->rm_metaBuf, BUFFER_LOCK_UNLOCK); - ReleaseBuffer(buf); + UnlockReleaseBuffer(buf); return; } - LockBuffer(buf, BUFFER_LOCK_EXCLUSIVE); page = BufferGetPage(buf); - - if (needLock) - UnlockRelationForExtension(irel, ExclusiveLock); } /* Check that it's a regular block (or an empty page) */ diff --git a/src/backend/access/gin/gininsert.c b/src/backend/access/gin/gininsert.c index d5d748009e..be1841de7b 100644 --- a/src/backend/access/gin/gininsert.c +++ b/src/backend/access/gin/gininsert.c @@ -440,12 +440,10 @@ ginbuildempty(Relation index) MetaBuffer; /* An empty GIN index has two pages. */ - MetaBuffer = - ReadBufferExtended(index, INIT_FORKNUM, P_NEW, RBM_NORMAL, NULL); - LockBuffer(MetaBuffer, BUFFER_LOCK_EXCLUSIVE); - RootBuffer = - ReadBufferExtended(index, INIT_FORKNUM, P_NEW, RBM_NORMAL, NULL); - LockBuffer(RootBuffer, BUFFER_LOCK_EXCLUSIVE); + MetaBuffer = ExtendBufferedRel(EB_REL(index), INIT_FORKNUM, NULL, + EB_LOCK_FIRST | EB_SKIP_EXTENSION_LOCK); + RootBuffer = ExtendBufferedRel(EB_REL(index), INIT_FORKNUM, NULL, + EB_LOCK_FIRST | EB_SKIP_EXTENSION_LOCK); /* Initialize and xlog metabuffer and root buffer. */ START_CRIT_SECTION(); diff --git a/src/backend/access/gin/ginutil.c b/src/backend/access/gin/ginutil.c index 03fec1704e..437f24753c 100644 --- a/src/backend/access/gin/ginutil.c +++ b/src/backend/access/gin/ginutil.c @@ -299,7 +299,6 @@ Buffer GinNewBuffer(Relation index) { Buffer buffer; - bool needLock; /* First, try to get a page from FSM */ for (;;) @@ -328,15 +327,8 @@ GinNewBuffer(Relation index) } /* Must extend the file */ - needLock = !RELATION_IS_LOCAL(index); - if (needLock) - LockRelationForExtension(index, ExclusiveLock); - - buffer = ReadBuffer(index, P_NEW); - LockBuffer(buffer, GIN_EXCLUSIVE); - - if (needLock) - UnlockRelationForExtension(index, ExclusiveLock); + buffer = ExtendBufferedRel(EB_REL(index), MAIN_FORKNUM, NULL, + EB_LOCK_FIRST); return buffer; } diff --git a/src/backend/access/gist/gist.c b/src/backend/access/gist/gist.c index c3a3d49bca..b5c1754e78 100644 --- a/src/backend/access/gist/gist.c +++ b/src/backend/access/gist/gist.c @@ -134,8 +134,8 @@ gistbuildempty(Relation index) Buffer buffer; /* Initialize the root page */ - buffer = ReadBufferExtended(index, INIT_FORKNUM, P_NEW, RBM_NORMAL, NULL); - LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE); + buffer = ExtendBufferedRel(EB_REL(index), INIT_FORKNUM, NULL, + EB_SKIP_EXTENSION_LOCK | EB_LOCK_FIRST); /* Initialize and xlog buffer */ START_CRIT_SECTION(); diff --git a/src/backend/access/gist/gistutil.c b/src/backend/access/gist/gistutil.c index a607464b97..f9f51152b8 100644 --- a/src/backend/access/gist/gistutil.c +++ b/src/backend/access/gist/gistutil.c @@ -824,7 +824,6 @@ Buffer gistNewBuffer(Relation r, Relation heaprel) { Buffer buffer; - bool needLock; /* First, try to get a page from FSM */ for (;;) @@ -878,16 +877,8 @@ gistNewBuffer(Relation r, Relation heaprel) } /* Must extend the file */ - needLock = !RELATION_IS_LOCAL(r); - - if (needLock) - LockRelationForExtension(r, ExclusiveLock); - - buffer = ReadBuffer(r, P_NEW); - LockBuffer(buffer, GIST_EXCLUSIVE); - - if (needLock) - UnlockRelationForExtension(r, ExclusiveLock); + buffer = ExtendBufferedRel(EB_REL(r), MAIN_FORKNUM, NULL, + EB_LOCK_FIRST); return buffer; } diff --git a/src/backend/access/hash/hashpage.c b/src/backend/access/hash/hashpage.c index 2d8fdec98e..6d8af42260 100644 --- a/src/backend/access/hash/hashpage.c +++ b/src/backend/access/hash/hashpage.c @@ -206,14 +206,14 @@ _hash_getnewbuf(Relation rel, BlockNumber blkno, ForkNumber forkNum) elog(ERROR, "access to noncontiguous page in hash index \"%s\"", RelationGetRelationName(rel)); - /* smgr insists we use P_NEW to extend the relation */ + /* smgr insists we explicitly extend the relation */ if (blkno == nblocks) { - buf = ReadBufferExtended(rel, forkNum, P_NEW, RBM_NORMAL, NULL); + buf = ExtendBufferedRel(EB_REL(rel), forkNum, NULL, + EB_LOCK_FIRST | EB_SKIP_EXTENSION_LOCK); if (BufferGetBlockNumber(buf) != blkno) elog(ERROR, "unexpected hash relation size: %u, should be %u", BufferGetBlockNumber(buf), blkno); - LockBuffer(buf, HASH_WRITE); } else { diff --git a/src/backend/access/nbtree/nbtpage.c b/src/backend/access/nbtree/nbtpage.c index 0144c3ab57..41aa1c4ccd 100644 --- a/src/backend/access/nbtree/nbtpage.c +++ b/src/backend/access/nbtree/nbtpage.c @@ -882,7 +882,6 @@ _bt_getbuf(Relation rel, Relation heaprel, BlockNumber blkno, int access) } else { - bool needLock; Page page; Assert(access == BT_WRITE); @@ -963,31 +962,16 @@ _bt_getbuf(Relation rel, Relation heaprel, BlockNumber blkno, int access) } /* - * Extend the relation by one page. - * - * We have to use a lock to ensure no one else is extending the rel at - * the same time, else we will both try to initialize the same new - * page. We can skip locking for new or temp relations, however, - * since no one else could be accessing them. + * Extend the relation by one page. Need to use RBM_ZERO_AND_LOCK or + * we risk a race condition against btvacuumscan --- see comments + * therein. This forces us to repeat the valgrind request that + * _bt_lockbuf() otherwise would make, as we can't use _bt_lockbuf() + * without introducing a race. */ - needLock = !RELATION_IS_LOCAL(rel); - - if (needLock) - LockRelationForExtension(rel, ExclusiveLock); - - buf = ReadBuffer(rel, P_NEW); - - /* Acquire buffer lock on new page */ - _bt_lockbuf(rel, buf, BT_WRITE); - - /* - * Release the file-extension lock; it's now OK for someone else to - * extend the relation some more. Note that we cannot release this - * lock before we have buffer lock on the new page, or we risk a race - * condition against btvacuumscan --- see comments therein. - */ - if (needLock) - UnlockRelationForExtension(rel, ExclusiveLock); + buf = ExtendBufferedRel(EB_REL(rel), MAIN_FORKNUM, NULL, + EB_LOCK_FIRST); + if (!RelationUsesLocalBuffers(rel)) + VALGRIND_MAKE_MEM_DEFINED(BufferGetPage(buf), BLCKSZ); /* Initialize the new page before returning it */ page = BufferGetPage(buf); diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c index 409a2c1210..992f84834f 100644 --- a/src/backend/access/nbtree/nbtree.c +++ b/src/backend/access/nbtree/nbtree.c @@ -970,6 +970,9 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats, * write-lock on the left page before it adds a right page, so we must * already have processed any tuples due to be moved into such a page. * + * XXX: Now that new pages are locked with RBM_ZERO_AND_LOCK, I don't + * think the use of the extension lock is still required. + * * We can skip locking for new or temp relations, however, since no one * else could be accessing them. */ diff --git a/src/backend/access/spgist/spgutils.c b/src/backend/access/spgist/spgutils.c index 4e7ff1d160..190e4f76a9 100644 --- a/src/backend/access/spgist/spgutils.c +++ b/src/backend/access/spgist/spgutils.c @@ -366,7 +366,6 @@ Buffer SpGistNewBuffer(Relation index) { Buffer buffer; - bool needLock; /* First, try to get a page from FSM */ for (;;) @@ -406,16 +405,8 @@ SpGistNewBuffer(Relation index) ReleaseBuffer(buffer); } - /* Must extend the file */ - needLock = !RELATION_IS_LOCAL(index); - if (needLock) - LockRelationForExtension(index, ExclusiveLock); - - buffer = ReadBuffer(index, P_NEW); - LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE); - - if (needLock) - UnlockRelationForExtension(index, ExclusiveLock); + buffer = ExtendBufferedRel(EB_REL(index), MAIN_FORKNUM, NULL, + EB_LOCK_FIRST); return buffer; } diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c index f3d1779655..ef01449678 100644 --- a/src/backend/commands/sequence.c +++ b/src/backend/commands/sequence.c @@ -377,7 +377,8 @@ fill_seq_fork_with_data(Relation rel, HeapTuple tuple, ForkNumber forkNum) /* Initialize first page of relation with special magic number */ - buf = ReadBufferExtended(rel, forkNum, P_NEW, RBM_ZERO_AND_LOCK, NULL); + buf = ExtendBufferedRel(EB_REL(rel), forkNum, NULL, + EB_LOCK_FIRST | EB_SKIP_EXTENSION_LOCK); Assert(BufferGetBlockNumber(buf) == 0); page = BufferGetPage(buf);