database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-07-30 11:03:19 +03:00
Code Activity

Do all accesses to shared buffer headers through volatile-qualified

Browse Source 
pointers, to ensure that compilers won't rearrange accesses to occur
while we're not holding the buffer header spinlock.  It's probably
not necessary to mark volatile in every single place in bufmgr.c,
but better safe than sorry.  Per trouble report from Kevin Grittner.
This commit is contained in:
Tom Lane
2005-10-12 16:45:14 +00:00
parent 6b97e437ca
commit 07eeb9d109
4 changed files with 64 additions and 71 deletions
Download Patch File Download Diff File Expand all files Collapse all files

27
contrib/pg_buffercache/pg_buffercache_pages.c
View File

@ -3,7 +3,7 @@
* pg_buffercache_pages.c * pg_buffercache_pages.c
* display some contents of the buffer cache * display some contents of the buffer cache
* *
* $PostgreSQL: pgsql/contrib/pg_buffercache/pg_buffercache_pages.c,v 1.4 2005/05/31 00:07:47 tgl Exp $ * $PostgreSQL: pgsql/contrib/pg_buffercache/pg_buffercache_pages.c,v 1.5 2005/10/12 16:45:13 tgl Exp $
*------------------------------------------------------------------------- *-------------------------------------------------------------------------
*/ */
#include "postgres.h" #include "postgres.h"
@ -72,10 +72,8 @@ pg_buffercache_pages(PG_FUNCTION_ARGS)
if (SRF_IS_FIRSTCALL()) if (SRF_IS_FIRSTCALL())
{ {
RelFileNode rnode;
uint32 i; uint32 i;
BufferDesc *bufHdr; volatile BufferDesc *bufHdr;
funcctx = SRF_FIRSTCALL_INIT(); funcctx = SRF_FIRSTCALL_INIT();
@ -136,35 +134,24 @@ pg_buffercache_pages(PG_FUNCTION_ARGS)
/* Lock each buffer header before inspecting. */ /* Lock each buffer header before inspecting. */
LockBufHdr(bufHdr); LockBufHdr(bufHdr);
rnode = bufHdr->tag.rnode;
fctx->record[i].bufferid = BufferDescriptorGetBuffer(bufHdr); fctx->record[i].bufferid = BufferDescriptorGetBuffer(bufHdr);
fctx->record[i].relfilenode = rnode.relNode; fctx->record[i].relfilenode = bufHdr->tag.rnode.relNode;
fctx->record[i].reltablespace = rnode.spcNode; fctx->record[i].reltablespace = bufHdr->tag.rnode.spcNode;
fctx->record[i].reldatabase = rnode.dbNode; fctx->record[i].reldatabase = bufHdr->tag.rnode.dbNode;
fctx->record[i].blocknum = bufHdr->tag.blockNum; fctx->record[i].blocknum = bufHdr->tag.blockNum;
if ( bufHdr->flags & BM_DIRTY) if (bufHdr->flags & BM_DIRTY)
{
fctx->record[i].isdirty = true; fctx->record[i].isdirty = true;
}
else else
{
fctx->record[i].isdirty = false; fctx->record[i].isdirty = false;
}
/* Note if the buffer is valid, and has storage created */ /* Note if the buffer is valid, and has storage created */
if ( (bufHdr->flags & BM_VALID) && (bufHdr->flags & BM_TAG_VALID)) if ((bufHdr->flags & BM_VALID) && (bufHdr->flags & BM_TAG_VALID))
{
fctx->record[i].isvalid = true; fctx->record[i].isvalid = true;
}
else else
{
fctx->record[i].isvalid = false; fctx->record[i].isvalid = false;
}
UnlockBufHdr(bufHdr); UnlockBufHdr(bufHdr);
} }
/* Release Buffer map. */ /* Release Buffer map. */

89
src/backend/storage/buffer/bufmgr.c
View File

@ -8,7 +8,7 @@
* *
* *
* IDENTIFICATION * IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/storage/buffer/bufmgr.c,v 1.195 2005/08/12 23:13:54 momjian Exp $ * $PostgreSQL: pgsql/src/backend/storage/buffer/bufmgr.c,v 1.196 2005/10/12 16:45:13 tgl Exp $
* *
*------------------------------------------------------------------------- *-------------------------------------------------------------------------
*/ */
@ -76,24 +76,24 @@ long NDirectFileWrite; /* e.g., I/O in psort and hashjoin. */
/* local state for StartBufferIO and related functions */ /* local state for StartBufferIO and related functions */
static BufferDesc *InProgressBuf = NULL; static volatile BufferDesc *InProgressBuf = NULL;
static bool IsForInput; static bool IsForInput;
/* local state for LockBufferForCleanup */ /* local state for LockBufferForCleanup */
static BufferDesc *PinCountWaitBuf = NULL; static volatile BufferDesc *PinCountWaitBuf = NULL;
static bool PinBuffer(BufferDesc *buf); static bool PinBuffer(volatile BufferDesc *buf);
static void PinBuffer_Locked(BufferDesc *buf); static void PinBuffer_Locked(volatile BufferDesc *buf);
static void UnpinBuffer(BufferDesc *buf, bool fixOwner, bool trashOK); static void UnpinBuffer(volatile BufferDesc *buf, bool fixOwner, bool trashOK);
static bool SyncOneBuffer(int buf_id, bool skip_pinned); static bool SyncOneBuffer(int buf_id, bool skip_pinned);
static void WaitIO(BufferDesc *buf); static void WaitIO(volatile BufferDesc *buf);
static bool StartBufferIO(BufferDesc *buf, bool forInput); static bool StartBufferIO(volatile BufferDesc *buf, bool forInput);
static void TerminateBufferIO(BufferDesc *buf, bool clear_dirty, static void TerminateBufferIO(volatile BufferDesc *buf, bool clear_dirty,
int set_flag_bits); int set_flag_bits);
static void buffer_write_error_callback(void *arg); static void buffer_write_error_callback(void *arg);
static BufferDesc *BufferAlloc(Relation reln, BlockNumber blockNum, static volatile BufferDesc *BufferAlloc(Relation reln, BlockNumber blockNum,
bool *foundPtr); bool *foundPtr);
static void FlushBuffer(BufferDesc *buf, SMgrRelation reln); static void FlushBuffer(volatile BufferDesc *buf, SMgrRelation reln);
static void AtProcExit_Buffers(int code, Datum arg); static void AtProcExit_Buffers(int code, Datum arg);
static void write_buffer(Buffer buffer, bool unpin); static void write_buffer(Buffer buffer, bool unpin);
@ -116,7 +116,7 @@ static void write_buffer(Buffer buffer, bool unpin);
Buffer Buffer
ReadBuffer(Relation reln, BlockNumber blockNum) ReadBuffer(Relation reln, BlockNumber blockNum)
{ {
BufferDesc *bufHdr; volatile BufferDesc *bufHdr;
Block bufBlock; Block bufBlock;
bool found; bool found;
bool isExtend; bool isExtend;
@ -255,7 +255,7 @@ ReadBuffer(Relation reln, BlockNumber blockNum)
* *
* No locks are held either at entry or exit. * No locks are held either at entry or exit.
*/ */
static BufferDesc * static volatile BufferDesc *
BufferAlloc(Relation reln, BufferAlloc(Relation reln,
BlockNumber blockNum, BlockNumber blockNum,
bool *foundPtr) bool *foundPtr)
@ -264,7 +264,7 @@ BufferAlloc(Relation reln,
BufferTag oldTag; BufferTag oldTag;
BufFlags oldFlags; BufFlags oldFlags;
int buf_id; int buf_id;
BufferDesc *buf; volatile BufferDesc *buf;
bool valid; bool valid;
/* create a tag so we can lookup the buffer */ /* create a tag so we can lookup the buffer */
@ -512,7 +512,7 @@ BufferAlloc(Relation reln,
* to acquire the necessary locks; if so, don't mess it up. * to acquire the necessary locks; if so, don't mess it up.
*/ */
static void static void
InvalidateBuffer(BufferDesc *buf) InvalidateBuffer(volatile BufferDesc *buf)
{ {
BufferTag oldTag; BufferTag oldTag;
BufFlags oldFlags; BufFlags oldFlags;
@ -602,7 +602,7 @@ retry:
static void static void
write_buffer(Buffer buffer, bool unpin) write_buffer(Buffer buffer, bool unpin)
{ {
BufferDesc *bufHdr; volatile BufferDesc *bufHdr;
if (!BufferIsValid(buffer)) if (!BufferIsValid(buffer))
elog(ERROR, "bad buffer id: %d", buffer); elog(ERROR, "bad buffer id: %d", buffer);
@ -679,7 +679,7 @@ ReleaseAndReadBuffer(Buffer buffer,
Relation relation, Relation relation,
BlockNumber blockNum) BlockNumber blockNum)
{ {
BufferDesc *bufHdr; volatile BufferDesc *bufHdr;
if (BufferIsValid(buffer)) if (BufferIsValid(buffer))
{ {
@ -722,7 +722,7 @@ ReleaseAndReadBuffer(Buffer buffer,
* some callers to avoid an extra spinlock cycle. * some callers to avoid an extra spinlock cycle.
*/ */
static bool static bool
PinBuffer(BufferDesc *buf) PinBuffer(volatile BufferDesc *buf)
{ {
int b = buf->buf_id; int b = buf->buf_id;
bool result; bool result;
@ -760,7 +760,7 @@ PinBuffer(BufferDesc *buf)
* its state can change under us. * its state can change under us.
*/ */
static void static void
PinBuffer_Locked(BufferDesc *buf) PinBuffer_Locked(volatile BufferDesc *buf)
{ {
int b = buf->buf_id; int b = buf->buf_id;
@ -787,7 +787,7 @@ PinBuffer_Locked(BufferDesc *buf)
* used recently, and trashOK is true, send the buffer to the freelist. * used recently, and trashOK is true, send the buffer to the freelist.
*/ */
static void static void
UnpinBuffer(BufferDesc *buf, bool fixOwner, bool trashOK) UnpinBuffer(volatile BufferDesc *buf, bool fixOwner, bool trashOK)
{ {
int b = buf->buf_id; int b = buf->buf_id;
@ -967,7 +967,7 @@ BgBufferSync(void)
static bool static bool
SyncOneBuffer(int buf_id, bool skip_pinned) SyncOneBuffer(int buf_id, bool skip_pinned)
{ {
BufferDesc *bufHdr = &BufferDescriptors[buf_id]; volatile BufferDesc *bufHdr = &BufferDescriptors[buf_id];
/* /*
* Check whether buffer needs writing. * Check whether buffer needs writing.
@ -1114,7 +1114,7 @@ AtProcExit_Buffers(int code, Datum arg)
{ {
if (PrivateRefCount[i] != 0) if (PrivateRefCount[i] != 0)
{ {
BufferDesc *buf = &(BufferDescriptors[i]); volatile BufferDesc *buf = &(BufferDescriptors[i]);
/* /*
* We don't worry about updating ResourceOwner; if we even got * We don't worry about updating ResourceOwner; if we even got
@ -1136,7 +1136,7 @@ AtProcExit_Buffers(int code, Datum arg)
void void
PrintBufferLeakWarning(Buffer buffer) PrintBufferLeakWarning(Buffer buffer)
{ {
BufferDesc *buf; volatile BufferDesc *buf;
int32 loccount; int32 loccount;
Assert(BufferIsValid(buffer)); Assert(BufferIsValid(buffer));
@ -1199,7 +1199,7 @@ BufmgrCommit(void)
BlockNumber BlockNumber
BufferGetBlockNumber(Buffer buffer) BufferGetBlockNumber(Buffer buffer)
{ {
BufferDesc *bufHdr; volatile BufferDesc *bufHdr;
Assert(BufferIsPinned(buffer)); Assert(BufferIsPinned(buffer));
@ -1222,7 +1222,7 @@ BufferGetBlockNumber(Buffer buffer)
RelFileNode RelFileNode
BufferGetFileNode(Buffer buffer) BufferGetFileNode(Buffer buffer)
{ {
BufferDesc *bufHdr; volatile BufferDesc *bufHdr;
if (BufferIsLocal(buffer)) if (BufferIsLocal(buffer))
bufHdr = &(LocalBufferDescriptors[-buffer - 1]); bufHdr = &(LocalBufferDescriptors[-buffer - 1]);
@ -1252,7 +1252,7 @@ BufferGetFileNode(Buffer buffer)
* as the second parameter. If not, pass NULL. * as the second parameter. If not, pass NULL.
*/ */
static void static void
FlushBuffer(BufferDesc *buf, SMgrRelation reln) FlushBuffer(volatile BufferDesc *buf, SMgrRelation reln)
{ {
XLogRecPtr recptr; XLogRecPtr recptr;
ErrorContextCallback errcontext; ErrorContextCallback errcontext;
@ -1267,7 +1267,7 @@ FlushBuffer(BufferDesc *buf, SMgrRelation reln)
/* Setup error traceback support for ereport() */ /* Setup error traceback support for ereport() */
errcontext.callback = buffer_write_error_callback; errcontext.callback = buffer_write_error_callback;
errcontext.arg = buf; errcontext.arg = (void *) buf;
errcontext.previous = error_context_stack; errcontext.previous = error_context_stack;
error_context_stack = &errcontext; error_context_stack = &errcontext;
@ -1375,7 +1375,7 @@ DropRelFileNodeBuffers(RelFileNode rnode, bool istemp,
BlockNumber firstDelBlock) BlockNumber firstDelBlock)
{ {
int i; int i;
BufferDesc *bufHdr; volatile BufferDesc *bufHdr;
if (istemp) if (istemp)
{ {
@ -1427,7 +1427,7 @@ void
DropBuffers(Oid dbid) DropBuffers(Oid dbid)
{ {
int i; int i;
BufferDesc *bufHdr; volatile BufferDesc *bufHdr;
/* /*
* We needn't consider local buffers, since by assumption the target * We needn't consider local buffers, since by assumption the target
@ -1457,7 +1457,7 @@ void
PrintBufferDescs(void) PrintBufferDescs(void)
{ {
int i; int i;
BufferDesc *buf = BufferDescriptors; volatile BufferDesc *buf = BufferDescriptors;
for (i = 0; i < NBuffers; ++i, ++buf) for (i = 0; i < NBuffers; ++i, ++buf)
{ {
@ -1479,7 +1479,7 @@ void
PrintPinnedBufs(void) PrintPinnedBufs(void)
{ {
int i; int i;
BufferDesc *buf = BufferDescriptors; volatile BufferDesc *buf = BufferDescriptors;
for (i = 0; i < NBuffers; ++i, ++buf) for (i = 0; i < NBuffers; ++i, ++buf)
{ {
@ -1522,7 +1522,7 @@ void
FlushRelationBuffers(Relation rel) FlushRelationBuffers(Relation rel)
{ {
int i; int i;
BufferDesc *bufHdr; volatile BufferDesc *bufHdr;
/* Open rel at the smgr level if not already done */ /* Open rel at the smgr level if not already done */
RelationOpenSmgr(rel); RelationOpenSmgr(rel);
@ -1539,7 +1539,7 @@ FlushRelationBuffers(Relation rel)
/* Setup error traceback support for ereport() */ /* Setup error traceback support for ereport() */
errcontext.callback = buffer_write_error_callback; errcontext.callback = buffer_write_error_callback;
errcontext.arg = bufHdr; errcontext.arg = (void *) bufHdr;
errcontext.previous = error_context_stack; errcontext.previous = error_context_stack;
error_context_stack = &errcontext; error_context_stack = &errcontext;
@ -1586,7 +1586,7 @@ FlushRelationBuffers(Relation rel)
void void
ReleaseBuffer(Buffer buffer) ReleaseBuffer(Buffer buffer)
{ {
BufferDesc *bufHdr; volatile BufferDesc *bufHdr;
if (!BufferIsValid(buffer)) if (!BufferIsValid(buffer))
elog(ERROR, "bad buffer id: %d", buffer); elog(ERROR, "bad buffer id: %d", buffer);
@ -1657,7 +1657,7 @@ IncrBufferRefCount(Buffer buffer)
void void
SetBufferCommitInfoNeedsSave(Buffer buffer) SetBufferCommitInfoNeedsSave(Buffer buffer)
{ {
BufferDesc *bufHdr; volatile BufferDesc *bufHdr;
if (!BufferIsValid(buffer)) if (!BufferIsValid(buffer))
elog(ERROR, "bad buffer id: %d", buffer); elog(ERROR, "bad buffer id: %d", buffer);
@ -1694,7 +1694,7 @@ SetBufferCommitInfoNeedsSave(Buffer buffer)
void void
UnlockBuffers(void) UnlockBuffers(void)
{ {
BufferDesc *buf = PinCountWaitBuf; volatile BufferDesc *buf = PinCountWaitBuf;
if (buf) if (buf)
{ {
@ -1727,7 +1727,7 @@ UnlockBuffers(void)
void void
LockBuffer(Buffer buffer, int mode) LockBuffer(Buffer buffer, int mode)
{ {
BufferDesc *buf; volatile BufferDesc *buf;
Assert(BufferIsValid(buffer)); Assert(BufferIsValid(buffer));
if (BufferIsLocal(buffer)) if (BufferIsLocal(buffer))
@ -1765,7 +1765,7 @@ LockBuffer(Buffer buffer, int mode)
bool bool
ConditionalLockBuffer(Buffer buffer) ConditionalLockBuffer(Buffer buffer)
{ {
BufferDesc *buf; volatile BufferDesc *buf;
Assert(BufferIsValid(buffer)); Assert(BufferIsValid(buffer));
if (BufferIsLocal(buffer)) if (BufferIsLocal(buffer))
@ -1809,7 +1809,7 @@ ConditionalLockBuffer(Buffer buffer)
void void
LockBufferForCleanup(Buffer buffer) LockBufferForCleanup(Buffer buffer)
{ {
BufferDesc *bufHdr; volatile BufferDesc *bufHdr;
Assert(BufferIsValid(buffer)); Assert(BufferIsValid(buffer));
Assert(PinCountWaitBuf == NULL); Assert(PinCountWaitBuf == NULL);
@ -1875,7 +1875,7 @@ LockBufferForCleanup(Buffer buffer)
* WaitIO -- Block until the IO_IN_PROGRESS flag on 'buf' is cleared. * WaitIO -- Block until the IO_IN_PROGRESS flag on 'buf' is cleared.
*/ */
static void static void
WaitIO(BufferDesc *buf) WaitIO(volatile BufferDesc *buf)
{ {
/* /*
* Changed to wait until there's no IO - Inoue 01/13/2000 * Changed to wait until there's no IO - Inoue 01/13/2000
@ -1922,7 +1922,7 @@ WaitIO(BufferDesc *buf)
* FALSE if someone else already did the work. * FALSE if someone else already did the work.
*/ */
static bool static bool
StartBufferIO(BufferDesc *buf, bool forInput) StartBufferIO(volatile BufferDesc *buf, bool forInput)
{ {
Assert(!InProgressBuf); Assert(!InProgressBuf);
@ -1989,7 +1989,8 @@ StartBufferIO(BufferDesc *buf, bool forInput)
* be 0, or BM_VALID if we just finished reading in the page. * be 0, or BM_VALID if we just finished reading in the page.
*/ */
static void static void
TerminateBufferIO(BufferDesc *buf, bool clear_dirty, int set_flag_bits) TerminateBufferIO(volatile BufferDesc *buf, bool clear_dirty,
int set_flag_bits)
{ {
Assert(buf == InProgressBuf); Assert(buf == InProgressBuf);
@ -2021,7 +2022,7 @@ TerminateBufferIO(BufferDesc *buf, bool clear_dirty, int set_flag_bits)
void void
AbortBufferIO(void) AbortBufferIO(void)
{ {
BufferDesc *buf = InProgressBuf; volatile BufferDesc *buf = InProgressBuf;
if (buf) if (buf)
{ {
@ -2075,7 +2076,7 @@ AbortBufferIO(void)
static void static void
buffer_write_error_callback(void *arg) buffer_write_error_callback(void *arg)
{ {
BufferDesc *bufHdr = (BufferDesc *) arg; volatile BufferDesc *bufHdr = (volatile BufferDesc *) arg;
/* Buffer is pinned, so we can read the tag without locking the spinlock */ /* Buffer is pinned, so we can read the tag without locking the spinlock */
if (bufHdr != NULL) if (bufHdr != NULL)

8
src/backend/storage/buffer/freelist.c
View File

@ -9,7 +9,7 @@
* *
* *
* IDENTIFICATION * IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/storage/buffer/freelist.c,v 1.52 2005/08/20 23:26:17 tgl Exp $ * $PostgreSQL: pgsql/src/backend/storage/buffer/freelist.c,v 1.53 2005/10/12 16:45:13 tgl Exp $
* *
*------------------------------------------------------------------------- *-------------------------------------------------------------------------
*/ */
@ -55,10 +55,10 @@ bool strategy_hint_vacuum = false;
* means that we return with the BufFreelistLock still held, as well; * means that we return with the BufFreelistLock still held, as well;
* the caller must release that lock once the spinlock is dropped. * the caller must release that lock once the spinlock is dropped.
*/ */
BufferDesc * volatile BufferDesc *
StrategyGetBuffer(void) StrategyGetBuffer(void)
{ {
BufferDesc *buf; volatile BufferDesc *buf;
int trycounter; int trycounter;
LWLockAcquire(BufFreelistLock, LW_EXCLUSIVE); LWLockAcquire(BufFreelistLock, LW_EXCLUSIVE);
@ -138,7 +138,7 @@ StrategyGetBuffer(void)
* quickly the buffer is reused. * quickly the buffer is reused.
*/ */
void void
StrategyFreeBuffer(BufferDesc *buf, bool at_head) StrategyFreeBuffer(volatile BufferDesc *buf, bool at_head)
{ {
LWLockAcquire(BufFreelistLock, LW_EXCLUSIVE); LWLockAcquire(BufFreelistLock, LW_EXCLUSIVE);

11
src/include/storage/buf_internals.h
View File

@ -8,7 +8,7 @@
* Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California * Portions Copyright (c) 1994, Regents of the University of California
* *
* $PostgreSQL: pgsql/src/include/storage/buf_internals.h,v 1.79 2005/08/20 23:26:33 tgl Exp $ * $PostgreSQL: pgsql/src/include/storage/buf_internals.h,v 1.80 2005/10/12 16:45:14 tgl Exp $
* *
*------------------------------------------------------------------------- *-------------------------------------------------------------------------
*/ */
@ -145,6 +145,11 @@ typedef struct sbufdesc
* NoHoldoff cases may be used when we know that we hold some LWLock * NoHoldoff cases may be used when we know that we hold some LWLock
* and therefore interrupts are already held off. Do not apply these * and therefore interrupts are already held off. Do not apply these
* to local buffers! * to local buffers!
*
* Note: as a general coding rule, if you are using these then you probably
* want to be using a volatile-qualified pointer to the buffer header, to
* ensure that the compiler doesn't rearrange accesses to the header to
* occur before or after the spinlock is acquired/released.
*/ */
#define LockBufHdr(bufHdr) \ #define LockBufHdr(bufHdr) \
SpinLockAcquire(&(bufHdr)->buf_hdr_lock) SpinLockAcquire(&(bufHdr)->buf_hdr_lock)
@ -179,8 +184,8 @@ extern long int LocalBufferFlushCount;
*/ */
/* freelist.c */ /* freelist.c */
extern BufferDesc *StrategyGetBuffer(void); extern volatile BufferDesc *StrategyGetBuffer(void);
extern void StrategyFreeBuffer(BufferDesc *buf, bool at_head); extern void StrategyFreeBuffer(volatile BufferDesc *buf, bool at_head);
extern int StrategySyncStart(void); extern int StrategySyncStart(void);
extern Size StrategyShmemSize(void); extern Size StrategyShmemSize(void);
extern void StrategyInitialize(bool init); extern void StrategyInitialize(bool init);