From: Ming Lei <[email protected]>
To: Pavel Begunkov <[email protected]>
Cc: [email protected], Jens Axboe <[email protected]>,
[email protected], [email protected],
[email protected]
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 <[email protected]>
> ---
> 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 \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
/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