[PATCH liburing 0/3] fix _io_uring_get_cqe()

[PATCH liburing 0/3] fix _io_uring_get_cqe()

[Pavel Begunkov]

address live-locking of _io_uring_get_cqe() reported
by Victor.

Pavel Begunkov (3):
  src/queue: don't wait for less than expected
  src/queue: clean _io_uring_get_cqe() err handling
  src/queue: don't loop when don't enter

 src/queue.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

-- 
2.24.0

[PATCH liburing 1/3] src/queue: don't wait for less than expected

[Pavel Begunkov]

__io_uring_peek_cqe() doesn't consume cqe it returns, no need to
decrease wait_nr because we check against the number in CQ as well as
the kernel do. One exception for that behaviour is IOPOLL, but that
kernel will return if there is anything in CQ, so will work just fine.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 src/queue.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/queue.c b/src/queue.c
index 94f791e..dd1df2a 100644
--- a/src/queue.c
+++ b/src/queue.c
@@ -106,8 +106,6 @@ static int _io_uring_get_cqe(struct io_uring *ring, struct io_uring_cqe **cqe_pt
 			}
 			cq_overflow_flush = true;
 		}
-		if (data->wait_nr && cqe)
-			data->wait_nr--;
 		if (data->wait_nr || cq_overflow_flush)
 			flags = IORING_ENTER_GETEVENTS | data->get_flags;
 		if (data->submit)
-- 
2.24.0

[PATCH liburing 2/3] src/queue: clean _io_uring_get_cqe() err handling

[Pavel Begunkov]

Doesn't change behaviour, just a little cleanup.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 src/queue.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/queue.c b/src/queue.c
index dd1df2a..be461c6 100644
--- a/src/queue.c
+++ b/src/queue.c
@@ -117,8 +117,11 @@ static int _io_uring_get_cqe(struct io_uring *ring, struct io_uring_cqe **cqe_pt
 					data->sz);
 		if (ret < 0) {
 			err = -errno;
-		} else if (ret == (int)data->submit) {
-			data->submit = 0;
+			break;
+		}
+
+		data->submit -= ret;
+		if (ret == (int)data->submit) {
 			/*
 			 * When SETUP_IOPOLL is set, __sys_io_uring enter()
 			 * must be called to reap new completions but the call
@@ -127,12 +130,10 @@ static int _io_uring_get_cqe(struct io_uring *ring, struct io_uring_cqe **cqe_pt
 			 */
 			if (!(ring->flags & IORING_SETUP_IOPOLL))
 				data->wait_nr = 0;
-		} else {
-			data->submit -= ret;
 		}
 		if (cqe)
 			break;
-	} while (!err);
+	} while (1);
 
 	*cqe_ptr = cqe;
 	return err;
-- 
2.24.0

[PATCH liburing 3/3] src/queue: don't loop when don't enter

[Pavel Begunkov]

_io_uring_get_cqe() can live-lock in some cases, always return if we're
not going to do __sys_io_uring_enter().

Reported-by: Victor Stewart <v@nametag.social>
Suggested-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 src/queue.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/src/queue.c b/src/queue.c
index be461c6..8c394dd 100644
--- a/src/queue.c
+++ b/src/queue.c
@@ -89,12 +89,13 @@ static int _io_uring_get_cqe(struct io_uring *ring, struct io_uring_cqe **cqe_pt
 {
 	struct io_uring_cqe *cqe = NULL;
 	const int to_wait = data->wait_nr;
-	int ret = 0, err;
+	int err;
 
 	do {
 		bool cq_overflow_flush = false;
 		unsigned flags = 0;
 		unsigned nr_available;
+		int ret;
 
 		err = __io_uring_peek_cqe(ring, &cqe, &nr_available);
 		if (err)
@@ -110,11 +111,13 @@ static int _io_uring_get_cqe(struct io_uring *ring, struct io_uring_cqe **cqe_pt
 			flags = IORING_ENTER_GETEVENTS | data->get_flags;
 		if (data->submit)
 			sq_ring_needs_enter(ring, &flags);
-		if (data->wait_nr > nr_available || data->submit ||
-		    cq_overflow_flush)
-			ret = __sys_io_uring_enter2(ring->ring_fd, data->submit,
-					data->wait_nr, flags, data->arg,
-					data->sz);
+		if (data->wait_nr <= nr_available && !data->submit &&
+		    !cq_overflow_flush)
+			break;
+
+		ret = __sys_io_uring_enter2(ring->ring_fd, data->submit,
+				data->wait_nr, flags, data->arg,
+				data->sz);
 		if (ret < 0) {
 			err = -errno;
 			break;
-- 
2.24.0

Re: [PATCH liburing 0/3] fix _io_uring_get_cqe()

[Jens Axboe]

On 2/7/21 4:32 PM, Pavel Begunkov wrote:
> address live-locking of _io_uring_get_cqe() reported
> by Victor.
> 
> Pavel Begunkov (3):
>   src/queue: don't wait for less than expected
>   src/queue: clean _io_uring_get_cqe() err handling
>   src/queue: don't loop when don't enter
> 
>  src/queue.c | 28 +++++++++++++++-------------
>  1 file changed, 15 insertions(+), 13 deletions(-)

Nice, thanks Pavel! Applied.

-- 
Jens Axboe