database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-10-19 15:49:24 +03:00
Code Activity

bufmgr: Fix valgrind checking for buffers pinned in StrategyGetBuffer()

Browse Source 
In 5e89985928 I made StrategyGetBuffer() pin buffers with a single CAS,
instead of using PinBuffer_Locked(). Unfortunately I missed that
PinBuffer_Locked() marked the page as defined for valgrind.

Fix this oversight by centralizing the valgrind initialization into
TrackNewBufferPin(), which also allows us to reduce the number of places doing
VALGRIND_MAKE_MEM_DEFINED.

Per buildfarm animal skink and Amit Langote.

Discussion: https://postgr.es/m/fvfmkr5kk4nyex56ejgxj3uzi63isfxovp2biecb4bspbjrze7@az2pljabhnff
Discussion: https://postgr.es/m/CA+HiwqGKJ6nEXEPQW7EpykVsEtzxp5-up_xhtcUAkWFtATVQvQ@mail.gmail.com
This commit is contained in:
Andres Freund
2025-10-09 16:27:08 -04:00
parent 9d46b86529
commit c819d1017d
1 changed files with 16 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -3113,15 +3113,6 @@ PinBuffer(BufferDesc *buf, BufferAccessStrategy strategy,
result = (buf_state & BM_VALID) != 0; result = (buf_state & BM_VALID) != 0;
TrackNewBufferPin(b); TrackNewBufferPin(b);
/*
* Assume that we acquired a buffer pin for the purposes of
* Valgrind buffer client checks (even in !result case) to
* keep things simple. Buffers that are unsafe to access are
* not generally guaranteed to be marked undefined or
* non-accessible in any case.
*/
VALGRIND_MAKE_MEM_DEFINED(BufHdrGetBlock(buf), BLCKSZ);
break; break;
} }
} }
@@ -3186,13 +3177,6 @@ PinBuffer_Locked(BufferDesc *buf)
*/ */
Assert(GetPrivateRefCountEntry(BufferDescriptorGetBuffer(buf), false) == NULL); Assert(GetPrivateRefCountEntry(BufferDescriptorGetBuffer(buf), false) == NULL);
/*
* Buffer can't have a preexisting pin, so mark its page as defined to
* Valgrind (this is similar to the PinBuffer() case where the backend
* doesn't already have a buffer pin)
*/
VALGRIND_MAKE_MEM_DEFINED(BufHdrGetBlock(buf), BLCKSZ);
/* /*
* Since we hold the buffer spinlock, we can update the buffer state and * Since we hold the buffer spinlock, we can update the buffer state and
* release the lock in one operation. * release the lock in one operation.
@@ -3320,6 +3304,10 @@ UnpinBufferNoOwner(BufferDesc *buf)
} }
} }
/*
* Set up backend-local tracking of a buffer pinned the first time by this
* backend.
*/
inline void inline void
TrackNewBufferPin(Buffer buf) TrackNewBufferPin(Buffer buf)
{ {
@@ -3329,6 +3317,18 @@ TrackNewBufferPin(Buffer buf)
ref->refcount++; ref->refcount++;
ResourceOwnerRememberBuffer(CurrentResourceOwner, buf); ResourceOwnerRememberBuffer(CurrentResourceOwner, buf);
/*
* This is the first pin for this page by this backend, mark its page as
* defined to valgrind. While the page contents might not actually be
* valid yet, we don't currently guarantee that such pages are marked
* undefined or non-accessible.
*
* It's not necessarily the prettiest to do this here, but otherwise we'd
* need this block of code in multiple places.
*/
VALGRIND_MAKE_MEM_DEFINED(BufHdrGetBlock(GetBufferDescriptor(buf - 1)),
BLCKSZ);
} }
#define ST_SORT sort_checkpoint_bufferids #define ST_SORT sort_checkpoint_bufferids