Attention is currently required from: fixeria, pespin.
daniel has posted comments on this change. (
https://gerrit.osmocom.org/c/libosmocore/+/32536 )
Change subject: osmo_io: Add io_uring backend
......................................................................
Patch Set 11:
(15 comments)
File src/core/osmo_io_uring.c:
https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/2cb864d2_dc8a8633
PS10, Line 118: return
Looks like `msg` is leaked here. […]
I also
combined submit_read() and submit_recvmsg() into one function since they are nearly
identical
https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/5a0f9c27_c3519917
PS10, Line 126: Could not get io_uring_sqe
missing `\n`
Done
https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/0b8eac9f_12888db1
PS10, Line 130: // Prep msgb/iov
cosmetic: use the `/* ... […]
Done
https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/4c811e23_9aec738b
PS10, Line 135: // NOTE: This only works if we have one read per fd
cosmetic: use the `/* ... […]
Done
https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/c4490aee_cb46df3b
PS10, Line 153: return
Again, looks like `msg` is leaked here.
Done
https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/84577c9c_84851497
PS10, Line 165: Could not get io_uring_sqe
Again missing `\n`.
Done
https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/1ac096b2_2394de44
PS10, Line 275: /* Fallthrough */
This comment is not really needed since there is no
code preceding it.
Done
https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/cd73e497_51a04bba
PS10, Line 295: io_uring_peek_cqe
Are you sure this is correct? […]
No, it looks
wrong. Seems like I still got all cqes because peek would (eventually) return the same
completion until its io_uring_cqe_seen()
https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/71151ea0_c55ac0f9
PS10, Line 326: Could not get io_uring_sqe
missing '\n'
Done
https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/fd1a42a5_7defc65a
PS10, Line 360: if (!sqe)
: OSMO_ASSERT(0);
`OSMO_ASSERT(sqe != NULL)`
Ack
https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/ef12b6cb_28afeab0
PS10, Line 362: (void *)0x0
Why not `NULL`?
Done
https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/f4f7cacc_5236b267
PS10, Line 369: if (!sqe)
: OSMO_ASSERT(0);
`OSMO_ASSERT(sqe != NULL)`
Done
https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/517eb248_f5f18ca1
PS10, Line 397: return
`msg` is leaked here.
Done
https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/34689a92_515ad1e0
PS10, Line 404: Could not get io_uring_sqe
missing `\n`
Done
https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/2a74067c_3fa9d3b9
PS10, Line 438:
ws
Done
--
To view, visit
https://gerrit.osmocom.org/c/libosmocore/+/32536
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I5152129eb84b31ccc9e02bc2a5c5bdb046d331bc
Gerrit-Change-Number: 32536
Gerrit-PatchSet: 11
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: daniel <dwillmann(a)sysmocom.de>
Gerrit-CC: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 22 Aug 2023 14:35:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: comment