Re: [PATCH 1/2] io_uring: fix no lock protection for ctx->cq_extra

[Pavel Begunkov <asml.silence@gmail.com>]

On 11/25/21 09:21, Hao Xu wrote:
> ctx->cq_extra should be protected by completion lock so that the
> req_need_defer() does the right check.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Hao Xu <haoxu@linux.alibaba.com>
> ---
>   fs/io_uring.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index f666a0e7f5e8..ae9534382b26 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -6537,12 +6537,15 @@ static __cold void io_drain_req(struct io_kiocb *req)
>   	u32 seq = io_get_sequence(req);
>   
>   	/* Still need defer if there is pending req in defer list. */
> +	spin_lock(&ctx->completion_lock);
>   	if (!req_need_defer(req, seq) && list_empty_careful(&ctx->defer_list)) {
> +		spin_unlock(&ctx->completion_lock);

I haven't checked the sync assumptions, but it was as this since
the very beginning, so curious if you see any hangs or other
problems?

In any case, as drain is pushed to slow path, I'm all for
simplifying synchronisation here.

>   queue:
>   		ctx->drain_active = false;
>   		io_req_task_queue(req);
>   		return;
>   	}
> +	spin_unlock(&ctx->completion_lock);
>   
>   	ret = io_req_prep_async(req);
>   	if (ret) {
> 

-- 
Pavel Begunkov