Attached patch fixes oversights converting buf_id to Buffer in
PrintBufferDescs() and InvalidateBuffer(). Especially for the latter,
the reason we haven't seen any reports of the issue might be that it
needs certain concurrent conditions to be true.
Along the line, it also changes all direct maths against buf_id to use
BufferDescriptorGetBuffer() instead.
Regards,
Qingqing
diff --git a/src/backend/storage/buffer/bufmgr.c
b/src/backend/storage/buffer/bufmgr.c
new file mode 100644
index e4b2558..2e9a7c7
*** a/src/backend/storage/buffer/bufmgr.c
--- b/src/backend/storage/buffer/bufmgr.c
*************** retry:
*** 1273,1279 ****
UnlockBufHdr(buf);
LWLockRelease(oldPartitionLock);
/* safety check: should definitely not be our *own* pin */
! if (GetPrivateRefCount(buf->buf_id) > 0)
elog(ERROR, "buffer is pinned in InvalidateBuffer");
WaitIO(buf);
goto retry;
--- 1273,1279 ----
UnlockBufHdr(buf);
LWLockRelease(oldPartitionLock);
/* safety check: should definitely not be our *own* pin */
! if (GetPrivateRefCount(BufferDescriptorGetBuffer(buf)) > 0)
elog(ERROR, "buffer is pinned in InvalidateBuffer");
WaitIO(buf);
goto retry;
*************** ReleaseAndReadBuffer(Buffer buffer,
*** 1426,1441 ****
static bool
PinBuffer(volatile BufferDesc *buf, BufferAccessStrategy strategy)
{
! int b = buf->buf_id;
bool result;
PrivateRefCountEntry *ref;
! ref = GetPrivateRefCountEntry(b + 1, true);
if (ref == NULL)
{
ReservePrivateRefCountEntry();
! ref = NewPrivateRefCountEntry(b + 1);
LockBufHdr(buf);
buf->refcount++;
--- 1426,1441 ----
static bool
PinBuffer(volatile BufferDesc *buf, BufferAccessStrategy strategy)
{
! Buffer b = BufferDescriptorGetBuffer(buf);
bool result;
PrivateRefCountEntry *ref;
! ref = GetPrivateRefCountEntry(b, true);
if (ref == NULL)
{
ReservePrivateRefCountEntry();
! ref = NewPrivateRefCountEntry(b);
LockBufHdr(buf);
buf->refcount++;
*************** PinBuffer(volatile BufferDesc *buf, Buff
*** 1460,1467 ****
ref->refcount++;
Assert(ref->refcount > 0);
! ResourceOwnerRememberBuffer(CurrentResourceOwner,
!
BufferDescriptorGetBuffer(buf));
return result;
}
--- 1460,1466 ----
ref->refcount++;
Assert(ref->refcount > 0);
! ResourceOwnerRememberBuffer(CurrentResourceOwner, b);
return result;
}
*************** PinBuffer(volatile BufferDesc *buf, Buff
*** 1489,1511 ****
static void
PinBuffer_Locked(volatile BufferDesc *buf)
{
! int b = buf->buf_id;
PrivateRefCountEntry *ref;
/*
* As explained, We don't expect any preexisting pins. That allows us to
* manipulate the PrivateRefCount after releasing the spinlock
*/
! Assert(GetPrivateRefCountEntry(b + 1, false) == NULL);
buf->refcount++;
UnlockBufHdr(buf);
! ref = NewPrivateRefCountEntry(b + 1);
ref->refcount++;
! ResourceOwnerRememberBuffer(CurrentResourceOwner,
!
BufferDescriptorGetBuffer(buf));
}
/*
--- 1488,1509 ----
static void
PinBuffer_Locked(volatile BufferDesc *buf)
{
! Buffer b = BufferDescriptorGetBuffer(buf);
PrivateRefCountEntry *ref;
/*
* As explained, We don't expect any preexisting pins. That allows us to
* manipulate the PrivateRefCount after releasing the spinlock
*/
! Assert(GetPrivateRefCountEntry(b, false) == NULL);
buf->refcount++;
UnlockBufHdr(buf);
! ref = NewPrivateRefCountEntry(b);
ref->refcount++;
! ResourceOwnerRememberBuffer(CurrentResourceOwner, b);
}
/*
*************** static void
*** 1520,1533 ****
UnpinBuffer(volatile BufferDesc *buf, bool fixOwner)
{
PrivateRefCountEntry *ref;
/* not moving as we're likely deleting it soon anyway */
! ref = GetPrivateRefCountEntry(buf->buf_id + 1, false);
Assert(ref != NULL);
if (fixOwner)
! ResourceOwnerForgetBuffer(CurrentResourceOwner,
!
BufferDescriptorGetBuffer(buf));
Assert(ref->refcount > 0);
ref->refcount--;
--- 1518,1531 ----
UnpinBuffer(volatile BufferDesc *buf, bool fixOwner)
{
PrivateRefCountEntry *ref;
+ Buffer b = BufferDescriptorGetBuffer(buf);
/* not moving as we're likely deleting it soon anyway */
! ref = GetPrivateRefCountEntry(b, false);
Assert(ref != NULL);
if (fixOwner)
! ResourceOwnerForgetBuffer(CurrentResourceOwner, b);
Assert(ref->refcount > 0);
ref->refcount--;
*************** PrintBufferDescs(void)
*** 2739,2753 ****
for (i = 0; i < NBuffers; ++i)
{
volatile BufferDesc *buf = GetBufferDescriptor(i);
/* theoretically we should lock the bufhdr here */
elog(LOG,
"[%02d] (freeNext=%d, rel=%s, "
"blockNum=%u, flags=0x%x, refcount=%u %d)",
i, buf->freeNext,
! relpathbackend(buf->tag.rnode, InvalidBackendId,
buf->tag.forkNum),
buf->tag.blockNum, buf->flags,
! buf->refcount, GetPrivateRefCount(i));
}
}
#endif
--- 2737,2753 ----
for (i = 0; i < NBuffers; ++i)
{
volatile BufferDesc *buf = GetBufferDescriptor(i);
+ Buffer b = BufferDescriptorGetBuffer(buf);
/* theoretically we should lock the bufhdr here */
+ Assert (b == i + 1);
elog(LOG,
"[%02d] (freeNext=%d, rel=%s, "
"blockNum=%u, flags=0x%x, refcount=%u %d)",
i, buf->freeNext,
! relpathbackend(buf->tag.rnode, InvalidBackendId,
buf->tag.forkNum),
buf->tag.blockNum, buf->flags,
! buf->refcount, GetPrivateRefCount(b));
}
}
#endif
*************** PrintPinnedBufs(void)
*** 2761,2768 ****
for (i = 0; i < NBuffers; ++i)
{
volatile BufferDesc *buf = GetBufferDescriptor(i);
! if (GetPrivateRefCount(i + 1) > 0)
{
/* theoretically we should lock the bufhdr here */
elog(LOG,
--- 2761,2770 ----
for (i = 0; i < NBuffers; ++i)
{
volatile BufferDesc *buf = GetBufferDescriptor(i);
+ Buffer b = BufferDescriptorGetBuffer(buf);
! Assert (b == i + 1);
! if (GetPrivateRefCount(b) > 0)
{
/* theoretically we should lock the bufhdr here */
elog(LOG,
*************** PrintPinnedBufs(void)
*** 2771,2777 ****
i, buf->freeNext,
relpathperm(buf->tag.rnode, buf->tag.forkNum),
buf->tag.blockNum, buf->flags,
! buf->refcount, GetPrivateRefCount(i + 1));
}
}
}
--- 2773,2779 ----
i, buf->freeNext,
relpathperm(buf->tag.rnode, buf->tag.forkNum),
buf->tag.blockNum, buf->flags,
! buf->refcount, GetPrivateRefCount(b));
}
}
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
