[PATCH for-next 0/5] random 5.20 patches

[PATCH for-next 0/5] random 5.20 patches

[Pavel Begunkov]

Just random patches here and there. The nicest one is 5/5, which removes
ctx->refs pinning from io_uring_enter.

Pavel Begunkov (5):
  io_uring: improve io_fail_links()
  io_uring: fuse fallback_node and normal tw node
  io_uring: remove extra TIF_NOTIFY_SIGNAL check
  io_uring: don't check file ops of registered rings
  io_uring: remove ctx->refs pinning on enter

 include/linux/io_uring_types.h |  5 +----
 io_uring/io_uring.c            | 37 ++++++++++++----------------------
 io_uring/timeout.c             | 36 ++++++++++++++++++++++-----------
 3 files changed, 38 insertions(+), 40 deletions(-)

-- 
2.36.1

[PATCH for-next 1/5] io_uring: improve io_fail_links()

[Pavel Begunkov]

io_fail_links() is called with ->completion_lock held and for that
reason we'd want to keep it as small as we can. Instead of doing
__io_req_complete_post() for each linked request under the lock, fail
them in a task_work handler under ->uring_lock.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/timeout.c | 36 ++++++++++++++++++++++++------------
 1 file changed, 24 insertions(+), 12 deletions(-)

diff --git a/io_uring/timeout.c b/io_uring/timeout.c
index 4af074b8f6b7..2f9e56935479 100644
--- a/io_uring/timeout.c
+++ b/io_uring/timeout.c
@@ -101,32 +101,44 @@ __cold void io_flush_timeouts(struct io_ring_ctx *ctx)
 	spin_unlock_irq(&ctx->timeout_lock);
 }
 
-static void io_fail_links(struct io_kiocb *req)
-	__must_hold(&req->ctx->completion_lock)
+static void io_req_tw_fail_links(struct io_kiocb *link, bool *locked)
 {
-	struct io_kiocb *nxt, *link = req->link;
-	bool ignore_cqes = req->flags & REQ_F_SKIP_LINK_CQES;
-
-	req->link = NULL;
+	io_tw_lock(link->ctx, locked);
 	while (link) {
+		struct io_kiocb *nxt = link->link;
 		long res = -ECANCELED;
 
 		if (link->flags & REQ_F_FAIL)
 			res = link->cqe.res;
-
-		nxt = link->link;
 		link->link = NULL;
+		io_req_set_res(link, res, 0);
+		io_req_task_complete(link, locked);
+		link = nxt;
+	}
+}
 
-		trace_io_uring_fail_link(req, link);
+static void io_fail_links(struct io_kiocb *req)
+	__must_hold(&req->ctx->completion_lock)
+{
+	struct io_kiocb *link = req->link;
+	bool ignore_cqes = req->flags & REQ_F_SKIP_LINK_CQES;
+
+	if (!link)
+		return;
 
+	while (link) {
 		if (ignore_cqes)
 			link->flags |= REQ_F_CQE_SKIP;
 		else
 			link->flags &= ~REQ_F_CQE_SKIP;
-		io_req_set_res(link, res, 0);
-		__io_req_complete_post(link);
-		link = nxt;
+		trace_io_uring_fail_link(req, link);
+		link = link->link;
 	}
+
+	link = req->link;
+	link->io_task_work.func = io_req_tw_fail_links;
+	io_req_task_work_add(link);
+	req->link = NULL;
 }
 
 static inline void io_remove_next_linked(struct io_kiocb *req)
-- 
2.36.1

[PATCH for-next 2/5] io_uring: fuse fallback_node and normal tw node

[Pavel Begunkov]

Now as both normal and fallback paths use llist, just keep one node head
in struct io_task_work and kill off ->fallback_node.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 include/linux/io_uring_types.h | 5 +----
 io_uring/io_uring.c            | 5 ++---
 2 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/include/linux/io_uring_types.h b/include/linux/io_uring_types.h
index 918165a20053..3ca8f363f504 100644
--- a/include/linux/io_uring_types.h
+++ b/include/linux/io_uring_types.h
@@ -427,10 +427,7 @@ enum {
 typedef void (*io_req_tw_func_t)(struct io_kiocb *req, bool *locked);
 
 struct io_task_work {
-	union {
-		struct llist_node	node;
-		struct llist_node	fallback_node;
-	};
+	struct llist_node		node;
 	io_req_tw_func_t		func;
 };
 
diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 45538b3c3a76..86a0b0c6f5bf 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -233,7 +233,7 @@ static __cold void io_fallback_req_func(struct work_struct *work)
 	bool locked = false;
 
 	percpu_ref_get(&ctx->refs);
-	llist_for_each_entry_safe(req, tmp, node, io_task_work.fallback_node)
+	llist_for_each_entry_safe(req, tmp, node, io_task_work.node)
 		req->io_task_work.func(req, &locked);
 
 	if (locked) {
@@ -1091,13 +1091,12 @@ void io_req_task_work_add(struct io_kiocb *req)
 	if (likely(!task_work_add(req->task, &tctx->task_work, ctx->notify_method)))
 		return;
 
-
 	node = llist_del_all(&tctx->task_list);
 
 	while (node) {
 		req = container_of(node, struct io_kiocb, io_task_work.node);
 		node = node->next;
-		if (llist_add(&req->io_task_work.fallback_node,
+		if (llist_add(&req->io_task_work.node,
 			      &req->ctx->fallback_llist))
 			schedule_delayed_work(&req->ctx->fallback_work, 1);
 	}
-- 
2.36.1

[PATCH for-next 3/5] io_uring: remove extra TIF_NOTIFY_SIGNAL check

[Pavel Begunkov]

io_run_task_work() accounts for TIF_NOTIFY_SIGNAL, so no need to have an
second check in io_run_task_work_sig().

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

diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 86a0b0c6f5bf..f40526426db8 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -2205,8 +2205,6 @@ int io_run_task_work_sig(void)
 {
 	if (io_run_task_work())
 		return 1;
-	if (test_thread_flag(TIF_NOTIFY_SIGNAL))
-		return -ERESTARTSYS;
 	if (task_sigpending(current))
 		return -EINTR;
 	return 0;
-- 
2.36.1

[PATCH for-next 4/5] io_uring: don't check file ops of registered rings

[Pavel Begunkov]

Registered rings are per definitions io_uring files, so we don't need to
additionally verify them.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/io_uring.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index f40526426db8..e1e8dcd17df3 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -3036,22 +3036,22 @@ SYSCALL_DEFINE6(io_uring_enter, unsigned int, fd, u32, to_submit,
 	if (flags & IORING_ENTER_REGISTERED_RING) {
 		struct io_uring_task *tctx = current->io_uring;
 
-		if (!tctx || fd >= IO_RINGFD_REG_MAX)
+		if (unlikely(!tctx || fd >= IO_RINGFD_REG_MAX))
 			return -EINVAL;
 		fd = array_index_nospec(fd, IO_RINGFD_REG_MAX);
 		f.file = tctx->registered_rings[fd];
 		f.flags = 0;
+		if (unlikely(!f.file))
+			return -EBADF;
 	} else {
 		f = fdget(fd);
+		if (unlikely(!f.file))
+			return -EBADF;
+		ret = -EOPNOTSUPP;
+		if (unlikely(!io_is_uring_fops(f.file)))
+			goto out_fput;
 	}
 
-	if (unlikely(!f.file))
-		return -EBADF;
-
-	ret = -EOPNOTSUPP;
-	if (unlikely(!io_is_uring_fops(f.file)))
-		goto out_fput;
-
 	ret = -ENXIO;
 	ctx = f.file->private_data;
 	if (unlikely(!percpu_ref_tryget(&ctx->refs)))
-- 
2.36.1

[PATCH for-next 5/5] io_uring: remove ctx->refs pinning on enter

[Pavel Begunkov]

io_uring_enter() takes ctx->refs, which was previously preventing racing
with register quiesce. However, as register now doesn't touch the refs,
we can freely kill extra ctx pinning and rely on the fact that we're
holding a file reference preventing the ring from being destroyed.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/io_uring.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index e1e8dcd17df3..070ee9ec9ee7 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -3049,14 +3049,10 @@ SYSCALL_DEFINE6(io_uring_enter, unsigned int, fd, u32, to_submit,
 			return -EBADF;
 		ret = -EOPNOTSUPP;
 		if (unlikely(!io_is_uring_fops(f.file)))
-			goto out_fput;
+			goto out;
 	}
 
-	ret = -ENXIO;
 	ctx = f.file->private_data;
-	if (unlikely(!percpu_ref_tryget(&ctx->refs)))
-		goto out_fput;
-
 	ret = -EBADFD;
 	if (unlikely(ctx->flags & IORING_SETUP_R_DISABLED))
 		goto out;
@@ -3141,10 +3137,7 @@ SYSCALL_DEFINE6(io_uring_enter, unsigned int, fd, u32, to_submit,
 					  &ctx->check_cq);
 		}
 	}
-
 out:
-	percpu_ref_put(&ctx->refs);
-out_fput:
 	fdput(f);
 	return ret;
 }
@@ -3730,11 +3723,10 @@ static int __io_uring_register(struct io_ring_ctx *ctx, unsigned opcode,
 	int ret;
 
 	/*
-	 * We're inside the ring mutex, if the ref is already dying, then
-	 * someone else killed the ctx or is already going through
-	 * io_uring_register().
+	 * We don't quiesce the refs for register anymore and so it can't be
+	 * dying as we're holding a file ref here.
 	 */
-	if (percpu_ref_is_dying(&ctx->refs))
+	if (WARN_ON_ONCE(percpu_ref_is_dying(&ctx->refs)))
 		return -ENXIO;
 
 	if (ctx->restricted) {
-- 
2.36.1

Re: [PATCH for-next 0/5] random 5.20 patches

[Jens Axboe]

On Sat, 25 Jun 2022 11:52:57 +0100, Pavel Begunkov wrote:
> Just random patches here and there. The nicest one is 5/5, which removes
> ctx->refs pinning from io_uring_enter.
> 
> Pavel Begunkov (5):
>   io_uring: improve io_fail_links()
>   io_uring: fuse fallback_node and normal tw node
>   io_uring: remove extra TIF_NOTIFY_SIGNAL check
>   io_uring: don't check file ops of registered rings
>   io_uring: remove ctx->refs pinning on enter
> 
> [...]

Applied, thanks!

[1/5] io_uring: improve io_fail_links()
      commit: 149e51e72cc0d87b7eb452e928b29a906501981d
[2/5] io_uring: fuse fallback_node and normal tw node
      commit: aacc96447edf1ea1f057fb5dd3c53ee495e21487
[3/5] io_uring: remove extra TIF_NOTIFY_SIGNAL check
      commit: 8b5e7937ac521ff33c2bac66e3c1a0385ad7087e
[4/5] io_uring: don't check file ops of registered rings
      commit: 9c59698445f94fbdf208b3f50286e4fbfd295571
[5/5] io_uring: remove ctx->refs pinning on enter
      commit: e8584adba8863d531cb25233a650fdb9b50c2e2b

Best regards,
-- 
Jens Axboe

Re: [PATCH for-next 0/5] random 5.20 patches

[Jens Axboe]

On 6/25/22 4:52 AM, Pavel Begunkov wrote:
> Just random patches here and there. The nicest one is 5/5, which removes
> ctx->refs pinning from io_uring_enter.

Nice!

-- 
Jens Axboe