Skip to content

Commit fc65a3e

Browse files
Jan WieckJan Wieck
Jan Wieck
authored and
Jan Wieck
committed
Fixed bug where FlushRelationBuffers() did call StrategyInvalidateBuffer()
for already empty buffers because their buffer tag was not cleard out when the buffers have been invalidated before. Also removed the misnamed BM_FREE bufhdr flag and replaced the checks, which effectively ask if the buffer is unpinned, with checks against the refcount field. Jan
1 parent 76f02b5 commit fc65a3e

File tree

4 files changed

+37
-123
lines changed

4 files changed

+37
-123
lines changed

‎src/backend/storage/buffer/buf_init.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/storage/buffer/buf_init.c,v 1.61 2004/01/15 16:14:26 wieck Exp $
11+
* $PostgreSQL: pgsql/src/backend/storage/buffer/buf_init.c,v 1.62 2004/02/12 15:06:56 wieck Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -164,7 +164,7 @@ InitBufferPool(void)
164164
buf->buf_id = i;
165165

166166
buf->data = MAKE_OFFSET(block);
167-
buf->flags = (BM_DELETED | BM_FREE | BM_VALID);
167+
buf->flags = (BM_DELETED | BM_VALID);
168168
buf->refcount = 0;
169169
buf->io_in_progress_lock = LWLockAssign();
170170
buf->cntx_lock = LWLockAssign();

‎src/backend/storage/buffer/bufmgr.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/storage/buffer/bufmgr.c,v 1.158 2004/02/10 03:42:45 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/storage/buffer/bufmgr.c,v 1.159 2004/02/12 15:06:56 wieck Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -1190,7 +1190,7 @@ DropRelFileNodeBuffers(RelFileNode rnode, bool istemp)
11901190
* Release any refcount we may have. If someone else has a
11911191
* pin on the buffer, we got trouble.
11921192
*/
1193-
if (!(bufHdr->flags & BM_FREE))
1193+
if (bufHdr->refcount != 0)
11941194
{
11951195
/* the sole pin should be ours */
11961196
if (bufHdr->refcount != 1 || PrivateRefCount[i - 1] == 0)
@@ -1268,7 +1268,7 @@ DropBuffers(Oid dbid)
12681268
* The thing should be free, if caller has checked that no
12691269
* backends are running in that database.
12701270
*/
1271-
Assert(bufHdr->flags & BM_FREE);
1271+
Assert(bufHdr->refcount == 0);
12721272

12731273
/*
12741274
* And mark the buffer as no longer occupied by this page.
@@ -1501,7 +1501,7 @@ FlushRelationBuffers(Relation rel, BlockNumber firstDelBlock)
15011501
}
15021502
UnpinBuffer(bufHdr);
15031503
}
1504-
if (!(bufHdr->flags & BM_FREE))
1504+
if (bufHdr->refcount != 0)
15051505
{
15061506
LWLockRelease(BufMgrLock);
15071507
error_context_stack = errcontext.previous;

‎src/backend/storage/buffer/freelist.c

Lines changed: 26 additions & 111 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/storage/buffer/freelist.c,v 1.40 2004/02/06 19:36:18 wieck Exp $
11+
* $PostgreSQL: pgsql/src/backend/storage/buffer/freelist.c,v 1.41 2004/02/12 15:06:56 wieck Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -662,51 +662,44 @@ void
662662
StrategyInvalidateBuffer(BufferDesc *buf)
663663
{
664664
int cdb_id;
665-
#ifdef USE_ASSERT_CHECKING
666-
int buf_id;
667-
#endif
668665
BufferStrategyCDB *cdb;
669666

670667
/* The buffer cannot be dirty or pinned */
671668
Assert(!(buf->flags & BM_DIRTY));
672669
Assert(buf->refcount == 0);
673670

674671
/*
675-
* If we have the buffer somewhere in the directory, remove it,
676-
* add the CDB to the list of unused CDB's. and the buffer to
677-
* the list of free buffers
672+
* Lookup the cache directory block for this buffer
678673
*/
679674
cdb_id = BufTableLookup(&(buf->tag));
680-
if (cdb_id >= 0)
681-
{
682-
cdb = &StrategyCDB[cdb_id];
683-
BufTableDelete(&(cdb->buf_tag));
684-
STRAT_LIST_REMOVE(cdb);
685-
cdb->buf_id = -1;
686-
cdb->next = StrategyControl->listUnusedCDB;
687-
StrategyControl->listUnusedCDB = cdb_id;
688-
689-
buf->bufNext = StrategyControl->listFreeBuffers;
690-
StrategyControl->listFreeBuffers = buf->buf_id;
691-
return;
692-
}
675+
if (cdb_id < 0)
676+
elog(ERROR, "StrategyInvalidateBuffer() buffer %d not in directory",
677+
buf->buf_id);
678+
cdb = &StrategyCDB[cdb_id];
693679

694-
#ifdef USE_ASSERT_CHECKING
695680
/*
696-
* Check that we have this buffer in the freelist already.
681+
* Remove the CDB from the hashtable and the ARC queue it is
682+
* currently on.
697683
*/
698-
buf_id = StrategyControl->listFreeBuffers;
699-
while (buf_id >= 0)
700-
{
701-
if (buf == &BufferDescriptors[buf_id])
702-
return;
684+
BufTableDelete(&(cdb->buf_tag));
685+
STRAT_LIST_REMOVE(cdb);
703686

704-
buf_id = BufferDescriptors[buf_id].bufNext;
705-
}
687+
/*
688+
* Clear out the CDB's buffer tag and association with the buffer
689+
* and add it to the list of unused CDB's
690+
*/
691+
CLEAR_BUFFERTAG(&(cdb->buf_tag));
692+
cdb->buf_id = -1;
693+
cdb->next = StrategyControl->listUnusedCDB;
694+
StrategyControl->listUnusedCDB = cdb_id;
706695

707-
elog(ERROR, "StrategyInvalidateBuffer() buffer %d not in directory or freelist",
708-
buf->buf_id);
709-
#endif
696+
/*
697+
* Clear out the buffers tag and add it to the list of
698+
* currently unused buffers.
699+
*/
700+
CLEAR_BUFFERTAG(&(buf->tag));
701+
buf->bufNext = StrategyControl->listFreeBuffers;
702+
StrategyControl->listFreeBuffers = buf->buf_id;
710703
}
711704

712705

@@ -869,12 +862,6 @@ PinBuffer(BufferDesc *buf)
869862
{
870863
int b = BufferDescriptorGetBuffer(buf) - 1;
871864

872-
if (buf->refcount == 0)
873-
{
874-
/* mark buffer as no longer free */
875-
buf->flags &= ~BM_FREE;
876-
}
877-
878865
if (PrivateRefCount[b] == 0)
879866
buf->refcount++;
880867
PrivateRefCount[b]++;
@@ -917,12 +904,7 @@ UnpinBuffer(BufferDesc *buf)
917904
if (PrivateRefCount[b] == 0)
918905
buf->refcount--;
919906

920-
if (buf->refcount == 0)
921-
{
922-
/* buffer is now unpinned */
923-
buf->flags |= BM_FREE;
924-
}
925-
else if ((buf->flags & BM_PIN_COUNT_WAITER) != 0 &&
907+
if ((buf->flags & BM_PIN_COUNT_WAITER) != 0 &&
926908
buf->refcount == 1)
927909
{
928910
/* we just released the last pin other than the waiter's */
@@ -953,70 +935,3 @@ refcount = %ld, file: %s, line: %d\n",
953935
#endif
954936

955937

956-
/*
957-
* print out the free list and check for breaks.
958-
*/
959-
#ifdef NOT_USED
960-
void
961-
DBG_FreeListCheck(int nfree)
962-
{
963-
int i;
964-
BufferDesc *buf;
965-
966-
buf = &(BufferDescriptors[SharedFreeList->freeNext]);
967-
for (i = 0; i < nfree; i++, buf = &(BufferDescriptors[buf->freeNext]))
968-
{
969-
if (!(buf->flags & (BM_FREE)))
970-
{
971-
if (buf != SharedFreeList)
972-
printf("\tfree list corrupted: %d flags %x\n",
973-
buf->buf_id, buf->flags);
974-
else
975-
printf("\tfree list corrupted: too short -- %d not %d\n",
976-
i, nfree);
977-
}
978-
if ((BufferDescriptors[buf->freeNext].freePrev != buf->buf_id) ||
979-
(BufferDescriptors[buf->freePrev].freeNext != buf->buf_id))
980-
printf("\tfree list links corrupted: %d %ld %ld\n",
981-
buf->buf_id, buf->freePrev, buf->freeNext);
982-
}
983-
if (buf != SharedFreeList)
984-
printf("\tfree list corrupted: %d-th buffer is %d\n",
985-
nfree, buf->buf_id);
986-
}
987-
#endif
988-
989-
#ifdef NOT_USED
990-
/*
991-
* PrintBufferFreeList -
992-
* prints the buffer free list, for debugging
993-
*/
994-
static void
995-
PrintBufferFreeList()
996-
{
997-
BufferDesc *buf;
998-
999-
if (SharedFreeList->freeNext == Free_List_Descriptor)
1000-
{
1001-
printf("free list is empty.\n");
1002-
return;
1003-
}
1004-
1005-
buf = &(BufferDescriptors[SharedFreeList->freeNext]);
1006-
for (;;)
1007-
{
1008-
int i = (buf - BufferDescriptors);
1009-
1010-
printf("[%-2d] (%s, %d) flags=0x%x, refcnt=%d %ld, nxt=%ld prv=%ld)\n",
1011-
i, buf->blind.relname, buf->tag.blockNum,
1012-
buf->flags, buf->refcount, PrivateRefCount[i],
1013-
buf->freeNext, buf->freePrev);
1014-
1015-
if (buf->freeNext == Free_List_Descriptor)
1016-
break;
1017-
1018-
buf = &(BufferDescriptors[buf->freeNext]);
1019-
}
1020-
}
1021-
1022-
#endif

‎src/include/storage/buf_internals.h

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
* Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group
99
* Portions Copyright (c) 1994, Regents of the University of California
1010
*
11-
* $PostgreSQL: pgsql/src/include/storage/buf_internals.h,v 1.67 2004/01/15 16:14:26 wieck Exp $
11+
* $PostgreSQL: pgsql/src/include/storage/buf_internals.h,v 1.68 2004/02/12 15:06:56 wieck Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -36,11 +36,10 @@ extern int ShowPinTrace;
3636
#define BM_DIRTY (1 << 0)
3737
#define BM_VALID (1 << 1)
3838
#define BM_DELETED (1 << 2)
39-
#define BM_FREE (1 << 3)
40-
#define BM_IO_IN_PROGRESS (1 << 4)
41-
#define BM_IO_ERROR (1 << 5)
42-
#define BM_JUST_DIRTIED (1 << 6)
43-
#define BM_PIN_COUNT_WAITER (1 << 7)
39+
#define BM_IO_IN_PROGRESS (1 << 3)
40+
#define BM_IO_ERROR (1 << 4)
41+
#define BM_JUST_DIRTIED (1 << 5)
42+
#define BM_PIN_COUNT_WAITER (1 << 6)
4443

4544
typedef bits16 BufFlags;
4645

0 commit comments

Comments
 (0)