FAILED: patch "[PATCH] io_uring: hold 'ctx' reference around task_work queue +" failed to apply to 5.8-stable tree

PATCH [email protected] 2 participants 8 messages Aug 17, 2020

Subsystems (2)

Stable branch VFS

lore

Share & tools

lore

[email protected]

author maintainer

2020-08-17 10:43 UTC #0 lore raw

The patch below does not apply to the 5.8-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 6d816e088c359866f9867057e04f244c608c42fe Mon Sep 17 00:00:00 2001
From: Jens Axboe <[email protected]>
Date: Tue, 11 Aug 2020 08:04:14 -0600
Subject: [PATCH] io_uring: hold 'ctx' reference around task_work queue +
execute

We're holding the request reference, but we need to go one higher to ensure that the ctx remains valid after the request has finished. If the ring is closed with pending task_work inflight, and the given io_kiocb finishes sync during issue, then we need a reference to the ring itself around the task_work execution cycle.

Cc [email protected] # v5.7+

Reported-by [email protected]

Signed-off-by Jens Axboe <[email protected]>

fs/io_uring.c +15 -0

index 5488698189da..99582cf5106b 100644
@@ -1821,8 +1821,10 @@ static void __io_req_task_submit(struct io_kiocb *req)
 static void io_req_task_submit(struct callback_head *cb)
 {
 	struct io_kiocb *req = container_of(cb, struct io_kiocb, task_work);
+	struct io_ring_ctx *ctx = req->ctx;
 
 	__io_req_task_submit(req);
+	percpu_ref_put(&ctx->refs);
 }
 
 static void io_req_task_queue(struct io_kiocb *req)

@@ -1830,6 +1832,7 @@ static void io_req_task_queue(struct io_kiocb *req)
 	int ret;
 
 	init_task_work(&req->task_work, io_req_task_submit);
+	percpu_ref_get(&req->ctx->refs);
 
 	ret = io_req_task_work_add(req, &req->task_work);
 	if (unlikely(ret)) {

@@ -2318,6 +2321,8 @@ static void io_rw_resubmit(struct callback_head *cb)
 		refcount_inc(&req->refs);
 		io_queue_async_work(req);
 	}
+
+	percpu_ref_put(&ctx->refs);
 }
 #endif

@@ -2330,6 +2335,8 @@ static bool io_rw_reissue(struct io_kiocb *req, long res)
 		return false;
 
 	init_task_work(&req->task_work, io_rw_resubmit);
+	percpu_ref_get(&req->ctx->refs);
+
 	ret = io_req_task_work_add(req, &req->task_work);
 	if (!ret)
 		return true;

@@ -3033,6 +3040,8 @@ static int io_async_buf_func(struct wait_queue_entry *wait, unsigned mode,
 	list_del_init(&wait->entry);
 
 	init_task_work(&req->task_work, io_req_task_submit);
+	percpu_ref_get(&req->ctx->refs);
+
 	/* submit ref gets dropped, acquire a new one */
 	refcount_inc(&req->refs);
 	ret = io_req_task_work_add(req, &req->task_work);

@@ -4565,6 +4574,8 @@ static int __io_async_wake(struct io_kiocb *req, struct io_poll_iocb *poll,
 
 	req->result = mask;
 	init_task_work(&req->task_work, func);
+	percpu_ref_get(&req->ctx->refs);
+
 	/*
 	 * If this fails, then the task is exiting. When a task exits, the
 	 * work gets canceled, so just cancel this request as well instead

@@ -4652,11 +4663,13 @@ static void io_poll_task_handler(struct io_kiocb *req, struct io_kiocb **nxt)
 static void io_poll_task_func(struct callback_head *cb)
 {
 	struct io_kiocb *req = container_of(cb, struct io_kiocb, task_work);
+	struct io_ring_ctx *ctx = req->ctx;
 	struct io_kiocb *nxt = NULL;
 
 	io_poll_task_handler(req, &nxt);
 	if (nxt)
 		__io_req_task_submit(nxt);
+	percpu_ref_put(&ctx->refs);
 }
 
 static int io_poll_double_wake(struct wait_queue_entry *wait, unsigned mode,

@@ -4752,6 +4765,7 @@ static void io_async_task_func(struct callback_head *cb)
 
 	if (io_poll_rewait(req, &apoll->poll)) {
 		spin_unlock_irq(&ctx->completion_lock);
+		percpu_ref_put(&ctx->refs);
 		return;
 	}

@@ -4767,6 +4781,7 @@ static void io_async_task_func(struct callback_head *cb)
 	else
 		__io_req_task_cancel(req, -ECANCELED);
 
+	percpu_ref_put(&ctx->refs);
 	kfree(apoll->double_poll);
 	kfree(apoll);
 }

Jens Axboe

2020-08-17 13:10 UTC #1 lore raw

Quoted 5 lines · On 8/17/20 3:44 AM, [email protected] wrote:

The patch below does not apply to the 5.8-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

Here's a 5.8 version.

Signature · 1 line

Jens Axboe

From 4805555b82f6eb78903ee2841ebae13707d9da13 Mon Sep 17 00:00:00 2001
From: Jens Axboe <[email protected]>
Date: Tue, 11 Aug 2020 08:04:14 -0600
Subject: [PATCH] io_uring: hold 'ctx' reference around task_work queue +
execute

Cc [email protected] # v5.7+

Reported-by [email protected]

Signed-off-by Jens Axboe <[email protected]>

---
 fs/io_uring.c | 6 ++++++
 1 file changed, 6 insertions(+)

fs/io_uring.c +6 -0

index 3da73e58759e..c7aefd3da135 100644
@@ -4141,6 +4141,8 @@ static int __io_async_wake(struct io_kiocb *req, struct io_poll_iocb *poll,
 	tsk = req->task;
 	req->result = mask;
 	init_task_work(&req->task_work, func);
+	percpu_ref_get(&req->ctx->refs);
+
 	/*
 	 * If this fails, then the task is exiting. When a task exits, the
 	 * work gets canceled, so just cancel this request as well instead

@@ -4225,6 +4227,7 @@ static void io_poll_task_handler(struct io_kiocb *req, struct io_kiocb **nxt)
 static void io_poll_task_func(struct callback_head *cb)
 {
 	struct io_kiocb *req = container_of(cb, struct io_kiocb, task_work);
+	struct io_ring_ctx *ctx = req->ctx;
 	struct io_kiocb *nxt = NULL;
 
 	io_poll_task_handler(req, &nxt);

@@ -4235,6 +4238,7 @@ static void io_poll_task_func(struct callback_head *cb)
 		__io_queue_sqe(nxt, NULL);
 		mutex_unlock(&ctx->uring_lock);
 	}
+	percpu_ref_put(&ctx->refs);
 }
 
 static int io_poll_double_wake(struct wait_queue_entry *wait, unsigned mode,

@@ -4349,6 +4353,7 @@ static void io_async_task_func(struct callback_head *cb)
 
 	if (io_poll_rewait(req, &apoll->poll)) {
 		spin_unlock_irq(&ctx->completion_lock);
+		percpu_ref_put(&ctx->refs);
 		return;
 	}

@@ -4387,6 +4392,7 @@ static void io_async_task_func(struct callback_head *cb)
 		req_set_fail_links(req);
 		io_double_put_req(req);
 	}
+	percpu_ref_put(&ctx->refs);
 }
 
 static int io_async_wake(struct wait_queue_entry *wait, unsigned mode, int sync,

Signature · 1 line

2.28.0

Greg KH

author maintainer Applied

2020-08-17 13:14 UTC #2 lore raw

Quoted 8 lines · Jens Axboe

Quoted 5 lines · On 8/17/20 3:44 AM, [email protected] wrote:
The patch below does not apply to the 5.8-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
Here's a 5.8 version.

Applied, thanks!

Looks like it applies to 5.7 too, want me to take this for that as well?

greg k-h

Jens Axboe

2020-08-17 13:21 UTC #3 lore raw

Quoted 8 lines · Greg KH

Quoted 7 lines · Jens Axboe
Quoted 5 lines · On 8/17/20 3:44 AM, [email protected] wrote:
The patch below does not apply to the 5.8-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

Quoted 5 lines

Quoted 1 line
Here's a 5.8 version.
Applied, thanks!
Looks like it applies to 5.7 too, want me to take this for that as well?

Heh, didn't see this email, just going through this by kernel revision. Either one should work, sent a specific set for that too.

BTW, for both 5.7 and 5.8, could you please queue up:

commit ebf0d100df0731901c16632f78d78d35f4123bc4

Author: Jens Axboe <[email protected]>
Date:   Thu Aug 13 09:01:38 2020 -0600

task_work: only grab task signal lock when needed

as well, to avoid a perf regression with the TWA_SIGNAL change? Thanks!

Signature · 1 line

Jens Axboe

Greg KH

author maintainer

2020-08-17 13:43 UTC #4 lore raw

Quoted 9 lines · Jens Axboe

Quoted 8 lines · Greg KH
Quoted 7 lines · Jens Axboe
Quoted 5 lines · On 8/17/20 3:44 AM, [email protected] wrote:
The patch below does not apply to the 5.8-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

Quoted 8 lines

Quoted 5 lines
Quoted 1 line
Here's a 5.8 version.
Applied, thanks!
Looks like it applies to 5.7 too, want me to take this for that as well?
Heh, didn't see this email, just going through this by kernel revision. Either one should work, sent a specific set for that too.

Oops, it did not build on 5.7, so I still need a working backport for that.

Quoted 9 lines

BTW, for both 5.7 and 5.8, could you please queue up:
commit ebf0d100df0731901c16632f78d78d35f4123bc4
Author: Jens Axboe <[email protected]>
Date:   Thu Aug 13 09:01:38 2020 -0600
task_work: only grab task signal lock when needed
as well, to avoid a perf regression with the TWA_SIGNAL change? Thanks!

Also now queued up, thanks.

greg k-h

Jens Axboe

2020-08-17 13:48 UTC #5 lore raw

Quoted 14 lines · Greg KH

Quoted 13 lines · Jens Axboe
Quoted 12 lines · Greg KH
Quoted 8 lines · Jens Axboe
Quoted 5 lines · On 8/17/20 3:44 AM, [email protected] wrote:
The patch below does not apply to the 5.8-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
Here's a 5.8 version.
Applied, thanks!

Quoted 7 lines

Quoted 4 lines
Quoted 1 line
Looks like it applies to 5.7 too, want me to take this for that as well?
Heh, didn't see this email, just going through this by kernel revision. Either one should work, sent a specific set for that too.
Oops, it did not build on 5.7, so I still need a working backport for that.

Maybe I missed that, in any case, here it is. This one is for 5.7, to be specific.

Quoted 9 lines

Quoted 7 lines
commit ebf0d100df0731901c16632f78d78d35f4123bc4
Author: Jens Axboe <[email protected]>
Date:   Thu Aug 13 09:01:38 2020 -0600
task_work: only grab task signal lock when needed
as well, to avoid a perf regression with the TWA_SIGNAL change? Thanks!
Also now queued up, thanks.

Thanks!

Signature · 1 line

Jens Axboe

From bf96cb6381c6765e8ae84aef546ea0b3f970599a Mon Sep 17 00:00:00 2001
From: Jens Axboe <[email protected]>
Date: Tue, 11 Aug 2020 08:04:14 -0600
Subject: [PATCH] io_uring: hold 'ctx' reference around task_work queue +
execute

Cc [email protected] # v5.7+

Reported-by [email protected]

Signed-off-by Jens Axboe <[email protected]>

---
 fs/io_uring.c | 6 ++++++
 1 file changed, 6 insertions(+)

fs/io_uring.c +6 -0

index 5e6bbcb60fc4..2e7cbe61f64c 100644
@@ -4202,6 +4202,8 @@ static int __io_async_wake(struct io_kiocb *req, struct io_poll_iocb *poll,
 	tsk = req->task;
 	req->result = mask;
 	init_task_work(&req->task_work, func);
+	percpu_ref_get(&req->ctx->refs);
+
 	/*
 	 * If this fails, then the task is exiting. When a task exits, the
 	 * work gets canceled, so just cancel this request as well instead

@@ -4301,6 +4303,7 @@ static void io_poll_task_handler(struct io_kiocb *req, struct io_kiocb **nxt)
 static void io_poll_task_func(struct callback_head *cb)
 {
 	struct io_kiocb *req = container_of(cb, struct io_kiocb, task_work);
+	struct io_ring_ctx *ctx = req->ctx;
 	struct io_kiocb *nxt = NULL;
 
 	io_poll_task_handler(req, &nxt);

@@ -4311,6 +4314,7 @@ static void io_poll_task_func(struct callback_head *cb)
 		__io_queue_sqe(nxt, NULL);
 		mutex_unlock(&ctx->uring_lock);
 	}
+	percpu_ref_put(&ctx->refs);
 }
 
 static int io_poll_double_wake(struct wait_queue_entry *wait, unsigned mode,

@@ -4427,6 +4431,7 @@ static void io_async_task_func(struct callback_head *cb)
 
 	if (io_poll_rewait(req, &apoll->poll)) {
 		spin_unlock_irq(&ctx->completion_lock);
+		percpu_ref_put(&ctx->refs);
 		return;
 	}

@@ -4465,6 +4470,7 @@ static void io_async_task_func(struct callback_head *cb)
 
 	kfree(apoll->double_poll);
 	kfree(apoll);
+	percpu_ref_put(&ctx->refs);
 }
 
 static int io_async_wake(struct wait_queue_entry *wait, unsigned mode, int sync,

Signature · 1 line

2.28.0

Greg KH

author maintainer

2020-08-17 13:55 UTC #6 lore raw

Quoted 17 lines · Jens Axboe

Quoted 16 lines · Greg KH
Quoted 15 lines · Jens Axboe
Quoted 13 lines · Greg KH
Quoted 8 lines · Jens Axboe
Quoted 5 lines · On 8/17/20 3:44 AM, [email protected] wrote:
The patch below does not apply to the 5.8-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
Here's a 5.8 version.
Applied, thanks!
Looks like it applies to 5.7 too, want me to take this for that as well?

Quoted 8 lines

Quoted 5 lines
Quoted 2 lines
Heh, didn't see this email, just going through this by kernel revision. Either one should work, sent a specific set for that too.
Oops, it did not build on 5.7, so I still need a working backport for that.
Maybe I missed that, in any case, here it is. This one is for 5.7, to be specific.

That worked, thanks!

greg k-h

Jens Axboe

2020-08-17 14:03 UTC #7 lore raw

Quoted 21 lines · Greg KH

Quoted 20 lines · Jens Axboe
Quoted 19 lines · Greg KH
Quoted 17 lines · Jens Axboe
Quoted 8 lines · Greg KH
Quoted 7 lines · Jens Axboe
Quoted 5 lines · On 8/17/20 3:44 AM, [email protected] wrote:
The patch below does not apply to the 5.8-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
Quoted 5 lines
Quoted 1 line
Here's a 5.8 version.
Applied, thanks!
Looks like it applies to 5.7 too, want me to take this for that as well?
Heh, didn't see this email, just going through this by kernel revision. Either one should work, sent a specific set for that too.

Quoted 7 lines

Quoted 5 lines
Quoted 2 lines
Oops, it did not build on 5.7, so I still need a working backport for that.
Maybe I missed that, in any case, here it is. This one is for 5.7, to be specific.
That worked, thanks!

Great, thanks. I think that concludes the stable fest from me this morning :-)

Signature · 1 line

Jens Axboe