Re: [RFC PATCH] io_uring: don't submit sqes when ctx->refs is dying

[Xiaoguang Wang <xiaoguang.wang@linux.alibaba.com>]

hi,

> hi,
> 
>> On 5/13/20 6:37 AM, Xiaoguang Wang wrote:
>>> When IORING_SETUP_SQPOLL is enabled, io_ring_ctx_wait_and_kill() will wait
>>> for sq thread to idle by busy loop:
>>>      while (ctx->sqo_thread && !wq_has_sleeper(&ctx->sqo_wait))
>>>          cond_resched();
>>> Above codes are not friendly, indeed I think this busy loop will introduce a
>>> cpu burst in current cpu, though it maybe short.
>>>
>>> In this patch, if ctx->refs is dying, we forbids sq_thread from submitting
>>> sqes anymore, just discard leftover sqes.
>>
>> I don't think this really changes anything. What happens if:
>>
>>> @@ -6051,7 +6053,8 @@ static int io_sq_thread(void *data)
>>>           }
>>>           mutex_lock(&ctx->uring_lock);
>>> -        ret = io_submit_sqes(ctx, to_submit, NULL, -1, true);
>>> +        if (likely(!percpu_ref_is_dying(&ctx->refs)))
>>> +            ret = io_submit_sqes(ctx, to_submit, NULL, -1, true);
>>>           mutex_unlock(&ctx->uring_lock);
>>>           timeout = jiffies + ctx->sq_thread_idle;
>>
>> You check for dying here, but that could change basically while you're
>> checking it. So you're still submitting sqes with a ref that's going
>> away. You've only reduced the window, you haven't eliminated it.
> Look at codes, we call percpu_ref_kill() under uring_lock, so isn't it safe
> to check the refs' dying status? Thanks.
Cloud you please have a look at my explanation again? Thanks.

Regards,
Xiaoguang Wang

> 
> Regards,
> Xiaouang Wang
> 
> 
>>