From: Pavel Begunkov <asml.silence@gmail.com>
To: David Stevens <stevensd@chromium.org>, io-uring@vger.kernel.org
Cc: Jens Axboe <axboe@kernel.dk>
Subject: Re: [PATCH] io_uring: fix short read/write with linked ops
Date: Mon, 3 Oct 2022 11:35:12 +0100 [thread overview]
Message-ID: <6932613a-e900-3cd3-cecf-5b982ae84a19@gmail.com> (raw)
In-Reply-To: <20221003091923.2096150-1-stevensd@google.com>
On 10/3/22 10:19, David Stevens wrote:
> From: David Stevens <stevensd@chromium.org>
>
> When continuing a short read/write, account for any completed bytes when
> calculating the operation's target length. The operation's actual
> accumulated length is fixed up by kiocb_done, and the target and actual
> lengths are then compared by __io_complete_rw_common. That function
> already propagated the actual length to userspace, but the incorrect
> target length was causing it to always cancel linked operations, even
> with a successfully completed read/write.
The issue looks same as fixed with
https://git.kernel.dk/cgit/linux-block/commit/?h=for-6.1/io_uring&id=bf68b5b34311ee57ed40749a1257a30b46127556
Can you check if for-6.1 works for you?
git://git.kernel.dk/linux.git for-6.1/io_uring
https://git.kernel.dk/cgit/linux-block/log/?h=for-6.1/io_uring
> Fixes: 227c0c9673d8 ("io_uring: internally retry short reads")
> Signed-off-by: David Stevens <stevensd@chromium.org>
> ---
> io_uring/rw.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/io_uring/rw.c b/io_uring/rw.c
> index 76ebcfebc9a6..aa9967a52dfd 100644
> --- a/io_uring/rw.c
> +++ b/io_uring/rw.c
> @@ -706,13 +706,14 @@ int io_read(struct io_kiocb *req, unsigned int issue_flags)
> struct kiocb *kiocb = &rw->kiocb;
> bool force_nonblock = issue_flags & IO_URING_F_NONBLOCK;
> struct io_async_rw *io;
> - ssize_t ret, ret2;
> + ssize_t ret, ret2, target_len;
> loff_t *ppos;
>
> if (!req_has_async_data(req)) {
> ret = io_import_iovec(READ, req, &iovec, s, issue_flags);
> if (unlikely(ret < 0))
> return ret;
> + target_len = iov_iter_count(&s->iter);
> } else {
> io = req->async_data;
> s = &io->s;
> @@ -733,6 +734,7 @@ int io_read(struct io_kiocb *req, unsigned int issue_flags)
> * need to make this conditional.
> */
> iov_iter_restore(&s->iter, &s->iter_state);
> + target_len = iov_iter_count(&s->iter) + io->bytes_done;
> iovec = NULL;
> }
> ret = io_rw_init_file(req, FMODE_READ);
> @@ -740,7 +742,7 @@ int io_read(struct io_kiocb *req, unsigned int issue_flags)
> kfree(iovec);
> return ret;
> }
> - req->cqe.res = iov_iter_count(&s->iter);
> + req->cqe.res = target_len;
>
> if (force_nonblock) {
> /* If the file doesn't support async, just async punt */
> @@ -850,18 +852,20 @@ int io_write(struct io_kiocb *req, unsigned int issue_flags)
> struct iovec *iovec;
> struct kiocb *kiocb = &rw->kiocb;
> bool force_nonblock = issue_flags & IO_URING_F_NONBLOCK;
> - ssize_t ret, ret2;
> + ssize_t ret, ret2, target_len;
> loff_t *ppos;
>
> if (!req_has_async_data(req)) {
> ret = io_import_iovec(WRITE, req, &iovec, s, issue_flags);
> if (unlikely(ret < 0))
> return ret;
> + target_len = iov_iter_count(&s->iter);
> } else {
> struct io_async_rw *io = req->async_data;
>
> s = &io->s;
> iov_iter_restore(&s->iter, &s->iter_state);
> + target_len = iov_iter_count(&s->iter) + io->bytes_done;
> iovec = NULL;
> }
> ret = io_rw_init_file(req, FMODE_WRITE);
> @@ -869,7 +873,7 @@ int io_write(struct io_kiocb *req, unsigned int issue_flags)
> kfree(iovec);
> return ret;
> }
> - req->cqe.res = iov_iter_count(&s->iter);
> + req->cqe.res = target_len;
>
> if (force_nonblock) {
> /* If the file doesn't support async, just async punt */
--
Pavel Begunkov
next prev parent reply other threads:[~2022-10-03 10:40 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-03 9:19 [PATCH] io_uring: fix short read/write with linked ops David Stevens
2022-10-03 10:35 ` Pavel Begunkov [this message]
2022-10-04 2:17 ` David Stevens
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=6932613a-e900-3cd3-cecf-5b982ae84a19@gmail.com \
--to=asml.silence@gmail.com \
--cc=axboe@kernel.dk \
--cc=io-uring@vger.kernel.org \
--cc=stevensd@chromium.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.