Re: [PATCH] io_uring: uring_cmd: add multishot support without poll

[Jens Axboe <axboe@kernel.dk>]

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