Re: [PATCH 3/4] io_uring: tweak iopoll return for REQ_F_CQE_SKIP

[Jens Axboe <axboe@kernel.dk>]

On 12/4/21 1:49 PM, Pavel Begunkov wrote:
> Currently, IOPOLL returns the number of completed requests, but with
> REQ_F_CQE_SKIP there are not the same thing anymore. That may be
> confusing as non-iopoll wait cares only about CQEs, so make io_do_iopoll
> return the number of posted CQEs.
> 
> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> ---
>  fs/io_uring.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index 64add8260abb..ea7a0daa0b3b 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -2538,10 +2538,10 @@ static int io_do_iopoll(struct io_ring_ctx *ctx, bool force_nonspin)
>  		/* order with io_complete_rw_iopoll(), e.g. ->result updates */
>  		if (!smp_load_acquire(&req->iopoll_completed))
>  			break;
> +		if (unlikely(req->flags & REQ_F_CQE_SKIP))
> +			continue;
>  
> -		if (!(req->flags & REQ_F_CQE_SKIP))
> -			__io_fill_cqe(ctx, req->user_data, req->result,
> -				      io_put_kbuf(req));
> +		__io_fill_cqe(ctx, req->user_data, req->result, io_put_kbuf(req));
>  		nr_events++;
>  	}
>  

Not sure I follow the logic behind this change. Places like
io_iopoll_try_reap_events() just need a "did we find anything" return,
which is independent on whether or not we actually posted CQEs or not.
Other callers either don't care what the return value is or if it's < 0
or not (which this change won't affect).

I feel like I'm missing something here, or that the commit message
better needs to explain why this change is done.

-- 
Jens Axboe