public inbox for io-uring@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] io_uring: uring_cmd: add multishot support without poll
@ 2025-08-10  2:50 Ming Lei
  2025-08-18  2:16 ` Ming Lei
  2025-08-18 15:31 ` Jens Axboe
  0 siblings, 2 replies; 10+ messages in thread
From: Ming Lei @ 2025-08-10  2:50 UTC (permalink / raw)
  To: Jens Axboe, io-uring, Pavel Begunkov; +Cc: Caleb Sander Mateos, Ming Lei

Add UAPI flag IORING_URING_CMD_MULTISHOT for supporting naive multishot
uring_cmd:

- for notifying userspace to handle event, typical use case is to notify
userspace for handle device interrupt event, really generic use case

- needn't device to support poll() because the event may be originated
from multiple source in device wide, such as multi queues

- add two APIs, io_uring_cmd_select_buffer() is for getting the provided
group buffer, io_uring_mshot_cmd_post_cqe() is for post CQE after event
data is pushed to the provided buffer

Follows one ublk use case:

https://github.com/ming1/linux/commits/ublk-devel/

Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 include/linux/io_uring/cmd.h  | 27 +++++++++++++++
 include/uapi/linux/io_uring.h |  9 ++++-
 io_uring/opdef.c              |  1 +
 io_uring/uring_cmd.c          | 64 ++++++++++++++++++++++++++++++++++-
 4 files changed, 99 insertions(+), 2 deletions(-)

diff --git a/include/linux/io_uring/cmd.h b/include/linux/io_uring/cmd.h
index cfa6d0c0c322..5a72399bfa77 100644
--- a/include/linux/io_uring/cmd.h
+++ b/include/linux/io_uring/cmd.h
@@ -70,6 +70,22 @@ void io_uring_cmd_mark_cancelable(struct io_uring_cmd *cmd,
 /* Execute the request from a blocking context */
 void io_uring_cmd_issue_blocking(struct io_uring_cmd *ioucmd);
 
+/*
+ * Select a buffer from the provided buffer group for multishot uring_cmd.
+ * Returns the selected buffer address and size.
+ */
+int io_uring_cmd_select_buffer(struct io_uring_cmd *ioucmd,
+			       unsigned buf_group,
+			       void **buf, size_t *len,
+			       unsigned int issue_flags);
+
+/*
+ * Complete a multishot uring_cmd event. This will post a CQE to the completion
+ * queue and update the provided buffer.
+ */
+bool io_uring_mshot_cmd_post_cqe(struct io_uring_cmd *ioucmd,
+				 ssize_t ret, unsigned int issue_flags);
+
 #else
 static inline int
 io_uring_cmd_import_fixed(u64 ubuf, unsigned long len, int rw,
@@ -102,6 +118,17 @@ static inline void io_uring_cmd_mark_cancelable(struct io_uring_cmd *cmd,
 static inline void io_uring_cmd_issue_blocking(struct io_uring_cmd *ioucmd)
 {
 }
+static inline int io_uring_cmd_select_buffer(struct io_uring_cmd *ioucmd,
+				unsigned buf_group,
+				void **buf, size_t *len,
+				unsigned int issue_flags)
+{
+	return -EOPNOTSUPP;
+}
+static inline void io_uring_mshot_cmd_post_cqe(struct io_uring_cmd *ioucmd,
+				ssize_t ret, unsigned int issue_flags)
+{
+}
 #endif
 
 /*
diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
index 6957dc539d83..e8afb4f5b56a 100644
--- a/include/uapi/linux/io_uring.h
+++ b/include/uapi/linux/io_uring.h
@@ -297,10 +297,17 @@ enum io_uring_op {
 /*
  * sqe->uring_cmd_flags		top 8bits aren't available for userspace
  * IORING_URING_CMD_FIXED	use registered buffer; pass this flag
+ *				along with setting sqe->buf_index,
+ *				IORING_URING_CMD_MULTISHOT can't be set
+ *				at the same time
+ * IORING_URING_CMD_MULTISHOT	use buffer select; pass this flag
  *				along with setting sqe->buf_index.
+ *				IORING_URING_CMD_FIXED can't be set
+ *				at the same time
  */
 #define IORING_URING_CMD_FIXED	(1U << 0)
-#define IORING_URING_CMD_MASK	IORING_URING_CMD_FIXED
+#define IORING_URING_CMD_MULTISHOT	(1U << 1)
+#define IORING_URING_CMD_MASK	(IORING_URING_CMD_FIXED | IORING_URING_CMD_MULTISHOT)
 
 
 /*
diff --git a/io_uring/opdef.c b/io_uring/opdef.c
index 9568785810d9..932319633eac 100644
--- a/io_uring/opdef.c
+++ b/io_uring/opdef.c
@@ -413,6 +413,7 @@ const struct io_issue_def io_issue_defs[] = {
 #endif
 	},
 	[IORING_OP_URING_CMD] = {
+		.buffer_select		= 1,
 		.needs_file		= 1,
 		.plug			= 1,
 		.iopoll			= 1,
diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c
index 053bac89b6c0..f0539e73f9d7 100644
--- a/io_uring/uring_cmd.c
+++ b/io_uring/uring_cmd.c
@@ -11,6 +11,7 @@
 #include "io_uring.h"
 #include "alloc_cache.h"
 #include "rsrc.h"
+#include "kbuf.h"
 #include "uring_cmd.h"
 #include "poll.h"
 
@@ -194,8 +195,20 @@ int io_uring_cmd_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	if (ioucmd->flags & ~IORING_URING_CMD_MASK)
 		return -EINVAL;
 
-	if (ioucmd->flags & IORING_URING_CMD_FIXED)
+	if ((ioucmd->flags & IORING_URING_CMD_FIXED) && (ioucmd->flags &
+				IORING_URING_CMD_MULTISHOT))
+		return -EINVAL;
+
+	if (ioucmd->flags & IORING_URING_CMD_FIXED) {
+		if (req->flags & REQ_F_BUFFER_SELECT)
+			return -EINVAL;
 		req->buf_index = READ_ONCE(sqe->buf_index);
+	}
+
+	if (ioucmd->flags & IORING_URING_CMD_MULTISHOT) {
+		if (!(req->flags & REQ_F_BUFFER_SELECT))
+			return -EINVAL;
+	}
 
 	ioucmd->cmd_op = READ_ONCE(sqe->cmd_op);
 
@@ -251,6 +264,11 @@ int io_uring_cmd(struct io_kiocb *req, unsigned int issue_flags)
 	}
 
 	ret = file->f_op->uring_cmd(ioucmd, issue_flags);
+	if (ioucmd->flags & IORING_URING_CMD_MULTISHOT) {
+		if (ret >= 0)
+			return IOU_ISSUE_SKIP_COMPLETE;
+		io_kbuf_recycle(req, issue_flags);
+	}
 	if (ret == -EAGAIN) {
 		ioucmd->flags |= IORING_URING_CMD_REISSUE;
 		return ret;
@@ -333,3 +351,47 @@ bool io_uring_cmd_post_mshot_cqe32(struct io_uring_cmd *cmd,
 		return false;
 	return io_req_post_cqe32(req, cqe);
 }
+
+int io_uring_cmd_select_buffer(struct io_uring_cmd *ioucmd,
+			       unsigned buf_group,
+			       void __user **buf, size_t *len,
+			       unsigned int issue_flags)
+{
+	struct io_kiocb *req = cmd_to_io_kiocb(ioucmd);
+	void __user *ubuf;
+
+	if (!(req->flags & REQ_F_BUFFER_SELECT))
+		return -EINVAL;
+
+	ubuf = io_buffer_select(req, len, buf_group, issue_flags);
+	if (!ubuf)
+		return -ENOBUFS;
+
+	*buf = ubuf;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(io_uring_cmd_select_buffer);
+
+/*
+ * Return true if this multishot uring_cmd needs to be completed, otherwise
+ * the event CQE is posted successfully.
+ *
+ * Should only be used from a task_work
+ *
+ */
+bool io_uring_mshot_cmd_post_cqe(struct io_uring_cmd *ioucmd,
+				 ssize_t ret, unsigned int issue_flags)
+{
+	struct io_kiocb *req = cmd_to_io_kiocb(ioucmd);
+	unsigned int cflags = 0;
+
+	if (ret > 0) {
+		cflags = io_put_kbuf(req, ret, issue_flags);
+		if (io_req_post_cqe(req, ret, cflags | IORING_CQE_F_MORE))
+			return false;
+	}
+
+	io_req_set_res(req, ret, cflags);
+	return true;
+}
+EXPORT_SYMBOL_GPL(io_uring_mshot_cmd_post_cqe);
-- 
2.47.1


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

* Re: [PATCH] io_uring: uring_cmd: add multishot support without poll
  2025-08-10  2:50 [PATCH] io_uring: uring_cmd: add multishot support without poll Ming Lei
@ 2025-08-18  2:16 ` Ming Lei
  2025-08-18 15:31 ` Jens Axboe
  1 sibling, 0 replies; 10+ messages in thread
From: Ming Lei @ 2025-08-18  2:16 UTC (permalink / raw)
  To: Jens Axboe, io-uring, Pavel Begunkov; +Cc: Caleb Sander Mateos

On Sun, Aug 10, 2025 at 10:50 AM Ming Lei <ming.lei@redhat.com> wrote:
>
> Add UAPI flag IORING_URING_CMD_MULTISHOT for supporting naive multishot
> uring_cmd:
>
> - for notifying userspace to handle event, typical use case is to notify
> userspace for handle device interrupt event, really generic use case
>
> - needn't device to support poll() because the event may be originated
> from multiple source in device wide, such as multi queues
>
> - add two APIs, io_uring_cmd_select_buffer() is for getting the provided
> group buffer, io_uring_mshot_cmd_post_cqe() is for post CQE after event
> data is pushed to the provided buffer
>
> Follows one ublk use case:
>
> https://github.com/ming1/linux/commits/ublk-devel/
>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>

Hello Guys,

Ping...

Thanks,


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

* Re: [PATCH] io_uring: uring_cmd: add multishot support without poll
  2025-08-10  2:50 [PATCH] io_uring: uring_cmd: add multishot support without poll Ming Lei
  2025-08-18  2:16 ` Ming Lei
@ 2025-08-18 15:31 ` Jens Axboe
  2025-08-19 11:19   ` Ming Lei
  1 sibling, 1 reply; 10+ messages in thread
From: Jens Axboe @ 2025-08-18 15:31 UTC (permalink / raw)
  To: Ming Lei, io-uring, Pavel Begunkov; +Cc: Caleb Sander Mateos

On 8/9/25 8:50 PM, Ming Lei wrote:
> Add UAPI flag IORING_URING_CMD_MULTISHOT for supporting naive multishot
> uring_cmd:
> 
> - for notifying userspace to handle event, typical use case is to notify
> userspace for handle device interrupt event, really generic use case
> 
> - needn't device to support poll() because the event may be originated
> from multiple source in device wide, such as multi queues
> 
> - add two APIs, io_uring_cmd_select_buffer() is for getting the provided
> group buffer, io_uring_mshot_cmd_post_cqe() is for post CQE after event
> data is pushed to the provided buffer
> 
> Follows one ublk use case:
> 
> https://github.com/ming1/linux/commits/ublk-devel/
> 
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
>  include/linux/io_uring/cmd.h  | 27 +++++++++++++++
>  include/uapi/linux/io_uring.h |  9 ++++-
>  io_uring/opdef.c              |  1 +
>  io_uring/uring_cmd.c          | 64 ++++++++++++++++++++++++++++++++++-
>  4 files changed, 99 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/io_uring/cmd.h b/include/linux/io_uring/cmd.h
> index cfa6d0c0c322..5a72399bfa77 100644
> --- a/include/linux/io_uring/cmd.h
> +++ b/include/linux/io_uring/cmd.h
> @@ -70,6 +70,22 @@ void io_uring_cmd_mark_cancelable(struct io_uring_cmd *cmd,
>  /* Execute the request from a blocking context */
>  void io_uring_cmd_issue_blocking(struct io_uring_cmd *ioucmd);
>  
> +/*
> + * Select a buffer from the provided buffer group for multishot uring_cmd.
> + * Returns the selected buffer address and size.
> + */
> +int io_uring_cmd_select_buffer(struct io_uring_cmd *ioucmd,
> +			       unsigned buf_group,
> +			       void **buf, size_t *len,
> +			       unsigned int issue_flags);
> +
> +/*
> + * Complete a multishot uring_cmd event. This will post a CQE to the completion
> + * queue and update the provided buffer.
> + */
> +bool io_uring_mshot_cmd_post_cqe(struct io_uring_cmd *ioucmd,
> +				 ssize_t ret, unsigned int issue_flags);
> +
>  #else
>  static inline int
>  io_uring_cmd_import_fixed(u64 ubuf, unsigned long len, int rw,
> @@ -102,6 +118,17 @@ static inline void io_uring_cmd_mark_cancelable(struct io_uring_cmd *cmd,
>  static inline void io_uring_cmd_issue_blocking(struct io_uring_cmd *ioucmd)
>  {
>  }
> +static inline int io_uring_cmd_select_buffer(struct io_uring_cmd *ioucmd,
> +				unsigned buf_group,
> +				void **buf, size_t *len,
> +				unsigned int issue_flags)
> +{
> +	return -EOPNOTSUPP;
> +}
> +static inline void io_uring_mshot_cmd_post_cqe(struct io_uring_cmd *ioucmd,
> +				ssize_t ret, unsigned int issue_flags)
> +{
> +}
>  #endif
>  
>  /*
> diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
> index 6957dc539d83..e8afb4f5b56a 100644
> --- a/include/uapi/linux/io_uring.h
> +++ b/include/uapi/linux/io_uring.h
> @@ -297,10 +297,17 @@ enum io_uring_op {
>  /*
>   * sqe->uring_cmd_flags		top 8bits aren't available for userspace
>   * IORING_URING_CMD_FIXED	use registered buffer; pass this flag
> + *				along with setting sqe->buf_index,
> + *				IORING_URING_CMD_MULTISHOT can't be set
> + *				at the same time
> + * IORING_URING_CMD_MULTISHOT	use buffer select; pass this flag
>   *				along with setting sqe->buf_index.
> + *				IORING_URING_CMD_FIXED can't be set
> + *				at the same time

This reads very confusingly, as if setting IORING_URING_CMD_MULTISHOT is
what decides this request selects a buffer. In practice, the rule is
that you MUST set IOSQE_BUFFER_SELECT, using IORING_URING_CMD_MULTISHOT
without that is invalid. So I think that just needs a bit of updating.
Something ala:

Must be used with buffer select, like other multishot commands. Not
compatible with IORING_URING_CMD_FIXED, for now.

> @@ -194,8 +195,20 @@ int io_uring_cmd_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>  	if (ioucmd->flags & ~IORING_URING_CMD_MASK)
>  		return -EINVAL;
>  
> -	if (ioucmd->flags & IORING_URING_CMD_FIXED)
> +	if ((ioucmd->flags & IORING_URING_CMD_FIXED) && (ioucmd->flags &
> +				IORING_URING_CMD_MULTISHOT))
> +		return -EINVAL;

I think the more idiomatic way to write that is:

	if ((ioucmd->flags & (IORING_URING_CMD_FIXED|IORING_URING_CMD_MULTISHOT) == (IORING_URING_CMD_FIXED|IORING_URING_CMD_MULTISHOT)

But since you have the separate checks below, why not do fold that check
into those instead? Eg in here:

> +	if (ioucmd->flags & IORING_URING_CMD_FIXED) {
> +		if (req->flags & REQ_F_BUFFER_SELECT)
> +			return -EINVAL;
>  		req->buf_index = READ_ONCE(sqe->buf_index);
> +	}
> +
> +	if (ioucmd->flags & IORING_URING_CMD_MULTISHOT) {
> +		if (!(req->flags & REQ_F_BUFFER_SELECT))
> +			return -EINVAL;
> +	}

each section can just check the other flag.

> @@ -251,6 +264,11 @@ int io_uring_cmd(struct io_kiocb *req, unsigned int issue_flags)
>  	}
>  
>  	ret = file->f_op->uring_cmd(ioucmd, issue_flags);
> +	if (ioucmd->flags & IORING_URING_CMD_MULTISHOT) {
> +		if (ret >= 0)
> +			return IOU_ISSUE_SKIP_COMPLETE;
> +		io_kbuf_recycle(req, issue_flags);
> +	}
>  	if (ret == -EAGAIN) {
>  		ioucmd->flags |= IORING_URING_CMD_REISSUE;
>  		return ret;

Missing recycle for -EAGAIN?

> @@ -333,3 +351,47 @@ bool io_uring_cmd_post_mshot_cqe32(struct io_uring_cmd *cmd,
>  		return false;
>  	return io_req_post_cqe32(req, cqe);
>  }
> +
> +int io_uring_cmd_select_buffer(struct io_uring_cmd *ioucmd,
> +			       unsigned buf_group,
> +			       void __user **buf, size_t *len,
> +			       unsigned int issue_flags)
> +{
> +	struct io_kiocb *req = cmd_to_io_kiocb(ioucmd);
> +	void __user *ubuf;
> +
> +	if (!(req->flags & REQ_F_BUFFER_SELECT))
> +		return -EINVAL;
> +
> +	ubuf = io_buffer_select(req, len, buf_group, issue_flags);
> +	if (!ubuf)
> +		return -ENOBUFS;
> +
> +	*buf = ubuf;
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(io_uring_cmd_select_buffer);
> +
> +/*
> + * Return true if this multishot uring_cmd needs to be completed, otherwise
> + * the event CQE is posted successfully.
> + *
> + * Should only be used from a task_work
> + *
> + */
> +bool io_uring_mshot_cmd_post_cqe(struct io_uring_cmd *ioucmd,
> +				 ssize_t ret, unsigned int issue_flags)
> +{
> +	struct io_kiocb *req = cmd_to_io_kiocb(ioucmd);
> +	unsigned int cflags = 0;
> +
> +	if (ret > 0) {
> +		cflags = io_put_kbuf(req, ret, issue_flags);
> +		if (io_req_post_cqe(req, ret, cflags | IORING_CQE_F_MORE))
> +			return false;
> +	}
> +
> +	io_req_set_res(req, ret, cflags);
> +	return true;
> +}
> +EXPORT_SYMBOL_GPL(io_uring_mshot_cmd_post_cqe);

Missing req_set_fail()?

-- 
Jens Axboe

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

* Re: [PATCH] io_uring: uring_cmd: add multishot support without poll
  2025-08-18 15:31 ` Jens Axboe
@ 2025-08-19 11:19   ` Ming Lei
  2025-08-19 14:01     ` Jens Axboe
  0 siblings, 1 reply; 10+ messages in thread
From: Ming Lei @ 2025-08-19 11:19 UTC (permalink / raw)
  To: Jens Axboe; +Cc: io-uring, Pavel Begunkov, Caleb Sander Mateos

On Mon, Aug 18, 2025 at 09:31:35AM -0600, Jens Axboe wrote:
> On 8/9/25 8:50 PM, Ming Lei wrote:
> > Add UAPI flag IORING_URING_CMD_MULTISHOT for supporting naive multishot
> > uring_cmd:
> > 
> > - for notifying userspace to handle event, typical use case is to notify
> > userspace for handle device interrupt event, really generic use case
> > 
> > - needn't device to support poll() because the event may be originated
> > from multiple source in device wide, such as multi queues
> > 
> > - add two APIs, io_uring_cmd_select_buffer() is for getting the provided
> > group buffer, io_uring_mshot_cmd_post_cqe() is for post CQE after event
> > data is pushed to the provided buffer
> > 
> > Follows one ublk use case:
> > 
> > https://github.com/ming1/linux/commits/ublk-devel/
> > 
> > Signed-off-by: Ming Lei <ming.lei@redhat.com>
> > ---
> >  include/linux/io_uring/cmd.h  | 27 +++++++++++++++
> >  include/uapi/linux/io_uring.h |  9 ++++-
> >  io_uring/opdef.c              |  1 +
> >  io_uring/uring_cmd.c          | 64 ++++++++++++++++++++++++++++++++++-
> >  4 files changed, 99 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/linux/io_uring/cmd.h b/include/linux/io_uring/cmd.h
> > index cfa6d0c0c322..5a72399bfa77 100644
> > --- a/include/linux/io_uring/cmd.h
> > +++ b/include/linux/io_uring/cmd.h
> > @@ -70,6 +70,22 @@ void io_uring_cmd_mark_cancelable(struct io_uring_cmd *cmd,
> >  /* Execute the request from a blocking context */
> >  void io_uring_cmd_issue_blocking(struct io_uring_cmd *ioucmd);
> >  
> > +/*
> > + * Select a buffer from the provided buffer group for multishot uring_cmd.
> > + * Returns the selected buffer address and size.
> > + */
> > +int io_uring_cmd_select_buffer(struct io_uring_cmd *ioucmd,
> > +			       unsigned buf_group,
> > +			       void **buf, size_t *len,
> > +			       unsigned int issue_flags);
> > +
> > +/*
> > + * Complete a multishot uring_cmd event. This will post a CQE to the completion
> > + * queue and update the provided buffer.
> > + */
> > +bool io_uring_mshot_cmd_post_cqe(struct io_uring_cmd *ioucmd,
> > +				 ssize_t ret, unsigned int issue_flags);
> > +
> >  #else
> >  static inline int
> >  io_uring_cmd_import_fixed(u64 ubuf, unsigned long len, int rw,
> > @@ -102,6 +118,17 @@ static inline void io_uring_cmd_mark_cancelable(struct io_uring_cmd *cmd,
> >  static inline void io_uring_cmd_issue_blocking(struct io_uring_cmd *ioucmd)
> >  {
> >  }
> > +static inline int io_uring_cmd_select_buffer(struct io_uring_cmd *ioucmd,
> > +				unsigned buf_group,
> > +				void **buf, size_t *len,
> > +				unsigned int issue_flags)
> > +{
> > +	return -EOPNOTSUPP;
> > +}
> > +static inline void io_uring_mshot_cmd_post_cqe(struct io_uring_cmd *ioucmd,
> > +				ssize_t ret, unsigned int issue_flags)
> > +{
> > +}
> >  #endif
> >  
> >  /*
> > diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
> > index 6957dc539d83..e8afb4f5b56a 100644
> > --- a/include/uapi/linux/io_uring.h
> > +++ b/include/uapi/linux/io_uring.h
> > @@ -297,10 +297,17 @@ enum io_uring_op {
> >  /*
> >   * sqe->uring_cmd_flags		top 8bits aren't available for userspace
> >   * IORING_URING_CMD_FIXED	use registered buffer; pass this flag
> > + *				along with setting sqe->buf_index,
> > + *				IORING_URING_CMD_MULTISHOT can't be set
> > + *				at the same time
> > + * IORING_URING_CMD_MULTISHOT	use buffer select; pass this flag
> >   *				along with setting sqe->buf_index.
> > + *				IORING_URING_CMD_FIXED can't be set
> > + *				at the same time
> 
> This reads very confusingly, as if setting IORING_URING_CMD_MULTISHOT is
> what decides this request selects a buffer. In practice, the rule is
> that you MUST set IOSQE_BUFFER_SELECT, using IORING_URING_CMD_MULTISHOT
> without that is invalid. So I think that just needs a bit of updating.
> Something ala:
> 
> Must be used with buffer select, like other multishot commands. Not
> compatible with IORING_URING_CMD_FIXED, for now.

OK

> 
> > @@ -194,8 +195,20 @@ int io_uring_cmd_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
> >  	if (ioucmd->flags & ~IORING_URING_CMD_MASK)
> >  		return -EINVAL;
> >  
> > -	if (ioucmd->flags & IORING_URING_CMD_FIXED)
> > +	if ((ioucmd->flags & IORING_URING_CMD_FIXED) && (ioucmd->flags &
> > +				IORING_URING_CMD_MULTISHOT))
> > +		return -EINVAL;
> 
> I think the more idiomatic way to write that is:
> 
> 	if ((ioucmd->flags & (IORING_URING_CMD_FIXED|IORING_URING_CMD_MULTISHOT) == (IORING_URING_CMD_FIXED|IORING_URING_CMD_MULTISHOT)
> 
> But since you have the separate checks below, why not do fold that check
> into those instead? Eg in here:
> 
> > +	if (ioucmd->flags & IORING_URING_CMD_FIXED) {
> > +		if (req->flags & REQ_F_BUFFER_SELECT)
> > +			return -EINVAL;
> >  		req->buf_index = READ_ONCE(sqe->buf_index);
> > +	}
> > +
> > +	if (ioucmd->flags & IORING_URING_CMD_MULTISHOT) {
> > +		if (!(req->flags & REQ_F_BUFFER_SELECT))
> > +			return -EINVAL;
> > +	}
> 
> each section can just check the other flag.

OK

> 
> > @@ -251,6 +264,11 @@ int io_uring_cmd(struct io_kiocb *req, unsigned int issue_flags)
> >  	}
> >  
> >  	ret = file->f_op->uring_cmd(ioucmd, issue_flags);
> > +	if (ioucmd->flags & IORING_URING_CMD_MULTISHOT) {
> > +		if (ret >= 0)
> > +			return IOU_ISSUE_SKIP_COMPLETE;
> > +		io_kbuf_recycle(req, issue_flags);
> > +	}
> >  	if (ret == -EAGAIN) {
> >  		ioucmd->flags |= IORING_URING_CMD_REISSUE;
> >  		return ret;
> 
> Missing recycle for -EAGAIN?

io_kbuf_recycle() is done above if `ret < 0`

> 
> > @@ -333,3 +351,47 @@ bool io_uring_cmd_post_mshot_cqe32(struct io_uring_cmd *cmd,
> >  		return false;
> >  	return io_req_post_cqe32(req, cqe);
> >  }
> > +
> > +int io_uring_cmd_select_buffer(struct io_uring_cmd *ioucmd,
> > +			       unsigned buf_group,
> > +			       void __user **buf, size_t *len,
> > +			       unsigned int issue_flags)
> > +{
> > +	struct io_kiocb *req = cmd_to_io_kiocb(ioucmd);
> > +	void __user *ubuf;
> > +
> > +	if (!(req->flags & REQ_F_BUFFER_SELECT))
> > +		return -EINVAL;
> > +
> > +	ubuf = io_buffer_select(req, len, buf_group, issue_flags);
> > +	if (!ubuf)
> > +		return -ENOBUFS;
> > +
> > +	*buf = ubuf;
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(io_uring_cmd_select_buffer);
> > +
> > +/*
> > + * Return true if this multishot uring_cmd needs to be completed, otherwise
> > + * the event CQE is posted successfully.
> > + *
> > + * Should only be used from a task_work
> > + *
> > + */
> > +bool io_uring_mshot_cmd_post_cqe(struct io_uring_cmd *ioucmd,
> > +				 ssize_t ret, unsigned int issue_flags)
> > +{
> > +	struct io_kiocb *req = cmd_to_io_kiocb(ioucmd);
> > +	unsigned int cflags = 0;
> > +
> > +	if (ret > 0) {
> > +		cflags = io_put_kbuf(req, ret, issue_flags);
> > +		if (io_req_post_cqe(req, ret, cflags | IORING_CQE_F_MORE))
> > +			return false;
> > +	}
> > +
> > +	io_req_set_res(req, ret, cflags);
> > +	return true;
> > +}
> > +EXPORT_SYMBOL_GPL(io_uring_mshot_cmd_post_cqe);
> 
> Missing req_set_fail()?

Will add it.


Thanks, 
Ming


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

* Re: [PATCH] io_uring: uring_cmd: add multishot support without poll
  2025-08-19 11:19   ` Ming Lei
@ 2025-08-19 14:01     ` Jens Axboe
  2025-08-19 14:28       ` Ming Lei
  0 siblings, 1 reply; 10+ messages in thread
From: Jens Axboe @ 2025-08-19 14:01 UTC (permalink / raw)
  To: Ming Lei; +Cc: io-uring, Pavel Begunkov, Caleb Sander Mateos

On 8/19/25 5:19 AM, Ming Lei wrote:
>>> @@ -251,6 +264,11 @@ int io_uring_cmd(struct io_kiocb *req, unsigned int issue_flags)
>>>  	}
>>>  
>>>  	ret = file->f_op->uring_cmd(ioucmd, issue_flags);
>>> +	if (ioucmd->flags & IORING_URING_CMD_MULTISHOT) {
>>> +		if (ret >= 0)
>>> +			return IOU_ISSUE_SKIP_COMPLETE;
>>> +		io_kbuf_recycle(req, issue_flags);
>>> +	}
>>>  	if (ret == -EAGAIN) {
>>>  		ioucmd->flags |= IORING_URING_CMD_REISSUE;
>>>  		return ret;
>>
>> Missing recycle for -EAGAIN?
> 
> io_kbuf_recycle() is done above if `ret < 0`

Inside the multishot case. I don't see anywhere where it's forbidden to
use IOSQE_BUFFER_SELECT without having multishot set? Either that needs
to be explicit for now, or the recycling should happen generically.
Probably the former I would suspect.

-- 
Jens Axboe

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

* Re: [PATCH] io_uring: uring_cmd: add multishot support without poll
  2025-08-19 14:01     ` Jens Axboe
@ 2025-08-19 14:28       ` Ming Lei
  2025-08-19 14:31         ` Jens Axboe
  0 siblings, 1 reply; 10+ messages in thread
From: Ming Lei @ 2025-08-19 14:28 UTC (permalink / raw)
  To: Jens Axboe; +Cc: io-uring, Pavel Begunkov, Caleb Sander Mateos

On Tue, Aug 19, 2025 at 08:01:18AM -0600, Jens Axboe wrote:
> On 8/19/25 5:19 AM, Ming Lei wrote:
> >>> @@ -251,6 +264,11 @@ int io_uring_cmd(struct io_kiocb *req, unsigned int issue_flags)
> >>>  	}
> >>>  
> >>>  	ret = file->f_op->uring_cmd(ioucmd, issue_flags);
> >>> +	if (ioucmd->flags & IORING_URING_CMD_MULTISHOT) {
> >>> +		if (ret >= 0)
> >>> +			return IOU_ISSUE_SKIP_COMPLETE;
> >>> +		io_kbuf_recycle(req, issue_flags);
> >>> +	}
> >>>  	if (ret == -EAGAIN) {
> >>>  		ioucmd->flags |= IORING_URING_CMD_REISSUE;
> >>>  		return ret;
> >>
> >> Missing recycle for -EAGAIN?
> > 
> > io_kbuf_recycle() is done above if `ret < 0`
> 
> Inside the multishot case. I don't see anywhere where it's forbidden to
> use IOSQE_BUFFER_SELECT without having multishot set? Either that needs

REQ_F_BUFFER_SELECT is supposed to be allowed for IORING_URING_CMD_MULTISHOT
only, and it is checked in io_uring_cmd_prep().

> to be explicit for now, or the recycling should happen generically.
> Probably the former I would suspect.

Yes, the former is exactly what the patch is doing.


Thanks,
Ming


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

* Re: [PATCH] io_uring: uring_cmd: add multishot support without poll
  2025-08-19 14:28       ` Ming Lei
@ 2025-08-19 14:31         ` Jens Axboe
  2025-08-19 14:40           ` Ming Lei
  0 siblings, 1 reply; 10+ messages in thread
From: Jens Axboe @ 2025-08-19 14:31 UTC (permalink / raw)
  To: Ming Lei; +Cc: io-uring, Pavel Begunkov, Caleb Sander Mateos

On 8/19/25 8:28 AM, Ming Lei wrote:
> On Tue, Aug 19, 2025 at 08:01:18AM -0600, Jens Axboe wrote:
>> On 8/19/25 5:19 AM, Ming Lei wrote:
>>>>> @@ -251,6 +264,11 @@ int io_uring_cmd(struct io_kiocb *req, unsigned int issue_flags)
>>>>>  	}
>>>>>  
>>>>>  	ret = file->f_op->uring_cmd(ioucmd, issue_flags);
>>>>> +	if (ioucmd->flags & IORING_URING_CMD_MULTISHOT) {
>>>>> +		if (ret >= 0)
>>>>> +			return IOU_ISSUE_SKIP_COMPLETE;
>>>>> +		io_kbuf_recycle(req, issue_flags);
>>>>> +	}
>>>>>  	if (ret == -EAGAIN) {
>>>>>  		ioucmd->flags |= IORING_URING_CMD_REISSUE;
>>>>>  		return ret;
>>>>
>>>> Missing recycle for -EAGAIN?
>>>
>>> io_kbuf_recycle() is done above if `ret < 0`
>>
>> Inside the multishot case. I don't see anywhere where it's forbidden to
>> use IOSQE_BUFFER_SELECT without having multishot set? Either that needs
> 
> REQ_F_BUFFER_SELECT is supposed to be allowed for IORING_URING_CMD_MULTISHOT
> only, and it is checked in io_uring_cmd_prep().
> 
>> to be explicit for now, or the recycling should happen generically.
>> Probably the former I would suspect.
> 
> Yes, the former is exactly what the patch is doing.

Is it? Because looking at v2, you check if IORING_URING_CMD_FIXED is
set, and you fail for that case if REQ_F_BUFFER_SELECT is set. Then you
have a IORING_URING_CMD_MULTISHOT where the opposite is true, which
obviously makes sense.

But no checks if neither is set?

You could add that in io_uring_cmd_select_buffer(), eg fail if
IORING_URING_CMD_MULTISHOT isn't set. Which if done, then the prep side
checking could probably just go away.

-- 
Jens Axboe

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

* Re: [PATCH] io_uring: uring_cmd: add multishot support without poll
  2025-08-19 14:31         ` Jens Axboe
@ 2025-08-19 14:40           ` Ming Lei
  2025-08-19 14:43             ` Jens Axboe
  0 siblings, 1 reply; 10+ messages in thread
From: Ming Lei @ 2025-08-19 14:40 UTC (permalink / raw)
  To: Jens Axboe; +Cc: io-uring, Pavel Begunkov, Caleb Sander Mateos

On Tue, Aug 19, 2025 at 08:31:32AM -0600, Jens Axboe wrote:
> On 8/19/25 8:28 AM, Ming Lei wrote:
> > On Tue, Aug 19, 2025 at 08:01:18AM -0600, Jens Axboe wrote:
> >> On 8/19/25 5:19 AM, Ming Lei wrote:
> >>>>> @@ -251,6 +264,11 @@ int io_uring_cmd(struct io_kiocb *req, unsigned int issue_flags)
> >>>>>  	}
> >>>>>  
> >>>>>  	ret = file->f_op->uring_cmd(ioucmd, issue_flags);
> >>>>> +	if (ioucmd->flags & IORING_URING_CMD_MULTISHOT) {
> >>>>> +		if (ret >= 0)
> >>>>> +			return IOU_ISSUE_SKIP_COMPLETE;
> >>>>> +		io_kbuf_recycle(req, issue_flags);
> >>>>> +	}
> >>>>>  	if (ret == -EAGAIN) {
> >>>>>  		ioucmd->flags |= IORING_URING_CMD_REISSUE;
> >>>>>  		return ret;
> >>>>
> >>>> Missing recycle for -EAGAIN?
> >>>
> >>> io_kbuf_recycle() is done above if `ret < 0`
> >>
> >> Inside the multishot case. I don't see anywhere where it's forbidden to
> >> use IOSQE_BUFFER_SELECT without having multishot set? Either that needs
> > 
> > REQ_F_BUFFER_SELECT is supposed to be allowed for IORING_URING_CMD_MULTISHOT
> > only, and it is checked in io_uring_cmd_prep().
> > 
> >> to be explicit for now, or the recycling should happen generically.
> >> Probably the former I would suspect.
> > 
> > Yes, the former is exactly what the patch is doing.
> 
> Is it? Because looking at v2, you check if IORING_URING_CMD_FIXED is
> set, and you fail for that case if REQ_F_BUFFER_SELECT is set. Then you
> have a IORING_URING_CMD_MULTISHOT where the opposite is true, which
> obviously makes sense.
> 
> But no checks if neither is set?

Indeed, thanks for the catch, and the REQ_F_BUFFER_SELECT check in IORING_URING_CMD_FIXED
branch can be moved to the branch of !IORING_URING_CMD_MULTISHOT.

> 
> You could add that in io_uring_cmd_select_buffer(), eg fail if
> IORING_URING_CMD_MULTISHOT isn't set. Which if done, then the prep side
> checking could probably just go away.

Looks this way is good too.

Thanks,
Ming


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

* Re: [PATCH] io_uring: uring_cmd: add multishot support without poll
  2025-08-19 14:40           ` Ming Lei
@ 2025-08-19 14:43             ` Jens Axboe
  2025-08-19 14:55               ` Ming Lei
  0 siblings, 1 reply; 10+ messages in thread
From: Jens Axboe @ 2025-08-19 14:43 UTC (permalink / raw)
  To: Ming Lei; +Cc: io-uring, Pavel Begunkov, Caleb Sander Mateos

On 8/19/25 8:40 AM, Ming Lei wrote:
> On Tue, Aug 19, 2025 at 08:31:32AM -0600, Jens Axboe wrote:
>> On 8/19/25 8:28 AM, Ming Lei wrote:
>>> On Tue, Aug 19, 2025 at 08:01:18AM -0600, Jens Axboe wrote:
>>>> On 8/19/25 5:19 AM, Ming Lei wrote:
>>>>>>> @@ -251,6 +264,11 @@ int io_uring_cmd(struct io_kiocb *req, unsigned int issue_flags)
>>>>>>>  	}
>>>>>>>  
>>>>>>>  	ret = file->f_op->uring_cmd(ioucmd, issue_flags);
>>>>>>> +	if (ioucmd->flags & IORING_URING_CMD_MULTISHOT) {
>>>>>>> +		if (ret >= 0)
>>>>>>> +			return IOU_ISSUE_SKIP_COMPLETE;
>>>>>>> +		io_kbuf_recycle(req, issue_flags);
>>>>>>> +	}
>>>>>>>  	if (ret == -EAGAIN) {
>>>>>>>  		ioucmd->flags |= IORING_URING_CMD_REISSUE;
>>>>>>>  		return ret;
>>>>>>
>>>>>> Missing recycle for -EAGAIN?
>>>>>
>>>>> io_kbuf_recycle() is done above if `ret < 0`
>>>>
>>>> Inside the multishot case. I don't see anywhere where it's forbidden to
>>>> use IOSQE_BUFFER_SELECT without having multishot set? Either that needs
>>>
>>> REQ_F_BUFFER_SELECT is supposed to be allowed for IORING_URING_CMD_MULTISHOT
>>> only, and it is checked in io_uring_cmd_prep().
>>>
>>>> to be explicit for now, or the recycling should happen generically.
>>>> Probably the former I would suspect.
>>>
>>> Yes, the former is exactly what the patch is doing.
>>
>> Is it? Because looking at v2, you check if IORING_URING_CMD_FIXED is
>> set, and you fail for that case if REQ_F_BUFFER_SELECT is set. Then you
>> have a IORING_URING_CMD_MULTISHOT where the opposite is true, which
>> obviously makes sense.
>>
>> But no checks if neither is set?
> 
> Indeed, thanks for the catch, and the REQ_F_BUFFER_SELECT check in IORING_URING_CMD_FIXED
> branch can be moved to the branch of !IORING_URING_CMD_MULTISHOT.
> 
>>
>> You could add that in io_uring_cmd_select_buffer(), eg fail if
>> IORING_URING_CMD_MULTISHOT isn't set. Which if done, then the prep side
>> checking could probably just go away.
> 
> Looks this way is good too.

Want to spin a v3 for this?

-- 
Jens Axboe

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

* Re: [PATCH] io_uring: uring_cmd: add multishot support without poll
  2025-08-19 14:43             ` Jens Axboe
@ 2025-08-19 14:55               ` Ming Lei
  0 siblings, 0 replies; 10+ messages in thread
From: Ming Lei @ 2025-08-19 14:55 UTC (permalink / raw)
  To: Jens Axboe; +Cc: io-uring, Pavel Begunkov, Caleb Sander Mateos

On Tue, Aug 19, 2025 at 08:43:58AM -0600, Jens Axboe wrote:
> On 8/19/25 8:40 AM, Ming Lei wrote:
> > On Tue, Aug 19, 2025 at 08:31:32AM -0600, Jens Axboe wrote:
> >> On 8/19/25 8:28 AM, Ming Lei wrote:
> >>> On Tue, Aug 19, 2025 at 08:01:18AM -0600, Jens Axboe wrote:
> >>>> On 8/19/25 5:19 AM, Ming Lei wrote:
> >>>>>>> @@ -251,6 +264,11 @@ int io_uring_cmd(struct io_kiocb *req, unsigned int issue_flags)
> >>>>>>>  	}
> >>>>>>>  
> >>>>>>>  	ret = file->f_op->uring_cmd(ioucmd, issue_flags);
> >>>>>>> +	if (ioucmd->flags & IORING_URING_CMD_MULTISHOT) {
> >>>>>>> +		if (ret >= 0)
> >>>>>>> +			return IOU_ISSUE_SKIP_COMPLETE;
> >>>>>>> +		io_kbuf_recycle(req, issue_flags);
> >>>>>>> +	}
> >>>>>>>  	if (ret == -EAGAIN) {
> >>>>>>>  		ioucmd->flags |= IORING_URING_CMD_REISSUE;
> >>>>>>>  		return ret;
> >>>>>>
> >>>>>> Missing recycle for -EAGAIN?
> >>>>>
> >>>>> io_kbuf_recycle() is done above if `ret < 0`
> >>>>
> >>>> Inside the multishot case. I don't see anywhere where it's forbidden to
> >>>> use IOSQE_BUFFER_SELECT without having multishot set? Either that needs
> >>>
> >>> REQ_F_BUFFER_SELECT is supposed to be allowed for IORING_URING_CMD_MULTISHOT
> >>> only, and it is checked in io_uring_cmd_prep().
> >>>
> >>>> to be explicit for now, or the recycling should happen generically.
> >>>> Probably the former I would suspect.
> >>>
> >>> Yes, the former is exactly what the patch is doing.
> >>
> >> Is it? Because looking at v2, you check if IORING_URING_CMD_FIXED is
> >> set, and you fail for that case if REQ_F_BUFFER_SELECT is set. Then you
> >> have a IORING_URING_CMD_MULTISHOT where the opposite is true, which
> >> obviously makes sense.
> >>
> >> But no checks if neither is set?
> > 
> > Indeed, thanks for the catch, and the REQ_F_BUFFER_SELECT check in IORING_URING_CMD_FIXED
> > branch can be moved to the branch of !IORING_URING_CMD_MULTISHOT.
> > 
> >>
> >> You could add that in io_uring_cmd_select_buffer(), eg fail if
> >> IORING_URING_CMD_MULTISHOT isn't set. Which if done, then the prep side
> >> checking could probably just go away.
> > 
> > Looks this way is good too.
> 
> Want to spin a v3 for this?

Yeah, will send V3 out after test is done.

Thanks,
Ming


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

end of thread, other threads:[~2025-08-19 14:55 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-10  2:50 [PATCH] io_uring: uring_cmd: add multishot support without poll Ming Lei
2025-08-18  2:16 ` Ming Lei
2025-08-18 15:31 ` Jens Axboe
2025-08-19 11:19   ` Ming Lei
2025-08-19 14:01     ` Jens Axboe
2025-08-19 14:28       ` Ming Lei
2025-08-19 14:31         ` Jens Axboe
2025-08-19 14:40           ` Ming Lei
2025-08-19 14:43             ` Jens Axboe
2025-08-19 14:55               ` Ming Lei

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