On 3/25/19 4:16 AM, Roman Penyaev wrote:
> Hi Jens,
>
> I gave a try to use io_uring and stumbled upon -EAGAIN on write path
> in direct mode if page cache is already populated or has been populated
> in-between by some buffered read. I'am talking about
> generic_file_direct_write() call, which checks filemap_range_has_page()
> on IOCB_NOWAIT path.
>
> To proceed further with tests I simply did the same thing, like you did
> in io_read(), and in case of -EAGAIN async worker does the rest. So the
> following chunk works well:
>
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index 6aaa30580a2b..ccb656168ae4 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -1022,6 +1022,8 @@ static int io_write(struct io_kiocb *req, const
> struct sqe_submit *s,
>
> ret = rw_verify_area(WRITE, file, &kiocb->ki_pos, iov_count);
> if (!ret) {
> + ssize_t ret2;
> +
> /*
> * Open-code file_start_write here to grab freeze
> protection,
> * which will be released by another thread in
> @@ -1036,7 +1038,19 @@ static int io_write(struct io_kiocb *req, const
> struct sqe_submit *s,
> SB_FREEZE_WRITE);
> }
> kiocb->ki_flags |= IOCB_WRITE;
> - io_rw_done(kiocb, call_write_iter(file, kiocb, &iter));
> +
> + ret2 = call_write_iter(file, kiocb, &iter);
> + if (!force_nonblock || ret2 != -EAGAIN) {
> + io_rw_done(kiocb, ret2);
> + } else {
> + /*
> + * If ->needs_lock is true, we're already in
> async
> + * context.
> + */
> + if (!s->needs_lock)
> + io_async_list_note(WRITE, req,
> iov_count);
> + ret = -EAGAIN;
> + }
>
>
> Does it make sense?
Yeah it makes sense, we need to punt that to async as well if we hit
EAGAIN at that point. Can you send a signed-off-by patch?
--
Jens Axboe
