| Message ID | 20230421172822.8053-2-kch@nvidia.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | io_uring: honor I/O nowait flag for read/write | expand |
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).
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;
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(-)