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?usp=email : 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?usp=email : 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?usp=email : 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?usp=email : 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?usp=email : 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?usp=email : PS2, Line 175: is there code to clean up the old ring after completion of the last cqe of the old ring? -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/40725?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Id9230146acc8d54bfd44834e783c31b37bd64bca Gerrit-Change-Number: 40725 Gerrit-PatchSet: 2 Gerrit-Owner: jolly <andr...@eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <lafo...@osmocom.org> Gerrit-CC: pespin <pes...@sysmocom.de> Gerrit-Attention: jolly <andr...@eversberg.eu> Gerrit-Attention: pespin <pes...@sysmocom.de> Gerrit-Comment-Date: Wed, 23 Jul 2025 11:16:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No
