[PATCH 0/4] random io_uring cleanups

[PATCH 0/4] random io_uring cleanups

[Pavel Begunkov]

A small cleanup series partly following up after recent fixes.

Pavel Begunkov (4):
  io_uring: remove FFS_SCM
  io_uring: kill hot path fixed file bitmap debug checks
  io_uring: reuse io_alloc_req()
  io_uring: don't iopoll from io_ring_ctx_wait_and_kill()

 io_uring/filetable.h | 16 ++--------------
 io_uring/io_uring.c  | 24 +++++++-----------------
 io_uring/rsrc.c      |  7 ++-----
 io_uring/rsrc.h      |  4 ----
 4 files changed, 11 insertions(+), 40 deletions(-)

-- 
2.38.0

[PATCH 1/4] io_uring: remove FFS_SCM

[Pavel Begunkov]

scm'ed files lifetime is bound to ring_sock, which is destroyed strictly
after we're done with registered file tables, so no need for the FFS_SCM
hack.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/filetable.h | 15 +--------------
 io_uring/io_uring.c  |  2 --
 io_uring/rsrc.c      |  7 ++-----
 io_uring/rsrc.h      |  4 ----
 4 files changed, 3 insertions(+), 25 deletions(-)

diff --git a/io_uring/filetable.h b/io_uring/filetable.h
index ff3a712e11bf..19d2aed66c72 100644
--- a/io_uring/filetable.h
+++ b/io_uring/filetable.h
@@ -5,22 +5,9 @@
 #include <linux/file.h>
 #include <linux/io_uring_types.h>
 
-/*
- * FFS_SCM is only available on 64-bit archs, for 32-bit we just define it as 0
- * and define IO_URING_SCM_ALL. For this case, we use SCM for all files as we
- * can't safely always dereference the file when the task has exited and ring
- * cleanup is done. If a file is tracked and part of SCM, then unix gc on
- * process exit may reap it before __io_sqe_files_unregister() is run.
- */
 #define FFS_NOWAIT		0x1UL
 #define FFS_ISREG		0x2UL
-#if defined(CONFIG_64BIT)
-#define FFS_SCM			0x4UL
-#else
-#define IO_URING_SCM_ALL
-#define FFS_SCM			0x0UL
-#endif
-#define FFS_MASK		~(FFS_NOWAIT|FFS_ISREG|FFS_SCM)
+#define FFS_MASK		~(FFS_NOWAIT|FFS_ISREG)
 
 bool io_alloc_file_tables(struct io_file_table *table, unsigned nr_files);
 void io_free_file_tables(struct io_file_table *table);
diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index de08d9902b30..18aa39709fae 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -1587,8 +1587,6 @@ unsigned int io_file_get_flags(struct file *file)
 		res |= FFS_ISREG;
 	if (__io_file_supports_nowait(file, mode))
 		res |= FFS_NOWAIT;
-	if (io_file_need_scm(file))
-		res |= FFS_SCM;
 	return res;
 }
 
diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c
index 012fdb04ec23..55d4ab96fb92 100644
--- a/io_uring/rsrc.c
+++ b/io_uring/rsrc.c
@@ -757,20 +757,17 @@ int io_queue_rsrc_removal(struct io_rsrc_data *data, unsigned idx,
 
 void __io_sqe_files_unregister(struct io_ring_ctx *ctx)
 {
-#if !defined(IO_URING_SCM_ALL)
 	int i;
 
 	for (i = 0; i < ctx->nr_user_files; i++) {
 		struct file *file = io_file_from_index(&ctx->file_table, i);
 
-		if (!file)
-			continue;
-		if (io_fixed_file_slot(&ctx->file_table, i)->file_ptr & FFS_SCM)
+		/* skip scm accounted files, they'll be freed by ->ring_sock */
+		if (!file || io_file_need_scm(file))
 			continue;
 		io_file_bitmap_clear(&ctx->file_table, i);
 		fput(file);
 	}
-#endif
 
 #if defined(CONFIG_UNIX)
 	if (ctx->ring_sock) {
diff --git a/io_uring/rsrc.h b/io_uring/rsrc.h
index 9bce15665444..81445a477622 100644
--- a/io_uring/rsrc.h
+++ b/io_uring/rsrc.h
@@ -82,11 +82,7 @@ int __io_scm_file_account(struct io_ring_ctx *ctx, struct file *file);
 #if defined(CONFIG_UNIX)
 static inline bool io_file_need_scm(struct file *filp)
 {
-#if defined(IO_URING_SCM_ALL)
-	return true;
-#else
 	return !!unix_get_socket(filp);
-#endif
 }
 #else
 static inline bool io_file_need_scm(struct file *filp)
-- 
2.38.0

[PATCH 2/4] io_uring: kill hot path fixed file bitmap debug checks

[Pavel Begunkov]

We test file_table.bitmap in io_file_get_fixed() to check invariants,
don't do it, it's expensive and was showing up in profiles.

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

diff --git a/io_uring/filetable.h b/io_uring/filetable.h
index 19d2aed66c72..351111ff8882 100644
--- a/io_uring/filetable.h
+++ b/io_uring/filetable.h
@@ -25,6 +25,7 @@ unsigned int io_file_get_flags(struct file *file);
 
 static inline void io_file_bitmap_clear(struct io_file_table *table, int bit)
 {
+	WARN_ON_ONCE(!test_bit(bit, table->bitmap));
 	__clear_bit(bit, table->bitmap);
 	table->alloc_hint = bit;
 }
diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 18aa39709fae..6e50f548de1a 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -1858,7 +1858,6 @@ inline struct file *io_file_get_fixed(struct io_kiocb *req, int fd,
 	/* mask in overlapping REQ_F and FFS bits */
 	req->flags |= (file_ptr << REQ_F_SUPPORT_NOWAIT_BIT);
 	io_req_set_rsrc_node(req, ctx, 0);
-	WARN_ON_ONCE(file && !test_bit(fd, ctx->file_table.bitmap));
 out:
 	io_ring_submit_unlock(ctx, issue_flags);
 	return file;
-- 
2.38.0

[PATCH 3/4] io_uring: reuse io_alloc_req()

[Pavel Begunkov]

Don't duplicate io_alloc_req() in io_req_caches_free() but reuse the
helper.

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

diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 6e50f548de1a..62be51fbf39c 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -2560,18 +2560,14 @@ static int io_eventfd_unregister(struct io_ring_ctx *ctx)
 
 static void io_req_caches_free(struct io_ring_ctx *ctx)
 {
-	struct io_submit_state *state = &ctx->submit_state;
 	int nr = 0;
 
 	mutex_lock(&ctx->uring_lock);
-	io_flush_cached_locked_reqs(ctx, state);
+	io_flush_cached_locked_reqs(ctx, &ctx->submit_state);
 
 	while (!io_req_cache_empty(ctx)) {
-		struct io_wq_work_node *node;
-		struct io_kiocb *req;
+		struct io_kiocb *req = io_alloc_req(ctx);
 
-		node = wq_stack_extract(&state->free_list);
-		req = container_of(node, struct io_kiocb, comp_list);
 		kmem_cache_free(req_cachep, req);
 		nr++;
 	}
-- 
2.38.0

[PATCH 4/4] io_uring: don't iopoll from io_ring_ctx_wait_and_kill()

[Pavel Begunkov]

We should not be completing requests from a task context that already
undergone io_uring cancellations, i.e. __io_uring_cancel(), as there are
some assumptions, e.g. aroundd cached task refs draining. Remove
iopolling from io_ring_ctx_wait_and_kill() as it can be called later
after PF_EXITING is set with the last task_work run.

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

diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 62be51fbf39c..6cc16e39b27f 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -2804,15 +2804,12 @@ static __cold void io_ring_ctx_wait_and_kill(struct io_ring_ctx *ctx)
 		io_poll_remove_all(ctx, NULL, true);
 	mutex_unlock(&ctx->uring_lock);
 
-	/* failed during ring init, it couldn't have issued any requests */
-	if (ctx->rings) {
+	/*
+	 * If we failed setting up the ctx, we might not have any rings
+	 * and therefore did not submit any requests
+	 */
+	if (ctx->rings)
 		io_kill_timeouts(ctx, NULL, true);
-		/* if we failed setting up the ctx, we might not have any rings */
-		io_iopoll_try_reap_events(ctx);
-		/* drop cached put refs after potentially doing completions */
-		if (current->io_uring)
-			io_uring_drop_tctx_refs(current);
-	}
 
 	INIT_WORK(&ctx->exit_work, io_ring_exit_work);
 	/*
-- 
2.38.0

Re: [PATCH 0/4] random io_uring cleanups

[Jens Axboe]

On Sun, 16 Oct 2022 21:30:47 +0100, Pavel Begunkov wrote:
> A small cleanup series partly following up after recent fixes.
> 
> Pavel Begunkov (4):
>   io_uring: remove FFS_SCM
>   io_uring: kill hot path fixed file bitmap debug checks
>   io_uring: reuse io_alloc_req()
>   io_uring: don't iopoll from io_ring_ctx_wait_and_kill()
> 
> [...]

Applied, thanks!

[1/4] io_uring: remove FFS_SCM
      commit: 38eddb2c75fb99b9cd78445094ca0e1bda08d102
[2/4] io_uring: kill hot path fixed file bitmap debug checks
      commit: 4d5059512d283dab7372d282c2fbd43c7f5a2456
[3/4] io_uring: reuse io_alloc_req()
      commit: 34f0bc427e94065e7f828e70690f8fe1e01b3a9d
[4/4] io_uring: don't iopoll from io_ring_ctx_wait_and_kill()
      commit: 02bac94bd8efd75f615ac7515dd2def75b43e5b9

Best regards,
-- 
Jens Axboe