public inbox for [email protected]
 help / color / mirror / Atom feed
* [PATCH for-next] io_uring: fix not released cached task refs
@ 2022-01-09  0:53 Pavel Begunkov
  2022-01-09 16:23 ` Jens Axboe
  0 siblings, 1 reply; 3+ messages in thread
From: Pavel Begunkov @ 2022-01-09  0:53 UTC (permalink / raw)
  To: Jens Axboe, io-uring; +Cc: asml.silence, Lukas Bulwahn

tctx_task_work() may get run after io_uring cancellation and so there
will be no one to put cached in tctx task refs that may have been added
back by tw handlers using inline completion infra, Call
io_uring_drop_tctx_refs() at the end of the main tw handler to release
them.

Cc: [email protected] # 5.15+
Reported-by: Lukas Bulwahn <[email protected]>
Fixes: e98e49b2bbf7 ("io_uring: extend task put optimisations")
Signed-off-by: Pavel Begunkov <[email protected]>
---
 fs/io_uring.c | 34 +++++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index aed1625a26e1..684d77c179a0 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -1827,6 +1827,18 @@ static inline void io_get_task_refs(int nr)
 		io_task_refs_refill(tctx);
 }
 
+static __cold void io_uring_drop_tctx_refs(struct task_struct *task)
+{
+	struct io_uring_task *tctx = task->io_uring;
+	unsigned int refs = tctx->cached_refs;
+
+	if (refs) {
+		tctx->cached_refs = 0;
+		percpu_counter_sub(&tctx->inflight, refs);
+		put_task_struct_many(task, refs);
+	}
+}
+
 static bool io_cqring_event_overflow(struct io_ring_ctx *ctx, u64 user_data,
 				     s32 res, u32 cflags)
 {
@@ -2319,6 +2331,10 @@ static void tctx_task_work(struct callback_head *cb)
 	}
 
 	ctx_flush_and_put(ctx, &uring_locked);
+
+	/* relaxed read is enough as only the task itself sets ->in_idle */
+	if (unlikely(atomic_read(&tctx->in_idle)))
+		io_uring_drop_tctx_refs(current);
 }
 
 static void io_req_task_work_add(struct io_kiocb *req, bool priority)
@@ -9803,18 +9819,6 @@ static s64 tctx_inflight(struct io_uring_task *tctx, bool tracked)
 	return percpu_counter_sum(&tctx->inflight);
 }
 
-static __cold void io_uring_drop_tctx_refs(struct task_struct *task)
-{
-	struct io_uring_task *tctx = task->io_uring;
-	unsigned int refs = tctx->cached_refs;
-
-	if (refs) {
-		tctx->cached_refs = 0;
-		percpu_counter_sub(&tctx->inflight, refs);
-		put_task_struct_many(task, refs);
-	}
-}
-
 /*
  * Find any io_uring ctx that this task has registered or done IO on, and cancel
  * requests. @sqd should be not-null IIF it's an SQPOLL thread cancellation.
@@ -9870,10 +9874,14 @@ static __cold void io_uring_cancel_generic(bool cancel_all,
 			schedule();
 		finish_wait(&tctx->wait, &wait);
 	} while (1);
-	atomic_dec(&tctx->in_idle);
 
 	io_uring_clean_tctx(tctx);
 	if (cancel_all) {
+		/*
+		 * We shouldn't run task_works after cancel, so just leave
+		 * ->in_idle set for normal exit.
+		 */
+		atomic_dec(&tctx->in_idle);
 		/* for exec all current's requests should be gone, kill tctx */
 		__io_uring_free(current);
 	}
-- 
2.33.1


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

* Re: [PATCH for-next] io_uring: fix not released cached task refs
  2022-01-09  0:53 [PATCH for-next] io_uring: fix not released cached task refs Pavel Begunkov
@ 2022-01-09 16:23 ` Jens Axboe
  2022-01-10 13:20   ` Lukas Bulwahn
  0 siblings, 1 reply; 3+ messages in thread
From: Jens Axboe @ 2022-01-09 16:23 UTC (permalink / raw)
  To: io-uring, Pavel Begunkov; +Cc: Lukas Bulwahn

On Sun, 9 Jan 2022 00:53:22 +0000, Pavel Begunkov wrote:
> tctx_task_work() may get run after io_uring cancellation and so there
> will be no one to put cached in tctx task refs that may have been added
> back by tw handlers using inline completion infra, Call
> io_uring_drop_tctx_refs() at the end of the main tw handler to release
> them.
> 
> 
> [...]

Applied, thanks!

[1/1] io_uring: fix not released cached task refs
      commit: 3cc7fdb9f90a25ae92250bf9e6cf3b9556b230e9

Best regards,
-- 
Jens Axboe



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

* Re: [PATCH for-next] io_uring: fix not released cached task refs
  2022-01-09 16:23 ` Jens Axboe
@ 2022-01-10 13:20   ` Lukas Bulwahn
  0 siblings, 0 replies; 3+ messages in thread
From: Lukas Bulwahn @ 2022-01-10 13:20 UTC (permalink / raw)
  To: Jens Axboe; +Cc: io-uring, Pavel Begunkov

On Sun, Jan 9, 2022 at 5:23 PM Jens Axboe <[email protected]> wrote:
>
> On Sun, 9 Jan 2022 00:53:22 +0000, Pavel Begunkov wrote:
> > tctx_task_work() may get run after io_uring cancellation and so there
> > will be no one to put cached in tctx task refs that may have been added
> > back by tw handlers using inline completion infra, Call
> > io_uring_drop_tctx_refs() at the end of the main tw handler to release
> > them.
> >
> >
> > [...]
>
> Applied, thanks!
>
> [1/1] io_uring: fix not released cached task refs
>       commit: 3cc7fdb9f90a25ae92250bf9e6cf3b9556b230e9
>

The memory leak, reported in
https://lore.kernel.org/all/CAKXUXMzHUi3q4K-OpiBKyMAsQ2K=FOsVzULC76v05nCUKNCA+Q@mail.gmail.com/:

    - does     trigger on next-20220107.

    - does NOT trigger on next-20220107 + cherry-pick
3cc7fdb9f90a25ae92250bf9e6cf3b9556b230e9.

    - does NOT trigger on next-20220110, which already includes commit
3cc7fdb9f90a25ae92250bf9e6cf3b9556b230e9.


So, with that I think this patch resolves the reported memory leak for good:

Tested-by: Lukas Bulwahn <[email protected]>

Pavel, thanks for the quick fix.

I guess that the patch has already landed in linux-next, so the tag
above will not be applied to the commit, but is only for our own
historic reference.

Lukas

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

end of thread, other threads:[~2022-01-10 13:20 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-01-09  0:53 [PATCH for-next] io_uring: fix not released cached task refs Pavel Begunkov
2022-01-09 16:23 ` Jens Axboe
2022-01-10 13:20   ` Lukas Bulwahn

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