public inbox for [email protected]
help / color / mirror / Atom feedbufmgr: pass through I/O stats context in FlushUnlockedBuffer()
3+ messages / 2 participants
[nested] [flat]
* bufmgr: pass through I/O stats context in FlushUnlockedBuffer()
@ 2026-04-01 02:15 Chao Li <[email protected]>
0 siblings, 1 reply; 3+ messages in thread
From: Chao Li @ 2026-04-01 02:15 UTC (permalink / raw)
To: pgsql-hackers; +Cc: Andres Freund <[email protected]>; Melanie Plageman <[email protected]>
Hi,
I noticed that FlushUnlockedBuffer() accepts io_object and io_context, but then ignores them and hardcodes IOOBJECT_RELATIONand IOCONTEXT_NORMAL instead:
```
static void
FlushUnlockedBuffer(BufferDesc *buf, SMgrRelation reln,
IOObject io_object, IOContext io_context)
{
Buffer buffer = BufferDescriptorGetBuffer(buf);
BufferLockAcquire(buffer, buf, BUFFER_LOCK_SHARE_EXCLUSIVE);
FlushBuffer(buf, reln, IOOBJECT_RELATION, IOCONTEXT_NORMAL); // <== HERE
BufferLockUnlock(buffer, buf);
}
```
Unless I am missing something, if a function accepts these parameters, they should generally be used.
FlushBuffer() seems to have the same issue. It takes both io_object and io_context:
```
static void
FlushBuffer(BufferDesc *buf, SMgrRelation reln, IOObject io_object,
IOContext io_context)
```
but while io_context is used, io_object is ignored:
```
pgstat_count_io_op_time(IOOBJECT_RELATION, io_context,
IOOP_WRITE, io_start, 1, BLCKSZ);
```
For comparison, in AsyncReadBuffers(), where io_object is also available locally, it is passed through and used directly:
```
pgstat_count_io_op_time(io_object, io_context, IOOP_READ,
io_start, 1, io_buffers_len * BLCKSZ);
```
I raised the same point while reviewing patch [1], but that patch has not been updated yet.
This tiny patch just makes FlushUnlockedBuffer() and FlushBuffer() use the io_object and io_context values passed in by the caller.
[1] Discussion: https://postgr.es/m/[email protected]
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Attachments:
[application/octet-stream] v1-0001-bufmgr-pass-through-I-O-stats-context-in-FlushUnl.patch (1.8K, 2-v1-0001-bufmgr-pass-through-I-O-stats-context-in-FlushUnl.patch)
download | inline diff:
From 118dfb42dfce72ada4e8a1f7ce081bf5da149809 Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <[email protected]>
Date: Wed, 1 Apr 2026 09:52:41 +0800
Subject: [PATCH v1] bufmgr: pass through I/O stats context in
FlushUnlockedBuffer()
FlushUnlockedBuffer() accepted io_object and io_context arguments, but
ignored them and always called FlushBuffer() with IOOBJECT_RELATION and
IOCONTEXT_NORMAL.
Fix that by passing the caller-provided io_object and io_context through
to FlushBuffer(). While here, make FlushBuffer() use its io_object
parameter when reporting I/O timing, instead of hardcoding
IOOBJECT_RELATION.
Author: Chao Li <[email protected]>
Reviewed-by:
Discussion: https://postgr.es/m/
---
src/backend/storage/buffer/bufmgr.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index 17499451ad2..0619d7b8d62 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -4583,7 +4583,7 @@ FlushBuffer(BufferDesc *buf, SMgrRelation reln, IOObject io_object,
* When a strategy is not in use, the write can only be a "regular" write
* of a dirty shared buffer (IOCONTEXT_NORMAL IOOP_WRITE).
*/
- pgstat_count_io_op_time(IOOBJECT_RELATION, io_context,
+ pgstat_count_io_op_time(io_object, io_context,
IOOP_WRITE, io_start, 1, BLCKSZ);
pgBufferUsage.shared_blks_written++;
@@ -4614,7 +4614,7 @@ FlushUnlockedBuffer(BufferDesc *buf, SMgrRelation reln,
Buffer buffer = BufferDescriptorGetBuffer(buf);
BufferLockAcquire(buffer, buf, BUFFER_LOCK_SHARE_EXCLUSIVE);
- FlushBuffer(buf, reln, IOOBJECT_RELATION, IOCONTEXT_NORMAL);
+ FlushBuffer(buf, reln, io_object, io_context);
BufferLockUnlock(buffer, buf);
}
--
2.50.1 (Apple Git-155)
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: bufmgr: pass through I/O stats context in FlushUnlockedBuffer()
@ 2026-04-21 21:52 Melanie Plageman <[email protected]>
parent: Chao Li <[email protected]>
0 siblings, 1 reply; 3+ messages in thread
From: Melanie Plageman @ 2026-04-21 21:52 UTC (permalink / raw)
To: Chao Li <[email protected]>; +Cc: pgsql-hackers; Andres Freund <[email protected]>
On Tue, Mar 31, 2026 at 10:15 PM Chao Li <[email protected]> wrote:
>
> I noticed that FlushUnlockedBuffer() accepts io_object and io_context, but then ignores them and hardcodes IOOBJECT_RELATIONand IOCONTEXT_NORMAL instead:
> ```
> static void
> FlushUnlockedBuffer(BufferDesc *buf, SMgrRelation reln,
> IOObject io_object, IOContext io_context)
> {
> Buffer buffer = BufferDescriptorGetBuffer(buf);
>
> BufferLockAcquire(buffer, buf, BUFFER_LOCK_SHARE_EXCLUSIVE);
> FlushBuffer(buf, reln, IOOBJECT_RELATION, IOCONTEXT_NORMAL); // <== HERE
> BufferLockUnlock(buffer, buf);
> }
> ```
>
> Unless I am missing something, if a function accepts these parameters, they should generally be used.
Thanks for the patch. Committed in 31b0544b32b
- Melanie
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: bufmgr: pass through I/O stats context in FlushUnlockedBuffer()
@ 2026-04-21 22:24 Chao Li <[email protected]>
parent: Melanie Plageman <[email protected]>
0 siblings, 0 replies; 3+ messages in thread
From: Chao Li @ 2026-04-21 22:24 UTC (permalink / raw)
To: Melanie Plageman <[email protected]>; +Cc: pgsql-hackers; Andres Freund <[email protected]>
> On Apr 22, 2026, at 05:52, Melanie Plageman <[email protected]> wrote:
>
> On Tue, Mar 31, 2026 at 10:15 PM Chao Li <[email protected]> wrote:
>>
>> I noticed that FlushUnlockedBuffer() accepts io_object and io_context, but then ignores them and hardcodes IOOBJECT_RELATIONand IOCONTEXT_NORMAL instead:
>> ```
>> static void
>> FlushUnlockedBuffer(BufferDesc *buf, SMgrRelation reln,
>> IOObject io_object, IOContext io_context)
>> {
>> Buffer buffer = BufferDescriptorGetBuffer(buf);
>>
>> BufferLockAcquire(buffer, buf, BUFFER_LOCK_SHARE_EXCLUSIVE);
>> FlushBuffer(buf, reln, IOOBJECT_RELATION, IOCONTEXT_NORMAL); // <== HERE
>> BufferLockUnlock(buffer, buf);
>> }
>> ```
>>
>> Unless I am missing something, if a function accepts these parameters, they should generally be used.
>
> Thanks for the patch. Committed in 31b0544b32b
>
> - Melanie
Hi Melanie, thank you very much for pushing.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2026-04-21 22:24 UTC | newest]
Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-04-01 02:15 bufmgr: pass through I/O stats context in FlushUnlockedBuffer() Chao Li <[email protected]>
2026-04-21 21:52 ` Melanie Plageman <[email protected]>
2026-04-21 22:24 ` Chao Li <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox