Skip to content

Commit acab1b0

Browse files
anarazelanarazel
committed
Convert many uses of ReadBuffer[Extended](P_NEW) to ExtendBufferedRel()
A few places are not converted. Some because they are tackled in later commits (e.g. hio.c, xlogutils.c), some because they are more complicated (e.g. brin_pageops.c). Having a few users of ReadBuffer(P_NEW) is good anyway, to ensure the backward compat path stays working. Discussion: https://postgr.es/m/20221029025420.eplyow6k7tgu6he3@awork3.anarazel.de
1 parent fcdda1e commit acab1b0

File tree

13 files changed

+42
-96
lines changed

13 files changed

+42
-96
lines changed

‎contrib/bloom/blutils.c

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -353,7 +353,6 @@ Buffer
353353
BloomNewBuffer(Relation index)
354354
{
355355
Buffer buffer;
356-
bool needLock;
357356

358357
/* First, try to get a page from FSM */
359358
for (;;)
@@ -387,15 +386,8 @@ BloomNewBuffer(Relation index)
387386
}
388387

389388
/* Must extend the file */
390-
needLock = !RELATION_IS_LOCAL(index);
391-
if (needLock)
392-
LockRelationForExtension(index, ExclusiveLock);
393-
394-
buffer = ReadBuffer(index, P_NEW);
395-
LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
396-
397-
if (needLock)
398-
UnlockRelationForExtension(index, ExclusiveLock);
389+
buffer = ExtendBufferedRel(EB_REL(index), MAIN_FORKNUM, NULL,
390+
EB_LOCK_FIRST);
399391

400392
return buffer;
401393
}

‎src/backend/access/brin/brin.c

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -837,9 +837,9 @@ brinbuild(Relation heap, Relation index, IndexInfo *indexInfo)
837837
* whole relation will be rolled back.
838838
*/
839839

840-
meta = ReadBuffer(index, P_NEW);
840+
meta = ExtendBufferedRel(EB_REL(index), MAIN_FORKNUM, NULL,
841+
EB_LOCK_FIRST | EB_SKIP_EXTENSION_LOCK);
841842
Assert(BufferGetBlockNumber(meta) == BRIN_METAPAGE_BLKNO);
842-
LockBuffer(meta, BUFFER_LOCK_EXCLUSIVE);
843843

844844
brin_metapage_init(BufferGetPage(meta), BrinGetPagesPerRange(index),
845845
BRIN_CURRENT_VERSION);
@@ -904,9 +904,8 @@ brinbuildempty(Relation index)
904904
Buffer metabuf;
905905

906906
/* An empty BRIN index has a metapage only. */
907-
metabuf =
908-
ReadBufferExtended(index, INIT_FORKNUM, P_NEW, RBM_NORMAL, NULL);
909-
LockBuffer(metabuf, BUFFER_LOCK_EXCLUSIVE);
907+
metabuf = ExtendBufferedRel(EB_REL(index), INIT_FORKNUM, NULL,
908+
EB_LOCK_FIRST | EB_SKIP_EXTENSION_LOCK);
910909

911910
/* Initialize and xlog metabuffer. */
912911
START_CRIT_SECTION();

‎src/backend/access/brin/brin_pageops.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -730,6 +730,10 @@ brin_getinsertbuffer(Relation irel, Buffer oldbuf, Size itemsz,
730730
* There's not enough free space in any existing index page,
731731
* according to the FSM: extend the relation to obtain a shiny new
732732
* page.
733+
*
734+
* XXX: It's likely possible to use RBM_ZERO_AND_LOCK here,
735+
* which'd avoid the need to hold the extension lock during buffer
736+
* reclaim.
733737
*/
734738
if (!RELATION_IS_LOCAL(irel))
735739
{

‎src/backend/access/brin/brin_revmap.c

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -538,7 +538,6 @@ revmap_physical_extend(BrinRevmap *revmap)
538538
BlockNumber mapBlk;
539539
BlockNumber nblocks;
540540
Relation irel = revmap->rm_irel;
541-
bool needLock = !RELATION_IS_LOCAL(irel);
542541

543542
/*
544543
* Lock the metapage. This locks out concurrent extensions of the revmap,
@@ -570,10 +569,8 @@ revmap_physical_extend(BrinRevmap *revmap)
570569
}
571570
else
572571
{
573-
if (needLock)
574-
LockRelationForExtension(irel, ExclusiveLock);
575-
576-
buf = ReadBuffer(irel, P_NEW);
572+
buf = ExtendBufferedRel(EB_REL(irel), MAIN_FORKNUM, NULL,
573+
EB_LOCK_FIRST);
577574
if (BufferGetBlockNumber(buf) != mapBlk)
578575
{
579576
/*
@@ -582,17 +579,11 @@ revmap_physical_extend(BrinRevmap *revmap)
582579
* up and have caller start over. We will have to evacuate that
583580
* page from under whoever is using it.
584581
*/
585-
if (needLock)
586-
UnlockRelationForExtension(irel, ExclusiveLock);
587582
LockBuffer(revmap->rm_metaBuf, BUFFER_LOCK_UNLOCK);
588-
ReleaseBuffer(buf);
583+
UnlockReleaseBuffer(buf);
589584
return;
590585
}
591-
LockBuffer(buf, BUFFER_LOCK_EXCLUSIVE);
592586
page = BufferGetPage(buf);
593-
594-
if (needLock)
595-
UnlockRelationForExtension(irel, ExclusiveLock);
596587
}
597588

598589
/* Check that it's a regular block (or an empty page) */

‎src/backend/access/gin/gininsert.c

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -440,12 +440,10 @@ ginbuildempty(Relation index)
440440
MetaBuffer;
441441

442442
/* An empty GIN index has two pages. */
443-
MetaBuffer =
444-
ReadBufferExtended(index, INIT_FORKNUM, P_NEW, RBM_NORMAL, NULL);
445-
LockBuffer(MetaBuffer, BUFFER_LOCK_EXCLUSIVE);
446-
RootBuffer =
447-
ReadBufferExtended(index, INIT_FORKNUM, P_NEW, RBM_NORMAL, NULL);
448-
LockBuffer(RootBuffer, BUFFER_LOCK_EXCLUSIVE);
443+
MetaBuffer = ExtendBufferedRel(EB_REL(index), INIT_FORKNUM, NULL,
444+
EB_LOCK_FIRST | EB_SKIP_EXTENSION_LOCK);
445+
RootBuffer = ExtendBufferedRel(EB_REL(index), INIT_FORKNUM, NULL,
446+
EB_LOCK_FIRST | EB_SKIP_EXTENSION_LOCK);
449447

450448
/* Initialize and xlog metabuffer and root buffer. */
451449
START_CRIT_SECTION();

‎src/backend/access/gin/ginutil.c

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -299,7 +299,6 @@ Buffer
299299
GinNewBuffer(Relation index)
300300
{
301301
Buffer buffer;
302-
bool needLock;
303302

304303
/* First, try to get a page from FSM */
305304
for (;;)
@@ -328,15 +327,8 @@ GinNewBuffer(Relation index)
328327
}
329328

330329
/* Must extend the file */
331-
needLock = !RELATION_IS_LOCAL(index);
332-
if (needLock)
333-
LockRelationForExtension(index, ExclusiveLock);
334-
335-
buffer = ReadBuffer(index, P_NEW);
336-
LockBuffer(buffer, GIN_EXCLUSIVE);
337-
338-
if (needLock)
339-
UnlockRelationForExtension(index, ExclusiveLock);
330+
buffer = ExtendBufferedRel(EB_REL(index), MAIN_FORKNUM, NULL,
331+
EB_LOCK_FIRST);
340332

341333
return buffer;
342334
}

‎src/backend/access/gist/gist.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,8 @@ gistbuildempty(Relation index)
134134
Buffer buffer;
135135

136136
/* Initialize the root page */
137-
buffer = ReadBufferExtended(index, INIT_FORKNUM, P_NEW, RBM_NORMAL, NULL);
138-
LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
137+
buffer = ExtendBufferedRel(EB_REL(index), INIT_FORKNUM, NULL,
138+
EB_SKIP_EXTENSION_LOCK | EB_LOCK_FIRST);
139139

140140
/* Initialize and xlog buffer */
141141
START_CRIT_SECTION();

‎src/backend/access/gist/gistutil.c

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -824,7 +824,6 @@ Buffer
824824
gistNewBuffer(Relation r, Relation heaprel)
825825
{
826826
Buffer buffer;
827-
bool needLock;
828827

829828
/* First, try to get a page from FSM */
830829
for (;;)
@@ -878,16 +877,8 @@ gistNewBuffer(Relation r, Relation heaprel)
878877
}
879878

880879
/* Must extend the file */
881-
needLock = !RELATION_IS_LOCAL(r);
882-
883-
if (needLock)
884-
LockRelationForExtension(r, ExclusiveLock);
885-
886-
buffer = ReadBuffer(r, P_NEW);
887-
LockBuffer(buffer, GIST_EXCLUSIVE);
888-
889-
if (needLock)
890-
UnlockRelationForExtension(r, ExclusiveLock);
880+
buffer = ExtendBufferedRel(EB_REL(r), MAIN_FORKNUM, NULL,
881+
EB_LOCK_FIRST);
891882

892883
return buffer;
893884
}

‎src/backend/access/hash/hashpage.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -206,14 +206,14 @@ _hash_getnewbuf(Relation rel, BlockNumber blkno, ForkNumber forkNum)
206206
elog(ERROR, "access to noncontiguous page in hash index \"%s\"",
207207
RelationGetRelationName(rel));
208208

209-
/* smgr insists we use P_NEW to extend the relation */
209+
/* smgr insists we explicitly extend the relation */
210210
if (blkno == nblocks)
211211
{
212-
buf = ReadBufferExtended(rel, forkNum, P_NEW, RBM_NORMAL, NULL);
212+
buf = ExtendBufferedRel(EB_REL(rel), forkNum, NULL,
213+
EB_LOCK_FIRST | EB_SKIP_EXTENSION_LOCK);
213214
if (BufferGetBlockNumber(buf) != blkno)
214215
elog(ERROR, "unexpected hash relation size: %u, should be %u",
215216
BufferGetBlockNumber(buf), blkno);
216-
LockBuffer(buf, HASH_WRITE);
217217
}
218218
else
219219
{

‎src/backend/access/nbtree/nbtpage.c

Lines changed: 9 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -882,7 +882,6 @@ _bt_getbuf(Relation rel, Relation heaprel, BlockNumber blkno, int access)
882882
}
883883
else
884884
{
885-
bool needLock;
886885
Page page;
887886

888887
Assert(access == BT_WRITE);
@@ -963,31 +962,16 @@ _bt_getbuf(Relation rel, Relation heaprel, BlockNumber blkno, int access)
963962
}
964963

965964
/*
966-
* Extend the relation by one page.
967-
*
968-
* We have to use a lock to ensure no one else is extending the rel at
969-
* the same time, else we will both try to initialize the same new
970-
* page. We can skip locking for new or temp relations, however,
971-
* since no one else could be accessing them.
972-
*/
973-
needLock = !RELATION_IS_LOCAL(rel);
974-
975-
if (needLock)
976-
LockRelationForExtension(rel, ExclusiveLock);
977-
978-
buf = ReadBuffer(rel, P_NEW);
979-
980-
/* Acquire buffer lock on new page */
981-
_bt_lockbuf(rel, buf, BT_WRITE);
982-
983-
/*
984-
* Release the file-extension lock; it's now OK for someone else to
985-
* extend the relation some more. Note that we cannot release this
986-
* lock before we have buffer lock on the new page, or we risk a race
987-
* condition against btvacuumscan --- see comments therein.
965+
* Extend the relation by one page. Need to use RBM_ZERO_AND_LOCK or
966+
* we risk a race condition against btvacuumscan --- see comments
967+
* therein. This forces us to repeat the valgrind request that
968+
* _bt_lockbuf() otherwise would make, as we can't use _bt_lockbuf()
969+
* without introducing a race.
988970
*/
989-
if (needLock)
990-
UnlockRelationForExtension(rel, ExclusiveLock);
971+
buf = ExtendBufferedRel(EB_REL(rel), MAIN_FORKNUM, NULL,
972+
EB_LOCK_FIRST);
973+
if (!RelationUsesLocalBuffers(rel))
974+
VALGRIND_MAKE_MEM_DEFINED(BufferGetPage(buf), BLCKSZ);
991975

992976
/* Initialize the new page before returning it */
993977
page = BufferGetPage(buf);

‎src/backend/access/nbtree/nbtree.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -970,6 +970,9 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
970970
* write-lock on the left page before it adds a right page, so we must
971971
* already have processed any tuples due to be moved into such a page.
972972
*
973+
* XXX: Now that new pages are locked with RBM_ZERO_AND_LOCK, I don't
974+
* think the use of the extension lock is still required.
975+
*
973976
* We can skip locking for new or temp relations, however, since no one
974977
* else could be accessing them.
975978
*/

‎src/backend/access/spgist/spgutils.c

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -366,7 +366,6 @@ Buffer
366366
SpGistNewBuffer(Relation index)
367367
{
368368
Buffer buffer;
369-
bool needLock;
370369

371370
/* First, try to get a page from FSM */
372371
for (;;)
@@ -406,16 +405,8 @@ SpGistNewBuffer(Relation index)
406405
ReleaseBuffer(buffer);
407406
}
408407

409-
/* Must extend the file */
410-
needLock = !RELATION_IS_LOCAL(index);
411-
if (needLock)
412-
LockRelationForExtension(index, ExclusiveLock);
413-
414-
buffer = ReadBuffer(index, P_NEW);
415-
LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
416-
417-
if (needLock)
418-
UnlockRelationForExtension(index, ExclusiveLock);
408+
buffer = ExtendBufferedRel(EB_REL(index), MAIN_FORKNUM, NULL,
409+
EB_LOCK_FIRST);
419410

420411
return buffer;
421412
}

‎src/backend/commands/sequence.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,8 @@ fill_seq_fork_with_data(Relation rel, HeapTuple tuple, ForkNumber forkNum)
377377

378378
/* Initialize first page of relation with special magic number */
379379

380-
buf = ReadBufferExtended(rel, forkNum, P_NEW, RBM_ZERO_AND_LOCK, NULL);
380+
buf = ExtendBufferedRel(EB_REL(rel), forkNum, NULL,
381+
EB_LOCK_FIRST | EB_SKIP_EXTENSION_LOCK);
381382
Assert(BufferGetBlockNumber(buf) == 0);
382383

383384
page = BufferGetPage(buf);

0 commit comments

Comments
 (0)