public inbox for io-uring@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] io_uring: fix buffer auto-commit for multishot uring_cmd
@ 2025-10-23 10:43 Ming Lei
  2025-10-23 18:37 ` Jens Axboe
  0 siblings, 1 reply; 2+ messages in thread
From: Ming Lei @ 2025-10-23 10:43 UTC (permalink / raw)
  To: Jens Axboe, io-uring; +Cc: Caleb Sander Mateos, Ming Lei

Commit 620a50c92700 ("io_uring: uring_cmd: add multishot support") added
multishot uring_cmd support with explicit buffer upfront commit via
io_uring_mshot_cmd_post_cqe(). However, the buffer selection path in
io_ring_buffer_select() was auto-committing buffers for non-pollable files,
which conflicts with uring_cmd's explicit upfront commit model.

This way consumes the whole selected buffer immediately, and causes
failure on the following buffer selection.

Fix this by adding io_commit_kbuf_upfront() to identify operations that
handle buffer commit explicitly (currently only IORING_OP_URING_CMD),
and skip auto-commit for these operations.

Cc: Caleb Sander Mateos <csander@purestorage.com>
Fixes: 620a50c92700 ("io_uring: uring_cmd: add multishot support")
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 io_uring/kbuf.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/io_uring/kbuf.c b/io_uring/kbuf.c
index aad655e38672..e3f3dec8b135 100644
--- a/io_uring/kbuf.c
+++ b/io_uring/kbuf.c
@@ -155,6 +155,12 @@ static int io_provided_buffers_select(struct io_kiocb *req, size_t *len,
 	return 1;
 }
 
+/* So far, uring_cmd commits kbuf upfront, no need to auto-commit */
+static bool io_commit_kbuf_upfront(const struct io_kiocb *req)
+{
+	return req->opcode == IORING_OP_URING_CMD;
+}
+
 static struct io_br_sel io_ring_buffer_select(struct io_kiocb *req, size_t *len,
 					      struct io_buffer_list *bl,
 					      unsigned int issue_flags)
@@ -181,7 +187,8 @@ static struct io_br_sel io_ring_buffer_select(struct io_kiocb *req, size_t *len,
 	sel.buf_list = bl;
 	sel.addr = u64_to_user_ptr(buf->addr);
 
-	if (issue_flags & IO_URING_F_UNLOCKED || !io_file_can_poll(req)) {
+	if (issue_flags & IO_URING_F_UNLOCKED || (!io_file_can_poll(req) &&
+				!io_commit_kbuf_upfront(req))) {
 		/*
 		 * If we came in unlocked, we have no choice but to consume the
 		 * buffer here, otherwise nothing ensures that the buffer won't
-- 
2.47.1


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

* Re: [PATCH] io_uring: fix buffer auto-commit for multishot uring_cmd
  2025-10-23 10:43 [PATCH] io_uring: fix buffer auto-commit for multishot uring_cmd Ming Lei
@ 2025-10-23 18:37 ` Jens Axboe
  0 siblings, 0 replies; 2+ messages in thread
From: Jens Axboe @ 2025-10-23 18:37 UTC (permalink / raw)
  To: Ming Lei, io-uring; +Cc: Caleb Sander Mateos

On 10/23/25 4:43 AM, Ming Lei wrote:
> Commit 620a50c92700 ("io_uring: uring_cmd: add multishot support") added
> multishot uring_cmd support with explicit buffer upfront commit via
> io_uring_mshot_cmd_post_cqe(). However, the buffer selection path in
> io_ring_buffer_select() was auto-committing buffers for non-pollable files,
> which conflicts with uring_cmd's explicit upfront commit model.
> 
> This way consumes the whole selected buffer immediately, and causes
> failure on the following buffer selection.
> 
> Fix this by adding io_commit_kbuf_upfront() to identify operations that
> handle buffer commit explicitly (currently only IORING_OP_URING_CMD),
> and skip auto-commit for these operations.
> 
> Cc: Caleb Sander Mateos <csander@purestorage.com>
> Fixes: 620a50c92700 ("io_uring: uring_cmd: add multishot support")
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
>  io_uring/kbuf.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/io_uring/kbuf.c b/io_uring/kbuf.c
> index aad655e38672..e3f3dec8b135 100644
> --- a/io_uring/kbuf.c
> +++ b/io_uring/kbuf.c
> @@ -155,6 +155,12 @@ static int io_provided_buffers_select(struct io_kiocb *req, size_t *len,
>  	return 1;
>  }
>  
> +/* So far, uring_cmd commits kbuf upfront, no need to auto-commit */
> +static bool io_commit_kbuf_upfront(const struct io_kiocb *req)
> +{
> +	return req->opcode == IORING_OP_URING_CMD;
> +}
> +
>  static struct io_br_sel io_ring_buffer_select(struct io_kiocb *req, size_t *len,
>  					      struct io_buffer_list *bl,
>  					      unsigned int issue_flags)
> @@ -181,7 +187,8 @@ static struct io_br_sel io_ring_buffer_select(struct io_kiocb *req, size_t *len,
>  	sel.buf_list = bl;
>  	sel.addr = u64_to_user_ptr(buf->addr);
>  
> -	if (issue_flags & IO_URING_F_UNLOCKED || !io_file_can_poll(req)) {
> +	if (issue_flags & IO_URING_F_UNLOCKED || (!io_file_can_poll(req) &&
> +				!io_commit_kbuf_upfront(req))) {
>  		/*
>  		 * If we came in unlocked, we have no choice but to consume the
>  		 * buffer here, otherwise nothing ensures that the buffer won't

Might be cleaner to put this logic in a helper instead, ala:

static bool io_should_commit(req, issue_flags)
{
	if (issue_flags & IO_URING_F_UNLOCKED)
		return true;
	if (!io_file_can_poll(req) && req->opcode == IORING_OP_URING_CMD)
		return true;
	return false;
}

and just add the appropriate comments there, rather than add a new
random helper.

-- 
Jens Axboe

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

end of thread, other threads:[~2025-10-23 18:37 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-23 10:43 [PATCH] io_uring: fix buffer auto-commit for multishot uring_cmd Ming Lei
2025-10-23 18:37 ` Jens Axboe

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