public inbox for [email protected]
 help / color / mirror / Atom feed
* [PATCH 0/2] SQPOLL cancel files fix
@ 2021-02-10 11:45 Pavel Begunkov
  2021-02-10 11:45 ` [PATCH 1/2] io_uring: cancel files inflight counting Pavel Begunkov
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Pavel Begunkov @ 2021-02-10 11:45 UTC (permalink / raw)
  To: Jens Axboe, io-uring

2/2 is for the recent syzbot report. Quick and dirty, but easy to
backport. I plan to restructure (and clean) task vs files cancel later.

Pavel Begunkov (2):
  io_uring: cancel files inflight counting
  io_uring; fix files cancel hangs

 fs/io_uring.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

-- 
2.24.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/2] io_uring: cancel files inflight counting
  2021-02-10 11:45 [PATCH 0/2] SQPOLL cancel files fix Pavel Begunkov
@ 2021-02-10 11:45 ` Pavel Begunkov
  2021-02-10 11:45 ` [PATCH 2/2] io_uring; fix files cancel hangs Pavel Begunkov
  2021-02-10 20:30 ` [PATCH 0/2] SQPOLL cancel files fix Jens Axboe
  2 siblings, 0 replies; 4+ messages in thread
From: Pavel Begunkov @ 2021-02-10 11:45 UTC (permalink / raw)
  To: Jens Axboe, io-uring

To not skip events, io_uring_cancel_files() is pretty much counts on
io_uring_count_inflight() to be monotonous for the time it's used.
That's  not the case when it includes requests of other thats that are
PF_EXITING.

Cancel as before, but don't account extra in io_uring_count_inflight(),
we can hang otherwise.

Signed-off-by: Pavel Begunkov <[email protected]>
---
 fs/io_uring.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 17194e0d62ff..6b73e38aa1a9 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -1090,15 +1090,14 @@ static inline void io_set_resource_node(struct io_kiocb *req)
 	}
 }
 
-static bool io_match_task(struct io_kiocb *head,
-			  struct task_struct *task,
-			  struct files_struct *files)
+static bool __io_match_task(struct io_kiocb *head, struct task_struct *task,
+			    struct files_struct *files, bool match_exiting)
 {
 	struct io_kiocb *req;
 
 	if (task && head->task != task) {
 		/* in terms of cancelation, always match if req task is dead */
-		if (head->task->flags & PF_EXITING)
+		if (match_exiting && (head->task->flags & PF_EXITING))
 			return true;
 		return false;
 	}
@@ -1117,6 +1116,13 @@ static bool io_match_task(struct io_kiocb *head,
 	return false;
 }
 
+static bool io_match_task(struct io_kiocb *head,
+			  struct task_struct *task,
+			  struct files_struct *files)
+{
+	return __io_match_task(head, task, files, true);
+}
+
 static void io_sq_thread_drop_mm_files(void)
 {
 	struct files_struct *files = current->files;
@@ -9032,7 +9038,7 @@ static int io_uring_count_inflight(struct io_ring_ctx *ctx,
 
 	spin_lock_irq(&ctx->inflight_lock);
 	list_for_each_entry(req, &ctx->inflight_list, inflight_entry)
-		cnt += io_match_task(req, task, files);
+		cnt += __io_match_task(req, task, files, false);
 	spin_unlock_irq(&ctx->inflight_lock);
 	return cnt;
 }
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/2] io_uring; fix files cancel hangs
  2021-02-10 11:45 [PATCH 0/2] SQPOLL cancel files fix Pavel Begunkov
  2021-02-10 11:45 ` [PATCH 1/2] io_uring: cancel files inflight counting Pavel Begunkov
@ 2021-02-10 11:45 ` Pavel Begunkov
  2021-02-10 20:30 ` [PATCH 0/2] SQPOLL cancel files fix Jens Axboe
  2 siblings, 0 replies; 4+ messages in thread
From: Pavel Begunkov @ 2021-02-10 11:45 UTC (permalink / raw)
  To: Jens Axboe, io-uring; +Cc: stable, syzbot+695b03d82fa8e4901b06

We park SQPOLL task before going into io_uring_cancel_files(), so the
task won't run task_works including those that might be important for
the cancellation passes. In this case it's io_poll_remove_one(), which
frees requests via io_put_req_deferred().

Unpark it for while waiting, it's ok as we disable submissions
beforehand, so no new will be generated.

INFO: task syz-executor893:8493 blocked for more than 143 seconds.
Call Trace:
 context_switch kernel/sched/core.c:4327 [inline]
 __schedule+0x90c/0x21a0 kernel/sched/core.c:5078
 schedule+0xcf/0x270 kernel/sched/core.c:5157
 io_uring_cancel_files fs/io_uring.c:8912 [inline]
 io_uring_cancel_task_requests+0xe70/0x11a0 fs/io_uring.c:8979
 __io_uring_files_cancel+0x110/0x1b0 fs/io_uring.c:9067
 io_uring_files_cancel include/linux/io_uring.h:51 [inline]
 do_exit+0x2fe/0x2ae0 kernel/exit.c:780
 do_group_exit+0x125/0x310 kernel/exit.c:922
 __do_sys_exit_group kernel/exit.c:933 [inline]
 __se_sys_exit_group kernel/exit.c:931 [inline]
 __x64_sys_exit_group+0x3a/0x50 kernel/exit.c:931
 do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
 entry_SYSCALL_64_after_hwframe+0x44/0xa9

Cc: [email protected] # 5.5+
Reported-by: [email protected]
Signed-off-by: Pavel Begunkov <[email protected]>
---
 fs/io_uring.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 6b73e38aa1a9..1e803a9afc8e 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -9056,11 +9056,16 @@ static void io_uring_cancel_files(struct io_ring_ctx *ctx,
 			break;
 
 		io_uring_try_cancel_requests(ctx, task, files);
+
+		if (ctx->sq_data)
+			io_sq_thread_unpark(ctx->sq_data);
 		prepare_to_wait(&task->io_uring->wait, &wait,
 				TASK_UNINTERRUPTIBLE);
 		if (inflight == io_uring_count_inflight(ctx, task, files))
 			schedule();
 		finish_wait(&task->io_uring->wait, &wait);
+		if (ctx->sq_data)
+			io_sq_thread_park(ctx->sq_data);
 	}
 }
 
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 0/2] SQPOLL cancel files fix
  2021-02-10 11:45 [PATCH 0/2] SQPOLL cancel files fix Pavel Begunkov
  2021-02-10 11:45 ` [PATCH 1/2] io_uring: cancel files inflight counting Pavel Begunkov
  2021-02-10 11:45 ` [PATCH 2/2] io_uring; fix files cancel hangs Pavel Begunkov
@ 2021-02-10 20:30 ` Jens Axboe
  2 siblings, 0 replies; 4+ messages in thread
From: Jens Axboe @ 2021-02-10 20:30 UTC (permalink / raw)
  To: Pavel Begunkov, io-uring

On 2/10/21 4:45 AM, Pavel Begunkov wrote:
> 2/2 is for the recent syzbot report. Quick and dirty, but easy to
> backport. I plan to restructure (and clean) task vs files cancel later.
> 
> Pavel Begunkov (2):
>   io_uring: cancel files inflight counting
>   io_uring; fix files cancel hangs
> 
>  fs/io_uring.c | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)

Applied 2/2 for now, thanks.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-02-10 20:31 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-02-10 11:45 [PATCH 0/2] SQPOLL cancel files fix Pavel Begunkov
2021-02-10 11:45 ` [PATCH 1/2] io_uring: cancel files inflight counting Pavel Begunkov
2021-02-10 11:45 ` [PATCH 2/2] io_uring; fix files cancel hangs Pavel Begunkov
2021-02-10 20:30 ` [PATCH 0/2] SQPOLL cancel files fix Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox