public inbox for io-uring@vger.kernel.org
 help / color / mirror / Atom feed
From: Caleb Sander Mateos <csander@purestorage.com>
To: Ming Lei <ming.lei@redhat.com>
Cc: Jens Axboe <axboe@kernel.dk>,
	io-uring@vger.kernel.org,
	 Pavel Begunkov <asml.silence@gmail.com>
Subject: Re: [PATCH V5 2/2] io_uring: uring_cmd: add multishot support
Date: Thu, 21 Aug 2025 09:23:16 -0700	[thread overview]
Message-ID: <CADUfDZruvf+RTVRdH16X0xfUO-FmgLZAx6zvwHN3s0LoCcUAQA@mail.gmail.com> (raw)
In-Reply-To: <20250821040210.1152145-3-ming.lei@redhat.com>

On Wed, Aug 20, 2025 at 9:02 PM Ming Lei <ming.lei@redhat.com> wrote:
>
> Add UAPI flag IORING_URING_CMD_MULTISHOT for supporting multishot
> uring_cmd operations with provided buffer.
>
> This enables drivers to post multiple completion events from a single
> uring_cmd submission, which is useful for:
>
> - Notifying userspace of device events (e.g., interrupt handling)
> - Supporting devices with multiple event sources (e.g., multi-queue devices)
> - Avoiding the need for device poll() support when events originate
>   from multiple sources device-wide
>
> The implementation adds two new APIs:
> - io_uring_cmd_select_buffer(): selects a buffer from the provided
>   buffer group for multishot uring_cmd
> - io_uring_mshot_cmd_post_cqe(): posts a CQE after event data is
>   pushed to the provided buffer
>
> Multishot uring_cmd must be used with buffer select (IOSQE_BUFFER_SELECT)
> and is mutually exclusive with IORING_URING_CMD_FIXED for now.
>
> The ublk driver will be the first user of this functionality:
>
>         https://github.com/ming1/linux/commits/ublk-devel/
>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>

Sorry I was out for a while and didn't get a chance to look at this
earlier. It generally looks reasonable. I noticed a couple of small
things which I'll send out patches for.

> ---
>  include/linux/io_uring/cmd.h  | 27 +++++++++++++
>  include/uapi/linux/io_uring.h |  6 ++-
>  io_uring/opdef.c              |  1 +
>  io_uring/uring_cmd.c          | 71 ++++++++++++++++++++++++++++++++++-
>  4 files changed, 103 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/io_uring/cmd.h b/include/linux/io_uring/cmd.h
> index cfa6d0c0c322..856d343b8e2a 100644
> --- a/include/linux/io_uring/cmd.h
> +++ b/include/linux/io_uring/cmd.h
> @@ -70,6 +70,21 @@ 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.
> + */
> +struct io_br_sel io_uring_cmd_buffer_select(struct io_uring_cmd *ioucmd,
> +                                           unsigned buf_group, 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,
> +                                struct io_br_sel *sel, unsigned int issue_flags);
> +
>  #else
>  static inline int
>  io_uring_cmd_import_fixed(u64 ubuf, unsigned long len, int rw,
> @@ -102,6 +117,18 @@ 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 bool io_uring_mshot_cmd_post_cqe(struct io_uring_cmd *ioucmd,
> +                               ssize_t ret, unsigned int issue_flags)
> +{
> +       return true;
> +}
>  #endif
>
>  /*
> diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
> index 6957dc539d83..1e935f8901c5 100644
> --- a/include/uapi/linux/io_uring.h
> +++ b/include/uapi/linux/io_uring.h
> @@ -298,9 +298,13 @@ 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  must be used with buffer select, like other
> + *                             multishot commands. Not compatible with
> + *                             IORING_URING_CMD_FIXED, for now.
>   */
>  #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..3cfb5d51b88a 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,21 @@ 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) {
> +               if (ioucmd->flags & IORING_URING_CMD_MULTISHOT)
> +                       return -EINVAL;
>                 req->buf_index = READ_ONCE(sqe->buf_index);
> +       }
> +
> +       if (ioucmd->flags & IORING_URING_CMD_MULTISHOT) {
> +               if (ioucmd->flags & IORING_URING_CMD_FIXED)
> +                       return -EINVAL;
> +               if (!(req->flags & REQ_F_BUFFER_SELECT))
> +                       return -EINVAL;
> +       } else {
> +               if (req->flags & REQ_F_BUFFER_SELECT)
> +                       return -EINVAL;
> +       }
>
>         ioucmd->cmd_op = READ_ONCE(sqe->cmd_op);
>
> @@ -251,6 +265,10 @@ 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;
> +       }
>         if (ret == -EAGAIN) {
>                 ioucmd->flags |= IORING_URING_CMD_REISSUE;
>                 return ret;
> @@ -333,3 +351,54 @@ bool io_uring_cmd_post_mshot_cqe32(struct io_uring_cmd *cmd,
>                 return false;
>         return io_req_post_cqe32(req, cqe);
>  }
> +
> +/*
> + * Work with io_uring_mshot_cmd_post_cqe() together for committing the
> + * provided buffer upfront
> + */
> +struct io_br_sel io_uring_cmd_buffer_select(struct io_uring_cmd *ioucmd,
> +                                           unsigned buf_group, size_t *len,
> +                                           unsigned int issue_flags)
> +{
> +       struct io_kiocb *req = cmd_to_io_kiocb(ioucmd);
> +
> +       if (!(ioucmd->flags & IORING_URING_CMD_MULTISHOT))
> +               return (struct io_br_sel) { .val = -EINVAL };

Would this condition make more sense as a WARN_ON()? When would this
be called for a non-IORING_URING_CMD_MULTISHOT io_uring_cmd?

> +
> +       if (WARN_ON_ONCE(!io_do_buffer_select(req)))
> +               return (struct io_br_sel) { .val = -EINVAL };
> +
> +       return io_buffer_select(req, len, buf_group, issue_flags);
> +}
> +EXPORT_SYMBOL_GPL(io_uring_cmd_buffer_select);
> +
> +/*
> + * Return true if this multishot uring_cmd needs to be completed, otherwise
> + * the event CQE is posted successfully.
> + *
> + * This function must use `struct io_br_sel` returned from
> + * io_uring_cmd_buffer_select() for committing the buffer in the same
> + * uring_cmd submission context.
> + */
> +bool io_uring_mshot_cmd_post_cqe(struct io_uring_cmd *ioucmd,
> +                                struct io_br_sel *sel, unsigned int issue_flags)
> +{
> +       struct io_kiocb *req = cmd_to_io_kiocb(ioucmd);
> +       unsigned int cflags = 0;
> +
> +       if (!(ioucmd->flags & IORING_URING_CMD_MULTISHOT))
> +               return true;

Same here, a WARN_ON() seems like it would make more sense.

Best,
Caleb

> +
> +       if (sel->val > 0) {
> +               cflags = io_put_kbuf(req, sel->val, sel->buf_list);
> +               if (io_req_post_cqe(req, sel->val, cflags | IORING_CQE_F_MORE))
> +                       return false;
> +       }
> +
> +       io_kbuf_recycle(req, sel->buf_list, issue_flags);
> +       if (sel->val < 0)
> +               req_set_fail(req);
> +       io_req_set_res(req, sel->val, cflags);
> +       return true;
> +}
> +EXPORT_SYMBOL_GPL(io_uring_mshot_cmd_post_cqe);
> --
> 2.47.0
>

  reply	other threads:[~2025-08-21 16:23 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-21  4:02 [PATCH V5 0/2] io_uring: uring_cmd: add multishot support with provided buffer Ming Lei
2025-08-21  4:02 ` [PATCH V5 1/2] io-uring: move `struct io_br_sel` into io_uring_types.h Ming Lei
2025-08-21  4:02 ` [PATCH V5 2/2] io_uring: uring_cmd: add multishot support Ming Lei
2025-08-21 16:23   ` Caleb Sander Mateos [this message]
2025-08-21 16:37     ` Jens Axboe
2025-08-21 16:29   ` Caleb Sander Mateos
2025-08-21 16:38     ` Jens Axboe
2025-08-22  0:52       ` Ming Lei
2025-08-22  0:58         ` Jens Axboe
2025-08-21 11:41 ` [PATCH V5 0/2] io_uring: uring_cmd: add multishot support with provided buffer Jens Axboe
2025-08-21 11:44   ` Jens Axboe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CADUfDZruvf+RTVRdH16X0xfUO-FmgLZAx6zvwHN3s0LoCcUAQA@mail.gmail.com \
    --to=csander@purestorage.com \
    --cc=asml.silence@gmail.com \
    --cc=axboe@kernel.dk \
    --cc=io-uring@vger.kernel.org \
    --cc=ming.lei@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox