public inbox for [email protected]
 help / color / mirror / Atom feed
* [PATCH v2] liburing: Add io_uring_submit_and_wait_timeout function in API
@ 2021-10-04 16:56 Olivier Langlois
  2021-10-04 23:32 ` Jens Axboe
  0 siblings, 1 reply; 5+ messages in thread
From: Olivier Langlois @ 2021-10-04 16:56 UTC (permalink / raw)
  To: Jens Axboe, Pavel Begunkov, io-uring

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 <[email protected]>
---
 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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] liburing: Add io_uring_submit_and_wait_timeout function in API
  2021-10-04 16:56 [PATCH v2] liburing: Add io_uring_submit_and_wait_timeout function in API Olivier Langlois
@ 2021-10-04 23:32 ` Jens Axboe
  2021-10-05 22:30   ` [PATCH] Fix typo "timout" -> "timeout" Ammar Faizi
  0 siblings, 1 reply; 5+ messages in thread
From: Jens Axboe @ 2021-10-04 23:32 UTC (permalink / raw)
  To: Olivier Langlois, Pavel Begunkov, io-uring

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] Fix typo "timout" -> "timeout"
  2021-10-04 23:32 ` Jens Axboe
@ 2021-10-05 22:30   ` Ammar Faizi
  2021-10-05 22:41     ` Jens Axboe
  0 siblings, 1 reply; 5+ messages in thread
From: Ammar Faizi @ 2021-10-05 22:30 UTC (permalink / raw)
  To: Jens Axboe, Pavel Begunkov, io-uring; +Cc: Olivier Langlois, Ammar Faizi

Cc: Jens Axboe <[email protected]>
Cc: Olivier Langlois <[email protected]>
Fixes: a060c8e55a6116342a16b5b6ac0c4afed17c1cd7 ("liburing: Add io_uring_submit_and_wait_timeout function in API")
Signed-off-by: Ammar Faizi <[email protected]>
---

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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] Fix typo "timout" -> "timeout"
  2021-10-05 22:30   ` [PATCH] Fix typo "timout" -> "timeout" Ammar Faizi
@ 2021-10-05 22:41     ` Jens Axboe
  2021-12-06 19:45       ` Olivier Langlois
  0 siblings, 1 reply; 5+ messages in thread
From: Jens Axboe @ 2021-10-05 22:41 UTC (permalink / raw)
  To: Ammar Faizi, Pavel Begunkov, io-uring; +Cc: Olivier Langlois

On 10/5/21 4:30 PM, Ammar Faizi wrote:
> Cc: Jens Axboe <[email protected]>
> Cc: Olivier Langlois <[email protected]>
> Fixes: a060c8e55a6116342a16b5b6ac0c4afed17c1cd7 ("liburing: Add io_uring_submit_and_wait_timeout function in API")
> Signed-off-by: Ammar Faizi <[email protected]>
> ---
> 
> 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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Fix typo "timout" -> "timeout"
  2021-10-05 22:41     ` Jens Axboe
@ 2021-12-06 19:45       ` Olivier Langlois
  0 siblings, 0 replies; 5+ messages in thread
From: Olivier Langlois @ 2021-12-06 19:45 UTC (permalink / raw)
  To: Jens Axboe, Ammar Faizi, Pavel Begunkov, io-uring

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 <[email protected]>
> > Cc: Olivier Langlois <[email protected]>
> > Fixes: a060c8e55a6116342a16b5b6ac0c4afed17c1cd7 ("liburing: Add
> > io_uring_submit_and_wait_timeout function in API")
> > Signed-off-by: Ammar Faizi <[email protected]>
> > ---
> > 
> > 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,


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-12-06 20:24 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-10-04 16:56 [PATCH v2] liburing: Add io_uring_submit_and_wait_timeout function in API Olivier Langlois
2021-10-04 23:32 ` Jens Axboe
2021-10-05 22:30   ` [PATCH] Fix typo "timout" -> "timeout" Ammar Faizi
2021-10-05 22:41     ` Jens Axboe
2021-12-06 19:45       ` Olivier Langlois

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox