Hi, On 2025-06-30 13:57:28 -0500, Jim Nasby wrote: > +#if defined(HAVE_LIBURING_QUEUE_INIT_MEM) && defined(IORING_SETUP_NO_MMAP) > && 1 > > Is that && 1 intentional?
It was for testing both branches... > Nit: > + "mmap(%zu) to determine io_uring_queue_init_mem() support has failed: %m", > IMHO that would read better without "has". Agreed, fixed. > + /* 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... I was thinking of *lowering* it, given that the user, as you point out, can't do much with the information. > + 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. I don't think it's worth it - typically the user won't be able to do much, given that just upgrading the kernel is rarely easily possible. > It'd also be good to have a HINT either explaining the downside or pointing > to the docs. I don't know about that - outside of extreme cases the performance effects really aren't that meaningful. E.g. compiling with openssl support also has connection establishment performance overhead, yet we don't document that anywhere either, even though it's present even with ssl=off. > + * 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? No, it's trying to say that this is reserving space for alignment. > AFAICT pgaio_uring_ring_shmem_size() isn't even reserving space... That's all it does? It's used for sizing the shared memory allocation... Greetings, Andres Freund
