Re: [PATCH 2/4] io_uring: move locking inside overflow posting

[Jens Axboe <axboe@kernel.dk>]

On 5/14/25 2:07 AM, Pavel Begunkov wrote:
> A preparation patch moving locking protecting the overflow list into
> io_cqring_event_overflow(). The locking needs to be conditional because
> callers might already hold the lock. It's not the prettiest option, but
> it's not much different from the current state. Hopefully, one day we'll
> get rid of this nasty locking pattern.
> 
> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> ---
>  io_uring/io_uring.c | 53 +++++++++++++++++++++++++++------------------
>  1 file changed, 32 insertions(+), 21 deletions(-)
> 
> diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
> index 068e140b6bd8..5b253e2b6c49 100644
> --- a/io_uring/io_uring.c
> +++ b/io_uring/io_uring.c
> @@ -718,8 +718,9 @@ static __cold void io_uring_drop_tctx_refs(struct task_struct *task)
>  	}
>  }
>  
> -static bool io_cqring_event_overflow(struct io_ring_ctx *ctx, u64 user_data,
> -				     s32 res, u32 cflags, u64 extra1, u64 extra2)
> +static bool __io_cqring_event_overflow(struct io_ring_ctx *ctx,
> +				       u64 user_data, s32 res, u32 cflags,
> +				       u64 extra1, u64 extra2)
>  {
>  	struct io_overflow_cqe *ocqe;
>  	size_t ocq_size = sizeof(struct io_overflow_cqe);
> @@ -760,6 +761,24 @@ static bool io_cqring_event_overflow(struct io_ring_ctx *ctx, u64 user_data,
>  	return true;
>  }
>  
> +static bool io_cqring_event_overflow(struct io_ring_ctx *ctx, bool locked,
> +				     u64 user_data, s32 res, u32 cflags,
> +				     u64 extra1, u64 extra2)
> +{
> +	bool queued;
> +
> +	if (locked) {
> +		queued = __io_cqring_event_overflow(ctx, user_data, res, cflags,
> +						    extra1, extra2);
> +	} else {
> +		spin_lock(&ctx->completion_lock);
> +		queued = __io_cqring_event_overflow(ctx, user_data, res, cflags,
> +						    extra1, extra2);
> +		spin_unlock(&ctx->completion_lock);
> +	}
> +	return queued;
> +}

Really not a fan of passing in locking state and having a split helper
like that. It's also pretty unwieldy with 7 arguments.

Overall, why do we care about atomic vs non-atomic allocations for
overflows? If you're hitting overflow, you've messed up... And if it's
about cost, gfp atomic alloc will be predictable, vs GFP_KERNEL which
can be a lot more involved.

-- 
Jens Axboe