Attention is currently required from: jolly, laforge.
pespin 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 9:
(6 comments)
Commit Message:
https://gerrit.osmocom.org/c/libosmocore/+/40725/comment/bf3fa1ff_1903038e?u... : PS9, Line 23: to determine when all submissions are completed. Sounds like an easy thing to add right? It makes sense to implement this either her eor on a follow-up patch.
File src/core/osmo_io_internal.h:
https://gerrit.osmocom.org/c/libosmocore/+/40725/comment/b34815a0_ee55ba5c?u... : PS9, Line 110: bool read_enabled; (At some point it may make sense to properly put all these fields into a "read" and "write" substructs in here.)
File src/core/osmo_io_uring.c:
https://gerrit.osmocom.org/c/libosmocore/+/40725/comment/09634797_688ca5a4?u... : PS9, Line 126: g_ring = talloc_zero(OTC_GLOBAL, struct osmo_io_uring); are you sure OTC_GLOBAL is defined at this point? Or is it actually NULL? If it's still NULL better pass NULL explicitly to avoid creating confusion.
https://gerrit.osmocom.org/c/libosmocore/+/40725/comment/afc5fac1_bb6d44e8?u... : PS9, Line 175: g_io_uring_size *= 2; Once you move this to an unsigned, you can do "<< 1" here. BTW, I don't see you are making sure we don't go over the "maximum ring size allowed" in there, the one you are checking in previous commit.
https://gerrit.osmocom.org/c/libosmocore/+/40725/comment/6946c3e3_7a05083e?u... : PS9, Line 208: /* All subsequent read SQEs must be on the same ring. */ This should imho go inside iofd_uring_get_sqe(), based on "current_ring" being passed to it.
https://gerrit.osmocom.org/c/libosmocore/+/40725/comment/a8053e18_abe26da4?u... : PS9, Line 282: while (iofd->u.uring.reads_submitted < ((iofd->u.uring.num_read_sqes) ? : g_io_uring_read_sqes)) { why is the g_io_uring_read_sqes reference needed here? How can iofd have num_read_sqes = 0, and if it does, why are we overriding it?