On 31.07.25 19:17, Tom Lane wrote:
Also I see a "// XXX" in pg_get_aios, which I guess is a note
to confirm the data type to use for ioh_id?
Yes, the stuff returned from pgaio_io_get_id() should be int, but some
code uses uint32, and also UINT32_MAX as a special marker. Here is a
patch that tries to straighten that out by using int consistently and -1
as a special marker.
From e1bb97e13af226cd15c9a59929aef48bc92c4ac2 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Tue, 5 Aug 2025 19:16:38 +0200
Subject: [PATCH] Use consistent type for pgaio_io_get_id() result
The result of pgaio_io_get_id() was being assigned to a mix of int and
uint32 variables. This fixes it to use int consistently, which seems
the most correct. Also change the queue empty special value in
method_worker.c to -1 from UINT32_MAX.
---
src/backend/storage/aio/aio_funcs.c | 4 ++--
src/backend/storage/aio/method_worker.c | 14 +++++++-------
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/backend/storage/aio/aio_funcs.c
b/src/backend/storage/aio/aio_funcs.c
index 584e683371a..34f8f632733 100644
--- a/src/backend/storage/aio/aio_funcs.c
+++ b/src/backend/storage/aio/aio_funcs.c
@@ -56,7 +56,7 @@ pg_get_aios(PG_FUNCTION_ARGS)
for (uint64 i = 0; i < pgaio_ctl->io_handle_count; i++)
{
PgAioHandle *live_ioh = &pgaio_ctl->io_handles[i];
- uint32 ioh_id = pgaio_io_get_id(live_ioh);
+ int ioh_id = pgaio_io_get_id(live_ioh);
Datum values[PG_GET_AIOS_COLS] = {0};
bool nulls[PG_GET_AIOS_COLS] = {0};
ProcNumber owner;
@@ -152,7 +152,7 @@ pg_get_aios(PG_FUNCTION_ARGS)
nulls[0] = false;
/* column: IO's id */
- values[1] = ioh_id;
+ values[1] = Int32GetDatum(ioh_id);
/* column: IO's generation */
values[2] = Int64GetDatum(start_generation);
diff --git a/src/backend/storage/aio/method_worker.c
b/src/backend/storage/aio/method_worker.c
index bf8f77e6ff6..b5ac073a910 100644
--- a/src/backend/storage/aio/method_worker.c
+++ b/src/backend/storage/aio/method_worker.c
@@ -58,7 +58,7 @@ typedef struct PgAioWorkerSubmissionQueue
uint32 mask;
uint32 head;
uint32 tail;
- uint32 sqes[FLEXIBLE_ARRAY_MEMBER];
+ int sqes[FLEXIBLE_ARRAY_MEMBER];
} PgAioWorkerSubmissionQueue;
typedef struct PgAioWorkerSlot
@@ -107,7 +107,7 @@ pgaio_worker_queue_shmem_size(int *queue_size)
*queue_size = pg_nextpower2_32(io_worker_queue_size);
return offsetof(PgAioWorkerSubmissionQueue, sqes) +
- sizeof(uint32) * *queue_size;
+ sizeof(int) * *queue_size;
}
static size_t
@@ -198,15 +198,15 @@ pgaio_worker_submission_queue_insert(PgAioHandle *ioh)
return true;
}
-static uint32
+static int
pgaio_worker_submission_queue_consume(void)
{
PgAioWorkerSubmissionQueue *queue;
- uint32 result;
+ int result;
queue = io_worker_submission_queue;
if (queue->tail == queue->head)
- return UINT32_MAX; /* empty */
+ return -1; /* empty */
result = queue->sqes[queue->tail];
queue->tail = (queue->tail + 1) & (queue->size - 1);
@@ -470,7 +470,7 @@ IoWorkerMain(const void *startup_data, size_t
startup_data_len)
* to ensure that we don't see an outdated data in the handle.
*/
LWLockAcquire(AioWorkerSubmissionQueueLock, LW_EXCLUSIVE);
- if ((io_index = pgaio_worker_submission_queue_consume()) ==
UINT32_MAX)
+ if ((io_index = pgaio_worker_submission_queue_consume()) == -1)
{
/*
* Nothing to do. Mark self idle.
@@ -500,7 +500,7 @@ IoWorkerMain(const void *startup_data, size_t
startup_data_len)
for (int i = 0; i < nlatches; ++i)
SetLatch(latches[i]);
- if (io_index != UINT32_MAX)
+ if (io_index != -1)
{
PgAioHandle *ioh = NULL;
--
2.50.1
