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

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

hi,

> On 5/16/20 3:23 AM, Xiaoguang Wang wrote:
>> 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.
> 
> Sorry for the delay - you are right, we only kill it inside the ring
> mutex, so should be safe to check. Can we get by with just the very last
> check instead of checking all of the cases?
Sure, I'll send V2 soon, and thanks for reviewing, I need this patch because I'm
writing a prototype to implement idea in this url:
     https://lore.kernel.org/io-uring/c94098d2-279e-a552-91ec-8a8f177d770a@linux.alibaba.com/T/#t
which needs above patch.

Regards,
Xiaoguang Wang
>