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

[Jens Axboe <axboe@kernel.dk>]

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.

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);
> +		    }
>  		}
>  	}
>  
> 


-- 
Jens Axboe