Re: [PATCH v3 0/4] fast poll multishot mode

[Jens Axboe <axboe@kernel.dk>]

[-- Attachment #1: Type: text/plain, Size: 917 bytes --]

On 5/7/22 10:05 AM, Hao Xu wrote:
>> But we still need to consider direct accept with multishot... Should
>> probably be an add-on patch as I think it'd get a bit more complicated
>> if we need to be able to cheaply find an available free fixed fd slot.
>> I'll try and play with that.
> 
> I'm tending to use a new mail account to send v4 rather than the gmail
> account since the git issue seems to be network related.
> I'll also think about the fixed fd problem.

Two basic attached patches that attempt do just alloc a fixed file
descriptor for this case. Not tested at all... We return the fixed file
slot in this case since we have to, to let the application know what was
picked. I kind of wish we'd done that with direct open/accept to begin
with anyway, a bit annoying that fixed vs normal open/accept behave
differently.

Anyway, something to play with, and I'm sure it can be made better.

-- 
Jens Axboe

[-- Attachment #2: 0002-io_uring-have-multishot-accept-alloc-new-fixed-file-.patch --]
[-- Type: text/x-patch, Size: 2969 bytes --]

From 66e85e379d27394398a11d02bd01abf449d7bde3 Mon Sep 17 00:00:00 2001
From: Jens Axboe <axboe@kernel.dk>
Date: Sat, 7 May 2022 10:08:31 -0600
Subject: [PATCH 2/2] io_uring: have multishot accept alloc new fixed file slot

Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 fs/io_uring.c | 34 ++++++++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 13b99040ae90..88b95b828bed 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -258,6 +258,7 @@ struct io_rsrc_put {
 struct io_file_table {
 	struct io_fixed_file *files;
 	unsigned long *bitmap;
+	unsigned int alloc_hint;
 };
 
 struct io_rsrc_node {
@@ -5721,8 +5722,7 @@ static int io_accept_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 		return -EINVAL;
 
 	accept->file_slot = READ_ONCE(sqe->file_index);
-	if (accept->file_slot && ((accept->flags & SOCK_CLOEXEC) ||
-	   flags & IORING_ACCEPT_MULTISHOT))
+	if (accept->file_slot && (accept->flags & SOCK_CLOEXEC))
 		return -EINVAL;
 	if (accept->flags & ~(SOCK_CLOEXEC | SOCK_NONBLOCK))
 		return -EINVAL;
@@ -5750,6 +5750,18 @@ static inline void io_poll_clean(struct io_kiocb *req)
 		__io_poll_clean(req);
 }
 
+static int io_file_bitmap_get(struct io_ring_ctx *ctx)
+{
+	struct io_file_table *table = &ctx->file_table;
+	int ret;
+
+	ret = find_next_zero_bit(table->bitmap, ctx->nr_user_files,
+				 table->alloc_hint);
+	if (unlikely(ret == ctx->nr_user_files))
+		return -ENFILE;
+	return ret;
+}
+
 static int io_accept(struct io_kiocb *req, unsigned int issue_flags)
 {
 	struct io_ring_ctx *ctx = req->ctx;
@@ -5787,8 +5799,19 @@ static int io_accept(struct io_kiocb *req, unsigned int issue_flags)
 		fd_install(fd, file);
 		ret = fd;
 	} else {
-		ret = io_install_fixed_file(req, file, issue_flags,
-					    accept->file_slot - 1);
+		int fixed_slot = accept->file_slot - 1;
+
+		if (req->flags & REQ_F_APOLL_MULTISHOT && accept->file_slot) {
+			fixed_slot = io_file_bitmap_get(ctx);
+			if (unlikely(fixed_slot < 0)) {
+				ret = fixed_slot;
+				goto err;
+			}
+		}
+
+		ret = io_install_fixed_file(req, file, issue_flags, fixed_slot);
+		if (!ret && req->flags & REQ_F_APOLL_MULTISHOT)
+			ret = fixed_slot;
 	}
 
 	if (!(req->flags & REQ_F_APOLL_MULTISHOT)) {
@@ -5815,6 +5838,7 @@ static int io_accept(struct io_kiocb *req, unsigned int issue_flags)
 	 * since the upper layer who called io_queue_sqe() cannot get errors
 	 * happened here.
 	 */
+err:
 	io_poll_clean(req);
 	return ret;
 }
@@ -8736,11 +8760,13 @@ static void io_free_file_tables(struct io_file_table *table)
 static inline void io_file_bitmap_set(struct io_file_table *table, int bit)
 {
 	__set_bit(bit, table->bitmap);
+	table->alloc_hint = bit + 1;
 }
 
 static inline void io_file_bitmap_clear(struct io_file_table *table, int bit)
 {
 	__clear_bit(bit, table->bitmap);
+	table->alloc_hint = bit;
 }
 
 static void __io_sqe_files_unregister(struct io_ring_ctx *ctx)
-- 
2.35.1


[-- Attachment #3: 0001-io_uring-track-fixed-files-with-a-bitmap.patch --]
[-- Type: text/x-patch, Size: 3665 bytes --]

From f4801f95c01e744bd336d64ccfa5efd29a2869e2 Mon Sep 17 00:00:00 2001
From: Jens Axboe <axboe@kernel.dk>
Date: Sat, 7 May 2022 09:56:13 -0600
Subject: [PATCH 1/2] io_uring: track fixed files with a bitmap

Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 fs/io_uring.c | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 358c195e2d99..13b99040ae90 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -257,6 +257,7 @@ struct io_rsrc_put {
 
 struct io_file_table {
 	struct io_fixed_file *files;
+	unsigned long *bitmap;
 };
 
 struct io_rsrc_node {
@@ -7645,6 +7646,7 @@ static 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(!test_bit(fd, ctx->file_table.bitmap));
 out:
 	io_ring_submit_unlock(ctx, issue_flags);
 	return file;
@@ -8711,13 +8713,34 @@ static bool io_alloc_file_tables(struct io_file_table *table, unsigned nr_files)
 {
 	table->files = kvcalloc(nr_files, sizeof(table->files[0]),
 				GFP_KERNEL_ACCOUNT);
-	return !!table->files;
+	if (unlikely(!table->files))
+		return false;
+
+	table->bitmap = bitmap_zalloc(nr_files, GFP_KERNEL_ACCOUNT);
+	if (unlikely(!table->bitmap)) {
+		kvfree(table->files);
+		return false;
+	}
+
+	return true;
 }
 
 static void io_free_file_tables(struct io_file_table *table)
 {
 	kvfree(table->files);
+	bitmap_free(table->bitmap);
 	table->files = NULL;
+	table->bitmap = NULL;
+}
+
+static inline void io_file_bitmap_set(struct io_file_table *table, int bit)
+{
+	__set_bit(bit, table->bitmap);
+}
+
+static inline void io_file_bitmap_clear(struct io_file_table *table, int bit)
+{
+	__clear_bit(bit, table->bitmap);
 }
 
 static void __io_sqe_files_unregister(struct io_ring_ctx *ctx)
@@ -8732,6 +8755,7 @@ static void __io_sqe_files_unregister(struct io_ring_ctx *ctx)
 			continue;
 		if (io_fixed_file_slot(&ctx->file_table, i)->file_ptr & FFS_SCM)
 			continue;
+		io_file_bitmap_clear(&ctx->file_table, i);
 		fput(file);
 	}
 #endif
@@ -9135,6 +9159,7 @@ static int io_sqe_files_register(struct io_ring_ctx *ctx, void __user *arg,
 		}
 		file_slot = io_fixed_file_slot(&ctx->file_table, i);
 		io_fixed_file_set(file_slot, file);
+		io_file_bitmap_set(&ctx->file_table, i);
 	}
 
 	io_rsrc_node_switch(ctx, NULL);
@@ -9195,6 +9220,7 @@ static int io_install_fixed_file(struct io_kiocb *req, struct file *file,
 		if (ret)
 			goto err;
 		file_slot->file_ptr = 0;
+		io_file_bitmap_clear(&ctx->file_table, slot_index);
 		needs_switch = true;
 	}
 
@@ -9202,6 +9228,7 @@ static int io_install_fixed_file(struct io_kiocb *req, struct file *file,
 	if (!ret) {
 		*io_get_tag_slot(ctx->file_data, slot_index) = 0;
 		io_fixed_file_set(file_slot, file);
+		io_file_bitmap_set(&ctx->file_table, slot_index);
 	}
 err:
 	if (needs_switch)
@@ -9243,6 +9270,7 @@ static int io_close_fixed(struct io_kiocb *req, unsigned int issue_flags)
 		goto out;
 
 	file_slot->file_ptr = 0;
+	io_file_bitmap_clear(&ctx->file_table, offset);
 	io_rsrc_node_switch(ctx, ctx->file_data);
 	ret = 0;
 out:
@@ -9292,6 +9320,7 @@ static int __io_sqe_files_update(struct io_ring_ctx *ctx,
 			if (err)
 				break;
 			file_slot->file_ptr = 0;
+			io_file_bitmap_clear(&ctx->file_table, i);
 			needs_switch = true;
 		}
 		if (fd != -1) {
@@ -9320,6 +9349,7 @@ static int __io_sqe_files_update(struct io_ring_ctx *ctx,
 			}
 			*io_get_tag_slot(data, i) = tag;
 			io_fixed_file_set(file_slot, file);
+			io_file_bitmap_set(&ctx->file_table, i);
 		}
 	}
 
-- 
2.35.1