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>
Attention is currently required from: pespin.
Jenkins Builder 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:
(1 comment)
File src/core/osmo_io.c:
Robot Comment from checkpatch (run ID ):
https://gerrit.osmocom.org/c/libosmocore/+/40494/comment/85847f79_0de80a8c?… :
PS8, Line 860: /*! Set the number of SQEs that are submitted to an io_unring before completion is recevied.
'recevied' may be misspelled - perhaps 'received'?
--
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-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 28 Jul 2025 10:25:42 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: laforge, pespin.
Jenkins Builder 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 7:
(1 comment)
File src/core/osmo_io.c:
Robot Comment from checkpatch (run ID ):
https://gerrit.osmocom.org/c/libosmocore/+/40491/comment/fb9ed244_57caa6bc?… :
PS7, Line 768: switch (op) {
switch and case should be at the same indent
--
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: 7
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: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 28 Jul 2025 10:25:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: laforge, pespin.
jolly 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:
(6 comments)
File src/core/osmo_io_internal.h:
https://gerrit.osmocom.org/c/libosmocore/+/40725/comment/88d1e846_2dae4314?… :
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 th […]
Done
File src/core/osmo_io_uring.c:
https://gerrit.osmocom.org/c/libosmocore/+/40725/comment/71b86426_f122d2f5?… :
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. […]
Done
https://gerrit.osmocom.org/c/libosmocore/+/40725/comment/e38bd57a_d54e1f06?… :
PS2, Line 113: OSMO_IO_URING_QUEUE
> like the variable name in the code, the environment variable name shold be self-descriptive. […]
Done
https://gerrit.osmocom.org/c/libosmocore/+/40725/comment/720762d0_8db9c7c1?… :
PS2, Line 163: osmo_iofd_uring_get_sqe
> the 'osmo_' prefix is reserverd for exported functions. […]
Done
https://gerrit.osmocom.org/c/libosmocore/+/40725/comment/86884b81_0162c7d1?… :
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 […]
Done
https://gerrit.osmocom.org/c/libosmocore/+/40725/comment/13d47890_d1b52789?… :
PS2, Line 175:
> is there code to clean up the old ring after completion of the last cqe of the old ring?
No, it doesn't, because I don't track the number of submissions and the number of completions. I could do that. This would make it more complex, especially during cancellation process. I can add this if you want and test with the ttcn3 tests.
--
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: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 28 Jul 2025 10:19:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>