From: Ming Lei <ming.lei@redhat.com>
To: Pavel Begunkov <asml.silence@gmail.com>
Cc: io-uring@vger.kernel.org, Jens Axboe <axboe@kernel.dk>,
linux-block@vger.kernel.org, joshi.k@samsung.com,
ming.lei@redhat.com
Subject: Re: [PATCH 1/3] io_uring: split out cmd api into a separate header
Date: Thu, 23 Nov 2023 10:40:35 +0800 [thread overview]
Message-ID: <ZV67ozp4yizgWYYg@fedora> (raw)
In-Reply-To: <547e56560b97cd66f00bfc5b53db24f2fa1a8852.1700668641.git.asml.silence@gmail.com>
On Wed, Nov 22, 2023 at 04:01:09PM +0000, Pavel Begunkov wrote:
> linux/io_uring.h is slowly becoming a rubbish bin where we put
> anything exposed to other subsystems. For instance, the task exit
> hooks and io_uring cmd infra are completely orthogonal and don't need
> each other's definitions. Start cleaning it up by splitting out all
> command bits into a new header file.
>
> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> ---
> drivers/block/ublk_drv.c | 2 +-
> drivers/nvme/host/ioctl.c | 2 +-
> include/linux/io_uring.h | 89 +---------------------------------
> include/linux/io_uring/cmd.h | 81 +++++++++++++++++++++++++++++++
> include/linux/io_uring_types.h | 20 ++++++++
> io_uring/io_uring.c | 1 +
> io_uring/rw.c | 2 +-
> io_uring/uring_cmd.c | 2 +-
> 8 files changed, 107 insertions(+), 92 deletions(-)
> create mode 100644 include/linux/io_uring/cmd.h
>
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index 83600b45e12a..909377068a87 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -36,7 +36,7 @@
> #include <linux/sched/mm.h>
> #include <linux/uaccess.h>
> #include <linux/cdev.h>
> -#include <linux/io_uring.h>
> +#include <linux/io_uring/cmd.h>
> #include <linux/blk-mq.h>
> #include <linux/delay.h>
> #include <linux/mm.h>
> diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c
> index 529b9954d2b8..6864a6eeee93 100644
> --- a/drivers/nvme/host/ioctl.c
> +++ b/drivers/nvme/host/ioctl.c
> @@ -5,7 +5,7 @@
> */
> #include <linux/ptrace.h> /* for force_successful_syscall_return */
> #include <linux/nvme_ioctl.h>
> -#include <linux/io_uring.h>
> +#include <linux/io_uring/cmd.h>
> #include "nvme.h"
>
> enum {
> diff --git a/include/linux/io_uring.h b/include/linux/io_uring.h
> index aefb73eeeebf..d8fc93492dc5 100644
> --- a/include/linux/io_uring.h
> +++ b/include/linux/io_uring.h
> @@ -6,71 +6,13 @@
> #include <linux/xarray.h>
> #include <uapi/linux/io_uring.h>
>
> -enum io_uring_cmd_flags {
> - IO_URING_F_COMPLETE_DEFER = 1,
> - IO_URING_F_UNLOCKED = 2,
> - /* the request is executed from poll, it should not be freed */
> - IO_URING_F_MULTISHOT = 4,
> - /* executed by io-wq */
> - IO_URING_F_IOWQ = 8,
> - /* int's last bit, sign checks are usually faster than a bit test */
> - IO_URING_F_NONBLOCK = INT_MIN,
> -
> - /* ctx state flags, for URING_CMD */
> - IO_URING_F_SQE128 = (1 << 8),
> - IO_URING_F_CQE32 = (1 << 9),
> - IO_URING_F_IOPOLL = (1 << 10),
> -
> - /* set when uring wants to cancel a previously issued command */
> - IO_URING_F_CANCEL = (1 << 11),
> - IO_URING_F_COMPAT = (1 << 12),
> -};
> -
> -/* only top 8 bits of sqe->uring_cmd_flags for kernel internal use */
> -#define IORING_URING_CMD_CANCELABLE (1U << 30)
> -#define IORING_URING_CMD_POLLED (1U << 31)
> -
> -struct io_uring_cmd {
> - struct file *file;
> - const struct io_uring_sqe *sqe;
> - union {
> - /* callback to defer completions to task context */
> - void (*task_work_cb)(struct io_uring_cmd *cmd, unsigned);
> - /* used for polled completion */
> - void *cookie;
> - };
> - u32 cmd_op;
> - u32 flags;
> - u8 pdu[32]; /* available inline for free use */
> -};
> -
> -static inline const void *io_uring_sqe_cmd(const struct io_uring_sqe *sqe)
> -{
> - return sqe->cmd;
> -}
> -
> #if defined(CONFIG_IO_URING)
> -int io_uring_cmd_import_fixed(u64 ubuf, unsigned long len, int rw,
> - struct iov_iter *iter, void *ioucmd);
> -void io_uring_cmd_done(struct io_uring_cmd *cmd, ssize_t ret, ssize_t res2,
> - unsigned issue_flags);
> struct sock *io_uring_get_socket(struct file *file);
> void __io_uring_cancel(bool cancel_all);
> void __io_uring_free(struct task_struct *tsk);
> void io_uring_unreg_ringfd(void);
> const char *io_uring_get_opcode(u8 opcode);
> -void __io_uring_cmd_do_in_task(struct io_uring_cmd *ioucmd,
> - void (*task_work_cb)(struct io_uring_cmd *, unsigned),
> - unsigned flags);
> -/* users should follow semantics of IOU_F_TWQ_LAZY_WAKE */
> -void io_uring_cmd_do_in_task_lazy(struct io_uring_cmd *ioucmd,
> - void (*task_work_cb)(struct io_uring_cmd *, unsigned));
> -
> -static inline void io_uring_cmd_complete_in_task(struct io_uring_cmd *ioucmd,
> - void (*task_work_cb)(struct io_uring_cmd *, unsigned))
> -{
> - __io_uring_cmd_do_in_task(ioucmd, task_work_cb, 0);
> -}
> +int io_uring_cmd_sock(struct io_uring_cmd *cmd, unsigned int issue_flags);
>
> static inline void io_uring_files_cancel(void)
> {
> @@ -89,28 +31,7 @@ static inline void io_uring_free(struct task_struct *tsk)
> if (tsk->io_uring)
> __io_uring_free(tsk);
> }
> -int io_uring_cmd_sock(struct io_uring_cmd *cmd, unsigned int issue_flags);
> -void io_uring_cmd_mark_cancelable(struct io_uring_cmd *cmd,
> - unsigned int issue_flags);
> -struct task_struct *io_uring_cmd_get_task(struct io_uring_cmd *cmd);
> #else
> -static inline int io_uring_cmd_import_fixed(u64 ubuf, unsigned long len, int rw,
> - struct iov_iter *iter, void *ioucmd)
> -{
> - return -EOPNOTSUPP;
> -}
> -static inline void io_uring_cmd_done(struct io_uring_cmd *cmd, ssize_t ret,
> - ssize_t ret2, unsigned issue_flags)
> -{
> -}
> -static inline void io_uring_cmd_complete_in_task(struct io_uring_cmd *ioucmd,
> - void (*task_work_cb)(struct io_uring_cmd *, unsigned))
> -{
> -}
> -static inline void io_uring_cmd_do_in_task_lazy(struct io_uring_cmd *ioucmd,
> - void (*task_work_cb)(struct io_uring_cmd *, unsigned))
> -{
> -}
> static inline struct sock *io_uring_get_socket(struct file *file)
> {
> return NULL;
> @@ -133,14 +54,6 @@ static inline int io_uring_cmd_sock(struct io_uring_cmd *cmd,
> {
> return -EOPNOTSUPP;
> }
> -static inline void io_uring_cmd_mark_cancelable(struct io_uring_cmd *cmd,
> - unsigned int issue_flags)
> -{
> -}
> -static inline struct task_struct *io_uring_cmd_get_task(struct io_uring_cmd *cmd)
> -{
> - return NULL;
> -}
> #endif
>
> #endif
> diff --git a/include/linux/io_uring/cmd.h b/include/linux/io_uring/cmd.h
> new file mode 100644
> index 000000000000..62fcfaf6fcc9
> --- /dev/null
> +++ b/include/linux/io_uring/cmd.h
> @@ -0,0 +1,81 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +#ifndef _LINUX_IO_URING_CMD_H
> +#define _LINUX_IO_URING_CMD_H
> +
> +#include <uapi/linux/io_uring.h>
> +#include <linux/io_uring_types.h>
> +
> +/* only top 8 bits of sqe->uring_cmd_flags for kernel internal use */
> +#define IORING_URING_CMD_CANCELABLE (1U << 30)
> +#define IORING_URING_CMD_POLLED (1U << 31)
> +
> +struct io_uring_cmd {
> + struct file *file;
> + const struct io_uring_sqe *sqe;
> + union {
> + /* callback to defer completions to task context */
> + void (*task_work_cb)(struct io_uring_cmd *cmd, unsigned);
> + /* used for polled completion */
> + void *cookie;
> + };
> + u32 cmd_op;
> + u32 flags;
> + u8 pdu[32]; /* available inline for free use */
> +};
> +
> +static inline const void *io_uring_sqe_cmd(const struct io_uring_sqe *sqe)
> +{
> + return sqe->cmd;
> +}
> +
> +#if defined(CONFIG_IO_URING)
> +int io_uring_cmd_import_fixed(u64 ubuf, unsigned long len, int rw,
> + struct iov_iter *iter, void *ioucmd);
> +void io_uring_cmd_done(struct io_uring_cmd *cmd, ssize_t ret, ssize_t res2,
> + unsigned issue_flags);
> +void __io_uring_cmd_do_in_task(struct io_uring_cmd *ioucmd,
> + void (*task_work_cb)(struct io_uring_cmd *, unsigned),
> + unsigned flags);
> +/* users should follow semantics of IOU_F_TWQ_LAZY_WAKE */
> +void io_uring_cmd_do_in_task_lazy(struct io_uring_cmd *ioucmd,
> + void (*task_work_cb)(struct io_uring_cmd *, unsigned));
> +
> +static inline void io_uring_cmd_complete_in_task(struct io_uring_cmd *ioucmd,
> + void (*task_work_cb)(struct io_uring_cmd *, unsigned))
> +{
> + __io_uring_cmd_do_in_task(ioucmd, task_work_cb, 0);
> +}
> +
> +void io_uring_cmd_mark_cancelable(struct io_uring_cmd *cmd,
> + unsigned int issue_flags);
> +struct task_struct *io_uring_cmd_get_task(struct io_uring_cmd *cmd);
> +
> +#else
> +static inline int io_uring_cmd_import_fixed(u64 ubuf, unsigned long len, int rw,
> + struct iov_iter *iter, void *ioucmd)
> +{
> + return -EOPNOTSUPP;
> +}
> +static inline void io_uring_cmd_done(struct io_uring_cmd *cmd, ssize_t ret,
> + ssize_t ret2, unsigned issue_flags)
> +{
> +}
> +static inline void io_uring_cmd_complete_in_task(struct io_uring_cmd *ioucmd,
> + void (*task_work_cb)(struct io_uring_cmd *, unsigned))
> +{
> +}
> +static inline void io_uring_cmd_do_in_task_lazy(struct io_uring_cmd *ioucmd,
> + void (*task_work_cb)(struct io_uring_cmd *, unsigned))
> +{
> +}
> +static inline void io_uring_cmd_mark_cancelable(struct io_uring_cmd *cmd,
> + unsigned int issue_flags)
> +{
> +}
> +static inline struct task_struct *io_uring_cmd_get_task(struct io_uring_cmd *cmd)
> +{
> + return NULL;
> +}
> +#endif
> +
> +#endif /* _LINUX_IO_URING_CMD_H */
> diff --git a/include/linux/io_uring_types.h b/include/linux/io_uring_types.h
> index d3009d56af0b..0bcecb734af3 100644
> --- a/include/linux/io_uring_types.h
> +++ b/include/linux/io_uring_types.h
> @@ -7,6 +7,26 @@
> #include <linux/llist.h>
> #include <uapi/linux/io_uring.h>
>
> +enum io_uring_cmd_flags {
> + IO_URING_F_COMPLETE_DEFER = 1,
> + IO_URING_F_UNLOCKED = 2,
> + /* the request is executed from poll, it should not be freed */
> + IO_URING_F_MULTISHOT = 4,
> + /* executed by io-wq */
> + IO_URING_F_IOWQ = 8,
> + /* int's last bit, sign checks are usually faster than a bit test */
> + IO_URING_F_NONBLOCK = INT_MIN,
> +
> + /* ctx state flags, for URING_CMD */
> + IO_URING_F_SQE128 = (1 << 8),
> + IO_URING_F_CQE32 = (1 << 9),
> + IO_URING_F_IOPOLL = (1 << 10),
> +
> + /* set when uring wants to cancel a previously issued command */
> + IO_URING_F_CANCEL = (1 << 11),
> + IO_URING_F_COMPAT = (1 << 12),
> +};
I am wondering why you don't move io_uring_cmd_flags into
io_uring/cmd.h? And many above flags are used by driver now.
But most definitions in io_uring_types.h are actually io_uring
internal stuff.
Thanks,
Ming
next prev parent reply other threads:[~2023-11-23 2:40 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-22 16:01 [PATCH 0/3] clean up io_uring cmd header structure Pavel Begunkov
2023-11-22 16:01 ` [PATCH 1/3] io_uring: split out cmd api into a separate header Pavel Begunkov
2023-11-23 2:40 ` Ming Lei [this message]
2023-11-23 11:16 ` Pavel Begunkov
2023-11-24 2:05 ` Ming Lei
2023-11-24 13:20 ` kernel test robot
2023-11-24 15:23 ` kernel test robot
2023-11-22 16:01 ` [PATCH 2/3] io_uring/cmd: inline io_uring_cmd_do_in_task_lazy Pavel Begunkov
2023-11-22 16:01 ` [PATCH 3/3] io_uring/cmd: inline io_uring_cmd_get_task Pavel Begunkov
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=ZV67ozp4yizgWYYg@fedora \
--to=ming.lei@redhat.com \
--cc=asml.silence@gmail.com \
--cc=axboe@kernel.dk \
--cc=io-uring@vger.kernel.org \
--cc=joshi.k@samsung.com \
--cc=linux-block@vger.kernel.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.