[PATCH 0/3] task/files cancellation fixes

[PATCH 0/3] task/files cancellation fixes

[Pavel Begunkov]

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

[PATCH 1/3] io_uring: always progress task_work on task cancel

[Pavel Begunkov]

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 <asml.silence@gmail.com>
---
 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

[PATCH 2/3] io_uring: end waiting before task cancel attempts

[Pavel Begunkov]

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 <asml.silence@gmail.com>
---
 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

[PATCH 3/3] io_uring: fix cancellation hangs

[Pavel Begunkov]

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 <asml.silence@gmail.com>
---
 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

Re: [PATCH 0/3] task/files cancellation fixes

[Pavel Begunkov]

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

Re: [PATCH 0/3] task/files cancellation fixes

[Jens Axboe]

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