On 12/5/23 2:55 PM, Keith Busch wrote:
> From: Keith Busch <kbusch@xxxxxxxxxx>
>
> No need to rebuild the issue_flags on every IO: they're always the same.
>
> Suggested-by: Jens Axboe <axboe@xxxxxxxxx>
> Signed-off-by: Keith Busch <kbusch@xxxxxxxxxx>
> ---
> include/linux/io_uring_types.h | 1 +
> io_uring/io_uring.c | 15 ++++++++++++---
> io_uring/uring_cmd.c | 8 +-------
> 3 files changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/io_uring_types.h b/include/linux/io_uring_types.h
> index bebab36abce89..dd192d828f463 100644
> --- a/include/linux/io_uring_types.h
> +++ b/include/linux/io_uring_types.h
> @@ -228,6 +228,7 @@ struct io_ring_ctx {
> /* const or read-mostly hot data */
> struct {
> unsigned int flags;
> + unsigned int issue_flags;
> unsigned int drain_next: 1;
> unsigned int restricted: 1;
> unsigned int off_timeout_used: 1;
> diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
> index 1d254f2c997de..a338e3660ecb8 100644
> --- a/io_uring/io_uring.c
> +++ b/io_uring/io_uring.c
> @@ -3975,11 +3975,20 @@ static __cold int io_uring_create(unsigned entries, struct io_uring_params *p,
> * polling again, they can rely on io_sq_thread to do polling
> * work, which can reduce cpu usage and uring_lock contention.
> */
> - if (ctx->flags & IORING_SETUP_IOPOLL &&
> - !(ctx->flags & IORING_SETUP_SQPOLL))
> - ctx->syscall_iopoll = 1;
> + if (ctx->flags & IORING_SETUP_IOPOLL) {
> + ctx->issue_flags |= IO_URING_F_SQE128;
> + if (!(ctx->flags & IORING_SETUP_SQPOLL))
> + ctx->syscall_iopoll = 1;
> + }
This does not look correct?
> ctx->compat = in_compat_syscall();
> + if (ctx->compat)
> + ctx->issue_flags |= IO_URING_F_COMPAT;
> + if (ctx->flags & IORING_SETUP_SQE128)
> + ctx->issue_flags |= IO_URING_F_SQE128;
> + if (ctx->flags & IORING_SETUP_CQE32)
> + ctx->issue_flags |= IO_URING_F_CQE32;
> +
> if (!ns_capable_noaudit(&init_user_ns, CAP_IPC_LOCK))
> ctx->user = get_uid(current_user());
>
> diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c
> index 8a38b9f75d841..dbc0bfbfd0f05 100644
> --- a/io_uring/uring_cmd.c
> +++ b/io_uring/uring_cmd.c
> @@ -158,19 +158,13 @@ int io_uring_cmd(struct io_kiocb *req, unsigned int issue_flags)
> if (ret)
> return ret;
>
> - if (ctx->flags & IORING_SETUP_SQE128)
> - issue_flags |= IO_URING_F_SQE128;
> - if (ctx->flags & IORING_SETUP_CQE32)
> - issue_flags |= IO_URING_F_CQE32;
> - if (ctx->compat)
> - issue_flags |= IO_URING_F_COMPAT;
> if (ctx->flags & IORING_SETUP_IOPOLL) {
> if (!file->f_op->uring_cmd_iopoll)
> return -EOPNOTSUPP;
> - issue_flags |= IO_URING_F_IOPOLL;
> req->iopoll_completed = 0;
> }
>
> + issue_flags |= ctx->issue_flags;
> ret = file->f_op->uring_cmd(ioucmd, issue_flags);
> if (ret == -EAGAIN) {
> if (!req_has_async_data(req)) {
I obviously like this idea, but it should be accompanied by getting rid
of ->compat and ->syscall_iopoll in the ctx as well?
--
Jens Axboe
