From dc73e72c4e2a0fd046961a429985b0979f772440 Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Sat, 22 Oct 2022 10:39:50 -0600 Subject: [PATCH 1/5] test/recv-multishot.c: check for error specifically when submitting Signed-off-by: Jens Axboe --- test/recv-multishot.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/recv-multishot.c b/test/recv-multishot.c index e7c1a99d..2cfe6898 100644 --- a/test/recv-multishot.c +++ b/test/recv-multishot.c @@ -141,7 +141,7 @@ static int test(struct args *args) buffer_size, 1, 7, i); io_uring_sqe_set_data64(sqe, 0x999); memset(recv_buffs[i], 0xcc, buffer_size); - if (io_uring_submit_and_wait_timeout(&ring, &cqe, 1, &timeout, NULL) != 0) { + if (io_uring_submit_and_wait_timeout(&ring, &cqe, 1, &timeout, NULL) < 0) { fprintf(stderr, "provide buffers failed: %d\n", ret); ret = -1; goto cleanup; From 40cbfe3db2c0954fb3c0eb5f3832f08f049aefd7 Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Fri, 21 Oct 2022 13:55:37 -0700 Subject: [PATCH 2/5] src/queue: don't wait twice if looping in _io_uring_get_cqe() If the caller asks for > 1 events with a timeout and we get just 1 event, then we end up looping around and issuing an IORING_ENTER_GETEVENTS with the timeout set twice. This results in twice the timeout that the caller asked for. Add a ->has_ts argument to struct get_data and don't re-enter if we've already looped and it is set. Also be a bit saner in not overwriting the return value, if set, and return the initial value from io_uring_enter(). Signed-off-by: Jens Axboe --- src/queue.c | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/src/queue.c b/src/queue.c index d29d422b..c06bcc35 100644 --- a/src/queue.c +++ b/src/queue.c @@ -55,6 +55,7 @@ struct get_data { unsigned wait_nr; unsigned get_flags; int sz; + int has_ts; void *arg; }; @@ -64,7 +65,7 @@ static int _io_uring_get_cqe(struct io_uring *ring, { struct io_uring_cqe *cqe = NULL; bool looped = false; - int err; + int err = 0; do { bool need_enter = false; @@ -72,9 +73,12 @@ static int _io_uring_get_cqe(struct io_uring *ring, unsigned nr_available; int ret; - err = __io_uring_peek_cqe(ring, &cqe, &nr_available); - if (err) + ret = __io_uring_peek_cqe(ring, &cqe, &nr_available); + if (ret) { + if (!err) + err = ret; break; + } if (!cqe && !data->wait_nr && !data->submit) { /* * If we already looped once, we already entererd @@ -82,7 +86,8 @@ static int _io_uring_get_cqe(struct io_uring *ring, * wait for, don't keep retrying. */ if (looped || !cq_ring_needs_enter(ring)) { - err = -EAGAIN; + if (!err) + err = -EAGAIN; break; } need_enter = true; @@ -95,6 +100,13 @@ static int _io_uring_get_cqe(struct io_uring *ring, need_enter = true; if (!need_enter) break; + if (looped && data->has_ts) { + struct io_uring_getevents_arg *arg = data->arg; + + if (!cqe && arg->ts && !err) + err = -ETIME; + break; + } if (ring->int_flags & INT_FLAG_REG_RING) flags |= IORING_ENTER_REGISTERED_RING; @@ -102,14 +114,18 @@ static int _io_uring_get_cqe(struct io_uring *ring, data->wait_nr, flags, data->arg, data->sz); if (ret < 0) { - err = ret; + if (!err) + err = ret; break; } data->submit -= ret; if (cqe) break; - looped = true; + if (!looped) { + looped = true; + err = ret; + } } while (1); *cqe_ptr = cqe; @@ -233,6 +249,7 @@ static int io_uring_wait_cqes_new(struct io_uring *ring, .wait_nr = wait_nr, .get_flags = IORING_ENTER_EXT_ARG, .sz = sizeof(arg), + .has_ts = ts != NULL, .arg = &arg }; @@ -317,6 +334,7 @@ int io_uring_submit_and_wait_timeout(struct io_uring *ring, .wait_nr = wait_nr, .get_flags = IORING_ENTER_EXT_ARG, .sz = sizeof(arg), + .has_ts = ts != NULL, .arg = &arg }; From 54d785cb806ec8d8f8fecd9837acdaa8c34dcbe2 Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Sat, 22 Oct 2022 10:40:33 -0600 Subject: [PATCH 3/5] Revert "man/io_uring_submit_and_wait_timeout.3: fix return value description" This reverts commit c4c490c5db297c7d5168fea63c97b02716fe6043. Let's make this saner rather than document the odd return of it. Signed-off-by: Jens Axboe --- man/io_uring_submit_and_wait_timeout.3 | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/man/io_uring_submit_and_wait_timeout.3 b/man/io_uring_submit_and_wait_timeout.3 index 1767c098..80fe8891 100644 --- a/man/io_uring_submit_and_wait_timeout.3 +++ b/man/io_uring_submit_and_wait_timeout.3 @@ -38,14 +38,10 @@ After the caller retrieves a submission queue entry (SQE) with and prepares the SQE, it can be submitted with .BR io_uring_submit_and_wait_timeout (3) . -Unlike the other submit functions, this does not return the number of submitted entries. -The actual number submitted can be less than the total number of ready SQEs if there -was an error submitting an SQE. It is not an error to submit less than the total number of SQEs. - .SH RETURN VALUE On success .BR io_uring_submit_and_wait_timeout (3) -returns 0 and the cqe_ptr param is filled in. On failure it returns +returns the number of submitted submission queue entries. On failure it returns .BR -errno . The most common failure case is not receiving a completion within the specified timeout, From 1370f5a0675f30cacb00a09a090e4c42a8d6259a Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Fri, 21 Oct 2022 13:57:15 -0700 Subject: [PATCH 4/5] Add io_uring_submit_and_wait_timeout() test Signed-off-by: Jens Axboe --- test/Makefile | 1 + test/submit-and-wait.c | 108 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 109 insertions(+) create mode 100644 test/submit-and-wait.c diff --git a/test/Makefile b/test/Makefile index 4b74a4a9..e14eb514 100644 --- a/test/Makefile +++ b/test/Makefile @@ -163,6 +163,7 @@ test_srcs := \ sqpoll-sleep.c \ sq-space_left.c \ stdout.c \ + submit-and-wait.c \ submit-link-fail.c \ submit-reuse.c \ sync-cancel.c \ diff --git a/test/submit-and-wait.c b/test/submit-and-wait.c new file mode 100644 index 00000000..70d42d9b --- /dev/null +++ b/test/submit-and-wait.c @@ -0,0 +1,108 @@ +/* SPDX-License-Identifier: MIT */ +/* + * Description: Test that io_uring_submit_and_wait_timeout() returns the + * right value (submit count) and that it doesn't end up waiting twice. + * + */ +#include +#include +#include +#include +#include +#include +#include + +#include "liburing.h" +#include "test.h" + +static unsigned long long mtime_since(const struct timeval *s, + const struct timeval *e) +{ + long long sec, usec; + + sec = e->tv_sec - s->tv_sec; + usec = (e->tv_usec - s->tv_usec); + if (sec > 0 && usec < 0) { + sec--; + usec += 1000000; + } + + sec *= 1000; + usec /= 1000; + return sec + usec; +} + +static unsigned long long mtime_since_now(struct timeval *tv) +{ + struct timeval end; + + gettimeofday(&end, NULL); + return mtime_since(tv, &end); +} + +static int test(struct io_uring *ring) +{ + struct io_uring_cqe *cqe; + struct io_uring_sqe *sqe; + struct __kernel_timespec ts; + struct timeval tv; + int ret, i; + + for (i = 0; i < 1; i++) { + sqe = io_uring_get_sqe(ring); + if (!sqe) { + fprintf(stderr, "get sqe failed at %d\n", i); + goto err; + } + io_uring_prep_nop(sqe); + } + + ts.tv_sec = 1; + ts.tv_nsec = 0; + gettimeofday(&tv, NULL); + ret = io_uring_submit_and_wait_timeout(ring, &cqe, 2, &ts, NULL); + if (ret < 0) { + fprintf(stderr, "submit_and_wait_timeout: %d\n", ret); + goto err; + } + ret = mtime_since_now(&tv); + /* allow some slack, should be around 1s */ + if (ret > 1200) { + fprintf(stderr, "wait took too long: %d\n", ret); + goto err; + } + return 0; +err: + return 1; +} + +static int test_ring(void) +{ + struct io_uring ring; + struct io_uring_params p = { }; + int ret; + + p.flags = 0; + ret = io_uring_queue_init_params(8, &ring, &p); + if (ret) { + fprintf(stderr, "ring setup failed: %d\n", ret); + return 1; + } + + ret = test(&ring); + if (ret) { + fprintf(stderr, "test failed\n"); + goto err; + } +err: + io_uring_queue_exit(&ring); + return ret; +} + +int main(int argc, char *argv[]) +{ + if (argc > 1) + return 0; + + return test_ring(); +} From 4143cafd29a3c609dd2cc09cf944fb3a75edad43 Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Sat, 22 Oct 2022 15:11:47 -0600 Subject: [PATCH 5/5] man: io_uring_submit_and_wait(_timeout) updates Clarify that earlier versions of io_uring_submit_and_wait_timeout() returned 0 on success rather than the number of submitted entries. The man page has always been correct, the implementation just didn't match the description. Clarify we're submitting requests from the SQ ring for both man pages. Signed-off-by: Jens Axboe --- man/io_uring_submit_and_wait.3 | 2 +- man/io_uring_submit_and_wait_timeout.3 | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/man/io_uring_submit_and_wait.3 b/man/io_uring_submit_and_wait.3 index 1c9eb62c..ad4dc8e5 100644 --- a/man/io_uring_submit_and_wait.3 +++ b/man/io_uring_submit_and_wait.3 @@ -16,7 +16,7 @@ io_uring_submit_and_wait \- submit requests to the submission queue and wait for .PP The .BR io_uring_submit_and_wait (3) -function submits the next events to the submission queue belonging to the +function submits the next requests from the submission queue belonging to the .I ring and waits for .I wait_nr diff --git a/man/io_uring_submit_and_wait_timeout.3 b/man/io_uring_submit_and_wait_timeout.3 index 80fe8891..6533cec7 100644 --- a/man/io_uring_submit_and_wait_timeout.3 +++ b/man/io_uring_submit_and_wait_timeout.3 @@ -20,11 +20,11 @@ wait for the completion with timeout .PP The .BR io_uring_submit_and_wait_timeout (3) -function submits the next events to the submission queue belonging to the +function submits the next requests from the submission queue belonging to the .I ring and waits for .I wait_nr -completion events or until the timeout +completion events, or until the timeout .I ts expires. The completion events are stored in the .I cqe_ptr @@ -43,6 +43,8 @@ On success .BR io_uring_submit_and_wait_timeout (3) returns the number of submitted submission queue entries. On failure it returns .BR -errno . +Note that in earlier versions of the liburing library, the return value was 0 +on success. The most common failure case is not receiving a completion within the specified timeout, .B -ETIME