public inbox for [email protected]
help / color / mirror / Atom feedFrom: =?ISO-8859-1?B?emVuZ21hbg==?= <[email protected]>
To: =?ISO-8859-1?B?cGdzcWwtaGFja2Vycw==?= <[email protected]>
Cc: =?ISO-8859-1?B?YW5kcmVz?= <[email protected]>
Subject: [PATCH] aio: Refactor to deduplicate shared and local completion callbacks
Date: Fri, 2 Jan 2026 20:51:14 +0800
Message-ID: <[email protected]> (raw)
Hi,
I've observed that the functions `pgaio_io_call_complete_shared()` and` pgaio_io_call_complete_local()` contain nearly identical code,
with only minor differences in initialization and callback selection.
This code duplication was previously noted with an XXX comment.
I propose extracting the common logic into a new internal function `pgaio_io_call_complete_internal()`,
which uses a boolean parameter to distinguish between the two code paths.
What do you think of this approach?
--
Regards,
Man Zeng
www.openhalo.org
Attachments:
[application/octet-stream] 0001-aio-Deduplicate-shared-and-local-completion-callback.patch (5.6K, ../[email protected]/2-0001-aio-Deduplicate-shared-and-local-completion-callback.patch)
download | inline diff:
From 41513888063b2a7e2319468b8ad3686a3494071b Mon Sep 17 00:00:00 2001
From: Man Zeng <[email protected]>
Date: Fri, 2 Jan 2026 20:30:35 +0800
Subject: [PATCH] aio: Deduplicate shared and local completion callback
functions
The functions pgaio_io_call_complete_shared() and
pgaio_io_call_complete_local() contained nearly identical code with
only minor differences in initialization and callback selection. This
duplication was previously noted with an XXX comment.
Extract the common logic into a new internal function
pgaio_io_call_complete_internal() that uses a boolean parameter to
distinguish between the two code paths.
---
src/backend/storage/aio/aio_callback.c | 88 ++++++++++++--------------
1 file changed, 41 insertions(+), 47 deletions(-)
diff --git a/src/backend/storage/aio/aio_callback.c b/src/backend/storage/aio/aio_callback.c
index 206b81f5028..6ef51244a51 100644
--- a/src/backend/storage/aio/aio_callback.c
+++ b/src/backend/storage/aio/aio_callback.c
@@ -218,23 +218,31 @@ pgaio_io_call_stage(PgAioHandle *ioh)
}
/*
- * Internal function which invokes ->complete_shared for all the registered
+ * Internal function which invokes completion callbacks for all registered
* callbacks.
*/
-void
-pgaio_io_call_complete_shared(PgAioHandle *ioh)
+static PgAioResult
+pgaio_io_call_complete_internal(PgAioHandle *ioh, bool is_shared)
{
PgAioResult result;
-
- START_CRIT_SECTION();
+ const char *callback_name = is_shared ? "complete_shared" : "complete_local";
Assert(ioh->target > PGAIO_TID_INVALID && ioh->target < PGAIO_TID_COUNT);
Assert(ioh->op > PGAIO_OP_INVALID && ioh->op < PGAIO_OP_COUNT);
- result.status = PGAIO_RS_OK; /* low level IO is always considered OK */
- result.result = ioh->result;
- result.id = PGAIO_HCB_INVALID;
- result.error_data = 0;
+ /* Initialize result based on callback type */
+ if (is_shared)
+ {
+ result.status = PGAIO_RS_OK; /* low level IO is always considered OK */
+ result.result = ioh->result;
+ result.id = PGAIO_HCB_INVALID;
+ result.error_data = 0;
+ }
+ else
+ {
+ result = ioh->distilled_result;
+ Assert(result.status != PGAIO_RS_UNKNOWN);
+ }
/*
* Call callbacks with the last registered (innermost) callback first.
@@ -245,22 +253,41 @@ pgaio_io_call_complete_shared(PgAioHandle *ioh)
PgAioHandleCallbackID cb_id = ioh->callbacks[i - 1];
uint8 cb_data = ioh->callbacks_data[i - 1];
const PgAioHandleCallbacksEntry *ce = &aio_handle_cbs[cb_id];
+ PgAioHandleCallbackComplete cb_func = NULL;
+
+ /* Select the appropriate callback function based on type */
+ cb_func = is_shared ? ce->cb->complete_shared : ce->cb->complete_local;
- if (!ce->cb->complete_shared)
+ if (!cb_func)
continue;
pgaio_debug_io(DEBUG4, ioh,
- "calling cb #%d, id %d/%s->complete_shared(%u) with distilled result: (status %s, id %u, error_data %d, result %d)",
- i, cb_id, ce->name,
+ "calling cb #%d, id %d/%s->%s(%u) with distilled result: (status %s, id %u, error_data %d, result %d)",
+ i, cb_id, ce->name, callback_name,
cb_data,
pgaio_result_status_string(result.status),
result.id, result.error_data, result.result);
- result = ce->cb->complete_shared(ioh, result, cb_data);
+ result = cb_func(ioh, result, cb_data);
/* the callback should never transition to unknown */
Assert(result.status != PGAIO_RS_UNKNOWN);
}
+ return result;
+}
+
+/*
+ * Internal function which invokes ->complete_shared for all the registered
+ * callbacks.
+ */
+void
+pgaio_io_call_complete_shared(PgAioHandle *ioh)
+{
+ PgAioResult result;
+
+ START_CRIT_SECTION();
+
+ result = pgaio_io_call_complete_internal(ioh, true);
ioh->distilled_result = result;
pgaio_debug_io(DEBUG3, ioh,
@@ -278,8 +305,6 @@ pgaio_io_call_complete_shared(PgAioHandle *ioh)
*
* Returns ioh->distilled_result after, possibly, being modified by local
* callbacks.
- *
- * XXX: It'd be nice to deduplicate with pgaio_io_call_complete_shared().
*/
PgAioResult
pgaio_io_call_complete_local(PgAioHandle *ioh)
@@ -288,39 +313,8 @@ pgaio_io_call_complete_local(PgAioHandle *ioh)
START_CRIT_SECTION();
- Assert(ioh->target > PGAIO_TID_INVALID && ioh->target < PGAIO_TID_COUNT);
- Assert(ioh->op > PGAIO_OP_INVALID && ioh->op < PGAIO_OP_COUNT);
-
- /* start with distilled result from shared callback */
- result = ioh->distilled_result;
- Assert(result.status != PGAIO_RS_UNKNOWN);
-
- for (int i = ioh->num_callbacks; i > 0; i--)
- {
- PgAioHandleCallbackID cb_id = ioh->callbacks[i - 1];
- uint8 cb_data = ioh->callbacks_data[i - 1];
- const PgAioHandleCallbacksEntry *ce = &aio_handle_cbs[cb_id];
-
- if (!ce->cb->complete_local)
- continue;
-
- pgaio_debug_io(DEBUG4, ioh,
- "calling cb #%d, id %d/%s->complete_local(%u) with distilled result: status %s, id %u, error_data %d, result %d",
- i, cb_id, ce->name, cb_data,
- pgaio_result_status_string(result.status),
- result.id, result.error_data, result.result);
- result = ce->cb->complete_local(ioh, result, cb_data);
-
- /* the callback should never transition to unknown */
- Assert(result.status != PGAIO_RS_UNKNOWN);
- }
+ result = pgaio_io_call_complete_internal(ioh, false);
- /*
- * Note that we don't save the result in ioh->distilled_result, the local
- * callback's result should not ever matter to other waiters. However, the
- * local backend does care, so we return the result as modified by local
- * callbacks, which then can be passed to ioh->report_return->result.
- */
pgaio_debug_io(DEBUG3, ioh,
"after local completion: result: (status %s, id %u, error_data %d, result %d), raw_result: %d",
pgaio_result_status_string(result.status),
--
2.45.2
view thread (2+ messages)
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: [PATCH] aio: Refactor to deduplicate shared and local completion callbacks
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox