* [PATCH 1/2] io_uring: add one helper for reaping iopoll events
2023-08-25 9:09 [PATCH 0/2] io_uring: fix IO hang in io wq exit Ming Lei
@ 2023-08-25 9:09 ` Ming Lei
2023-08-25 9:09 ` [PATCH 2/2] io_uring: reap iopoll events before exiting io wq Ming Lei
1 sibling, 0 replies; 3+ messages in thread
From: Ming Lei @ 2023-08-25 9:09 UTC (permalink / raw)
To: Jens Axboe, io-uring, linux-block; +Cc: David Howells, Chengming Zhou, Ming Lei
Prepare for reaping iopoll events before exiting io_wq, which may depend on
iopoll.
Signed-off-by: Ming Lei <[email protected]>
---
io_uring/io_uring.c | 27 ++++++++++++++++++---------
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 88599852af82..c4adb44f1aa4 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -3214,6 +3214,23 @@ static __cold bool io_uring_try_cancel_iowq(struct io_ring_ctx *ctx)
return ret;
}
+static bool iopoll_reap_events(struct io_ring_ctx *ctx, bool reap_all)
+{
+ bool reapped = false;
+
+ /* SQPOLL thread does its own polling */
+ if ((!(ctx->flags & IORING_SETUP_SQPOLL) && reap_all) ||
+ (ctx->sq_data && ctx->sq_data->thread == current)) {
+ while (!wq_list_empty(&ctx->iopoll_list)) {
+ io_iopoll_try_reap_events(ctx);
+ reapped = true;
+ cond_resched();
+ }
+ }
+
+ return reapped;
+}
+
static __cold bool io_uring_try_cancel_requests(struct io_ring_ctx *ctx,
struct task_struct *task,
bool cancel_all)
@@ -3245,15 +3262,7 @@ static __cold bool io_uring_try_cancel_requests(struct io_ring_ctx *ctx,
ret |= (cret != IO_WQ_CANCEL_NOTFOUND);
}
- /* SQPOLL thread does its own polling */
- if ((!(ctx->flags & IORING_SETUP_SQPOLL) && cancel_all) ||
- (ctx->sq_data && ctx->sq_data->thread == current)) {
- while (!wq_list_empty(&ctx->iopoll_list)) {
- io_iopoll_try_reap_events(ctx);
- ret = true;
- cond_resched();
- }
- }
+ ret = iopoll_reap_events(ctx, cancel_all);
if ((ctx->flags & IORING_SETUP_DEFER_TASKRUN) &&
io_allowed_defer_tw_run(ctx))
--
2.40.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* [PATCH 2/2] io_uring: reap iopoll events before exiting io wq
2023-08-25 9:09 [PATCH 0/2] io_uring: fix IO hang in io wq exit Ming Lei
2023-08-25 9:09 ` [PATCH 1/2] io_uring: add one helper for reaping iopoll events Ming Lei
@ 2023-08-25 9:09 ` Ming Lei
1 sibling, 0 replies; 3+ messages in thread
From: Ming Lei @ 2023-08-25 9:09 UTC (permalink / raw)
To: Jens Axboe, io-uring, linux-block; +Cc: David Howells, Chengming Zhou, Ming Lei
io_uring delays reaping iopoll events into exit work, which is scheduled
in io_uring_release(). But io wq is exited inside do_exit(), and wq code
path may share resource with iopoll code path, such as request. If iopoll
events aren't reaped, io wq exit can't be done, and cause IO hang.
The issue can be triggered when terminating 't/io_uring -n4 /dev/nullb0' with
default null_blk parameters.
Fix it by reaping iopoll events in io_uring_clean_tctx() and moving io wq
exit into workqueue context.
Closes: https://lore.kernel.org/linux-block/[email protected]/
Reported-by: David Howells <[email protected]>
Signed-off-by: Ming Lei <[email protected]>
---
io_uring/io_uring.c | 2 +-
io_uring/io_uring.h | 1 +
io_uring/tctx.c | 60 +++++++++++++++++++++++++++++++++++++++------
3 files changed, 54 insertions(+), 9 deletions(-)
diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index c4adb44f1aa4..ab7844c3380c 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -3214,7 +3214,7 @@ static __cold bool io_uring_try_cancel_iowq(struct io_ring_ctx *ctx)
return ret;
}
-static bool iopoll_reap_events(struct io_ring_ctx *ctx, bool reap_all)
+bool iopoll_reap_events(struct io_ring_ctx *ctx, bool reap_all)
{
bool reapped = false;
diff --git a/io_uring/io_uring.h b/io_uring/io_uring.h
index 547c30582fb8..f1556666a064 100644
--- a/io_uring/io_uring.h
+++ b/io_uring/io_uring.h
@@ -63,6 +63,7 @@ void io_req_task_queue_fail(struct io_kiocb *req, int ret);
void io_req_task_submit(struct io_kiocb *req, struct io_tw_state *ts);
void tctx_task_work(struct callback_head *cb);
__cold void io_uring_cancel_generic(bool cancel_all, struct io_sq_data *sqd);
+bool iopoll_reap_events(struct io_ring_ctx *ctx, bool reap_all);
int io_uring_alloc_task_context(struct task_struct *task,
struct io_ring_ctx *ctx);
diff --git a/io_uring/tctx.c b/io_uring/tctx.c
index c043fe93a3f2..582b9149bab1 100644
--- a/io_uring/tctx.c
+++ b/io_uring/tctx.c
@@ -6,6 +6,7 @@
#include <linux/slab.h>
#include <linux/nospec.h>
#include <linux/io_uring.h>
+#include <linux/delay.h>
#include <uapi/linux/io_uring.h>
@@ -175,24 +176,67 @@ __cold void io_uring_del_tctx_node(unsigned long index)
kfree(node);
}
+struct wq_exit_work {
+ struct work_struct work;
+ struct io_wq *wq;
+ bool done;
+};
+
+static void io_uring_wq_exit_work(struct work_struct *work)
+{
+ struct wq_exit_work *wq_work =
+ container_of(work, struct wq_exit_work, work);
+ struct io_wq *wq = wq_work->wq;
+
+ /*
+ * Must be after io_uring_del_tctx_node() (removes nodes under
+ * uring_lock) to avoid race with io_uring_try_cancel_iowq().
+ */
+ io_wq_put_and_exit(wq);
+ wq_work->done = true;
+}
+
__cold void io_uring_clean_tctx(struct io_uring_task *tctx)
{
struct io_wq *wq = tctx->io_wq;
+ struct wq_exit_work work = {
+ .wq = wq,
+ .done = true,
+ };
struct io_tctx_node *node;
unsigned long index;
- xa_for_each(&tctx->xa, index, node) {
- io_uring_del_tctx_node(index);
- cond_resched();
- }
+ /*
+ * io_wq may depend on reaping iopoll events because pending
+ * requests in io_wq may share resource with polled requests,
+ * meantime new polled IO may be submitted from io_wq after
+ * getting resource.
+ *
+ * So io_wq has to be exited from workqueue context for avoiding
+ * IO hang.
+ */
if (wq) {
+ work.done = false;
+ INIT_WORK(&work.work, io_uring_wq_exit_work);
+ queue_work(system_unbound_wq, &work.work);
+ }
+
+ while (!work.done) {
+ xa_for_each(&tctx->xa, index, node)
+ iopoll_reap_events(node->ctx, true);
+
/*
- * Must be after io_uring_del_tctx_node() (removes nodes under
- * uring_lock) to avoid race with io_uring_try_cancel_iowq().
+ * Wait a little while and reap again since new polled
+ * IO may get resource from io_wq and be submitted.
*/
- io_wq_put_and_exit(wq);
- tctx->io_wq = NULL;
+ msleep(10);
+ }
+
+ xa_for_each(&tctx->xa, index, node) {
+ io_uring_del_tctx_node(index);
+ cond_resched();
}
+ tctx->io_wq = NULL;
}
void io_uring_unreg_ringfd(void)
--
2.40.1
^ permalink raw reply related [flat|nested] 3+ messages in thread