| From: | Xuneng Zhou <xunengzhou(at)gmail(dot)com> |
|---|---|
| To: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com> |
| Cc: | Lucas DRAESCHER <git(at)draescher(dot)fr>, Andrey Borodin <x4mmm(at)yandex-team(dot)ru>, Jakub Wartak <jakub(dot)wartak(at)enterprisedb(dot)com>, Laetitia Avrot <laetitia(dot)avrot(at)gmail(dot)com>, "tanswis42(at)gmail(dot)com" <tanswis42(at)gmail(dot)com>, cca5507 <cca5507(at)qq(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: [Bug Report + Patch] File descriptor leak when io_method=io_uring |
| Date: | 2026-11-06 06:33:52 |
| Message-ID: | CABPTF7VCnsd6YaN=Fn0cKKPdTJjJVuuTi206RCuNVDTkRJFXzA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Thu, Nov 5, 2026 at 10:37 PM Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>
> On Thu, Nov 5, 2026 at 6:25 PM Xuneng Zhou <xunengzhou(at)gmail(dot)com> wrote:
> > I could
> > write a follow-up patch or review it if you already did.
>
> I tried to write a patch that keeps postmaster-local copies of the io_uring
> descriptors and passes them to io_uring_queue_exit() for cleanup. Attached.
>
> Per [1], io_uring_queue_exit() seems to use sq.kring_entries. Since this is
> a pointer [2], simply copying the descriptor would leave it pointing into
> shared memory. So, the patch also saves the pointed-to value locally.
>
> But, I'm not sure this is the right approach... It relies on liburing's
> implementation rather than a documented guarantee, so future liburing changes
> would be really easily able to break it. This seems very fragile. Thought?
> Is there a better approach?
>
> Regards,
>
> [1] https://github.com/axboe/liburing/blob/liburing-2.7/src/setup.c#L430-L445
> [2] https://github.com/axboe/liburing/blob/liburing-2.7/src/include/liburing.h#L77-L83
It does seem fragile. I tried to look for similar use cases in other
projects and see how they do it. However, it seems few share io_uring
rings across fork() the way we do. Most create rings per thread or per
process. So they won't bother doing the local-copy. It maybe worth
asking the liburing maintainer whether a copy of struct io_uring is
valid input for io_uring_queue_exit().
There's a somewhat hacky alternative. On kernel 6.5+ with liburing
2.5+, we create the rings with io_uring_queue_init_mem() in our own
shared memory, so noting maps the ring fds. For those rings, closing
the fd is what the teardown has to do, which is also
io_uring_queue_exit() does from liburign 2.8 on. In theory, we could
just make local copy of the those fd and just close them during
cleanup, without invoking into liburing. However, this is against the
convention of paring registeration and teardown.
--
Regards,
Xuneng Zhou
HighGo Software Co., Ltd.
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Corey Huinker | 2026-11-06 06:37:00 | Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions |
| Previous Message | Amit Kapila | 2026-11-06 06:09:45 | Re: pg_dump --exclude-table loses FOR ALL TABLES EXCEPT publications |