+#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... On Mon, Jun 30, 2025 at 11:28 AM Andres Freund <and...@anarazel.de> wrote: > 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 <and...@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 > > >
