public inbox for [email protected]
 help / color / mirror / Atom feed
* [PATCH 0/3] task/files cancellation fixes
@ 2020-12-20 13:21 Pavel Begunkov
  2020-12-20 13:21 ` [PATCH 1/3] io_uring: always progress task_work on task cancel Pavel Begunkov
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Pavel Begunkov @ 2020-12-20 13:21 UTC (permalink / raw)
  To: Jens Axboe, io-uring

First two are only for task cancellation, the last one for both.

Jens, do you remember the exact reason why you added the check
removed in [3/3]? I have a clue, but that stuff doesn't look
right regardless. See "exit: do exit_task_work() before shooting
off mm" thread.

Pavel Begunkov (3):
  io_uring: always progress task_work on task cancel
  io_uring: end waiting before task cancel attempts
  io_uring: fix cancellation hangs

 fs/io_uring.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

-- 
2.24.0


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

* [PATCH 1/3] io_uring: always progress task_work on task cancel
  2020-12-20 13:21 [PATCH 0/3] task/files cancellation fixes Pavel Begunkov
@ 2020-12-20 13:21 ` Pavel Begunkov
  2020-12-20 13:21 ` [PATCH 2/3] io_uring: end waiting before task cancel attempts Pavel Begunkov
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Pavel Begunkov @ 2020-12-20 13:21 UTC (permalink / raw)
  To: Jens Axboe, io-uring

Might happen that __io_uring_cancel_task_requests() cancels nothing but
there are task_works pending. We need to always run them.

Signed-off-by: Pavel Begunkov <[email protected]>
---
 fs/io_uring.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index f3690dfdd564..051461b5bf52 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -8794,9 +8794,9 @@ static void __io_uring_cancel_task_requests(struct io_ring_ctx *ctx,
 
 		ret |= io_poll_remove_all(ctx, task, NULL);
 		ret |= io_kill_timeouts(ctx, task, NULL);
+		ret |= io_run_task_work();
 		if (!ret)
 			break;
-		io_run_task_work();
 		cond_resched();
 	}
 }
-- 
2.24.0


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

* [PATCH 2/3] io_uring: end waiting before task cancel attempts
  2020-12-20 13:21 [PATCH 0/3] task/files cancellation fixes Pavel Begunkov
  2020-12-20 13:21 ` [PATCH 1/3] io_uring: always progress task_work on task cancel Pavel Begunkov
@ 2020-12-20 13:21 ` Pavel Begunkov
  2020-12-20 13:21 ` [PATCH 3/3] io_uring: fix cancellation hangs Pavel Begunkov
  2020-12-20 18:01 ` [PATCH 0/3] task/files cancellation fixes Pavel Begunkov
  3 siblings, 0 replies; 6+ messages in thread
From: Pavel Begunkov @ 2020-12-20 13:21 UTC (permalink / raw)
  To: Jens Axboe, io-uring

Get rid of TASK_UNINTERRUPTIBLE and waiting with finish_wait before
going for next iteration in __io_uring_task_cancel(), because
__io_uring_files_cancel() doesn't expect that sheduling is disallowed.

Signed-off-by: Pavel Begunkov <[email protected]>
---
 fs/io_uring.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 051461b5bf52..30edf47a8f1a 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -8982,9 +8982,9 @@ void __io_uring_task_cancel(void)
 		if (inflight != tctx_inflight(tctx))
 			continue;
 		schedule();
+		finish_wait(&tctx->wait, &wait);
 	} while (1);
 
-	finish_wait(&tctx->wait, &wait);
 	atomic_dec(&tctx->in_idle);
 }
 
-- 
2.24.0


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

* [PATCH 3/3] io_uring: fix cancellation hangs
  2020-12-20 13:21 [PATCH 0/3] task/files cancellation fixes Pavel Begunkov
  2020-12-20 13:21 ` [PATCH 1/3] io_uring: always progress task_work on task cancel Pavel Begunkov
  2020-12-20 13:21 ` [PATCH 2/3] io_uring: end waiting before task cancel attempts Pavel Begunkov
@ 2020-12-20 13:21 ` Pavel Begunkov
  2020-12-20 18:01 ` [PATCH 0/3] task/files cancellation fixes Pavel Begunkov
  3 siblings, 0 replies; 6+ messages in thread
From: Pavel Begunkov @ 2020-12-20 13:21 UTC (permalink / raw)
  To: Jens Axboe, io-uring

We may enter files cancellation with requests that we want to cancel and
that are currently enqueued as task_work. However, before that happens
do_exit() sets PF_EXITING, disabling io_run_task_work() and so locking
up cancellation.

Also, if run between setting PF_EXITING and exit_task_work(), as the
case exit_files() and so io_uring cancellation, task_work_add() might
actually enqueue more task works.

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

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 30edf47a8f1a..941fe9b64fd9 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -2357,12 +2357,6 @@ static inline unsigned int io_put_rw_kbuf(struct io_kiocb *req)
 
 static inline bool io_run_task_work(void)
 {
-	/*
-	 * Not safe to run on exiting task, and the task_work handling will
-	 * not add work to such a task.
-	 */
-	if (unlikely(current->flags & PF_EXITING))
-		return false;
 	if (current->task_works) {
 		__set_current_state(TASK_RUNNING);
 		task_work_run();
-- 
2.24.0


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

* Re: [PATCH 0/3] task/files cancellation fixes
  2020-12-20 13:21 [PATCH 0/3] task/files cancellation fixes Pavel Begunkov
                   ` (2 preceding siblings ...)
  2020-12-20 13:21 ` [PATCH 3/3] io_uring: fix cancellation hangs Pavel Begunkov
@ 2020-12-20 18:01 ` Pavel Begunkov
  2020-12-20 18:05   ` Jens Axboe
  3 siblings, 1 reply; 6+ messages in thread
From: Pavel Begunkov @ 2020-12-20 18:01 UTC (permalink / raw)
  To: Jens Axboe, io-uring

On 20/12/2020 13:21, Pavel Begunkov wrote:
> First two are only for task cancellation, the last one for both.
> 
> Jens, do you remember the exact reason why you added the check
> removed in [3/3]? I have a clue, but that stuff doesn't look
> right regardless. See "exit: do exit_task_work() before shooting
> off mm" thread.

The question is answered, the bug is still here, but that's too
early for 3/3. Please consider first 2 patches.

> 
> Pavel Begunkov (3):
>   io_uring: always progress task_work on task cancel
>   io_uring: end waiting before task cancel attempts
>   io_uring: fix cancellation hangs
> 
>  fs/io_uring.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 

-- 
Pavel Begunkov

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

* Re: [PATCH 0/3] task/files cancellation fixes
  2020-12-20 18:01 ` [PATCH 0/3] task/files cancellation fixes Pavel Begunkov
@ 2020-12-20 18:05   ` Jens Axboe
  0 siblings, 0 replies; 6+ messages in thread
From: Jens Axboe @ 2020-12-20 18:05 UTC (permalink / raw)
  To: Pavel Begunkov, io-uring

On 12/20/20 11:01 AM, Pavel Begunkov wrote:
> On 20/12/2020 13:21, Pavel Begunkov wrote:
>> First two are only for task cancellation, the last one for both.
>>
>> Jens, do you remember the exact reason why you added the check
>> removed in [3/3]? I have a clue, but that stuff doesn't look
>> right regardless. See "exit: do exit_task_work() before shooting
>> off mm" thread.
> 
> The question is answered, the bug is still here, but that's too
> early for 3/3. Please consider first 2 patches.

Yep, first two look good to me, thanks!

-- 
Jens Axboe


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

end of thread, other threads:[~2020-12-20 18:06 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-12-20 13:21 [PATCH 0/3] task/files cancellation fixes Pavel Begunkov
2020-12-20 13:21 ` [PATCH 1/3] io_uring: always progress task_work on task cancel Pavel Begunkov
2020-12-20 13:21 ` [PATCH 2/3] io_uring: end waiting before task cancel attempts Pavel Begunkov
2020-12-20 13:21 ` [PATCH 3/3] io_uring: fix cancellation hangs Pavel Begunkov
2020-12-20 18:01 ` [PATCH 0/3] task/files cancellation fixes Pavel Begunkov
2020-12-20 18:05   ` Jens Axboe

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