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 4:
(1 comment)
Patchset:
PS4:
I think this patch should be split into 2:
* One first patch adding the functionality to configure the ring size over envvar
* The patch where the ring size is incremented x2 when it becomes full.
--
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: 4
Gerrit-Owner: jolly <andreas(a)eversberg.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: 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: Mon, 28 Jul 2025 11:21:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
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/recvmsg SQEs in advance
......................................................................
Patch Set 8: Code-Review+1
(3 comments)
File src/core/osmo_io_internal.h:
https://gerrit.osmocom.org/c/libosmocore/+/40494/comment/657ede80_e8db1a91?… :
PS8, Line 113: uint8_t read_sqes;
num_read_sqes?
https://gerrit.osmocom.org/c/libosmocore/+/40494/comment/2d0fe50f_685bd8b6?… :
PS8, Line 117: uint8_t read_count;
reads_in_transit? reads_in_progress?
File src/core/osmo_io_uring.c:
https://gerrit.osmocom.org/c/libosmocore/+/40494/comment/13ab340f_4efa41a4?… :
PS8, Line 152: msg = iofd_msgb_alloc(iofd);
I think having each loop iteration in a separate function should make this easier to read and maintain.
That also means this patch suddenly has half of modified lines.
--
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: 8
Gerrit-Owner: jolly <andreas(a)eversberg.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: jolly <andreas(a)eversberg.eu>
Gerrit-Comment-Date: Mon, 28 Jul 2025 11:18:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Attention is currently required from: jolly.
pespin has posted comments on this change by jolly. ( https://gerrit.osmocom.org/c/libosmocore/+/40760?usp=email )
Change subject: osmo-io: Put together message buffers when dequeued from tx queue
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
File src/core/osmo_io.c:
https://gerrit.osmocom.org/c/libosmocore/+/40760/comment/e88f2185_4e3fddf5?… :
PS1, Line 276: iofd_msghdr_free(next);
make sure you are not incidentally freeing msgbs which were previously part of the msghdr in case they were talloc children (not saying this is the case, just saying you check to make sure we don't have a problem here, we may need talloc_steal?)
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/40760?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: I97c366211dd266fd58ec252890ec017d6d334534
Gerrit-Change-Number: 40760
Gerrit-PatchSet: 1
Gerrit-Owner: jolly <andreas(a)eversberg.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: jolly <andreas(a)eversberg.eu>
Gerrit-Comment-Date: Mon, 28 Jul 2025 11:12:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
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 8:
(1 comment)
File src/core/osmo_io.c:
https://gerrit.osmocom.org/c/libosmocore/+/40493/comment/ba6fc25e_e62963db?… :
PS4, Line 444: /* Re-enqueue remaining buffers. */
> There will be a follow-up commit.
Marking as unresolved until I see the follow-up commit is submitted.
--
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: 8
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: Mon, 28 Jul 2025 11:04:01 +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/+/40492?usp=email )
Change subject: Add multiple messages buffers to io_uring read operations
......................................................................
Patch Set 7:
(3 comments)
File src/core/osmo_io_uring.c:
https://gerrit.osmocom.org/c/libosmocore/+/40492/comment/1b36d4d7_595e9d66?… :
PS7, Line 149: /* Set IO vectors and allocate additional read buffers.
I have the feeling all this code may need to go inside iofd_msghdr_alloc(), as well as the first iofd_msgb_alloc() above.
https://gerrit.osmocom.org/c/libosmocore/+/40492/comment/68bb6e03_5a8b6e12?… :
PS7, Line 154: msghdr->msg[idx] = iofd_msgb_alloc(iofd);
OSMO_ASSERT(msghdr->msg[idx])
https://gerrit.osmocom.org/c/libosmocore/+/40492/comment/44a55283_344b29c1?… :
PS7, Line 208: int idx;
this can be an unsigned int afaict.
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/40492?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: Ic4544b8fcbad5a266db748d6864d3ae93ee06bce
Gerrit-Change-Number: 40492
Gerrit-PatchSet: 7
Gerrit-Owner: jolly <andreas(a)eversberg.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: jolly <andreas(a)eversberg.eu>
Gerrit-Comment-Date: Mon, 28 Jul 2025 10:58:47 +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/+/40584?usp=email )
Change subject: Avoid reusing pending buffer; append incoming data instead
......................................................................
Patch Set 5: Code-Review-1
(2 comments)
File src/core/osmo_io.c:
https://gerrit.osmocom.org/c/libosmocore/+/40584/comment/9d8103ce_38c932d7?… :
PS5, Line 338: pending = iofd_msgb_alloc2(iofd, msgb_length(iofd->pending) + msgb_length(msg));
AFAICT this is not fullfilling the requirement that:
(msgb_length(iofd->pending) + msgb_length(msg)) >= iofd->msgb_alloc.size)
What I meant: msgb_length(iofd->pending) + msgb_length(msg) may be lower than iofd->msgb_alloc.size, which means user will end up with a msgb smaller than expected.
File src/core/osmo_io_internal.h:
https://gerrit.osmocom.org/c/libosmocore/+/40584/comment/d844b3a0_60f477c0?… :
PS5, Line 156: struct msgb *iofd_msgb_alloc2(struct osmo_io_fd *iofd, size_t extra);
this pnew param is named "size" now in the .c file.
--
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: 5
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-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Mon, 28 Jul 2025 10:52:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
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 5: Code-Review-1
(2 comments)
File src/core/osmo_io_uring.c:
https://gerrit.osmocom.org/c/libosmocore/+/40489/comment/1e55e1af_71945150?… :
PS4, Line 431: io_uring_submit(&g_ring.ring);
> Done
Not done
https://gerrit.osmocom.org/c/libosmocore/+/40489/comment/023b9662_88a06ae9?… :
PS4, Line 481: io_uring_submit(&g_ring.ring);
Since this pattern is repeated in several places, I'd add a static "inline void io_uring_sched_submit()" function which does this. Then we easily spot places where the same thing needs to be done, and we don't forget updating all of them like it happened already in this version of the patch.
--
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: 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: Mon, 28 Jul 2025 10:46:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: jolly <andreas(a)eversberg.eu>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>