Re: [RFC PATCH 1/1] io_uring: honor I/O nowait flag for read/write

[Jens Axboe <axboe@kernel.dk>]

On 4/21/23 11:28?AM, Chaitanya Kulkarni wrote:
> When IO_URING_F_NONBLOCK is set on io_kiocb req->flag in io_write() or
> io_read() IOCB_NOWAIT is set for kiocb when passed it to the respective
> rw_iter callback. This sets REQ_NOWAIT for underlaying I/O. The result
> is low level driver always sees block layer request as REQ_NOWAIT even
> if user has submitted request with nowait = 0 e.g. fio nowait=0.
> 
> That is not consistent behaviour with other fio ioengine such as
> libaio as it will issue non REQ_NOWAIT request with REQ_NOWAIT:-
> 
> libaio nowait = 0:-
> null_blk: fio:null_handle_rq 1288 *NOWAIT=FALSE* REQ_OP_WRITE
> 
> libaio nowait = 1:-
> null_blk: fio:null_handle_rq 1288 *NOWAIT=TRUE* REQ_OP_WRITE
> 
> * Without this patch with fio ioengine io_uring :-
> ---------------------------------------------------
> 
> iouring nowait = 0:-
> null_blk: fio:null_handle_rq 1288 *NOWAIT=TRUE* REQ_OP_WRITE
> 
> iouring nowait = 1:-
> null_blk: fio:null_handle_rq 1288 *NOWAIT=TRUE* REQ_OP_WRITE
> 
> * With this patch with fio ioengine io_uring :-
> ---------------------------------------------------
> 
> iouring nowait = 0:-
> null_blk: fio:null_handle_rq 1307 *REQ_NOWAIT=FALSE* WRITE
> 
> iouring nowait = 1:
> null_blk: fio:null_handle_rq 1307 *REQ_NOWAIT=TRUE* WRITE
> 
> Instead of only relying on IO_URING_F_NONBLOCK blindly in io_read() and
> io_write(), also make sure io_kiocb->io_rw->flags is set to RWF_NOWAIT
> before we mark kiocb->ki_flags = IOCB_NOWAIT.
> 
> Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
> ---
>  io_uring/rw.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/io_uring/rw.c b/io_uring/rw.c
> index 3f118ed46e4f..4b3a2c1df5f2 100644
> --- a/io_uring/rw.c
> +++ b/io_uring/rw.c
> @@ -745,7 +745,7 @@ int io_read(struct io_kiocb *req, unsigned int issue_flags)
>  	}
>  	req->cqe.res = iov_iter_count(&s->iter);
>  
> -	if (force_nonblock) {
> +	if (force_nonblock && (rw->flags & RWF_NOWAIT)) {
>  		/* If the file doesn't support async, just async punt */
>  		if (unlikely(!io_file_supports_nowait(req))) {
>  			ret = io_setup_async_rw(req, iovec, s, true);
> @@ -877,7 +877,7 @@ int io_write(struct io_kiocb *req, unsigned int issue_flags)
>  	}
>  	req->cqe.res = iov_iter_count(&s->iter);
>  
> -	if (force_nonblock) {
> +	if (force_nonblock && (rw->flags & RWF_NOWAIT)) {
>  		/* If the file doesn't support async, just async punt */
>  		if (unlikely(!io_file_supports_nowait(req)))
>  			goto copy_iov;

This is wrong. libaio doesn't care if it blocks for submission, and this
is actually one of the complains of aio/libaio. Is it async? Maybe?
io_uring, on the other hand, tries much harder to not block for
submission. Because if you do block, you've now starved your issue
pipeline. And how do you do that? By always having NOWAIT set on the
request, unless you are in a context (io-wq) where you want to block in
case you have to.

This has _nothing_ to do with the user setting RWF_NOWAIT or not, only
change for that is that if we did have that set, then we should
obviously not retry from blocking context. Rather, the io should be
complete with whatever we originally got (typically -EAGAIN).

-- 
Jens Axboe