Attention is currently required from: jolly, pespin.
laforge has posted comments on this change by jolly. ( https://gerrit.osmocom.org/c/libosmocore/+/40725?usp=email )
Change subject: Automatically increase io_uring, if too small. ......................................................................
Patch Set 2:
(6 comments)
File src/core/osmo_io_internal.h:
https://gerrit.osmocom.org/c/libosmocore/+/40725/comment/a53bcc51_10cebfac?u... : PS2, Line 110: void *read_ring[IOFD_MSGHDR_READ_SQES]; some comments here would be useful to explan what those fields are and how hey are used (not just the read_ring now, but also the previously-introduced read_msghdr, read_len, ...
Also: why are those pointers void? if it points to a ring, shouldn't it be 'struct io_uring *' for read_ring and write_ring?
File src/core/osmo_io_uring.c:
https://gerrit.osmocom.org/c/libosmocore/+/40725/comment/a0eb6d91_79685d7d?u... : PS2, Line 67: g_io_uring_queue pleaes use a more descriptive name. The variable is not a qeuue, but it is the size of the ring. Something like g_io_uring_size or g_io_uring_num_entries would be more descriptive.
https://gerrit.osmocom.org/c/libosmocore/+/40725/comment/06962c19_78cb567d?u... : PS2, Line 113: OSMO_IO_URING_QUEUE like the variable name in the code, the environment variable name shold be self-descriptive. Also, since it's the *initial* size, something like OSMO_IO_URING_INITIAL_{SIZE,NUM_ENTRIES} would make most sense.
Naming does matter...
https://gerrit.osmocom.org/c/libosmocore/+/40725/comment/76d8c11b_e4366e52?u... : PS2, Line 163: osmo_iofd_uring_get_sqe the 'osmo_' prefix is reserverd for exported functions. You are adding a static function, it should not have an osmo_ prefix.
https://gerrit.osmocom.org/c/libosmocore/+/40725/comment/4b3570e8_75a14c96?u... : PS2, Line 172: LOGP(DLIO, LOGL_NOTICE, "io_uring too small to handle all SQEs, increasing size to %d.\n", g_io_uring_queue); minor: while the current implementation alsways doubles the size, it might still be useful to print the old [too small] size?
https://gerrit.osmocom.org/c/libosmocore/+/40725/comment/40117db5_b6cf2823?u... : PS2, Line 175: is there code to clean up the old ring after completion of the last cqe of the old ring?