Re: [PATCH] io_uring: remove PF_EXITING checking in io_poll_rewait()

[Hao Xu <haoxu@linux.alibaba.com>]

在 2021/8/19 下午11:48, Jens Axboe 写道:
> We have two checks of task->flags & PF_EXITING left:
> 
> 1) In io_req_task_submit(), which is called in task_work and hence always
>     in the context of the original task. That means that
>     req->task == current, and hence checking ->flags is totally fine.
> 
> 2) In io_poll_rewait(), where we need to stop re-arming poll to prevent
>     it interfering with cancelation. Here, req->task is not necessarily
>     current, and hence the check is racy. Use the ctx refs state instead
>     to check if we need to cancel this request or not.
Hi Jens,
I saw cases that io_req_task_submit() and io_poll_rewait() in one
function, why one is safe and the other one not? btw, it seems both two
executes in task_work context..and task_work_add() may fail and then
work goes to system_wq, is that case safe?
> 
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> 
> ---
> 
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index 30edc329d803..ffce959c2370 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -2114,6 +2114,7 @@ static void io_req_task_submit(struct io_kiocb *req)
>   
>   	/* ctx stays valid until unlock, even if we drop all ours ctx->refs */
>   	mutex_lock(&ctx->uring_lock);
> +	/* req->task == current here, checking PF_EXITING is safe */
>   	if (likely(!(req->task->flags & PF_EXITING)))
>   		__io_queue_sqe(req);
>   	else
> @@ -4895,7 +4896,11 @@ static bool io_poll_rewait(struct io_kiocb *req, struct io_poll_iocb *poll)
>   {
>   	struct io_ring_ctx *ctx = req->ctx;
>   
> -	if (unlikely(req->task->flags & PF_EXITING))
> +	/*
> +	 * Pairs with spin_unlock() in percpu_ref_kill()
> +	 */
> +	smp_rmb();
> +	if (unlikely(percpu_ref_is_dying(&ctx->refs)))
>   		WRITE_ONCE(poll->canceled, true);
>   
>   	if (!req->result && !READ_ONCE(poll->canceled)) {
>