public inbox for [email protected]
 help / color / mirror / Atom feed
* [RFC PATCH 0/1] io_uring: honor I/O nowait flag for read/write
@ 2023-04-21 17:28 Chaitanya Kulkarni
  2023-04-21 17:28 ` [RFC PATCH 1/1] " Chaitanya Kulkarni
  0 siblings, 1 reply; 3+ messages in thread
From: Chaitanya Kulkarni @ 2023-04-21 17:28 UTC (permalink / raw)
  To: io-uring; +Cc: axboe, asml.silence, Chaitanya Kulkarni

Hi,

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.

Below is the deatailed testing log.

-ck

Chaitanya Kulkarni (1):
  io_uring: honor I/O nowait flag for read/write

 io_uring/rw.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

$ sh test-iouring-no-wait
* Without this patch :-
#######################
-------------------libaio nowait = 0--------------------------

++ grep -e ioengine -e rw -e nowait fio/randwrite-libaio-nowait-0.fio
ioengine=libaio
rw=randwrite
nowait=0   <-----------------------------
overwrite=0
++ fio fio/randwrite-libaio-nowait-0.fio --filename=/dev/nullb0
++ grep err
RANDWRITE: (groupid=0, jobs=1): err= 0: pid=40835: Thu Apr 20 21:31:09 2023
++ dmesg -c
[17978.613789] null_blk: disk nullb0 created
[17978.613814] null_blk: module loaded
__________________________________________________________________________
[17978.796560] null_blk: fio:null_handle_rq 1288 *NOWAIT=FALSE* REQ_OP_WRITE

-------------------libaio nowait = 1--------------------------

++ blkdiscard /dev/nullb0
++ grep -e ioengine -e rw -e nowait fio/randwrite-libaio-nowait-1.fio
ioengine=libaio
rw=randwrite
nowait=1   <-----------------------------
overwrite=0
++ fio fio/randwrite-libaio-nowait-1.fio --filename=/dev/nullb0
++ grep err
RANDWRITE: (groupid=0, jobs=1): err= 0: pid=40842: Thu Apr 20 21:31:09 2023
++ dmesg -c
__________________________________________________________________________
[17979.019595] null_blk: fio:null_handle_rq 1288 *NOWAIT=TRUE* REQ_OP_WRITE

-------------------iouring nowait = 0--------------------------

++ blkdiscard /dev/nullb0
++ grep -e ioengine -e rw -e nowait fio/randwrite-iouring-nowait-0.fio
ioengine=io_uring
rw=randwrite
nowait=0   <-----------------------------
overwrite=0
++ fio fio/randwrite-iouring-nowait-0.fio --filename=/dev/nullb0
++ grep err
RANDWRITE: (groupid=0, jobs=1): err= 0: pid=40849: Thu Apr 20 21:31:10 2023
++ dmesg -c
__________________________________________________________________________
[17979.242849] null_blk: fio:null_handle_rq 1288 *NOWAIT=TRUE* REQ_OP_WRITE

-------------------iouring nowait = 1--------------------------

++ blkdiscard /dev/nullb0
++ grep -e ioengine -e rw -e nowait fio/randwrite-iouring-nowait-1.fio
ioengine=io_uring
rw=randwrite
nowait=1   <-----------------------------
overwrite=0
++ fio fio/randwrite-iouring-nowait-1.fio --filename=/dev/nullb0
++ grep err
RANDWRITE: (groupid=0, jobs=1): err= 0: pid=40856: Thu Apr 20 21:31:10 2023
++ dmesg -c
__________________________________________________________________________
[17979.454102] null_blk: fio:null_handle_rq 1288 *NOWAIT=TRUE* REQ_OP_WRITE

* With this patch :-
####################
-------------------iouring nowait = 0--------------------------
+ blkdiscard /dev/nullb0
+ grep -e ioengine -e rw -e nowait fio/randwrite-iouring-nowait-0.fio
ioengine=io_uring
rw=randwrite
nowait=0   <-----------------------------
overwrite=0
+ fio fio/randwrite-iouring-nowait-0.fio --filename=/dev/nullb0
+ grep err
RANDWRITE: (groupid=0, jobs=1): err= 0: pid=2788: Thu Apr 20 23:35:40 2023
+ dmesg -c
__________________________________________________________________________
[  164.255136] null_blk: fio:null_handle_rq 1307 *REQ_NOWAIT=FALSE* WRITE
-------------------iouring nowait = 1--------------------------
+ blkdiscard /dev/nullb0
+ grep -e ioengine -e rw -e nowait fio/randwrite-iouring-nowait-1.fio
ioengine=io_uring
rw=randwrite
nowait=1   <-----------------------------
overwrite=0
+ fio fio/randwrite-iouring-nowait-1.fio --filename=/dev/nullb0
+ grep err
RANDWRITE: (groupid=0, jobs=1): err= 0: pid=2795: Thu Apr 20 23:35:41 2023
+ dmesg -c
__________________________________________________________________________
[  164.467420] null_blk: fio:null_handle_rq 1307 *REQ_NOWAIT=TRUE* WRITE


-- 
2.40.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [RFC PATCH 1/1] io_uring: honor I/O nowait flag for read/write
  2023-04-21 17:28 [RFC PATCH 0/1] io_uring: honor I/O nowait flag for read/write Chaitanya Kulkarni
@ 2023-04-21 17:28 ` Chaitanya Kulkarni
  2023-04-21 17:35   ` Jens Axboe
  0 siblings, 1 reply; 3+ messages in thread
From: Chaitanya Kulkarni @ 2023-04-21 17:28 UTC (permalink / raw)
  To: io-uring; +Cc: axboe, asml.silence, Chaitanya Kulkarni

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 <[email protected]>
---
 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;
-- 
2.40.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [RFC PATCH 1/1] io_uring: honor I/O nowait flag for read/write
  2023-04-21 17:28 ` [RFC PATCH 1/1] " Chaitanya Kulkarni
@ 2023-04-21 17:35   ` Jens Axboe
  0 siblings, 0 replies; 3+ messages in thread
From: Jens Axboe @ 2023-04-21 17:35 UTC (permalink / raw)
  To: Chaitanya Kulkarni, io-uring; +Cc: asml.silence

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 <[email protected]>
> ---
>  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


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-04-21 17:36 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-21 17:28 [RFC PATCH 0/1] io_uring: honor I/O nowait flag for read/write Chaitanya Kulkarni
2023-04-21 17:28 ` [RFC PATCH 1/1] " Chaitanya Kulkarni
2023-04-21 17:35   ` Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox