Hi,
> -----Original Message-----
> From: Jens Axboe <axboe@xxxxxxxxx>
> Sent: Tuesday, December 31, 2024 8:37 AM
> To: io-uring <io-uring@xxxxxxxxxxxxxxx>
> Subject: [PATCH for-next] io_uring: ensure io_queue_deferred() is out-of-line
>
> This is not the hot path, it's a slow path. Yet the locking for it is in the hot path,
> and __cold does not prevent it from being inlined.
>
> Move the locking to the function itself, and mark it noinline as well to avoid it
> polluting the icache of the hot path.
>
> Signed-off-by: Jens Axboe <axboe@xxxxxxxxx>
>
> ---
>
> diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c index
> 42d4cc5da73b..db198bd435b5 100644
> --- a/io_uring/io_uring.c
> +++ b/io_uring/io_uring.c
> @@ -550,8 +550,9 @@ void io_req_queue_iowq(struct io_kiocb *req)
> io_req_task_work_add(req);
> }
>
> -static __cold void io_queue_deferred(struct io_ring_ctx *ctx)
> +static __cold noinline void io_queue_deferred(struct io_ring_ctx *ctx)
> {
> + spin_lock(&ctx->completion_lock);
Just a digression, whether the io_uring subsystem welcomes scope-based cleanup helpers, this is somewhat
controversial in other submodules.
> while (!list_empty(&ctx->defer_list)) {
> struct io_defer_entry *de = list_first_entry(&ctx->defer_list,
> struct io_defer_entry, list);
> @@ -562,6 +563,7 @@ static __cold void io_queue_deferred(struct io_ring_ctx
> *ctx)
> io_req_task_queue(de->req);
> kfree(de);
> }
> + spin_unlock(&ctx->completion_lock);
> }
>
> void __io_commit_cqring_flush(struct io_ring_ctx *ctx) @@ -570,11 +572,8
> @@ void __io_commit_cqring_flush(struct io_ring_ctx *ctx)
> io_poll_wq_wake(ctx);
> if (ctx->off_timeout_used)
> io_flush_timeouts(ctx);
> - if (ctx->drain_active) {
> - spin_lock(&ctx->completion_lock);
> + if (ctx->drain_active)
> io_queue_deferred(ctx);
> - spin_unlock(&ctx->completion_lock);
> - }
> if (ctx->has_evfd)
> io_eventfd_flush_signal(ctx);
> }
>
> --
> Jens Axboe
>
Reviewed-by: Li Zetao<lizetao1@xxxxxxxxxx>
---
Li Zetao
