- gerrit-log - lists.osmocom.org

[M] Change in libosmocore[master]: Add multiple messages buffers to io_uring write operations

by jolly

Attention is currently required from: jolly. Hello Jenkins Builder, laforge, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/40493?usp=email to look at the new patch set (#12). The following approvals got outdated and were removed: Verified+1 by Jenkins Builder The change is no longer submittable: Verified is unsatisfied now. Change subject: Add multiple messages buffers to io_uring write operations ...................................................................... Add multiple messages buffers to io_uring write operations Multiple message buffers can be writen by sending a single SQE when using io_uring. If there is less data written, the completely written buffers are removed and the partly written buffers are truncated. Afterwards they are re-queued for next write operation. Having more than one buffer is optional and the number can be controlled via environment variable. Related: OS#6705 Change-Id: I8c4e0a785cf66becd7fb5b2caf718c9724b56686 --- M src/core/osmo_io.c M src/core/osmo_io_uring.c 2 files changed, 103 insertions(+), 46 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/93/40493/12 -- 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: newpatchset Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I8c4e0a785cf66becd7fb5b2caf718c9724b56686 Gerrit-Change-Number: 40493 Gerrit-PatchSet: 12 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>

11 hours, 51 minutes

1
0
0 0

[M] Change in libosmocore[master]: osmo_io: Add unit test to verify segmentation process

by jolly

Attention is currently required from: jolly. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/40854?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified-1 by Jenkins Builder Change subject: osmo_io: Add unit test to verify segmentation process ...................................................................... osmo_io: Add unit test to verify segmentation process Change-Id: I7d8feba9c8e8386c9fd144669f6ccd01d6bbbabb --- M tests/osmo_io/osmo_io_test.c M tests/osmo_io/osmo_io_test.ok 2 files changed, 126 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/54/40854/2 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/40854?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I7d8feba9c8e8386c9fd144669f6ccd01d6bbbabb Gerrit-Change-Number: 40854 Gerrit-PatchSet: 2 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Attention: jolly <andreas(a)eversberg.eu>

11 hours, 51 minutes

1
0
0 0

[M] Change in libosmocore[master]: osmo_io: Add unit test to verify segmentation process

by Jenkins Builder

Jenkins Builder has posted comments on this change by jolly. ( https://gerrit.osmocom.org/c/libosmocore/+/40854?usp=email ) Change subject: osmo_io: Add unit test to verify segmentation process ...................................................................... Patch Set 1: (1 comment) File tests/osmo_io/osmo_io_test.c: Robot Comment from checkpatch (run ID ): https://gerrit.osmocom.org/c/libosmocore/+/40854/comment/efd1b6d6_da5c03ad?… : PS1, Line 274: if (seg_number == 1) { braces {} are not necessary for any arm of this statement -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/40854?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: I7d8feba9c8e8386c9fd144669f6ccd01d6bbbabb Gerrit-Change-Number: 40854 Gerrit-PatchSet: 1 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-CC: Jenkins Builder Gerrit-Comment-Date: Fri, 08 Aug 2025 14:07:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

15 hours, 40 minutes

1
0
0 0

[M] Change in libosmocore[master]: Automatically increase io_uring, if too small.

by jolly

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 10: (7 comments) Commit Message: https://gerrit.osmocom.org/c/libosmocore/+/40725/comment/e95cf057_9b995286?… : PS9, Line 23: to determine when all submissions are completed. > Sounds like an easy thing to add right? It makes sense to implement this either her eor on a follow- […] I added a counter in a different patch and destry old rings, once empty. I hope that all cancelled SQE will be completed, not only the cancellation SQEs. If not, the ring would not be destroyed, so this would not cause any harm. File src/core/osmo_io_internal.h: https://gerrit.osmocom.org/c/libosmocore/+/40725/comment/a700383a_0fecc145?… : PS9, Line 110: bool read_enabled; > (At some point it may make sense to properly put all these fields into a "read" and "write" substruc […] There is a later patch that does this. File src/core/osmo_io_uring.c: https://gerrit.osmocom.org/c/libosmocore/+/40725/comment/64de0ab5_adfed9dc?… : PS2, Line 175: > ok, then please mention this fact explicitly in this c ommit in a TODO-comment and also in the commi […] I added the comments. I think that a counter cannot be used, as we don't know if we get one or two CQEs after cancelling an SQE. We always get the CQE for the cancellation SQE. If an SQE gets completed while sending a cancellation, we get two CQEs. File src/core/osmo_io_uring.c: https://gerrit.osmocom.org/c/libosmocore/+/40725/comment/123a753e_15996863?… : PS9, Line 126: g_ring = talloc_zero(OTC_GLOBAL, struct osmo_io_uring); > are you sure OTC_GLOBAL is defined at this point? Or is it actually NULL? […] IT is definitly defined and not NULL. The contructor in context.c "static __attribute__((constructor(101))) void on_dso_load_ctx(void)" has higher priority than any other constructor, including the initialization of osmo_io_uring. https://gerrit.osmocom.org/c/libosmocore/+/40725/comment/3368db83_9e78fefb?… : PS9, Line 175: g_io_uring_size *= 2; > Once you move this to an unsigned, you can do "<< 1" here. […] Done, see previous patch. https://gerrit.osmocom.org/c/libosmocore/+/40725/comment/c1be736e_aafa80c0?… : PS9, Line 208: /* All subsequent read SQEs must be on the same ring. */ > This should imho go inside iofd_uring_get_sqe(), based on "current_ring" being passed to it. iofd_uring_get_sqe() has no knowledge about the iofd, so it doesn't know if there are SQEs already submitted to a different ring than the current one. (iofd->u.uring.read_ring) https://gerrit.osmocom.org/c/libosmocore/+/40725/comment/69b446a2_baf1f6fc?… : PS9, Line 282: while (iofd->u.uring.reads_submitted < ((iofd->u.uring.num_read_sqes) ? : g_io_uring_read_sqes)) { > why is the g_io_uring_read_sqes reference needed here? How can iofd have num_read_sqes = 0, and if i […] g_io_uring_read_sques is 1 by default. This can be changed by environment variable, to allow benchmarking. The user may also define the number of of SQEs via API. This should override the default or what has been set by envrironment variable. if iofd instance is created, num_read_sqes (read.num_sques now) is not set by the user, so that it is 0. -- 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: 10 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: Fri, 08 Aug 2025 13:57:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: jolly <andreas(a)eversberg.eu> Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>

15 hours, 50 minutes

1
0
0 0

[S] Change in libosmocore[master]: Add environment variable to set io_uring size

by jolly

Attention is currently required from: pespin. jolly has posted comments on this change by jolly. ( https://gerrit.osmocom.org/c/libosmocore/+/40782?usp=email ) Change subject: Add environment variable to set io_uring size ...................................................................... Patch Set 3: (5 comments) File src/core/osmo_io_uring.c: https://gerrit.osmocom.org/c/libosmocore/+/40782/comment/1151667d_1b343984?… : PS1, Line 111: g_io_uring_size = atoi(env); > This should be parsed with osmo_str_to_int() to make sure we don't use some weird value as a result […] Done File src/core/osmo_io_uring.c: https://gerrit.osmocom.org/c/libosmocore/+/40782/comment/c7a16338_69ba78d6?… : PS2, Line 66: static int g_io_uring_size = IOFD_URING_INITIAL_SIZE; > size_t Actually "unsigned int". https://gerrit.osmocom.org/c/libosmocore/+/40782/comment/a0644196_69463f83?… : PS2, Line 111: rc = osmo_str_to_int(&g_io_uring_size, env, 10, 1, 32768); > I wonder where 32768 comes from :D This is the maximum. I added a comment. https://gerrit.osmocom.org/c/libosmocore/+/40782/comment/35f4b3c6_8164f3c4?… : PS2, Line 116: if ((g_io_uring_size & (g_io_uring_size - 1))) { > if (g_io_uring_size & 0x01) This condition hits at an odd value. We need a condition that hits if it is not a power of two. https://gerrit.osmocom.org/c/libosmocore/+/40782/comment/66a2c722_52ae26fc?… : PS2, Line 117: fprintf(stderr, "Error: Initial io_uring size must be a positive power of two.\n"); > Is this really needed as per io_uring API? It is not required, because the setup function will round it up to the power of two. Without the check the user would believe that the ring size equals the env settings. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/40782?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: I55289d9282e13aa1bf82f3931c85c196752f1484 Gerrit-Change-Number: 40782 Gerrit-PatchSet: 3 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: Fri, 08 Aug 2025 13:57:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>

15 hours, 50 minutes

1
0
0 0

[M] Change in libosmocore[master]: Send multiple read/recvfrom/recvmsg SQEs in advance

by jolly

Attention is currently required from: laforge, pespin. jolly 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 11: (2 comments) File src/core/osmo_io_internal.h: https://gerrit.osmocom.org/c/libosmocore/+/40494/comment/df7f2fa8_ddc75cbe?… : PS10, Line 112: /* ! requested number of simultaniously submitted read SQEs */ > simultaneously Done https://gerrit.osmocom.org/c/libosmocore/+/40494/comment/aa7f0554_0ee5d9f4?… : PS10, Line 116: /* ! current number of simultaniously submitted read SQEs */ > same: simultaneously. […] Done -- 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: 11 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: Fri, 08 Aug 2025 13:57:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge <laforge(a)osmocom.org>

15 hours, 50 minutes

1
0
0 0

[S] Change in libosmocore[master]: osmo-io: Put together message buffers when dequeued from tx queue

by jolly

Attention is currently required from: laforge. jolly 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 4: (1 comment) File src/core/osmo_io.c: https://gerrit.osmocom.org/c/libosmocore/+/40760/comment/2ec59388_27c8ad09?… : PS3, Line 261: * There can be empty buffers, when a msghdr is queued to the front with incomplete write. */ > do we have some unit test coverage for those rather hard-to-trigger code paths? I think it's hard to […] I tried to trigger an incomplete write, so that it is re-queued in front of the tx_queue. With regular non-nblocking IO it is possible to have partly written buffers, but with io_uring this seems not to happen. This means that this condition is never true, as there is no msghdr with unused write buffers in front of another msghdr. I prefer to remove it, since it is just an optimization. It is not required, even if io_uring would return an incomplete write. -- 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: 4 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-Comment-Date: Fri, 08 Aug 2025 13:57:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge <laforge(a)osmocom.org>

15 hours, 50 minutes

1
0
0 0

[M] Change in libosmocore[master]: osmo_io: Add unit test to verify segmentation process

by jolly

jolly has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/40854?usp=email ) Change subject: osmo_io: Add unit test to verify segmentation process ...................................................................... osmo_io: Add unit test to verify segmentation process Change-Id: I7d8feba9c8e8386c9fd144669f6ccd01d6bbbabb --- M tests/osmo_io/osmo_io_test.c M tests/osmo_io/osmo_io_test.ok 2 files changed, 127 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/54/40854/1 diff --git a/tests/osmo_io/osmo_io_test.c b/tests/osmo_io/osmo_io_test.c index 553368c..5d36c05 100644 --- a/tests/osmo_io/osmo_io_test.c +++ b/tests/osmo_io/osmo_io_test.c @@ -247,6 +247,113 @@ for (int i = 0; i < 128; i++) osmo_select_main(1); } + +int segmentation_cb(struct osmo_io_fd *iofd, struct msgb *msg) +{ + printf("%s: segmentation_cb() returning %d\n", osmo_iofd_get_name(iofd), 4); + return 4; +} + +static void segment_read_cb(struct osmo_io_fd *iofd, int rc, struct msgb *msg) +{ + static int seg_number = 0; + + printf("%s: read() msg with rc=%d\n", osmo_iofd_get_name(iofd), rc); + if (rc < 0) { + printf("%s: error: %s\n", osmo_iofd_get_name(iofd), strerror(-rc)); + OSMO_ASSERT(0); + } + OSMO_ASSERT(msg); + if (seg_number < 3) { + printf("%s\n", osmo_hexdump(msgb_data(msg), msgb_length(msg))); + printf("tailroom = %d\n", msgb_tailroom(msg)); + /* Our read buffer is 6 bytes, Our segment is 4 bytes, this results in tailroom of 2 bytes. + * When the pending 2 bytes are combined with subsequent read of 6 bytes, an extra buffer + * with 8 bytes is allocated. Our segment is 4 byte, then this results in a tailroom of 4 + * bytes. */ + if (seg_number == 1) { + OSMO_ASSERT(msgb_tailroom(msg) == 4); + } else { + OSMO_ASSERT(msgb_tailroom(msg) == 2); + } + OSMO_ASSERT(msgb_length(msg) == sizeof(TESTDATA) / 4); + seg_number++; + } else { + OSMO_ASSERT(rc == 0); + file_eof_read = true; + } + talloc_free(msg); +} + +static void test_segmentation(void) +{ + struct osmo_io_fd *iofd; + struct msgb *msg; + uint8_t *buf; + int fd[2] = { 0, 0 }; + int rc; + struct osmo_io_ops ioops; + + TEST_START(); + + /* Create pipe */ + rc = pipe(fd); + OSMO_ASSERT(rc == 0); + OSMO_ASSERT(fd[0]); + OSMO_ASSERT(fd[1]); + + /* First test writing to the pipe: */ + printf("Enable write\n"); + ioops = (struct osmo_io_ops){ .write_cb = file_write_cb }; + iofd = osmo_iofd_setup(ctx, fd[1], "seg_iofd", OSMO_IO_FD_MODE_READ_WRITE, &ioops, NULL); + osmo_iofd_register(iofd, fd[1]); + + msg = msgb_alloc(12, "Test data"); + buf = msgb_put(msg, 12); + memcpy(buf, TESTDATA, 12); + osmo_iofd_write_msgb(iofd, msg); + /* Allow enough cycles to handle the messages */ + file_bytes_write_compl = 0; + for (int i = 0; i < 128; i++) { + OSMO_ASSERT(file_bytes_write_compl <= 12); + if (file_bytes_write_compl == 12) + break; + osmo_select_main(1); + usleep(100 * 1000); + } + fflush(stdout); + OSMO_ASSERT(file_bytes_write_compl == 12); + + osmo_iofd_unregister(iofd); + close(fd[1]); + + /* Now, re-configure iofd to only read from the pipe. + * Reduce the read buffer size, to verify correct segmentation operation: */ + printf("Enable read\n"); + osmo_iofd_set_alloc_info(iofd, 6, 0); + osmo_iofd_register(iofd, fd[0]); + ioops = (struct osmo_io_ops){ .read_cb = segment_read_cb, .segmentation_cb2 = segmentation_cb }; + rc = osmo_iofd_set_ioops(iofd, &ioops); + OSMO_ASSERT(rc == 0); + /* Allow enough cycles to handle the message. We expect 3 reads, 4th read will return 0. */ + file_bytes_read = 0; + file_eof_read = false; + for (int i = 0; i < 128; i++) { + if (file_eof_read) + break; + osmo_select_main(1); + usleep(100 * 1000); + } + fflush(stdout); + OSMO_ASSERT(file_eof_read); + + osmo_iofd_free(iofd); + + for (int i = 0; i < 128; i++) + osmo_select_main(1); +} + + static const struct log_info_cat default_categories[] = { }; @@ -267,6 +374,7 @@ test_file(); test_connected(); test_unconnected(); + test_segmentation(); return EXIT_SUCCESS; } diff --git a/tests/osmo_io/osmo_io_test.ok b/tests/osmo_io/osmo_io_test.ok index 6b9e591..b24bfd2 100644 --- a/tests/osmo_io/osmo_io_test.ok +++ b/tests/osmo_io/osmo_io_test.ok @@ -16,3 +16,22 @@ ep1: sendto() returned rc=16 ep2: recvfrom() msg with len=16 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f 10 +Running test_segmentation +Enable write +seg_iofd: write() returned rc=12 +01 02 03 04 05 06 07 08 09 0a 0b 0c +Enable read +seg_iofd: segmentation_cb() returning 4 +seg_iofd: read() msg with rc=6 +01 02 03 04 +tailroom = 2 +seg_iofd: segmentation_cb() returning 4 +seg_iofd: segmentation_cb() returning 4 +seg_iofd: read() msg with rc=6 +05 06 07 08 +tailroom = 4 +seg_iofd: segmentation_cb() returning 4 +seg_iofd: read() msg with rc=6 +09 0a 0b 0c +tailroom = 2 +seg_iofd: read() msg with rc=0 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/40854?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I7d8feba9c8e8386c9fd144669f6ccd01d6bbbabb Gerrit-Change-Number: 40854 Gerrit-PatchSet: 1 Gerrit-Owner: jolly <andreas(a)eversberg.eu>

15 hours, 51 minutes

1
0
0 0

[S] Change in libosmocore[master]: Remove old empty io_uring

by jolly

jolly has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/40855?usp=email ) Change subject: Remove old empty io_uring ...................................................................... Remove old empty io_uring A previous patch creates a new io_uring, if it becomes too small to store all SQEs. When all SQEs of the old ring are completed, the old ring will be destroyed. A counter is incremented whenever an SQE is submitted to an io_uring. The counter is decrements whenever a CQE is received and handled. This counter will determine when a ring is empty and can be destroyed. Related: OS#6705 Change-Id: Id2d2a0400ad442198c684ea0ead4eaeaead4c53d --- M src/core/osmo_io_uring.c 1 file changed, 27 insertions(+), 4 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/55/40855/1 diff --git a/src/core/osmo_io_uring.c b/src/core/osmo_io_uring.c index 1a817dd..a867968 100644 --- a/src/core/osmo_io_uring.c +++ b/src/core/osmo_io_uring.c @@ -65,7 +65,7 @@ bool g_io_uring_batch = false; bool g_io_uring_submit_needed = false; -static unsigned int g_io_uring_size = IOFD_URING_INITIAL_SIZE; +static int g_io_uring_size = IOFD_URING_INITIAL_SIZE; static int g_io_uring_read_sqes = 1; @@ -73,6 +73,7 @@ struct osmo_fd event_ofd; struct io_uring ring; struct llist_head cancel_queue; + unsigned int in_flight; }; static __thread struct osmo_io_uring *g_ring = NULL; @@ -167,13 +168,27 @@ } } +static void osmo_iofd_uring_exit(struct osmo_io_uring *ring) +{ + LOGP(DLIO, LOGL_DEBUG, "Old empty io_uring will be destroyed."); + + io_uring_queue_exit(&ring->ring); + + osmo_fd_unregister(&ring->event_ofd); + close(ring->event_ofd.fd); + + talloc_free(ring); +} + static struct io_uring_sqe *iofd_uring_get_sqe(bool current_ring) { struct io_uring_sqe *sqe; sqe = io_uring_get_sqe(&g_ring->ring); - if (sqe) + if (sqe) { + g_ring->in_flight++; return sqe; + } if (g_io_uring_size < IOFD_URING_MAXIMUM_SIZE) { LOGP(DLIO, LOGL_NOTICE, "io_uring too small to handle all SQEs with its current size of %d. " @@ -197,6 +212,7 @@ sqe = io_uring_get_sqe(&g_ring->ring); OSMO_ASSERT(sqe); + g_ring->in_flight++; return sqe; } @@ -424,6 +440,7 @@ struct osmo_io_uring *orig_ring = container_of(ring, struct osmo_io_uring, ring); while (io_uring_peek_cqe(ring, &cqe) == 0) { + orig_ring->in_flight--; msghdr = io_uring_cqe_get_data(cqe); if (!msghdr) { @@ -448,13 +465,18 @@ } /* If there are unsubmitted cancel SQEs, try to queue them now. */ - if (OSMO_LIKELY(llist_empty(&orig_ring->cancel_queue))) + if (OSMO_LIKELY(llist_empty(&orig_ring->cancel_queue))) { + /* Old ring is empty, remove it. */ + if (OSMO_UNLIKELY(orig_ring != g_ring && orig_ring->in_flight == 0)) + osmo_iofd_uring_exit(orig_ring); return; + } llist_for_each_entry_safe(msghdr, msghdr2, &orig_ring->cancel_queue, list) { struct io_uring_sqe *sqe; sqe = io_uring_get_sqe(&orig_ring->ring); if (!sqe) break; + orig_ring->in_flight++; io_uring_sqe_set_data(sqe, NULL); LOGP(DLIO, LOGL_DEBUG, "Cancelling queued read/write\n"); io_uring_prep_cancel(sqe, msghdr, 0); @@ -462,7 +484,6 @@ msghdr->in_cancel_queue = false; } io_uring_submit(&orig_ring->ring); - } /*! will submit the next to-be-transmitted message for given iofd */ @@ -584,6 +605,7 @@ /* If the submission queue is full, use cancel queue. We cannot cancel SQEs on the new ring. */ sqe = io_uring_get_sqe(&ring->ring); if (sqe) { + ring->in_flight++; io_uring_sqe_set_data(sqe, NULL); LOGPIO(iofd, LOGL_DEBUG, "Cancelling read\n"); io_uring_prep_cancel(sqe, msghdr, 0); @@ -614,6 +636,7 @@ /* If the submission queue is full, use cancel queue. We cannot cancel SQEs on the new ring. */ sqe = io_uring_get_sqe(&ring->ring); if (sqe) { + ring->in_flight++; io_uring_sqe_set_data(sqe, NULL); LOGPIO(iofd, LOGL_DEBUG, "Cancelling write\n"); io_uring_prep_cancel(sqe, msghdr, 0); -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/40855?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Id2d2a0400ad442198c684ea0ead4eaeaead4c53d Gerrit-Change-Number: 40855 Gerrit-PatchSet: 1 Gerrit-Owner: jolly <andreas(a)eversberg.eu>

15 hours, 51 minutes

1
0
0 0

[M] Change in libosmocore[master]: Put all io_uring related read and write states into sub structures

by jolly

jolly has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/40856?usp=email ) Change subject: Put all io_uring related read and write states into sub structures ...................................................................... Put all io_uring related read and write states into sub structures Related: OS#6705 Change-Id: I3cf4ab6d9aebf5532ad174c90c7b0e9633491c88 --- M src/core/osmo_io.c M src/core/osmo_io_internal.h M src/core/osmo_io_uring.c 3 files changed, 69 insertions(+), 62 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/56/40856/1 diff --git a/src/core/osmo_io.c b/src/core/osmo_io.c index 6a5dcce..6e19231 100644 --- a/src/core/osmo_io.c +++ b/src/core/osmo_io.c @@ -893,7 +893,7 @@ if (sqes < 1 || sqes > IOFD_MSGHDR_MAX_READ_SQES) return -EINVAL; - iofd->u.uring.num_read_sqes = sqes; + iofd->u.uring.read.num_sqes = sqes; return 0; } diff --git a/src/core/osmo_io_internal.h b/src/core/osmo_io_internal.h index 3d92b85..b417e37 100644 --- a/src/core/osmo_io_internal.h +++ b/src/core/osmo_io_internal.h @@ -107,19 +107,26 @@ struct osmo_fd ofd; } poll; struct { - bool read_enabled; - bool write_enabled; - /*! requested number of simultaniously submitted read SQEs */ - uint8_t num_read_sqes; - /*! array of simultaneously submitted read SQEs */ - void *read_msghdr[IOFD_MSGHDR_MAX_READ_SQES]; - /*! ring the read SQEs have been submitted to */ - struct io_uring *read_ring; - /*! current number of simultaneously submitted read SQEs */ - uint8_t reads_submitted; - void *write_msghdr; - /*! ring the write SQE has been submitted to */ - struct io_uring *write_ring; + struct { + /*! read is enabled, due to registration of callback function */ + bool enabled; + /*! requested number of simultaniously submitted read SQEs */ + uint8_t num_sqes; + /*! array of simultaneously submitted read SQEs */ + void *msghdr[IOFD_MSGHDR_MAX_READ_SQES]; + /*! ring the read SQEs have been submitted to */ + struct io_uring *ring; + /*! current number of simultaneously submitted read SQEs */ + uint8_t sqes_submitted; + } read; + struct { + /*! write is enabled, due to pending msghdr in tx_queue */ + bool enabled; + /*! submitted write SQE */ + void *msghdr; + /*! ring the write SQE has been submitted to */ + struct io_uring *ring; + } write; /* TODO: index into array of registered fd's? */ /* osmo_fd for non-blocking connect handling */ struct osmo_fd connect_ofd; diff --git a/src/core/osmo_io_uring.c b/src/core/osmo_io_uring.c index a867968..b414c0d 100644 --- a/src/core/osmo_io_uring.c +++ b/src/core/osmo_io_uring.c @@ -232,13 +232,13 @@ uint8_t idx; /* All subsequent read SQEs must be on the same ring. */ - if (iofd->u.uring.reads_submitted > 0 && iofd->u.uring.read_ring != &g_ring->ring) + if (iofd->u.uring.read.sqes_submitted > 0 && iofd->u.uring.read.ring != &g_ring->ring) return -EINVAL; /* Tell iofd_uring_get_sqe() not to allocate a new ring, if we want to enqueue multiple read SQEs. */ - sqe = iofd_uring_get_sqe(iofd->u.uring.reads_submitted > 0); + sqe = iofd_uring_get_sqe(iofd->u.uring.read.sqes_submitted > 0); if (!sqe) { - if (iofd->u.uring.reads_submitted > 0) + if (iofd->u.uring.read.sqes_submitted > 0) return -EINVAL; LOGPIO(iofd, LOGL_ERROR, "Could not get io_uring_sqe\n"); OSMO_ASSERT(0); @@ -293,9 +293,9 @@ iofd_io_uring_submit(); - iofd->u.uring.read_msghdr[iofd->u.uring.reads_submitted] = msghdr; - iofd->u.uring.reads_submitted++; - iofd->u.uring.read_ring = &g_ring->ring; + iofd->u.uring.read.msghdr[iofd->u.uring.read.sqes_submitted] = msghdr; + iofd->u.uring.read.sqes_submitted++; + iofd->u.uring.read.ring = &g_ring->ring; return 0; } @@ -305,7 +305,7 @@ int rc; /* Submit more read SQEs in advance, if requested. */ - while (iofd->u.uring.reads_submitted < ((iofd->u.uring.num_read_sqes) ? : g_io_uring_read_sqes)) { + while (iofd->u.uring.read.sqes_submitted < ((iofd->u.uring.read.num_sqes) ? : g_io_uring_read_sqes)) { rc = iofd_uring_submit_recv_sqe(iofd, action); /* Stop, if we cannot enqueue multiple read SQEs in the same ring. */ if (rc < 0) @@ -320,19 +320,19 @@ uint8_t idx, i; /* Find which read_msghdr is completed and remove from list. */ - for (idx = 0; idx < iofd->u.uring.reads_submitted; idx++) { - if (iofd->u.uring.read_msghdr[idx] == msghdr) + for (idx = 0; idx < iofd->u.uring.read.sqes_submitted; idx++) { + if (iofd->u.uring.read.msghdr[idx] == msghdr) break; } - if (idx == iofd->u.uring.reads_submitted) { + if (idx == iofd->u.uring.read.sqes_submitted) { LOGP(DLIO, LOGL_FATAL, "Read SQE completion, but msghdr not found, please fix!\n"); return; } /* Remove entry at idx. */ - iofd->u.uring.reads_submitted--; - for (i = idx; i < iofd->u.uring.reads_submitted; i++) - iofd->u.uring.read_msghdr[i] = iofd->u.uring.read_msghdr[i + 1]; - iofd->u.uring.read_msghdr[i] = NULL; + iofd->u.uring.read.sqes_submitted--; + for (i = idx; i < iofd->u.uring.read.sqes_submitted; i++) + iofd->u.uring.read.msghdr[i] = iofd->u.uring.read.msghdr[i + 1]; + iofd->u.uring.read.msghdr[i] = NULL; for (idx = 0; idx < msghdr->io_len; idx++) { struct msgb *msg = msghdr->msg[idx]; @@ -351,7 +351,7 @@ } /* Check for every iteration, because iofd might get unregistered/closed during receive function. */ - if (iofd->u.uring.read_enabled && !IOFD_FLAG_ISSET(iofd, IOFD_FLAG_CLOSED)) + if (iofd->u.uring.read.enabled && !IOFD_FLAG_ISSET(iofd, IOFD_FLAG_CLOSED)) iofd_handle_recv(iofd, msg, chunk, msghdr); else msgb_free(msg); @@ -364,7 +364,7 @@ msghdr->msg[idx] = NULL; } - if (iofd->u.uring.read_enabled && !IOFD_FLAG_ISSET(iofd, IOFD_FLAG_CLOSED)) + if (iofd->u.uring.read.enabled && !IOFD_FLAG_ISSET(iofd, IOFD_FLAG_CLOSED)) iofd_uring_submit_recv(iofd, msghdr->action); iofd_msghdr_free(msghdr); @@ -380,12 +380,12 @@ /* Detach msghdr from iofd. It might get freed here or it is freed during iofd_handle_send_completion(). * If there is pending data to send, iofd_uring_submit_tx() will attach it again. * iofd_handle_send_completion() will invoke a callback function to signal the possibility of write/send. - * This callback function might close iofd, leading to the potential freeing of iofd->u.uring.write_msghdr if + * This callback function might close iofd, leading to the potential freeing of iofd->u.uring.write.msghdr if * still attached. Since iofd_handle_send_completion() frees msghdr at the end of the function, detaching * msghdr here prevents a double-free bug. */ - if (iofd->u.uring.write_msghdr == msghdr) { - iofd->u.uring.write_msghdr = NULL; - iofd->u.uring.write_ring = NULL; + if (iofd->u.uring.write.msghdr == msghdr) { + iofd->u.uring.write.msghdr = NULL; + iofd->u.uring.write.ring = NULL; } if (OSMO_UNLIKELY(IOFD_FLAG_ISSET(iofd, IOFD_FLAG_CLOSED))) { @@ -399,7 +399,7 @@ } /* submit the next to-be-transmitted message for this file descriptor */ - if (iofd->u.uring.write_enabled && !IOFD_FLAG_ISSET(iofd, IOFD_FLAG_CLOSED)) + if (iofd->u.uring.write.enabled && !IOFD_FLAG_ISSET(iofd, IOFD_FLAG_CLOSED)) iofd_uring_submit_tx(iofd); } @@ -427,7 +427,7 @@ IOFD_FLAG_UNSET(iofd, IOFD_FLAG_IN_CALLBACK); - if (IOFD_FLAG_ISSET(iofd, IOFD_FLAG_TO_FREE) && !iofd->u.uring.reads_submitted && !iofd->u.uring.write_msghdr) + if (IOFD_FLAG_ISSET(iofd, IOFD_FLAG_TO_FREE) && !iofd->u.uring.read.sqes_submitted && !iofd->u.uring.write.msghdr) talloc_free(iofd); } @@ -518,8 +518,8 @@ iofd_io_uring_submit(); - iofd->u.uring.write_msghdr = msghdr; - iofd->u.uring.write_ring = &g_ring->ring; + iofd->u.uring.write.msghdr = msghdr; + iofd->u.uring.write.ring = &g_ring->ring; return 0; } @@ -558,12 +558,12 @@ IOFD_FLAG_UNSET(iofd, IOFD_FLAG_IN_CALLBACK); /* If write/read notifications are pending, enable it now. */ - if (iofd->u.uring.write_enabled && !IOFD_FLAG_ISSET(iofd, IOFD_FLAG_CLOSED)) + if (iofd->u.uring.write.enabled && !IOFD_FLAG_ISSET(iofd, IOFD_FLAG_CLOSED)) iofd_uring_write_enable(iofd); - if (iofd->u.uring.read_enabled && !IOFD_FLAG_ISSET(iofd, IOFD_FLAG_CLOSED)) + if (iofd->u.uring.read.enabled && !IOFD_FLAG_ISSET(iofd, IOFD_FLAG_CLOSED)) iofd_uring_read_enable(iofd); - if (IOFD_FLAG_ISSET(iofd, IOFD_FLAG_TO_FREE) && !iofd->u.uring.reads_submitted && !iofd->u.uring.write_msghdr) + if (IOFD_FLAG_ISSET(iofd, IOFD_FLAG_TO_FREE) && !iofd->u.uring.read.sqes_submitted && !iofd->u.uring.write.msghdr) talloc_free(iofd); return 0; } @@ -595,10 +595,10 @@ struct iofd_msghdr *msghdr; uint8_t idx; - for (idx = 0; idx < iofd->u.uring.reads_submitted; idx++) { - struct osmo_io_uring *ring = container_of(iofd->u.uring.read_ring, struct osmo_io_uring, ring); - msghdr = iofd->u.uring.read_msghdr[idx]; - iofd->u.uring.read_msghdr[idx] = NULL; + for (idx = 0; idx < iofd->u.uring.read.sqes_submitted; idx++) { + struct osmo_io_uring *ring = container_of(iofd->u.uring.read.ring, struct osmo_io_uring, ring); + msghdr = iofd->u.uring.read.msghdr[idx]; + iofd->u.uring.read.msghdr[idx] = NULL; /* Submit SQEs of the current ring, if needed. */ if (&ring->ring == &g_ring->ring) osmo_io_uring_submit(); @@ -617,15 +617,15 @@ talloc_steal(OTC_GLOBAL, msghdr); msghdr->iofd = NULL; } - if (iofd->u.uring.reads_submitted) { - iofd->u.uring.read_ring = NULL; - iofd->u.uring.reads_submitted = 0; + if (iofd->u.uring.read.sqes_submitted) { + iofd->u.uring.read.ring = NULL; + iofd->u.uring.read.sqes_submitted = 0; } - if (iofd->u.uring.write_msghdr) { - struct osmo_io_uring *ring = container_of(iofd->u.uring.write_ring, struct osmo_io_uring, ring); - msghdr = iofd->u.uring.write_msghdr; - iofd->u.uring.write_msghdr = NULL; + if (iofd->u.uring.write.msghdr) { + struct osmo_io_uring *ring = container_of(iofd->u.uring.write.ring, struct osmo_io_uring, ring); + msghdr = iofd->u.uring.write.msghdr; + iofd->u.uring.write.msghdr = NULL; for (int idx = 0; idx < msghdr->io_len; idx++) { msgb_free(msghdr->msg[idx]); msghdr->msg[idx] = NULL; @@ -647,7 +647,7 @@ } talloc_steal(OTC_GLOBAL, msghdr); msghdr->iofd = NULL; - iofd->u.uring.write_ring = NULL; + iofd->u.uring.write.ring = NULL; } if (IOFD_FLAG_ISSET(iofd, IOFD_FLAG_NOTIFY_CONNECTED)) { @@ -660,9 +660,9 @@ static void iofd_uring_write_enable(struct osmo_io_fd *iofd) { - iofd->u.uring.write_enabled = true; + iofd->u.uring.write.enabled = true; - if (iofd->u.uring.write_msghdr) + if (iofd->u.uring.write.msghdr) return; /* This function is called again, once the socket is connected. */ @@ -699,21 +699,21 @@ iofd_io_uring_submit(); - iofd->u.uring.write_msghdr = msghdr; - iofd->u.uring.write_ring = &g_ring->ring; + iofd->u.uring.write.msghdr = msghdr; + iofd->u.uring.write.ring = &g_ring->ring; } } static void iofd_uring_write_disable(struct osmo_io_fd *iofd) { - iofd->u.uring.write_enabled = false; + iofd->u.uring.write.enabled = false; } static void iofd_uring_read_enable(struct osmo_io_fd *iofd) { - iofd->u.uring.read_enabled = true; + iofd->u.uring.read.enabled = true; - if (iofd->u.uring.reads_submitted) + if (iofd->u.uring.read.sqes_submitted) return; /* This function is called again, once the socket is connected. */ @@ -737,7 +737,7 @@ static void iofd_uring_read_disable(struct osmo_io_fd *iofd) { - iofd->u.uring.read_enabled = false; + iofd->u.uring.read.enabled = false; } static int iofd_uring_close(struct osmo_io_fd *iofd) @@ -753,7 +753,7 @@ } /* OSMO_IO_FD_MODE_RECVMSG_SENDMSG: Don't call this function after enabling read or write. */ - OSMO_ASSERT(!iofd->u.uring.write_enabled && !iofd->u.uring.read_enabled); + OSMO_ASSERT(!iofd->u.uring.write.enabled && !iofd->u.uring.read.enabled); /* Set flag to enable temporary osmo_fd during register() time: */ IOFD_FLAG_SET(iofd, IOFD_FLAG_NOTIFY_CONNECTED); -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/40856?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I3cf4ab6d9aebf5532ad174c90c7b0e9633491c88 Gerrit-Change-Number: 40856 Gerrit-PatchSet: 1 Gerrit-Owner: jolly <andreas(a)eversberg.eu>

15 hours, 51 minutes

1
0
0 0

2025

2024

2023

2022

gerrit-log