aio, bufmgr: Comment fixes/improvements

Some of these comments have been wrong for a while (12f3867f5534), some I
recently introduced (da7226993fd, 55b454d0e14). This includes an update to 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 <noah@leadboat.com>
Discussion: https://postgr.es/m/20250319212530.80.nmisch@google.com

4 files changed, 27 insertions, 9 deletions

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 5cac8cd7389..03317b49025 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -3929,9 +3929,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);
 
@@ -5499,9 +5500,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.
  */
 
diff --git a/src/include/storage/aio.h b/src/include/storage/aio.h
index 6b083040943..c6bf7c4ccf5 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 fb0425ccbfc..7f18da2c856 100644
--- a/src/include/storage/aio_internal.h
+++ b/src/include/storage/aio_internal.h
@@ -34,6 +34,11 @@
  * linearly through all states.
  *
  * State changes should all go through pgaio_io_update_state().
+ *
+ * Note that the externally visible functions to start IO
+ * (e.g. FileStartReadV(), via pgaio_io_start_readv()) move an IO from
+ * PGAIO_HS_HANDED_OUT to at least PGAIO_HS_STAGED and at most
+ * PGAIO_HS_COMPLETED_LOCAL (at which point the handle will be reused).
  */
 typedef enum PgAioHandleState
 {
@@ -285,6 +290,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.
@@ -293,12 +301,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);