On 9/8/23 07:22, Ming Lei wrote:
On Fri, Sep 08, 2023 at 02:03:11AM +0100, Pavel Begunkov wrote:
On 9/7/23 16:36, Jens Axboe wrote:
On 9/1/23 9:13 AM, Jens Axboe wrote:
On Fri, 01 Sep 2023 21:49:16 +0800, Ming Lei wrote:
io_wq_put_and_exit() is called from do_exit(), but all FIXED_FILE requests
in io_wq aren't canceled in io_uring_cancel_generic() called from do_exit().
Meantime io_wq IO code path may share resource with normal iopoll code
path.
So if any HIPRI request is submittd via io_wq, this request may not get resouce
for moving on, given iopoll isn't possible in io_wq_put_and_exit().
[...]
Applied, thanks!
[1/1] io_uring: fix IO hang in io_wq_put_and_exit from do_exit()
commit: b484a40dc1f16edb58e5430105a021e1916e6f27
This causes a regression with the test/thread-exit.t test case, as it's
canceling requests from other tasks as well. I will drop this patch for
now.
And this one has never hit my mailbox... Anyway, I'm confused with
the issue:
Thanks CC'ing while resending, I don't know what's up with lore
1) request tracking is done only for io_uring polling io_uring, which
request tracking isn't done on FIXED_FILE IO, which is used by t/io_uring.
shouldn't be the case for t/io_uring, so it's probably unrelated?
So io_uring_try_cancel_requests() won't be called because
tctx_inflight() returns zero.
That's true for fixed files, but it also holds for non fixed files
as well apart from a narrow case t/io_uring doesn't exercise
2) In case of iopoll, io-wq only submits a request but doesn't wait/poll
for it. If io_issue_sqe() returned -EAGAIN or an error, the request is
considered not yet submitted to block and can be just cancelled normally
without any dancing like io_iopoll_try_reap_events().
io_issue_sqe() may never return since IO_URING_F_NONBLOCK isn't set
for iopoll, and recently polled IO doesn't imply nowait in commit
missing nowait semantics should be fine, io_uring_clean_tctx()
sends a signal to workers, which should break them out of waiting.
...
We set the flag, interrupt workers (TIF_NOTIFY_SIGNAL + wake up), and
wait for them. Workers are woken up (or just return), see
the flag and return. At least that's how it was intended to work.
What's missing? Racing for IO_WQ_BIT_EXIT? Not breaking on IO_WQ_BIT_EXIT
correctly? Not honouring / clearing TIF_NOTIFY_SIGNAL?
I'll try to repro later
After applying your patch of 9256b9371204 ("io_uring: break out of iowq
iopoll on teardown") and the above patch, the issue still can be triggered,
Yeah, it doesn't appear that it'd help, it was rather something
randomly spotted than targeting your issue.
and the worker is keeping to call io_issue_sqe() for ever, and
io_wq_worker_stopped() returns false. So do_exit() isn't called on
t/io_uring pthread yet, meantime I guess either iopoll is terminated or not
get chance to run.
That prior to task exit then, right? I wonder if we're missing
signal checking in that loop like below or just need limit the
number of re-submission attempts.
diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 6cce8948bddf..c6f566200d04 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -1944,6 +1944,8 @@ void io_wq_submit_work(struct io_wq_work *work)
break;
if (io_wq_worker_stopped())
break;
+ if (signal_pending(current))
+ break;
cond_resched();
continue;
}
--
Pavel Begunkov
