From 14333b5b72f2d9f9e12d2abe72bda4ee6cc3e45c Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Mon, 17 Aug 2020 21:03:27 -0700 Subject: [PATCH 2/2] io_uring: internally retry short reads We've had a few application cases of not handling short reads properly, and it is understandable as short reads aren't really expected if the application isn't doing non-blocking IO. Now that we retain the iov_iter over retries, we can implement internal retry pretty trivially. This ensures that we don't return a short read, even for buffered reads on page cache conflicts. Cleanup the deep nesting and hard to read nature of io_read() as well, it's much more straight forward now to read and understand. Added a few comments explaining the logic as well. Signed-off-by: Jens Axboe --- fs/io_uring.c | 61 +++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 45 insertions(+), 16 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index bdb88146b285..618a4ca29159 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -487,6 +487,7 @@ struct io_async_rw { struct iovec fast_iov[UIO_FASTIOV]; const struct iovec *free_iovec; struct iov_iter iter; + size_t bytes_done; }; struct io_async_ctx { @@ -2160,6 +2161,14 @@ static void kiocb_done(struct kiocb *kiocb, ssize_t ret) { struct io_kiocb *req = container_of(kiocb, struct io_kiocb, rw.kiocb); + /* add previously done IO, if any */ + if (req->io && req->io->rw.bytes_done > 0) { + if (ret < 0) + ret = req->io->rw.bytes_done; + else + ret += req->io->rw.bytes_done; + } + if (req->flags & REQ_F_CUR_POS) req->file->f_pos = kiocb->ki_pos; if (ret >= 0 && kiocb->ki_complete == io_complete_rw) @@ -2507,6 +2516,7 @@ static void io_req_map_rw(struct io_kiocb *req, const struct iovec *iovec, memcpy(&rw->iter, iter, sizeof(*iter)); rw->free_iovec = NULL; + rw->bytes_done = 0; /* can only be fixed buffers, no need to do anything */ if (iter->type == ITER_BVEC) return; @@ -2543,9 +2553,9 @@ static int io_alloc_async_ctx(struct io_kiocb *req) static int io_setup_async_rw(struct io_kiocb *req, const struct iovec *iovec, const struct iovec *fast_iov, - struct iov_iter *iter) + struct iov_iter *iter, bool force) { - if (!io_op_defs[req->opcode].async_ctx) + if (!force && !io_op_defs[req->opcode].async_ctx) return 0; if (!req->io) { if (__io_alloc_async_ctx(req)) @@ -2608,6 +2618,7 @@ static int io_read(struct io_kiocb *req, bool force_nonblock) req->result = 0; io_size = ret; + ret = 0; if (req->flags & REQ_F_LINK_HEAD) req->result = io_size; @@ -2630,21 +2641,39 @@ static int io_read(struct io_kiocb *req, bool force_nonblock) else ret2 = -EINVAL; - /* Catch -EAGAIN return for forced non-blocking submission */ - if (!force_nonblock || ret2 != -EAGAIN) { - kiocb_done(kiocb, ret2); - } else { -copy_iov: - ret = io_setup_async_rw(req, iovec, inline_vecs, iter); - if (ret) - goto out_free; - /* any defer here is final, must blocking retry */ - if (!(req->flags & REQ_F_NOWAIT) && - !file_can_poll(req->file)) - req->flags |= REQ_F_MUST_PUNT; - return -EAGAIN; + if (!ret2) { + goto done; + } else if (ret2 == -EIOCBQUEUED) { + ret = 0; + goto out_free; + } else if (ret2 == -EAGAIN) { + ret2 = 0; + goto copy_iov; + } else if (ret2 < 0) { + ret = ret2; + goto out_free; + } + + /* read it all, or we did blocking attempt. no retry. */ + if (!iov_iter_count(iter) || !force_nonblock) { + ret = ret2; + goto done; } + +copy_iov: + ret = io_setup_async_rw(req, iovec, inline_vecs, iter, true); + if (ret) + goto out_free; + req->io->rw.bytes_done += ret2; + /* any defer here is final, must blocking retry */ + if (!(req->flags & REQ_F_NOWAIT) && + !file_can_poll(req->file)) + req->flags |= REQ_F_MUST_PUNT; + return -EAGAIN; } +done: + kiocb_done(kiocb, ret); + ret = 0; out_free: if (!(req->flags & REQ_F_NEED_CLEANUP)) kfree(iovec); @@ -2762,7 +2791,7 @@ static int io_write(struct io_kiocb *req, bool force_nonblock) kiocb_done(kiocb, ret2); } else { copy_iov: - ret = io_setup_async_rw(req, iovec, inline_vecs, iter); + ret = io_setup_async_rw(req, iovec, inline_vecs, iter, false); if (ret) goto out_free; /* any defer here is final, must blocking retry */ -- 2.28.0