From 71ad880d55e58e603d7a03ac1e57af00e8611826 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Thu, 20 Mar 2025 11:32:10 -0400 Subject: [PATCH v2.12 17/28] aio, bufmgr: Comment fixes Some of these comments have been wrong for a while (12f3867f5534), some I recently introduced (da7226993fd, 55b454d0e14). This also updates a comment in FlushBuffer(), which will be copied in a future commit. These changes seem big enough to be worth doing in separate commits. Suggested-by: Noah Misch Discussion: https://postgr.es/m/20250319212530.80.nmisch@google.com --- src/include/storage/aio.h | 2 +- src/include/storage/aio_internal.h | 17 ++++++++++++++++- src/backend/postmaster/postmaster.c | 2 +- src/backend/storage/buffer/bufmgr.c | 10 ++++------ 4 files changed, 22 insertions(+), 9 deletions(-) diff --git a/src/include/storage/aio.h b/src/include/storage/aio.h index 92f29cfdb71..9a0868a270c 100644 --- a/src/include/storage/aio.h +++ b/src/include/storage/aio.h @@ -80,7 +80,7 @@ typedef enum PgAioHandleFlags /* * The IO operations supported by the AIO subsystem. * - * This could be in aio_internal.h, as it is not pubicly referenced, but + * This could be in aio_internal.h, as it is not publicly referenced, but * PgAioOpData currently *does* need to be public, therefore keeping this * public seems to make sense. */ diff --git a/src/include/storage/aio_internal.h b/src/include/storage/aio_internal.h index 400c44206dd..0dff909e4ad 100644 --- a/src/include/storage/aio_internal.h +++ b/src/include/storage/aio_internal.h @@ -276,6 +276,9 @@ typedef struct IoMethodOps /* * Start executing passed in IOs. * + * Shall advance state to at least PGAIO_HS_SUBMITTED. (By the time this + * returns, other backends might have advanced the state further.) + * * Will not be called if ->needs_synchronous_execution() returned true. * * num_staged_ios is <= PGAIO_SUBMIT_BATCH_SIZE. @@ -284,12 +287,24 @@ typedef struct IoMethodOps */ int (*submit) (uint16 num_staged_ios, PgAioHandle **staged_ios); - /* + /* --- * Wait for the IO to complete. Optional. * + * On return, state shall be on of + * - PGAIO_HS_COMPLETED_IO + * - PGAIO_HS_COMPLETED_SHARED + * - PGAIO_HS_COMPLETED_LOCAL + * + * The callback must not block if the handle is already in one of those + * states, or has been reused (see pgaio_io_was_recycled()). If, on + * return, the state is PGAIO_HS_COMPLETED_IO, state will reach + * PGAIO_HS_COMPLETED_SHARED without further intervention by the IO + * method. + * * If not provided, it needs to be guaranteed that the IO method calls * pgaio_io_process_completion() without further interaction by the * issuing backend. + * --- */ void (*wait_one) (PgAioHandle *ioh, uint64 ref_generation); diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index a0c37532d2f..c966c2e83af 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -4401,7 +4401,7 @@ maybe_adjust_io_workers(void) ++io_worker_count; } else - break; /* XXX try again soon? */ + break; /* try again next time */ } /* Too many running? */ diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index fcc5eb5654e..7f9b58003b3 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -4263,9 +4263,10 @@ FlushBuffer(BufferDesc *buf, SMgrRelation reln, IOObject io_object, XLogFlush(recptr); /* - * Now it's safe to write buffer to disk. Note that no one else should - * have been able to write it while we were busy with log flushing because - * only one process at a time can set the BM_IO_IN_PROGRESS bit. + * Now it's safe to write the buffer to disk. Note that no one else should + * have been able to write it, while we were busy with log flushing, + * because we got the exclusive right to perform I/O by setting the + * BM_IO_IN_PROGRESS bit. */ bufBlock = BufHdrGetBlock(buf); @@ -5855,9 +5856,6 @@ IsBufferCleanupOK(Buffer buffer) /* * Functions for buffer I/O handling * - * Note: We assume that nested buffer I/O never occurs. - * i.e at most one BM_IO_IN_PROGRESS bit is set per proc. - * * Also note that these are used only for shared buffers, not local ones. */ -- 2.48.1.76.g4e746b1a31.dirty