All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ming Lei <ming.lei@redhat.com>
To: Jens Axboe <axboe@kernel.dk>, io-uring@vger.kernel.org
Cc: Ming Lei <ming.lei@redhat.com>, Mike Snitzer <snitzer@kernel.org>
Subject: [RFC PATCH] io_uring: reissue in case -EAGAIN is returned after io issue returns
Date: Sun,  3 Apr 2022 19:45:32 +0800	[thread overview]
Message-ID: <20220403114532.180945-1-ming.lei@redhat.com> (raw)

-EAGAIN still may return after io issue returns, and REQ_F_REISSUE is
set in io_complete_rw_iopoll(), but the req never gets chance to be handled.
io_iopoll_check doesn't handle this situation, and io hang can be caused.

Current dm io polling may return -EAGAIN after bio submission is
returned, also blk-throttle might trigger this situation too.

Cc: Mike Snitzer <snitzer@kernel.org>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 fs/io-wq.h    |  13 +++++
 fs/io_uring.c | 128 ++++++++++++++++++++++++++++----------------------
 2 files changed, 86 insertions(+), 55 deletions(-)

diff --git a/fs/io-wq.h b/fs/io-wq.h
index dbecd27656c7..4ca4863664fb 100644
--- a/fs/io-wq.h
+++ b/fs/io-wq.h
@@ -96,6 +96,19 @@ static inline void wq_list_add_head(struct io_wq_work_node *node,
 	WRITE_ONCE(list->first, node);
 }
 
+static inline void wq_list_remove(struct io_wq_work_list *list,
+				  struct io_wq_work_node *prev,
+				  struct io_wq_work_node *node)
+{
+	if (!prev)
+		WRITE_ONCE(list->first, node->next);
+	else
+		prev->next = node->next;
+
+	if (node == list->last)
+		list->last = prev;
+}
+
 static inline void wq_list_cut(struct io_wq_work_list *list,
 			       struct io_wq_work_node *last,
 			       struct io_wq_work_node *prev)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 59e54a6854b7..6db5514e10ca 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -2759,6 +2759,65 @@ static inline bool io_run_task_work(void)
 	return false;
 }
 
+#ifdef CONFIG_BLOCK
+static bool io_resubmit_prep(struct io_kiocb *req)
+{
+	struct io_async_rw *rw = req->async_data;
+
+	if (!req_has_async_data(req))
+		return !io_req_prep_async(req);
+	iov_iter_restore(&rw->s.iter, &rw->s.iter_state);
+	return true;
+}
+
+static bool io_rw_should_reissue(struct io_kiocb *req)
+{
+	umode_t mode = file_inode(req->file)->i_mode;
+	struct io_ring_ctx *ctx = req->ctx;
+
+	if (!S_ISBLK(mode) && !S_ISREG(mode))
+		return false;
+	if ((req->flags & REQ_F_NOWAIT) || (io_wq_current_is_worker() &&
+	    !(ctx->flags & IORING_SETUP_IOPOLL)))
+		return false;
+	/*
+	 * If ref is dying, we might be running poll reap from the exit work.
+	 * Don't attempt to reissue from that path, just let it fail with
+	 * -EAGAIN.
+	 */
+	if (percpu_ref_is_dying(&ctx->refs))
+		return false;
+	/*
+	 * Play it safe and assume not safe to re-import and reissue if we're
+	 * not in the original thread group (or in task context).
+	 */
+	if (!same_thread_group(req->task, current) || !in_task())
+		return false;
+	return true;
+}
+#else
+static bool io_resubmit_prep(struct io_kiocb *req)
+{
+	return false;
+}
+static bool io_rw_should_reissue(struct io_kiocb *req)
+{
+	return false;
+}
+#endif
+
+static void do_io_reissue(struct io_kiocb *req, int ret)
+{
+	if (req->flags & REQ_F_REISSUE) {
+		req->flags &= ~REQ_F_REISSUE;
+		if (io_resubmit_prep(req))
+			io_req_task_queue_reissue(req);
+		else
+			io_req_task_queue_fail(req, ret);
+	}
+}
+
+
 static int io_do_iopoll(struct io_ring_ctx *ctx, bool force_nonspin)
 {
 	struct io_wq_work_node *pos, *start, *prev;
@@ -2786,6 +2845,13 @@ static int io_do_iopoll(struct io_ring_ctx *ctx, bool force_nonspin)
 		if (READ_ONCE(req->iopoll_completed))
 			break;
 
+		/*
+		 * Once REISSUE flag is set, the req has been done, and we
+		 * have to retry
+		 */
+		if (req->flags & REQ_F_REISSUE)
+			break;
+
 		ret = kiocb->ki_filp->f_op->iopoll(kiocb, &iob, poll_flags);
 		if (unlikely(ret < 0))
 			return ret;
@@ -2807,6 +2873,12 @@ static int io_do_iopoll(struct io_ring_ctx *ctx, bool force_nonspin)
 	wq_list_for_each_resume(pos, prev) {
 		struct io_kiocb *req = container_of(pos, struct io_kiocb, comp_list);
 
+		if (req->flags & REQ_F_REISSUE) {
+			wq_list_remove(&ctx->iopoll_list, prev, pos);
+			do_io_reissue(req, -EIO);
+			break;
+		}
+
 		/* order with io_complete_rw_iopoll(), e.g. ->result updates */
 		if (!smp_load_acquire(&req->iopoll_completed))
 			break;
@@ -2924,53 +2996,6 @@ static void kiocb_end_write(struct io_kiocb *req)
 	}
 }
 
-#ifdef CONFIG_BLOCK
-static bool io_resubmit_prep(struct io_kiocb *req)
-{
-	struct io_async_rw *rw = req->async_data;
-
-	if (!req_has_async_data(req))
-		return !io_req_prep_async(req);
-	iov_iter_restore(&rw->s.iter, &rw->s.iter_state);
-	return true;
-}
-
-static bool io_rw_should_reissue(struct io_kiocb *req)
-{
-	umode_t mode = file_inode(req->file)->i_mode;
-	struct io_ring_ctx *ctx = req->ctx;
-
-	if (!S_ISBLK(mode) && !S_ISREG(mode))
-		return false;
-	if ((req->flags & REQ_F_NOWAIT) || (io_wq_current_is_worker() &&
-	    !(ctx->flags & IORING_SETUP_IOPOLL)))
-		return false;
-	/*
-	 * If ref is dying, we might be running poll reap from the exit work.
-	 * Don't attempt to reissue from that path, just let it fail with
-	 * -EAGAIN.
-	 */
-	if (percpu_ref_is_dying(&ctx->refs))
-		return false;
-	/*
-	 * Play it safe and assume not safe to re-import and reissue if we're
-	 * not in the original thread group (or in task context).
-	 */
-	if (!same_thread_group(req->task, current) || !in_task())
-		return false;
-	return true;
-}
-#else
-static bool io_resubmit_prep(struct io_kiocb *req)
-{
-	return false;
-}
-static bool io_rw_should_reissue(struct io_kiocb *req)
-{
-	return false;
-}
-#endif
-
 static bool __io_complete_rw_common(struct io_kiocb *req, long res)
 {
 	if (req->rw.kiocb.ki_flags & IOCB_WRITE)
@@ -3264,14 +3289,7 @@ static void kiocb_done(struct io_kiocb *req, ssize_t ret,
 		__io_complete_rw(req, ret, issue_flags);
 	else
 		io_rw_done(&req->rw.kiocb, ret);
-
-	if (req->flags & REQ_F_REISSUE) {
-		req->flags &= ~REQ_F_REISSUE;
-		if (io_resubmit_prep(req))
-			io_req_task_queue_reissue(req);
-		else
-			io_req_task_queue_fail(req, ret);
-	}
+	do_io_reissue(req, ret);
 }
 
 static int __io_import_fixed(struct io_kiocb *req, int rw, struct iov_iter *iter,
-- 
2.31.1


             reply	other threads:[~2022-04-03 11:45 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-03 11:45 Ming Lei [this message]
2022-04-04 16:51 ` [dm-devel] [RFC PATCH] io_uring: reissue in case -EAGAIN is returned after io issue returns Mike Snitzer
2022-04-04 16:51   ` Mike Snitzer
2022-04-06  2:09   ` [dm-devel] " Ming Lei
2022-04-06  2:09     ` Ming Lei
2022-04-06 16:52     ` [dm-devel] " Mike Snitzer
2022-04-06 16:52       ` Mike Snitzer
2022-04-07 15:36       ` [dm-devel] " Bryn M. Reeves
2022-04-06  2:20 ` Jens Axboe
2022-04-06  3:57   ` Ming Lei
2022-04-06 12:58     ` Jens Axboe
2022-04-06 13:21       ` Ming Lei
2022-04-06 16:38         ` Mike Snitzer

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=20220403114532.180945-1-ming.lei@redhat.com \
    --to=ming.lei@redhat.com \
    --cc=axboe@kernel.dk \
    --cc=io-uring@vger.kernel.org \
    --cc=snitzer@kernel.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.