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>
Attention is currently required from: pespin.
jolly 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:
(2 comments)
File src/core/osmo_io.c:
https://gerrit.osmocom.org/c/libosmocore/+/40493/comment/747f89ec_2678ed2c?… :
PS4, Line 444: /* Re-enqueue remaining buffers. */
> No, what I mean is that in here a function should be called to re-arrange msgs inside msghdrs in the […]
There will be a follow-up commit.
File src/core/osmo_io.c:
https://gerrit.osmocom.org/c/libosmocore/+/40493/comment/dc039d0b_33123e35?… :
PS7, Line 442: /* Re-enqueue the complete msgb. */
> This can be moved to the top of the function before the for loop, to simplify function now that you […]
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: 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:19: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: pespin.
jolly 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:
(2 comments)
File src/core/osmo_io_uring.c:
https://gerrit.osmocom.org/c/libosmocore/+/40492/comment/ba5f15dc_d48d8b9a?… :
PS3, Line 179: if (idx)
> Because it is already allocated above at: iofd_msgb_alloc(iodfd); and then added to the msghdr durin […]
Done
https://gerrit.osmocom.org/c/libosmocore/+/40492/comment/31c00280_a1b8f037?… :
PS3, Line 185:
> the cases below have all "/* fall-through */". I still need to check if action == IOFD_ACT_READ.
Done
--
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: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 28 Jul 2025 10:19: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: laforge, pespin.
jolly 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:
(8 comments)
File src/core/osmo_io.c:
https://gerrit.osmocom.org/c/libosmocore/+/40491/comment/ef361154_bc712529?… :
PS3, Line 165: hdr->msg_max = (g_io_backend == OSMO_IO_BACKEND_IO_URING) ? g_io_uring_iov : 1;
> this info should be obtained from osmo_io_fd struct.
Done
https://gerrit.osmocom.org/c/libosmocore/+/40491/comment/c1736551_2110a685?… :
PS3, Line 406: struct msgb *msg = msghdr->msg[0];
> I see now, because this commit is actually not yet adding support for >1 msgb per msghdr, it's only […]
Done
File src/core/osmo_io.c:
https://gerrit.osmocom.org/c/libosmocore/+/40491/comment/97886b07_25aada17?… :
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.
It is set by an API function now.
File src/core/osmo_io.c:
https://gerrit.osmocom.org/c/libosmocore/+/40491/comment/ecc7525f_6e3dcb0d?… :
PS6, Line 747: * If your osmo_io_fd is in OSMO_IO_FD_MODE_READ_WRITE mode, this API function can be used to tell the
> Please add proper doxygen documentation of params here.
Done
https://gerrit.osmocom.org/c/libosmocore/+/40491/comment/12c1d822_38847e71?… :
PS6, Line 748: * osmo_io code how many buffers should be read or written with a single read or write operation.
> "read or write". Extra space.
Done
https://gerrit.osmocom.org/c/libosmocore/+/40491/comment/5a0c93ff_615b47a6?… :
PS6, Line 751: int osmo_iofd_set_io_buffers(struct osmo_io_fd *iofd, int buffers)
> unsigned int buffers? […]
Done
File src/core/osmo_io_internal.h:
https://gerrit.osmocom.org/c/libosmocore/+/40491/comment/c79bae45_5b9c7630?… :
PS6, Line 143: int io_len;
> this can also be a unsigned int. And even actually a uint8_t.
Done
File src/core/osmo_io_uring.c:
https://gerrit.osmocom.org/c/libosmocore/+/40491/comment/84b14b5c_5b18e99a?… :
PS3, Line 110: g_io_uring_iov = atoi(env);
> afaict this is not some information which is needed really early in the startup of the program, befo […]
It is configured in an API function now.
--
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:19:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>