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 4: (8 comments) File src/core/osmo_io.c: https://gerrit.osmocom.org/c/libosmocore/+/40493/comment/f4ab2e23_66117fc5?usp=email : PS4, Line 248: lh = iofd->tx_queue.msg_queue.prev; fyi we have llist_last(). We may also want to add llist_last_entry_or_null() to linuxlist.h, we already have llist_first_entry_or_null(). https://gerrit.osmocom.org/c/libosmocore/+/40493/comment/ec17b58f_8a08236b?usp=email : PS4, Line 444: /* Re-enqueue remaining buffers. */ Here we should improve it by adding new msg from the tx_queue into the iov if we aren't yet doing it. https://gerrit.osmocom.org/c/libosmocore/+/40493/comment/7d307ae3_038efe13?usp=email : PS4, Line 451: if (rc < chunk) shouldn't this case be already handled in the if path in line 431? https://gerrit.osmocom.org/c/libosmocore/+/40493/comment/7d6fa351_bb7a5ab3?usp=email : PS4, Line 465: iofd->io_ops.write_cb(iofd, (rc < 0) ? rc : chunk, msg); Again, by incorporation the negative value case into chunk variable, we can simplify all these lines here which now have a ": ?". https://gerrit.osmocom.org/c/libosmocore/+/40493/comment/bcc08743_17cb65c4?usp=email : PS4, Line 479: msgb_free(msghdr->msg[idx]); msghdr->msg[idx] = NULL; https://gerrit.osmocom.org/c/libosmocore/+/40493/comment/48579013_26af2f86?usp=email : PS4, Line 517: msghdr = iofd_txqueue_tail(iofd); why are you picking the tail here? Because the assumption is that the other before it are already filled? Do you mind writing teh rationale in a comment? https://gerrit.osmocom.org/c/libosmocore/+/40493/comment/eddfcee1_12b0e64c?usp=email : PS4, Line 531: you can simply do "idx = msghdr->msg_len - 1;" here, and simplify both branches above. https://gerrit.osmocom.org/c/libosmocore/+/40493/comment/867d036a_1ebad94d?usp=email : PS4, Line 539: rc = iofd_txqueue_enqueue(iofd, msghdr); are you submitting to io_uring queue inside here? Because then it looks like yoy may be modifying stuff you already submitted to io_uring... -- 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: 4 Gerrit-Owner: jolly <andr...@eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: pespin <pes...@sysmocom.de> Gerrit-Attention: jolly <andr...@eversberg.eu> Gerrit-Comment-Date: Fri, 04 Jul 2025 14:28:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No