* [PATCH] io_uring: no read-retry on -EAGAIN error and O_NONBLOCK marked file
@ 2020-09-02 16:00 Jens Axboe
2020-09-02 16:09 ` Pavel Begunkov
0 siblings, 1 reply; 3+ messages in thread
From: Jens Axboe @ 2020-09-02 16:00 UTC (permalink / raw)
To: io-uring; +Cc: Norman Maurer
Actually two things that need fixing up here:
- The io_rw_reissue() -EAGAIN retry is explicit to block devices and
regular files, so don't ever attempt to do that on other types of
files.
- If we hit -EAGAIN on a nonblock marked file, don't arm poll handler for
it. It should just complete with -EAGAIN.
Cc: [email protected]
Reported-by: Norman Maurer <[email protected]>
Signed-off-by: Jens Axboe <[email protected]>
---
diff --git a/fs/io_uring.c b/fs/io_uring.c
index b1ccd7072d93..65656102bbeb 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -2300,8 +2300,11 @@ static bool io_resubmit_prep(struct io_kiocb *req, int error)
static bool io_rw_reissue(struct io_kiocb *req, long res)
{
#ifdef CONFIG_BLOCK
+ umode_t mode = file_inode(req->file)->i_mode;
int ret;
+ if (!S_ISBLK(mode) && !S_ISREG(mode))
+ return false;
if ((res != -EAGAIN && res != -EOPNOTSUPP) || io_wq_current_is_worker())
return false;
@@ -3146,6 +3149,9 @@ static int io_read(struct io_kiocb *req, bool force_nonblock,
/* IOPOLL retry should happen for io-wq threads */
if (!force_nonblock && !(req->ctx->flags & IORING_SETUP_IOPOLL))
goto done;
+ /* no retry on NONBLOCK marked file */
+ if (req->file->f_flags & O_NONBLOCK)
+ goto done;
/* some cases will consume bytes even on error returns */
iov_iter_revert(iter, iov_count - iov_iter_count(iter));
ret = io_setup_async_rw(req, iovec, inline_vecs, iter, false);
@@ -3291,8 +3297,12 @@ static int io_write(struct io_kiocb *req, bool force_nonblock,
/* IOPOLL retry should happen for io-wq threads */
if ((req->ctx->flags & IORING_SETUP_IOPOLL) && ret2 == -EAGAIN)
goto copy_iov;
+done:
kiocb_done(kiocb, ret2, cs);
} else {
+ /* no retry on NONBLOCK marked file */
+ if (req->file->f_flags & O_NONBLOCK)
+ goto done;
copy_iov:
/* some cases will consume bytes even on error returns */
iov_iter_revert(iter, iov_count - iov_iter_count(iter));
--
Jens Axboe
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] io_uring: no read-retry on -EAGAIN error and O_NONBLOCK marked file
2020-09-02 16:00 [PATCH] io_uring: no read-retry on -EAGAIN error and O_NONBLOCK marked file Jens Axboe
@ 2020-09-02 16:09 ` Pavel Begunkov
2020-09-02 16:32 ` Jens Axboe
0 siblings, 1 reply; 3+ messages in thread
From: Pavel Begunkov @ 2020-09-02 16:09 UTC (permalink / raw)
To: Jens Axboe, io-uring; +Cc: Norman Maurer
On 02/09/2020 19:00, Jens Axboe wrote:
> Actually two things that need fixing up here:
>
> - The io_rw_reissue() -EAGAIN retry is explicit to block devices and
> regular files, so don't ever attempt to do that on other types of
> files.
>
> - If we hit -EAGAIN on a nonblock marked file, don't arm poll handler for
> it. It should just complete with -EAGAIN.
>
> Cc: [email protected]
> Reported-by: Norman Maurer <[email protected]>
> Signed-off-by: Jens Axboe <[email protected]>
>
> ---
>
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index b1ccd7072d93..65656102bbeb 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -2300,8 +2300,11 @@ static bool io_resubmit_prep(struct io_kiocb *req, int error)
> static bool io_rw_reissue(struct io_kiocb *req, long res)
> {
> #ifdef CONFIG_BLOCK
> + umode_t mode = file_inode(req->file)->i_mode;
> int ret;
>
> + if (!S_ISBLK(mode) && !S_ISREG(mode))
> + return false;
> if ((res != -EAGAIN && res != -EOPNOTSUPP) || io_wq_current_is_worker())
> return false;
>
> @@ -3146,6 +3149,9 @@ static int io_read(struct io_kiocb *req, bool force_nonblock,
> /* IOPOLL retry should happen for io-wq threads */
> if (!force_nonblock && !(req->ctx->flags & IORING_SETUP_IOPOLL))
> goto done;
> + /* no retry on NONBLOCK marked file */
> + if (req->file->f_flags & O_NONBLOCK)
> + goto done;
Looks like it works with open(O_NONBLOCK) but not with IOCB_NOWAIT in the
request's flags. Is that so?
> /* some cases will consume bytes even on error returns */
> iov_iter_revert(iter, iov_count - iov_iter_count(iter));
> ret = io_setup_async_rw(req, iovec, inline_vecs, iter, false);
> @@ -3291,8 +3297,12 @@ static int io_write(struct io_kiocb *req, bool force_nonblock,
> /* IOPOLL retry should happen for io-wq threads */
> if ((req->ctx->flags & IORING_SETUP_IOPOLL) && ret2 == -EAGAIN)
> goto copy_iov;
> +done:
> kiocb_done(kiocb, ret2, cs);
> } else {
> + /* no retry on NONBLOCK marked file */
> + if (req->file->f_flags & O_NONBLOCK)
> + goto done;
> copy_iov:
> /* some cases will consume bytes even on error returns */
> iov_iter_revert(iter, iov_count - iov_iter_count(iter));
>
--
Pavel Begunkov
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] io_uring: no read-retry on -EAGAIN error and O_NONBLOCK marked file
2020-09-02 16:09 ` Pavel Begunkov
@ 2020-09-02 16:32 ` Jens Axboe
0 siblings, 0 replies; 3+ messages in thread
From: Jens Axboe @ 2020-09-02 16:32 UTC (permalink / raw)
To: Pavel Begunkov, io-uring; +Cc: Norman Maurer
On 9/2/20 10:09 AM, Pavel Begunkov wrote:
> On 02/09/2020 19:00, Jens Axboe wrote:
>> Actually two things that need fixing up here:
>>
>> - The io_rw_reissue() -EAGAIN retry is explicit to block devices and
>> regular files, so don't ever attempt to do that on other types of
>> files.
>>
>> - If we hit -EAGAIN on a nonblock marked file, don't arm poll handler for
>> it. It should just complete with -EAGAIN.
>>
>> Cc: [email protected]
>> Reported-by: Norman Maurer <[email protected]>
>> Signed-off-by: Jens Axboe <[email protected]>
>>
>> ---
>>
>> diff --git a/fs/io_uring.c b/fs/io_uring.c
>> index b1ccd7072d93..65656102bbeb 100644
>> --- a/fs/io_uring.c
>> +++ b/fs/io_uring.c
>> @@ -2300,8 +2300,11 @@ static bool io_resubmit_prep(struct io_kiocb *req, int error)
>> static bool io_rw_reissue(struct io_kiocb *req, long res)
>> {
>> #ifdef CONFIG_BLOCK
>> + umode_t mode = file_inode(req->file)->i_mode;
>> int ret;
>>
>> + if (!S_ISBLK(mode) && !S_ISREG(mode))
>> + return false;
>> if ((res != -EAGAIN && res != -EOPNOTSUPP) || io_wq_current_is_worker())
>> return false;
>>
>> @@ -3146,6 +3149,9 @@ static int io_read(struct io_kiocb *req, bool force_nonblock,
>> /* IOPOLL retry should happen for io-wq threads */
>> if (!force_nonblock && !(req->ctx->flags & IORING_SETUP_IOPOLL))
>> goto done;
>> + /* no retry on NONBLOCK marked file */
>> + if (req->file->f_flags & O_NONBLOCK)
>> + goto done;
>
> Looks like it works with open(O_NONBLOCK) but not with IOCB_NOWAIT in the
> request's flags. Is that so?
The IOCB_NOWAIT case should already be fine, it's just the O_NONBLOCK that
isn't covered. IOCB_NOWAIT on a socket unhelpfully fails anyway, since
FMODE_NOWAIT isn't available. Arguably something that should be fixed
separately.
--
Jens Axboe
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-09-02 16:32 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-09-02 16:00 [PATCH] io_uring: no read-retry on -EAGAIN error and O_NONBLOCK marked file Jens Axboe
2020-09-02 16:09 ` Pavel Begunkov
2020-09-02 16:32 ` Jens Axboe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox