Attention is currently required from: jolly.
pespin has posted comments on this change by jolly. ( https://gerrit.osmocom.org/c/libosmocore/+/40494?usp=email )
Change subject: Send multiple read/recvfrom SQEs in advance
......................................................................
Patch Set 6:
(1 comment)
File src/core/osmo_io_uring.c:
https://gerrit.osmocom.org/c/libosmocore/+/40494/comment/f4ccedd3_8e05937f?… :
PS4, Line 122: if ((env = getenv(OSMO_IO_URING_READ_SQE))) {
> According to the note (https://projects.osmocom. […]
Feel free to add an extra internal commit to force-enable it during benchmarking. I don't see the fact that we want to benchmark it as a reason to avoid making it configurable.
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/40494?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: Id50a8900fa2fe6de553e5025feae7e1e8d501e30
Gerrit-Change-Number: 40494
Gerrit-PatchSet: 6
Gerrit-Owner: jolly <andreas(a)eversberg.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: jolly <andreas(a)eversberg.eu>
Gerrit-Comment-Date: Tue, 22 Jul 2025 15:41:17 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: jolly <andreas(a)eversberg.eu>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Attention is currently required from: jolly.
pespin has posted comments on this change by jolly. ( https://gerrit.osmocom.org/c/libosmocore/+/40493?usp=email )
Change subject: Add multiple messages buffers to io_uring write operations
......................................................................
Patch Set 6:
(3 comments)
File src/core/osmo_io.c:
https://gerrit.osmocom.org/c/libosmocore/+/40493/comment/1b6fd4ed_62c4364d?… :
PS4, Line 248: lh = iofd->tx_queue.msg_queue.prev;
> I added the suggested llist_last_entry_or_null().
You added it to libosmocore but apparently not using it here?
https://gerrit.osmocom.org/c/libosmocore/+/40493/comment/ed8b74ac_3c8644c8?… :
PS4, Line 517: msghdr = iofd_txqueue_tail(iofd);
> Done
Not done, not answered, no comment explaining in code.
https://gerrit.osmocom.org/c/libosmocore/+/40493/comment/e3905381_70389350?… :
PS4, Line 539: rc = iofd_txqueue_enqueue(iofd, msghdr);
> If io_uring processes a msghdr, it has been removed from the queue before, so everything within the […]
Done
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/40493?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: I8c4e0a785cf66becd7fb5b2caf718c9724b56686
Gerrit-Change-Number: 40493
Gerrit-PatchSet: 6
Gerrit-Owner: jolly <andreas(a)eversberg.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: jolly <andreas(a)eversberg.eu>
Gerrit-Comment-Date: Tue, 22 Jul 2025 15:40:12 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: jolly <andreas(a)eversberg.eu>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Attention is currently required from: jolly, laforge.
pespin has posted comments on this change by jolly. ( https://gerrit.osmocom.org/c/libosmocore/+/40491?usp=email )
Change subject: Add multiple messages buffers to struct iofd_msghdr
......................................................................
Patch Set 5:
(3 comments)
File src/core/osmo_io.c:
https://gerrit.osmocom.org/c/libosmocore/+/40491/comment/b1b2e4c3_37cca0ee?… :
PS5, Line 727: iofd->io_buffers = (g_io_backend == OSMO_IO_BACKEND_IO_URING) ? g_io_uring_iov : 1;
sounds like we want to have this per-backend logic as part of the per-backend ops, and not here.
File src/core/osmo_io_internal.h:
https://gerrit.osmocom.org/c/libosmocore/+/40491/comment/2baf7e2c_71b350ea?… :
PS5, Line 143: int io_len;
I wonder whether this is needed or can be inferred when needed by looking up NULL ptrs in "msg" field below.
File src/core/osmo_io_internal.h:
https://gerrit.osmocom.org/c/libosmocore/+/40491/comment/a767b7c9_568dd343?… :
PS3, Line 140: int msg_max;
> According to the note (https://projects.osmocom. […]
This should definetly be configurable over API per osmo_iofd object. Then apps can decide what to use on per socket/interface based.
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/40491?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: I4fb1067de4615cc22cc6caf99b481491e7f2ef92
Gerrit-Change-Number: 40491
Gerrit-PatchSet: 5
Gerrit-Owner: jolly <andreas(a)eversberg.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Attention: jolly <andreas(a)eversberg.eu>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Tue, 22 Jul 2025 15:33:41 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: jolly <andreas(a)eversberg.eu>
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Attention is currently required from: jolly.
pespin has posted comments on this change by jolly. ( https://gerrit.osmocom.org/c/libosmocore/+/40584?usp=email )
Change subject: Avoid reusing pending buffer; append incoming data instead
......................................................................
Patch Set 3:
(5 comments)
Commit Message:
https://gerrit.osmocom.org/c/libosmocore/+/40584/comment/0f6c0276_c46f912e?… :
PS3, Line 17: This change ensures that newly received data is appended to the existing
First you say we don't want to reuse the pending buffer, but here you say "This change ensures that newly received data is appended to the existing pending buffer", which seems to be the opposite of what you first stated?
File src/core/osmo_io.c:
https://gerrit.osmocom.org/c/libosmocore/+/40584/comment/d76babb4_0c35e494?… :
PS3, Line 177: /*! convenience wrapper to call msgb_alloc with parameters from osmo_io_fd (with extra size) */
"with extra size" here doesn't apply anymore?
https://gerrit.osmocom.org/c/libosmocore/+/40584/comment/30485a7d_e24dd494?… :
PS3, Line 178: struct msgb *iofd_msgb_alloc2(struct osmo_io_fd *iofd, size_t size)
This is probably breaking the promise that msgb passed to the app are at least of a certain size configured by the user.
https://gerrit.osmocom.org/c/libosmocore/+/40584/comment/a19b37dd_247f0cc8?… :
PS3, Line 335: * If the pending message is not large enough, create a larger message. */
I may be wrong, but I'd expect user to configure iofd with a max_size big enough to allow whatever size is needed?
https://gerrit.osmocom.org/c/libosmocore/+/40584/comment/fa6c86b2_162a97d0?… :
PS3, Line 344: memcpy(msgb_put(iofd->pending, msgb_length(msg)), msgb_data(msg), msgb_length(msg));
Can we avoid 2 memcpys (here and above) in the specific code path?
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/40584?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: I08df9736ccc5e9a7df61ca6dcf94629ee010752f
Gerrit-Change-Number: 40584
Gerrit-PatchSet: 3
Gerrit-Owner: jolly <andreas(a)eversberg.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: jolly <andreas(a)eversberg.eu>
Gerrit-Comment-Date: Tue, 22 Jul 2025 14:44:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: jolly, laforge.
pespin has posted comments on this change by jolly. ( https://gerrit.osmocom.org/c/libosmocore/+/40489?usp=email )
Change subject: Allow io_uring_submit batching just ahead of poll/select
......................................................................
Patch Set 3:
(2 comments)
Patchset:
PS3:
I'm still not convinced we want to add more envvars. This is something we can probably configure through VTY.
Only reason I see for having a envvar is in the event we want to have this opt-in for a while and later on enable it and remove the envvar.
File src/core/select.c:
https://gerrit.osmocom.org/c/libosmocore/+/40489/comment/325868c5_e2d4e50a?… :
PS3, Line 440: osmo_io_uring_submit();
I'd say it makes more sense to have the OSMO_UNLIKELY if here directly, to avoid an extra call+ret on each loop iteration.
Maybe as a C macro if you want to keep it similar.
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/40489?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: Id34fe2ced32c63d15b14810e145744f7509064cc
Gerrit-Change-Number: 40489
Gerrit-PatchSet: 3
Gerrit-Owner: jolly <andreas(a)eversberg.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: jolly <andreas(a)eversberg.eu>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Tue, 22 Jul 2025 14:35:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No