AioCtl Shared Memory size fix
Hi Folks,
While reviewing AIO code, we found an issue with AioCtlShmemSize function.
The function was not accounting for io_handles which is used post shared
memory alloc in AioShmemInit. Please find a patch to account for
io_handles member of PgAioCtl.
The patch uses sizeof(PgAioCtl) instead of offsetof(PgAioCtl, io_handles)
in AioCtlShmemSize.
Thanks,
Madhukar
Attachments:
patch.diffapplication/octet-stream; name=patch.diffDownload
commit d5ddf37fffa8c7446458e8eaecc7d8405df5bc65
Author: Madhukar <madhukarprasad@google.com>
Date: Thu Sep 11 07:02:25 2025 +0000
Fix shared memory size for AioCtlShmemSize
Before this patch, allocated shared memory size was
offsetof(PgAioCtl, io_handles). `io_handles` was not accounted as part of the
computation.
Changed the AioCtlShmemSize to sizeof(PgAioCtl) to account for io_handles.
diff --git a/src/backend/storage/aio/aio_init.c b/src/backend/storage/aio/aio_init.c
index 885c3940c66..54ab84dd6f0 100644
--- a/src/backend/storage/aio/aio_init.c
+++ b/src/backend/storage/aio/aio_init.c
@@ -30,12 +30,8 @@
static Size
AioCtlShmemSize(void)
{
- Size sz;
-
/* pgaio_ctl itself */
- sz = offsetof(PgAioCtl, io_handles);
-
- return sz;
+ return sizeof(PgAioCtl);
}
static uint32
On Mon, 15 Sept 2025 at 13:33, Madhukar . <madhukarprasad@google.com> wrote:
Hi Folks,
While reviewing AIO code, we found an issue with AioCtlShmemSize function. The function was not accounting for io_handles which is used post shared memory alloc in AioShmemInit.
Good catch.
Presumably this was `PgAioHandle io_handles[]` at some point, but now
that it's a pointer it's a proper part of the struct's own size, and
should be treated as such for memory accounting.
Please find a patch to account for io_handles member of PgAioCtl.
The patch uses sizeof(PgAioCtl) instead of offsetof(PgAioCtl, io_handles) in AioCtlShmemSize.
LGTM.
Kind regards,
Matthias van de Meent
Databricks
On Mon, Sep 15, 2025 at 02:06:03PM +0200, Matthias van de Meent wrote:
Presumably this was `PgAioHandle io_handles[]` at some point, but now
that it's a pointer it's a proper part of the struct's own size, and
should be treated as such for memory accounting.
I would bet on a FLEXIBLE_ARRAY_MEMBER from a previous version..
Please find a patch to account for io_handles member of PgAioCtl.
The patch uses sizeof(PgAioCtl) instead of offsetof(PgAioCtl, io_handles) in AioCtlShmemSize.LGTM.
Yep, will fix. Thanks for the report, the patch and the review, to
both of you.
--
Michael
Hi,
On 2025-09-16 15:50:20 +0900, Michael Paquier wrote:
On Mon, Sep 15, 2025 at 02:06:03PM +0200, Matthias van de Meent wrote:
Presumably this was `PgAioHandle io_handles[]` at some point, but now
that it's a pointer it's a proper part of the struct's own size, and
should be treated as such for memory accounting.I would bet on a FLEXIBLE_ARRAY_MEMBER from a previous version..
Indeed. I don't remember for sure why I changed it, but I think it may have
been to make the different allocations more visible in pg_shmem_allocations.
Please find a patch to account for io_handles member of PgAioCtl.
The patch uses sizeof(PgAioCtl) instead of offsetof(PgAioCtl, io_handles) in AioCtlShmemSize.LGTM.
Yep, will fix. Thanks for the report, the patch and the review, to
both of you.
Thanks for finding and fixing!
Greetings,
Andres Freund