public inbox for [email protected]
 help / color / mirror / Atom feed
* [PATCH 5.15] io_uring: auto-removal for direct open/accept
@ 2021-09-14 13:37 Pavel Begunkov
  2021-09-14 13:59 ` Pavel Begunkov
  2021-09-14 14:02 ` Jens Axboe
  0 siblings, 2 replies; 5+ messages in thread
From: Pavel Begunkov @ 2021-09-14 13:37 UTC (permalink / raw)
  To: Jens Axboe, io-uring

It might be inconvenient that direct open/accept deviates from the
update semantics and fails if the slot is taken instead of removing a
file sitting there. Implement the auto-removal.

Note that removal might need to allocate and so may fail. However, if an
empty slot is specified, it's guaraneed to not fail on the fd
installation side. It's needed for users that can't tolerate spuriously
closed files, e.g. accepts where the other end doesn't expect it.

Signed-off-by: Pavel Begunkov <[email protected]>
---
 fs/io_uring.c | 59 +++++++++++++++++++++++++++++++++++----------------
 1 file changed, 41 insertions(+), 18 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index a864a94364c6..29bca3a1ddeb 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -7672,6 +7672,7 @@ static void io_rsrc_node_switch(struct io_ring_ctx *ctx,
 	}
 }
 
+/* before changing see io_install_fixed_file(), it might ignore errors */
 static int io_rsrc_node_switch_start(struct io_ring_ctx *ctx)
 {
 	if (ctx->rsrc_backup_node)
@@ -8287,11 +8288,27 @@ static int io_sqe_file_register(struct io_ring_ctx *ctx, struct file *file,
 #endif
 }
 
+static int io_queue_rsrc_removal(struct io_rsrc_data *data, unsigned idx,
+				 struct io_rsrc_node *node, void *rsrc)
+{
+	struct io_rsrc_put *prsrc;
+
+	prsrc = kzalloc(sizeof(*prsrc), GFP_KERNEL);
+	if (!prsrc)
+		return -ENOMEM;
+
+	prsrc->tag = *io_get_tag_slot(data, idx);
+	prsrc->rsrc = rsrc;
+	list_add(&prsrc->list, &node->rsrc_list);
+	return 0;
+}
+
 static int io_install_fixed_file(struct io_kiocb *req, struct file *file,
 				 unsigned int issue_flags, u32 slot_index)
 {
 	struct io_ring_ctx *ctx = req->ctx;
 	bool force_nonblock = issue_flags & IO_URING_F_NONBLOCK;
+	bool needs_switch = false;
 	struct io_fixed_file *file_slot;
 	int ret = -EBADF;
 
@@ -8304,12 +8321,31 @@ static int io_install_fixed_file(struct io_kiocb *req, struct file *file,
 	ret = -EINVAL;
 	if (slot_index >= ctx->nr_user_files)
 		goto err;
+	/*
+	 * Ignore error, ->rsrc_backup_node is not needed if the slot is empty,
+	 * and we'd rather not drop the file.
+	 */
+	io_rsrc_node_switch_start(ctx);
 
 	slot_index = array_index_nospec(slot_index, ctx->nr_user_files);
 	file_slot = io_fixed_file_slot(&ctx->file_table, slot_index);
-	ret = -EBADF;
-	if (file_slot->file_ptr)
-		goto err;
+
+	if (file_slot->file_ptr) {
+		struct file *old_file;
+
+		if (!ctx->rsrc_backup_node) {
+			ret = -ENOMEM;
+			goto err;
+		}
+
+		old_file = (struct file *)(file_slot->file_ptr & FFS_MASK);
+		ret = io_queue_rsrc_removal(ctx->file_data, slot_index,
+					    ctx->rsrc_node, old_file);
+		if (ret)
+			goto err;
+		file_slot->file_ptr = 0;
+		needs_switch = true;
+	}
 
 	*io_get_tag_slot(ctx->file_data, slot_index) = 0;
 	io_fixed_file_set(file_slot, file);
@@ -8321,27 +8357,14 @@ static int io_install_fixed_file(struct io_kiocb *req, struct file *file,
 
 	ret = 0;
 err:
+	if (needs_switch)
+		io_rsrc_node_switch(ctx, ctx->file_data);
 	io_ring_submit_unlock(ctx, !force_nonblock);
 	if (ret)
 		fput(file);
 	return ret;
 }
 
-static int io_queue_rsrc_removal(struct io_rsrc_data *data, unsigned idx,
-				 struct io_rsrc_node *node, void *rsrc)
-{
-	struct io_rsrc_put *prsrc;
-
-	prsrc = kzalloc(sizeof(*prsrc), GFP_KERNEL);
-	if (!prsrc)
-		return -ENOMEM;
-
-	prsrc->tag = *io_get_tag_slot(data, idx);
-	prsrc->rsrc = rsrc;
-	list_add(&prsrc->list, &node->rsrc_list);
-	return 0;
-}
-
 static int __io_sqe_files_update(struct io_ring_ctx *ctx,
 				 struct io_uring_rsrc_update2 *up,
 				 unsigned nr_args)
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 5.15] io_uring: auto-removal for direct open/accept
  2021-09-14 13:37 [PATCH 5.15] io_uring: auto-removal for direct open/accept Pavel Begunkov
@ 2021-09-14 13:59 ` Pavel Begunkov
  2021-09-14 14:02 ` Jens Axboe
  1 sibling, 0 replies; 5+ messages in thread
From: Pavel Begunkov @ 2021-09-14 13:59 UTC (permalink / raw)
  To: Jens Axboe, io-uring

On 9/14/21 2:37 PM, Pavel Begunkov wrote:
> It might be inconvenient that direct open/accept deviates from the
> update semantics and fails if the slot is taken instead of removing a
> file sitting there. Implement the auto-removal.
> 
> Note that removal might need to allocate and so may fail. However, if an
> empty slot is specified, it's guaraneed to not fail on the fd
> installation side. It's needed for users that can't tolerate spuriously
> closed files, e.g. accepts where the other end doesn't expect it.
> 
> Signed-off-by: Pavel Begunkov <[email protected]>
> ---
>  fs/io_uring.c | 59 +++++++++++++++++++++++++++++++++++----------------
>  1 file changed, 41 insertions(+), 18 deletions(-)
> 
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index a864a94364c6..29bca3a1ddeb 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c

[...]
>  static int io_install_fixed_file(struct io_kiocb *req, struct file *file,
>  				 unsigned int issue_flags, u32 slot_index)
>  {
>  	struct io_ring_ctx *ctx = req->ctx;
>  	bool force_nonblock = issue_flags & IO_URING_F_NONBLOCK;
> +	bool needs_switch = false;
>  	struct io_fixed_file *file_slot;
>  	int ret = -EBADF;
>  
> @@ -8304,12 +8321,31 @@ static int io_install_fixed_file(struct io_kiocb *req, struct file *file,
>  	ret = -EINVAL;
>  	if (slot_index >= ctx->nr_user_files)
>  		goto err;
> +	/*
> +	 * Ignore error, ->rsrc_backup_node is not needed if the slot is empty,
> +	 * and we'd rather not drop the file.
> +	 */
> +	io_rsrc_node_switch_start(ctx);

Can be made easier, will resend shortly

-- 
Pavel Begunkov

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 5.15] io_uring: auto-removal for direct open/accept
  2021-09-14 13:37 [PATCH 5.15] io_uring: auto-removal for direct open/accept Pavel Begunkov
  2021-09-14 13:59 ` Pavel Begunkov
@ 2021-09-14 14:02 ` Jens Axboe
  2021-09-14 14:10   ` Pavel Begunkov
  1 sibling, 1 reply; 5+ messages in thread
From: Jens Axboe @ 2021-09-14 14:02 UTC (permalink / raw)
  To: Pavel Begunkov, io-uring

On 9/14/21 7:37 AM, Pavel Begunkov wrote:
> It might be inconvenient that direct open/accept deviates from the
> update semantics and fails if the slot is taken instead of removing a
> file sitting there. Implement the auto-removal.
> 
> Note that removal might need to allocate and so may fail. However, if an
> empty slot is specified, it's guaraneed to not fail on the fd
> installation side. It's needed for users that can't tolerate spuriously
> closed files, e.g. accepts where the other end doesn't expect it.

I think this makes sense, just curious if this was driven by feedback
from a user, or if it's something that came about thinking about the use
cases? This is certainly more flexible and allows an application to open
a new file in an existing slot, rather than needing to explicitly close
it first.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 5.15] io_uring: auto-removal for direct open/accept
  2021-09-14 14:02 ` Jens Axboe
@ 2021-09-14 14:10   ` Pavel Begunkov
  2021-09-14 14:18     ` Jens Axboe
  0 siblings, 1 reply; 5+ messages in thread
From: Pavel Begunkov @ 2021-09-14 14:10 UTC (permalink / raw)
  To: Jens Axboe, io-uring

On 9/14/21 3:02 PM, Jens Axboe wrote:
> On 9/14/21 7:37 AM, Pavel Begunkov wrote:
>> It might be inconvenient that direct open/accept deviates from the
>> update semantics and fails if the slot is taken instead of removing a
>> file sitting there. Implement the auto-removal.
>>
>> Note that removal might need to allocate and so may fail. However, if an
>> empty slot is specified, it's guaraneed to not fail on the fd
>> installation side. It's needed for users that can't tolerate spuriously
>> closed files, e.g. accepts where the other end doesn't expect it.
> 
> I think this makes sense, just curious if this was driven by feedback
> from a user, or if it's something that came about thinking about the use
> cases? This is certainly more flexible and allows an application to open
> a new file in an existing slot, rather than needing to explicitly close
> it first.

Franz noticed that it would've been more convenient this way. Good idea
to add his suggested-by. I had been thinking to make it this way before
that, but without particular use cases, it just felt better.

-- 
Pavel Begunkov

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 5.15] io_uring: auto-removal for direct open/accept
  2021-09-14 14:10   ` Pavel Begunkov
@ 2021-09-14 14:18     ` Jens Axboe
  0 siblings, 0 replies; 5+ messages in thread
From: Jens Axboe @ 2021-09-14 14:18 UTC (permalink / raw)
  To: Pavel Begunkov, io-uring

On 9/14/21 8:10 AM, Pavel Begunkov wrote:
> On 9/14/21 3:02 PM, Jens Axboe wrote:
>> On 9/14/21 7:37 AM, Pavel Begunkov wrote:
>>> It might be inconvenient that direct open/accept deviates from the
>>> update semantics and fails if the slot is taken instead of removing a
>>> file sitting there. Implement the auto-removal.
>>>
>>> Note that removal might need to allocate and so may fail. However, if an
>>> empty slot is specified, it's guaraneed to not fail on the fd
>>> installation side. It's needed for users that can't tolerate spuriously
>>> closed files, e.g. accepts where the other end doesn't expect it.
>>
>> I think this makes sense, just curious if this was driven by feedback
>> from a user, or if it's something that came about thinking about the use
>> cases? This is certainly more flexible and allows an application to open
>> a new file in an existing slot, rather than needing to explicitly close
>> it first.
> 
> Franz noticed that it would've been more convenient this way. Good idea
> to add his suggested-by. I had been thinking to make it this way before
> that, but without particular use cases, it just felt better.

OK good, and as mentioned, I do like it and think this is how it should
work. Just add his suggested-by for the v2.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-09-14 14:19 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-09-14 13:37 [PATCH 5.15] io_uring: auto-removal for direct open/accept Pavel Begunkov
2021-09-14 13:59 ` Pavel Begunkov
2021-09-14 14:02 ` Jens Axboe
2021-09-14 14:10   ` Pavel Begunkov
2021-09-14 14:18     ` Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox