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

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

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.

Regards,
Xiaouang Wang


>