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 6: Code-Review-1

(5 comments)

File src/core/osmo_io_internal.h:

https://gerrit.osmocom.org/c/libosmocore/+/40725/comment/6799a0a9_4e2cf970?usp=email
 :
PS6, Line 116:                  /* ! array of rings the submitted read SQEs 
have been submitted */
> "have been submitted to"
same comment like in the other patch regarding "*!" vs "* !"


File src/core/osmo_io_uring.c:

https://gerrit.osmocom.org/c/libosmocore/+/40725/comment/14dd5fc1_ad400304?usp=email
 :
PS2, Line 175:
> No, it doesn't, because I don't track the number of submissions and the 
> number of completions. […]
ok, then please mention this fact explicitly in this c ommit in a TODO-comment 
and also in the commit log.  At least this way it's clear that there is no 
related cleanup yet.  Implementation could then follow at a later patch, but at 
least it is known to everyone that this is the situation.

I would consider having a single per-ring counter of the number of 
not-yet-completed sqe's would be sufficient.  If that counter reaches zero, and 
we are not the currently active ring, we can destroy it?


File src/core/osmo_io_uring.c:

https://gerrit.osmocom.org/c/libosmocore/+/40725/comment/c5d522d0_aeb3e8ef?usp=email
 :
PS6, Line 172:  sqe = io_uring_get_sqe(&g_ring->ring);
> so here we end up with 2 read SQEs on 2 different queues. […]
does it matter? Do we ever care? I think there's no such guarantee even with 
two read sqe's for the same fd on a single ring?  In the end, they are all 
empty msgb's waiting for some incoming data. It doesn't matter which is 
used/filled/reported-completed first?


https://gerrit.osmocom.org/c/libosmocore/+/40725/comment/5501dd65_4c9eabef?usp=email
 :
PS6, Line 527:          while ((sqe = 
io_uring_get_sqe(iofd->u.uring.read_ring[idx])) == NULL);
> that looks like a really nice tight non-blocking loop filling the CPU?
yeah. It's one problem stalling the process itself by waiting for something to 
happen eventually - it's another problem to waste a full CPU core while doing 
that, potentially slowing down the event that we're waiting for.

It seems `io_uring_sqring_wait` was introduced for this: 
https://man7.org/linux/man-pages/man3/io_uring_sqring_wait.3.html
- we may have to check
1) at compile time that a sufficient enough liburing version is present
2) at runtime that the kernel supports it (-EINVAL is returned otherwise)

It might also make sense to log something just before we call 
io_uiring_sqring_wait... that might help debugging if we ever ended up spending 
significant time in it.

So my strategy would be:

* fast path trying io_uring_get_sqe
* only if that fails, log the situation and use io_uring_sqring_wait
* if io_uring_sqring_wait fails, fall-back to busy-waiting, but maybe with some 
usleep in the loop

Alternatively, we could try calling io_uring_sqring_wait once at ring creation 
time (when we know it will never block as there are always free SQEs inside), 
and cache whether or not it is supported (positive value) or unsupported 
(-EINVAL) in a global variable.  The code here and below could then avoid the 
trial-and-error every time we unregister an fd.


https://gerrit.osmocom.org/c/libosmocore/+/40725/comment/c930b6b7_3dd558ea?usp=email
 :
PS6, Line 543:          while ((sqe = 
io_uring_get_sqe(iofd->u.uring.write_ring)) == NULL);
> that looks like a really nice tight non-blocking loop filling the CPU?
same as above



--
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: 6
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: Thu, 31 Jul 2025 14:00:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: jolly <andr...@eversberg.eu>
Comment-In-Reply-To: laforge <lafo...@osmocom.org>
Comment-In-Reply-To: pespin <pes...@sysmocom.de>

Reply via email to