[PATCH v2] liburing: Add io_uring_submit_and_wait_timeout function in API

[PATCH v2] liburing: Add io_uring_submit_and_wait_timeout function in API

[Olivier Langlois]

before commit 0ea4ccd1c0e4 ("src/queue: don't flush SQ ring for new wait interface"),
io_uring_wait_cqes() was serving the purpose of submit sqe and wait for cqe up to a certain timeout value.

Since the commit, a new function is needed to fill this gap.

Fixes: https://github.com/axboe/liburing/issues/440
Signed-off-by: Olivier Langlois <olivier@trillion01.com>
---
 src/include/liburing.h |  5 +++
 src/liburing.map       |  5 +++
 src/queue.c            | 79 ++++++++++++++++++++++++++++++++----------
 3 files changed, 71 insertions(+), 18 deletions(-)

diff --git a/src/include/liburing.h b/src/include/liburing.h
index 0c2c5c2..fe8bfbe 100644
--- a/src/include/liburing.h
+++ b/src/include/liburing.h
@@ -122,6 +122,11 @@ int io_uring_wait_cqe_timeout(struct io_uring *ring,
 			      struct __kernel_timespec *ts);
 int io_uring_submit(struct io_uring *ring);
 int io_uring_submit_and_wait(struct io_uring *ring, unsigned wait_nr);
+int io_uring_submit_and_wait_timout(struct io_uring *ring,
+				    struct io_uring_cqe **cqe_ptr,
+				    unsigned wait_nr,
+				    struct __kernel_timespec *ts,
+				    sigset_t *sigmask);
 struct io_uring_sqe *io_uring_get_sqe(struct io_uring *ring);
 
 int io_uring_register_buffers(struct io_uring *ring, const struct iovec *iovecs,
diff --git a/src/liburing.map b/src/liburing.map
index 6692a3b..09f4275 100644
--- a/src/liburing.map
+++ b/src/liburing.map
@@ -44,3 +44,8 @@ LIBURING_2.1 {
 		io_uring_unregister_iowq_aff;
 		io_uring_register_iowq_max_workers;
 } LIBURING_2.0;
+
+LIBURING_2.2 {
+	global:
+		io_uring_submit_and_wait_timout;
+} LIBURING_2.1;
diff --git a/src/queue.c b/src/queue.c
index 31aa17c..074ea12 100644
--- a/src/queue.c
+++ b/src/queue.c
@@ -271,36 +271,79 @@ static int io_uring_wait_cqes_new(struct io_uring *ring,
  * hence this function is safe to use for applications that split SQ and CQ
  * handling between two threads.
  */
+
+static int __io_uring_submit_timeout(struct io_uring *ring, unsigned wait_nr,
+				     struct __kernel_timespec *ts)
+{
+	struct io_uring_sqe *sqe;
+	int ret;
+
+	/*
+	 * If the SQ ring is full, we may need to submit IO first
+	 */
+	sqe = io_uring_get_sqe(ring);
+	if (!sqe) {
+		ret = io_uring_submit(ring);
+		if (ret < 0)
+			return ret;
+		sqe = io_uring_get_sqe(ring);
+		if (!sqe)
+			return -EAGAIN;
+	}
+	io_uring_prep_timeout(sqe, ts, wait_nr, 0);
+	sqe->user_data = LIBURING_UDATA_TIMEOUT;
+	return __io_uring_flush_sq(ring);
+}
+
 int io_uring_wait_cqes(struct io_uring *ring, struct io_uring_cqe **cqe_ptr,
 		       unsigned wait_nr, struct __kernel_timespec *ts,
 		       sigset_t *sigmask)
 {
-	unsigned to_submit = 0;
+	int to_submit = 0;
 
 	if (ts) {
-		struct io_uring_sqe *sqe;
-		int ret;
-
 		if (ring->features & IORING_FEAT_EXT_ARG)
 			return io_uring_wait_cqes_new(ring, cqe_ptr, wait_nr,
 							ts, sigmask);
+		to_submit = __io_uring_submit_timeout(ring, wait_nr, ts);
+		if (to_submit < 0)
+			return to_submit;
+	}
 
-		/*
-		 * If the SQ ring is full, we may need to submit IO first
-		 */
-		sqe = io_uring_get_sqe(ring);
-		if (!sqe) {
-			ret = io_uring_submit(ring);
-			if (ret < 0)
-				return ret;
-			sqe = io_uring_get_sqe(ring);
-			if (!sqe)
-				return -EAGAIN;
+	return __io_uring_get_cqe(ring, cqe_ptr, to_submit, wait_nr, sigmask);
+}
+
+int io_uring_submit_and_wait_timout(struct io_uring *ring,
+				    struct io_uring_cqe **cqe_ptr,
+				    unsigned wait_nr,
+				    struct __kernel_timespec *ts,
+				    sigset_t *sigmask)
+{
+	int to_submit;
+
+	if (ts) {
+		if (ring->features & IORING_FEAT_EXT_ARG) {
+			struct io_uring_getevents_arg arg = {
+				.sigmask	= (unsigned long) sigmask,
+				.sigmask_sz	= _NSIG / 8,
+				.ts		= (unsigned long) ts
+			};
+			struct get_data data = {
+				.submit		= __io_uring_flush_sq(ring),
+				.wait_nr	= wait_nr,
+				.get_flags	= IORING_ENTER_EXT_ARG,
+				.sz		= sizeof(arg),
+				.arg		= &arg
+			};
+
+			return _io_uring_get_cqe(ring, cqe_ptr, &data);
 		}
-		io_uring_prep_timeout(sqe, ts, wait_nr, 0);
-		sqe->user_data = LIBURING_UDATA_TIMEOUT;
-		to_submit = __io_uring_flush_sq(ring);
+		to_submit = __io_uring_submit_timeout(ring, wait_nr, ts);
+		if (to_submit < 0)
+			return to_submit;
 	}
+	else
+		to_submit = __io_uring_flush_sq(ring);
 
 	return __io_uring_get_cqe(ring, cqe_ptr, to_submit, wait_nr, sigmask);
 }
-- 
2.33.0

Re: [PATCH v2] liburing: Add io_uring_submit_and_wait_timeout function in API

[Jens Axboe]

On 10/4/21 10:56 AM, Olivier Langlois wrote:
> before commit 0ea4ccd1c0e4 ("src/queue: don't flush SQ ring for new wait interface"),
> io_uring_wait_cqes() was serving the purpose of submit sqe and wait for cqe up to a certain timeout value.

I fixed up this to wrap at 74 chars (standard style) to avoid wrapping
in git log reading, and fixed up:

> +		if (to_submit < 0)
> +			return to_submit;
>  	}
> +	else
> +		to_submit = __io_uring_flush_sq(ring);
>  
>  	return __io_uring_get_cqe(ring, cqe_ptr, to_submit, wait_nr, sigmask);
>  }
> 

this else condition which was still on a separate line. Applied, thanks.

-- 
Jens Axboe

[PATCH] Fix typo "timout" -> "timeout"

[Ammar Faizi]

Cc: Jens Axboe <axboe@kernel.dk>
Cc: Olivier Langlois <olivier@trillion01.com>
Fixes: a060c8e55a6116342a16b5b6ac0c4afed17c1cd7 ("liburing: Add io_uring_submit_and_wait_timeout function in API")
Signed-off-by: Ammar Faizi <ammar.faizi@students.amikom.ac.id>
---

It seems Olivier got rushed a bit when writing this. How did you
test this?

 src/include/liburing.h | 10 +++++-----
 src/liburing.map       |  2 +-
 src/queue.c            | 10 +++++-----
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/src/include/liburing.h b/src/include/liburing.h
index fe8bfbe..99f4f37 100644
--- a/src/include/liburing.h
+++ b/src/include/liburing.h
@@ -122,11 +122,11 @@ int io_uring_wait_cqe_timeout(struct io_uring *ring,
 			      struct __kernel_timespec *ts);
 int io_uring_submit(struct io_uring *ring);
 int io_uring_submit_and_wait(struct io_uring *ring, unsigned wait_nr);
-int io_uring_submit_and_wait_timout(struct io_uring *ring,
-				    struct io_uring_cqe **cqe_ptr,
-				    unsigned wait_nr,
-				    struct __kernel_timespec *ts,
-				    sigset_t *sigmask);
+int io_uring_submit_and_wait_timeout(struct io_uring *ring,
+				     struct io_uring_cqe **cqe_ptr,
+				     unsigned wait_nr,
+				     struct __kernel_timespec *ts,
+				     sigset_t *sigmask);
 struct io_uring_sqe *io_uring_get_sqe(struct io_uring *ring);
 
 int io_uring_register_buffers(struct io_uring *ring, const struct iovec *iovecs,
diff --git a/src/liburing.map b/src/liburing.map
index 09f4275..7f1eeb7 100644
--- a/src/liburing.map
+++ b/src/liburing.map
@@ -47,5 +47,5 @@ LIBURING_2.1 {
 
 LIBURING_2.2 {
 	global:
-		io_uring_submit_and_wait_timout;
+		io_uring_submit_and_wait_timeout;
 } LIBURING_2.1;
diff --git a/src/queue.c b/src/queue.c
index b985056..9af29d5 100644
--- a/src/queue.c
+++ b/src/queue.c
@@ -313,11 +313,11 @@ int io_uring_wait_cqes(struct io_uring *ring, struct io_uring_cqe **cqe_ptr,
 	return __io_uring_get_cqe(ring, cqe_ptr, to_submit, wait_nr, sigmask);
 }
 
-int io_uring_submit_and_wait_timout(struct io_uring *ring,
-				    struct io_uring_cqe **cqe_ptr,
-				    unsigned wait_nr,
-				    struct __kernel_timespec *ts,
-				    sigset_t *sigmask)
+int io_uring_submit_and_wait_timeout(struct io_uring *ring,
+				     struct io_uring_cqe **cqe_ptr,
+				     unsigned wait_nr,
+				     struct __kernel_timespec *ts,
+				     sigset_t *sigmask)
 {
 	int to_submit;
 
-- 
2.30.2

Re: [PATCH] Fix typo "timout" -> "timeout"

[Jens Axboe]

On 10/5/21 4:30 PM, Ammar Faizi wrote:
> Cc: Jens Axboe <axboe@kernel.dk>
> Cc: Olivier Langlois <olivier@trillion01.com>
> Fixes: a060c8e55a6116342a16b5b6ac0c4afed17c1cd7 ("liburing: Add io_uring_submit_and_wait_timeout function in API")
> Signed-off-by: Ammar Faizi <ammar.faizi@students.amikom.ac.id>
> ---
> 
> It seems Olivier got rushed a bit when writing this. How did you
> test this?

Ugh indeed. Olivier, did you test this at all? I missed this when reviewing
it, but I would assume that writing a separate test would have caught it.
Said test should go into liburing as well, fwiw. Can you please submit it?

-- 
Jens Axboe

Re: [PATCH] Fix typo "timout" -> "timeout"

[Olivier Langlois]

On Tue, 2021-10-05 at 16:41 -0600, Jens Axboe wrote:
> On 10/5/21 4:30 PM, Ammar Faizi wrote:
> > Cc: Jens Axboe <axboe@kernel.dk>
> > Cc: Olivier Langlois <olivier@trillion01.com>
> > Fixes: a060c8e55a6116342a16b5b6ac0c4afed17c1cd7 ("liburing: Add
> > io_uring_submit_and_wait_timeout function in API")
> > Signed-off-by: Ammar Faizi <ammar.faizi@students.amikom.ac.id>
> > ---
> > 
> > It seems Olivier got rushed a bit when writing this. How did you
> > test this?
> 
> Ugh indeed. Olivier, did you test this at all? I missed this when
> reviewing
> it, but I would assume that writing a separate test would have caught
> it.
> Said test should go into liburing as well, fwiw. Can you please
> submit it?
> 
Jens, Ammar,

I am very sorry for the typo and yes I was in a rush because I have
been in a dev crunch for the last 2 months. I barely start to
resurface.

That beind said, I have been very careful in my testing.

I did run the liburing timeout unittest to make sure that the patch did
not break io_uring_wait_cqes() and I have tested the new function in my
own application where the problem got detected in the first place.

https://github.com/axboe/liburing/issues/429#issuecomment-917331678

I can assure you that the new function works perfectly well despite the
typo.

The silly typo has totally escaped my attention so thank you Ammar to
have spotted it and fixed it.

I should have some time soon to submit an addition to the timeout
unittest to test the new io_uring_submit_and_wait_timeout function. I
have put this small task on my todo list.

Greetings,