[PATCH 0/6] random fixes and patches for io_uring

[PATCH 0/6] random fixes and patches for io_uring

[Pavel Begunkov]

Patch 1 improves task exit cancellation. The problem is a mind
experiment, I haven't seen it anywhere, and should be rare as
it involves io_uring polling another io_uring.

Patch 2 fails netpolling with IOPOLL, as it's not supported

The rest is random cleanups.

Pavel Begunkov (6):
  io_uring: tighten task exit cancellations
  io_uring: don't allow netpolling with SETUP_IOPOLL
  io_uring: fix io_match_task must_hold
  io_uring: simplify io_uring_cmd return
  io_uring: kill REQ_F_CANCEL_SEQ
  io_uring: align iowq and task request error handling

 include/linux/io_uring_types.h | 3 ---
 io_uring/io_uring.c            | 7 +++++--
 io_uring/napi.c                | 2 ++
 io_uring/timeout.c             | 2 +-
 io_uring/uring_cmd.c           | 2 +-
 5 files changed, 9 insertions(+), 7 deletions(-)

-- 
2.44.0

[PATCH 1/6] io_uring: tighten task exit cancellations

[Pavel Begunkov]

io_uring_cancel_generic() should retry if any state changes like a
request is completed, however in case of a task exit it only goes for
another loop and avoids schedule() if any tracked (i.e. REQ_F_INFLIGHT)
request got completed.

Let's assume we have a non-tracked request executing in iowq and a
tracked request linked to it. Let's also assume
io_uring_cancel_generic() fails to find and cancel the request, i.e.
via io_run_local_work(), which may happen as io-wq has gaps.
Next, the request logically completes, io-wq still hold a ref but queues
it for completion via tw, which happens in
io_uring_try_cancel_requests(). After, right before prepare_to_wait()
io-wq puts the request, grabs the linked one and tries executes it, e.g.
arms polling. Finally the cancellation loop calls prepare_to_wait(),
there are no tw to run, no tracked request was completed, so the
tctx_inflight() check passes and the task is put to indefinite sleep.

Cc: stable@vger.kernel.org
Fixes: 3f48cf18f886c ("io_uring: unify files and task cancel")
Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/io_uring.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 8e6faa942a6f..10c409e56241 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -3031,8 +3031,11 @@ __cold void io_uring_cancel_generic(bool cancel_all, struct io_sq_data *sqd)
 		bool loop = false;
 
 		io_uring_drop_tctx_refs(current);
+		if (!tctx_inflight(tctx, !cancel_all))
+			break;
+
 		/* read completions before cancelations */
-		inflight = tctx_inflight(tctx, !cancel_all);
+		inflight = tctx_inflight(tctx, false);
 		if (!inflight)
 			break;
 
-- 
2.44.0

[PATCH 2/6] io_uring: don't allow netpolling with SETUP_IOPOLL

[Pavel Begunkov]

IORING_SETUP_IOPOLL rings don't have any netpoll handling, let's fail
attempts to register netpolling in this case, there might be people who
will mix up IOPOLL and netpoll.

Cc: stable@vger.kernel.org
Fixes: ef1186c1a875b ("io_uring: add register/unregister napi function")
Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/napi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/io_uring/napi.c b/io_uring/napi.c
index 762254a7ff3f..327e5f3a8abe 100644
--- a/io_uring/napi.c
+++ b/io_uring/napi.c
@@ -222,6 +222,8 @@ int io_register_napi(struct io_ring_ctx *ctx, void __user *arg)
 	};
 	struct io_uring_napi napi;
 
+	if (ctx->flags & IORING_SETUP_IOPOLL)
+		return -EINVAL;
 	if (copy_from_user(&napi, arg, sizeof(napi)))
 		return -EFAULT;
 	if (napi.pad[0] || napi.pad[1] || napi.pad[2] || napi.resv)
-- 
2.44.0

[PATCH 3/6] io_uring: fix io_match_task must_hold

[Pavel Begunkov]

The __must_hold annotation in io_match_task() uses a non existing
parameter "req", fix it.

Fixes: 6af3f48bf6156 ("io_uring: fix link traversal locking")
Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/timeout.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/io_uring/timeout.c b/io_uring/timeout.c
index 1c9bf07499b1..9973876d91b0 100644
--- a/io_uring/timeout.c
+++ b/io_uring/timeout.c
@@ -639,7 +639,7 @@ void io_queue_linked_timeout(struct io_kiocb *req)
 
 static bool io_match_task(struct io_kiocb *head, struct task_struct *task,
 			  bool cancel_all)
-	__must_hold(&req->ctx->timeout_lock)
+	__must_hold(&head->ctx->timeout_lock)
 {
 	struct io_kiocb *req;
 
-- 
2.44.0

[PATCH 4/6] io_uring: simplify io_uring_cmd return

[Pavel Begunkov]

We don't have to return error code from an op handler back to core
io_uring, so once io_uring_cmd() sets the results and handles errors we
can juts return IOU_OK and simplify the code.

Note, only valid with e0b23d9953b0c ("io_uring: optimise ltimeout for
inline execution"), there was a problem with iopoll before.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/uring_cmd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c
index a54163a83968..8391c7c7c1ec 100644
--- a/io_uring/uring_cmd.c
+++ b/io_uring/uring_cmd.c
@@ -265,7 +265,7 @@ int io_uring_cmd(struct io_kiocb *req, unsigned int issue_flags)
 		req_set_fail(req);
 	io_req_uring_cleanup(req, issue_flags);
 	io_req_set_res(req, ret, 0);
-	return ret < 0 ? ret : IOU_OK;
+	return IOU_OK;
 }
 
 int io_uring_cmd_import_fixed(u64 ubuf, unsigned long len, int rw,
-- 
2.44.0

[PATCH 5/6] io_uring: kill REQ_F_CANCEL_SEQ

[Pavel Begunkov]

We removed the reliance on the flag by the cancellation path and now
it's unused.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 include/linux/io_uring_types.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/include/linux/io_uring_types.h b/include/linux/io_uring_types.h
index 3bb6198d1523..e62aa9f0629f 100644
--- a/include/linux/io_uring_types.h
+++ b/include/linux/io_uring_types.h
@@ -461,7 +461,6 @@ enum {
 	REQ_F_SUPPORT_NOWAIT_BIT,
 	REQ_F_ISREG_BIT,
 	REQ_F_POLL_NO_LAZY_BIT,
-	REQ_F_CANCEL_SEQ_BIT,
 	REQ_F_CAN_POLL_BIT,
 	REQ_F_BL_EMPTY_BIT,
 	REQ_F_BL_NO_RECYCLE_BIT,
@@ -536,8 +535,6 @@ enum {
 	REQ_F_HASH_LOCKED	= IO_REQ_FLAG(REQ_F_HASH_LOCKED_BIT),
 	/* don't use lazy poll wake for this request */
 	REQ_F_POLL_NO_LAZY	= IO_REQ_FLAG(REQ_F_POLL_NO_LAZY_BIT),
-	/* cancel sequence is set and valid */
-	REQ_F_CANCEL_SEQ	= IO_REQ_FLAG(REQ_F_CANCEL_SEQ_BIT),
 	/* file is pollable */
 	REQ_F_CAN_POLL		= IO_REQ_FLAG(REQ_F_CAN_POLL_BIT),
 	/* buffer list was empty after selection of buffer */
-- 
2.44.0

[PATCH 6/6] io_uring: align iowq and task request error handling

[Pavel Begunkov]

There is a difference in how io_queue_sqe and io_wq_submit_work treat
error codes they get from io_issue_sqe. The first one fails anything
unknown but latter only fails when the code is negative.

It doesn't make sense to have this discrepancy, align them to the
io_queue_sqe behaviour.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/io_uring.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 10c409e56241..2626424f5d73 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -1849,7 +1849,7 @@ void io_wq_submit_work(struct io_wq_work *work)
 	} while (1);
 
 	/* avoid locking problems by failing it from a clean context */
-	if (ret < 0)
+	if (ret)
 		io_req_task_queue_fail(req, ret);
 }
 
-- 
2.44.0

Re: [PATCH 0/6] random fixes and patches for io_uring

[Jens Axboe]

On Wed, 24 Jul 2024 12:16:15 +0100, Pavel Begunkov wrote:
> Patch 1 improves task exit cancellation. The problem is a mind
> experiment, I haven't seen it anywhere, and should be rare as
> it involves io_uring polling another io_uring.
> 
> Patch 2 fails netpolling with IOPOLL, as it's not supported
> 
> The rest is random cleanups.
> 
> [...]

Applied, thanks!

[1/6] io_uring: tighten task exit cancellations
      (no commit info)
[2/6] io_uring: don't allow netpolling with SETUP_IOPOLL
      (no commit info)
[3/6] io_uring: fix io_match_task must_hold
      (no commit info)
[4/6] io_uring: simplify io_uring_cmd return
      (no commit info)
[5/6] io_uring: kill REQ_F_CANCEL_SEQ
      (no commit info)
[6/6] io_uring: align iowq and task request error handling
      (no commit info)

Best regards,
-- 
Jens Axboe