+#if defined(HAVE_LIBURING_QUEUE_INIT_MEM) && defined(IORING_SETUP_NO_MMAP) && 1
Is that && 1 intentional?
Nit:
+ "mmap(%zu) to determine io_uring_queue_init_mem() support has failed: %m",
IMHO that would read better without "has".
+ /* FIXME: This should probably not stay at DEBUG1? */
+ elog(DEBUG1,
+ "can use combined memory mapping for io_uring, each ring needs %d bytes",
+ ret);
Assuming my read that this is only executed at postmaster start is correct, I agree that NOTICE would also be reasonable. Though I'm not sure what a user could actually do with the info...
+ elog(DEBUG1,
+ "can't use combined memory mapping for io_uring, kernel or liburing too old");
OTOH this message would definitely be of interest to users; I'd say it should at least be NOTICE, possibly even WARNING. It'd also be good to have a HINT either explaining the downside or pointing to the docs.
+ * Memory for rings needs to be allocated to the page boundary,
+ * reserve space. Luckily it does not need to be aligned to hugepage
+ * boundaries, even if huge pages are used.
Is "reserve space" left over from something else? AFAICT pgaio_uring_ring_shmem_size() isn't even reserving space...
Hi,
On 2025-06-05 14:32:10 -0400, Andres Freund wrote:
> On 2025-06-05 12:47:52 -0400, Tom Lane wrote:
> > Andres Freund <andres@anarazel.de> writes:
> > > I think this is a big enough pitfall that it's, obviously assuming the patch
> > > has a sensible complexity, worth fixing this in 18. RMT, anyone, what do you
> > > think?
> >
> > Let's see the patch ... but yeah, I'd rather not ship 18 like this.
>
> I've attached a first draft.
>
> I can't make heads or tails of the ordering in configure.ac, so the function
> test is probably in the wrong place.
Any comments on that patch? I'd hoped for some review comments... Unless I'll
hear otherwise, I'll just do a bit more polish and push..
Greetings,
Andres
