Re: [PATCH] io_uring:IORING_SETUP_SQPOLL don't need to enter io_cqring_wait

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

hi,

> On 4/7/20 1:44 AM, wu860403@gmail.com wrote:
>> From: Liming Wu <19092205@suning.com>
>>
>> When SETUP_IOPOLL and SETUP_SQPOLL are both enabled, app don't
>> need to enter io_cqring_wait too. If I misunderstand, please give
>> me some advise.
> 
> The logic should be as follows:
> 
> flags			method
> ---------------------------------------------
> 0			io_cqring_wait()
> IOPOLL			io_iopoll_check()
> IOPOLL | SQPOLL		io_cqring_wait()
> SQPOLL			io_cqring_wait()
> 
> The reasoning being that we do want to enter cqring_wait() for SQPOLL,
> as the application may want to wait for completions. Even with IOPOLL
> set. As far as I can tell, the current code is correct, as long as we
> know SQPOLL will always poll for events for us.
Yes, agree.

Regards,
Xiaoguang Wang

> 
> So I'm curious why you think your patch is needed? Leaving it below and
> CC'ing Xiaoguang, who made the most recent change, so he can comment.
> 
>> Signed-off-by Liming Wu <19092205@suning.com>
>> ---
>>   io_uring.c | 11 ++++++-----
>>   1 file changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/io_uring.c b/io_uring.c
>> index b12d33b..36e884f 100644
>> --- a/io_uring.c
>> +++ b/io_uring.c
>> @@ -7418,11 +7418,12 @@ SYSCALL_DEFINE6(io_uring_enter, unsigned int, fd, u32, to_submit,
>>   		 * polling again, they can rely on io_sq_thread to do polling
>>   		 * work, which can reduce cpu usage and uring_lock contention.
>>   		 */
>> -		if (ctx->flags & IORING_SETUP_IOPOLL &&
>> -		    !(ctx->flags & IORING_SETUP_SQPOLL)) {
>> -			ret = io_iopoll_check(ctx, &nr_events, min_complete);
>> -		} else {
>> -			ret = io_cqring_wait(ctx, min_complete, sig, sigsz);
>> +		if (!(ctx->flags & IORING_SETUP_SQPOLL)) {
>> +		    if (ctx->flags & IORING_SETUP_IOPOLL) {
>> +		    	ret = io_iopoll_check(ctx, &nr_events, min_complete);
>> +		    } else {
>> +		    	ret = io_cqring_wait(ctx, min_complete, sig, sigsz);
>> +		    }
>>   		}
>>   	}
>>   
>>
> 
>