FAILED: patch "[PATCH] io_uring: lock overflowing for IOPOLL" failed to apply to 5.15-stable tree

PATCH [email protected] 2 participants 5 messages Jan 14, 2023 – Jan 16, 2023

Subsystems (2)

Stable branch io_uring

lore

Share & tools

lore

[email protected]

author maintainer

2023-01-14 09:52 UTC #0 lore raw

The patch below does not apply to the 5.15-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]>.

Possible dependencies:

544d163d659d ("io_uring: lock overflowing for IOPOLL") a8cf95f93610 ("io_uring: fix overflow handling regression") fa18fa2272c7 ("io_uring: inline __io_req_complete_put()") f9d567c75ec2 ("io_uring: inline __io_req_complete_post()") 52120f0fadcb ("io_uring: add allow_overflow to io_post_aux_cqe") e6130eba8a84 ("io_uring: add support for passing fixed file descriptors") 253993210bd8 ("io_uring: introduce locking helpers for CQE posting") 305bef988708 ("io_uring: hide eventfd assumptions in eventfd paths") d9dee4302a7c ("io_uring: remove ->flush_cqes optimisation") a830ffd28780 ("io_uring: move io_eventfd_signal()") 9046c6415be6 ("io_uring: reshuffle io_uring/io_uring.h") d142c3ec8d16 ("io_uring: remove extra io_commit_cqring()") 68494a65d0e2 ("io_uring: introduce io_req_cqe_overflow()") faf88dde060f ("io_uring: don't inline __io_get_cqe()") d245bca6375b ("io_uring: don't expose io_fill_cqe_aux()") aa1e90f64ee5 ("io_uring: move small helpers to headers") aa1e90f64ee5 ("io_uring: move small helpers to headers") aa1e90f64ee5 ("io_uring: move small helpers to headers") aa1e90f64ee5 ("io_uring: move small helpers to headers") aa1e90f64ee5 ("io_uring: move small helpers to headers")

thanks,

greg k-h

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

From 544d163d659d45a206d8929370d5a2984e546cb7 Mon Sep 17 00:00:00 2001
From: Pavel Begunkov <[email protected]>
Date: Thu, 12 Jan 2023 13:08:56 +0000
Subject: [PATCH] io_uring: lock overflowing for IOPOLL
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

syzbot reports an issue with overflow filling for IOPOLL:

WARNING: CPU: 0 PID: 28 at io_uring/io_uring.c:734 io_cqring_event_overflow+0x1c0/0x230 io_uring/io_uring.c:734
CPU: 0 PID: 28 Comm: kworker/u4:1 Not tainted 6.2.0-rc3-syzkaller-16369-g358a161a6a9e #0
Workqueue: events_unbound io_ring_exit_work
Call trace:
 io_cqring_event_overflow+0x1c0/0x230 io_uring/io_uring.c:734
 io_req_cqe_overflow+0x5c/0x70 io_uring/io_uring.c:773
 io_fill_cqe_req io_uring/io_uring.h:168 [inline]
 io_do_iopoll+0x474/0x62c io_uring/rw.c:1065
 io_iopoll_try_reap_events+0x6c/0x108 io_uring/io_uring.c:1513
 io_uring_try_cancel_requests+0x13c/0x258 io_uring/io_uring.c:3056
 io_ring_exit_work+0xec/0x390 io_uring/io_uring.c:2869
 process_one_work+0x2d8/0x504 kernel/workqueue.c:2289
 worker_thread+0x340/0x610 kernel/workqueue.c:2436
 kthread+0x12c/0x158 kernel/kthread.c:376
 ret_from_fork+0x10/0x20 arch/arm64/kernel/entry.S:863

There is no real problem for normal IOPOLL as flush is also called with uring_lock taken, but it's getting more complicated for IOPOLL|SQPOLL, for which __io_cqring_overflow_flush() happens from the CQ waiting path.

Reported-and-tested-by [email protected]

Cc [email protected] # 5.10+

Signed-off-by Pavel Begunkov <[email protected]>

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

io_uring/rw.c +5 -1

index 8227af2e1c0f..9c3ddd46a1ad 100644
@@ -1062,7 +1062,11 @@ int io_do_iopoll(struct io_ring_ctx *ctx, bool force_nonspin)
 			continue;
 
 		req->cqe.flags = io_put_kbuf(req, 0);
-		io_fill_cqe_req(req->ctx, req);
+		if (unlikely(!__io_fill_cqe_req(ctx, req))) {
+			spin_lock(&ctx->completion_lock);
+			io_req_cqe_overflow(req);
+			spin_unlock(&ctx->completion_lock);
+		}
 	}
 
 	if (unlikely(!nr_events))

Jens Axboe

maintainer

2023-01-14 16:15 UTC #1 lore raw

Quoted 5 lines · On 1/14/23 2:51 AM, [email protected] wrote:

The patch below does not apply to the 5.15-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]>.

This has to be done a bit differently, but this one should work. I tested it on 5.10-stable, but should apply to 5.15-stable as well as they are the same base now.

Signature · 1 line

Jens Axboe

From 313cc3d4c3f6a1c6d0df525f0368a2c5a7ce97c8 Mon Sep 17 00:00:00 2001
From: Pavel Begunkov <[email protected]>
Date: Sat, 14 Jan 2023 09:14:03 -0700
Subject: [PATCH] io_uring: lock overflowing for IOPOLL
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

commit 544d163d659d45a206d8929370d5a2984e546cb7 upstream.

syzbot reports an issue with overflow filling for IOPOLL:

WARNING: CPU: 0 PID: 28 at io_uring/io_uring.c:734 io_cqring_event_overflow+0x1c0/0x230 io_uring/io_uring.c:734
CPU: 0 PID: 28 Comm: kworker/u4:1 Not tainted 6.2.0-rc3-syzkaller-16369-g358a161a6a9e #0
Workqueue: events_unbound io_ring_exit_work
Call trace:
 io_cqring_event_overflow+0x1c0/0x230 io_uring/io_uring.c:734
 io_req_cqe_overflow+0x5c/0x70 io_uring/io_uring.c:773
 io_fill_cqe_req io_uring/io_uring.h:168 [inline]
 io_do_iopoll+0x474/0x62c io_uring/rw.c:1065
 io_iopoll_try_reap_events+0x6c/0x108 io_uring/io_uring.c:1513
 io_uring_try_cancel_requests+0x13c/0x258 io_uring/io_uring.c:3056
 io_ring_exit_work+0xec/0x390 io_uring/io_uring.c:2869
 process_one_work+0x2d8/0x504 kernel/workqueue.c:2289
 worker_thread+0x340/0x610 kernel/workqueue.c:2436
 kthread+0x12c/0x158 kernel/kthread.c:376
 ret_from_fork+0x10/0x20 arch/arm64/kernel/entry.S:863

Reported-and-tested-by [email protected]

Cc [email protected] # 5.10+

Signed-off-by Pavel Begunkov <[email protected]>

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

---
 io_uring/io_uring.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

io_uring/io_uring.c +15 -1

index 0c4d16afb9ef..1a3f360af373 100644
@@ -2478,10 +2478,24 @@ static void io_iopoll_complete(struct io_ring_ctx *ctx, unsigned int *nr_events,
 
 	io_init_req_batch(&rb);
 	while (!list_empty(done)) {
+		struct io_uring_cqe *cqe;
+		unsigned cflags;
+
 		req = list_first_entry(done, struct io_kiocb, inflight_entry);
 		list_del(&req->inflight_entry);
+		cflags = io_put_rw_kbuf(req);
 
-		io_fill_cqe_req(req, req->result, io_put_rw_kbuf(req));
+		cqe = io_get_cqe(ctx);
+		if (unlikely(!cqe)) {
+			spin_lock(&ctx->completion_lock);
+			io_cqring_event_overflow(ctx, req->user_data,
+							req->result, cflags);
+			spin_unlock(&ctx->completion_lock);
+			continue;
+		}
+		WRITE_ONCE(cqe->user_data, req->user_data);
+		WRITE_ONCE(cqe->res, req->result);
+		WRITE_ONCE(cqe->flags, cflags);
 		(*nr_events)++;
 
 		if (req_ref_put_and_test(req))

Signature · 1 line

2.39.0

Jens Axboe

maintainer

2023-01-14 16:21 UTC #2 lore raw

Quoted 10 lines · Jens Axboe

Quoted 5 lines · On 1/14/23 2:51 AM, [email protected] wrote:
The patch below does not apply to the 5.15-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]>.
This has to be done a bit differently, but this one should work. I tested it on 5.10-stable, but should apply to 5.15-stable as well as they are the same base now.

Oops, missed accounting for overflow. Please use this one instead! Sorry about that.

Signature · 1 line

Jens Axboe

From 929e5c54ff2ad116e9dcee967fc5df1dbb6b1d90 Mon Sep 17 00:00:00 2001
From: Pavel Begunkov <[email protected]>
Date: Sat, 14 Jan 2023 09:14:03 -0700
Subject: [PATCH] io_uring: lock overflowing for IOPOLL
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

commit 544d163d659d45a206d8929370d5a2984e546cb7 upstream.

syzbot reports an issue with overflow filling for IOPOLL:

WARNING: CPU: 0 PID: 28 at io_uring/io_uring.c:734 io_cqring_event_overflow+0x1c0/0x230 io_uring/io_uring.c:734
CPU: 0 PID: 28 Comm: kworker/u4:1 Not tainted 6.2.0-rc3-syzkaller-16369-g358a161a6a9e #0
Workqueue: events_unbound io_ring_exit_work
Call trace:
 io_cqring_event_overflow+0x1c0/0x230 io_uring/io_uring.c:734
 io_req_cqe_overflow+0x5c/0x70 io_uring/io_uring.c:773
 io_fill_cqe_req io_uring/io_uring.h:168 [inline]
 io_do_iopoll+0x474/0x62c io_uring/rw.c:1065
 io_iopoll_try_reap_events+0x6c/0x108 io_uring/io_uring.c:1513
 io_uring_try_cancel_requests+0x13c/0x258 io_uring/io_uring.c:3056
 io_ring_exit_work+0xec/0x390 io_uring/io_uring.c:2869
 process_one_work+0x2d8/0x504 kernel/workqueue.c:2289
 worker_thread+0x340/0x610 kernel/workqueue.c:2436
 kthread+0x12c/0x158 kernel/kthread.c:376
 ret_from_fork+0x10/0x20 arch/arm64/kernel/entry.S:863

Reported-and-tested-by [email protected]

Cc [email protected] # 5.10+

Signed-off-by Pavel Begunkov <[email protected]>

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

---
 io_uring/io_uring.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

io_uring/io_uring.c +16 -2

index 0c4d16afb9ef..c3838a2c5c6f 100644
@@ -2478,12 +2478,26 @@ static void io_iopoll_complete(struct io_ring_ctx *ctx, unsigned int *nr_events,
 
 	io_init_req_batch(&rb);
 	while (!list_empty(done)) {
+		struct io_uring_cqe *cqe;
+		unsigned cflags;
+
 		req = list_first_entry(done, struct io_kiocb, inflight_entry);
 		list_del(&req->inflight_entry);
-
-		io_fill_cqe_req(req, req->result, io_put_rw_kbuf(req));
+		cflags = io_put_rw_kbuf(req);
 		(*nr_events)++;
 
+		cqe = io_get_cqe(ctx);
+		if (unlikely(!cqe)) {
+			spin_lock(&ctx->completion_lock);
+			io_cqring_event_overflow(ctx, req->user_data,
+							req->result, cflags);
+			spin_unlock(&ctx->completion_lock);
+			continue;
+		}
+		WRITE_ONCE(cqe->user_data, req->user_data);
+		WRITE_ONCE(cqe->res, req->result);
+		WRITE_ONCE(cqe->flags, cflags);
+
 		if (req_ref_put_and_test(req))
 			io_req_free_batch(&rb, req, &ctx->submit_state);
 	}

Signature · 1 line

2.39.0

Jens Axboe

maintainer

2023-01-14 16:48 UTC #3 lore raw

Quoted 8 lines · Jens Axboe

Quoted 7 lines · Jens Axboe
Quoted 5 lines · On 1/14/23 2:51?AM, [email protected] wrote:
The patch below does not apply to the 5.15-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 6 lines

Quoted 3 lines
This has to be done a bit differently, but this one should work. I tested it on 5.10-stable, but should apply to 5.15-stable as well as they are the same base now.
Oops, missed accounting for overflow. Please use this one instead! Sorry about that.

Not batting 1000 on this one today. Wrote a test case for this specifically now and verified it, this one is good. For real.

Signature · 1 line

Jens Axboe

From 31f897396bbbd37010260119293e84007736dbc7 Mon Sep 17 00:00:00 2001
From: Pavel Begunkov <[email protected]>
Date: Sat, 14 Jan 2023 09:14:03 -0700
Subject: [PATCH] io_uring: lock overflowing for IOPOLL
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

commit 544d163d659d45a206d8929370d5a2984e546cb7 upstream.

syzbot reports an issue with overflow filling for IOPOLL:

WARNING: CPU: 0 PID: 28 at io_uring/io_uring.c:734 io_cqring_event_overflow+0x1c0/0x230 io_uring/io_uring.c:734
CPU: 0 PID: 28 Comm: kworker/u4:1 Not tainted 6.2.0-rc3-syzkaller-16369-g358a161a6a9e #0
Workqueue: events_unbound io_ring_exit_work
Call trace:
 io_cqring_event_overflow+0x1c0/0x230 io_uring/io_uring.c:734
 io_req_cqe_overflow+0x5c/0x70 io_uring/io_uring.c:773
 io_fill_cqe_req io_uring/io_uring.h:168 [inline]
 io_do_iopoll+0x474/0x62c io_uring/rw.c:1065
 io_iopoll_try_reap_events+0x6c/0x108 io_uring/io_uring.c:1513
 io_uring_try_cancel_requests+0x13c/0x258 io_uring/io_uring.c:3056
 io_ring_exit_work+0xec/0x390 io_uring/io_uring.c:2869
 process_one_work+0x2d8/0x504 kernel/workqueue.c:2289
 worker_thread+0x340/0x610 kernel/workqueue.c:2436
 kthread+0x12c/0x158 kernel/kthread.c:376
 ret_from_fork+0x10/0x20 arch/arm64/kernel/entry.S:863

Reported-and-tested-by [email protected]

Cc [email protected] # 5.10+

Signed-off-by Pavel Begunkov <[email protected]>

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

---
 io_uring/io_uring.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

io_uring/io_uring.c +16 -2

index 0c4d16afb9ef..cb83ba50b017 100644
@@ -2478,12 +2478,26 @@ static void io_iopoll_complete(struct io_ring_ctx *ctx, unsigned int *nr_events,
 
 	io_init_req_batch(&rb);
 	while (!list_empty(done)) {
+		struct io_uring_cqe *cqe;
+		unsigned cflags;
+
 		req = list_first_entry(done, struct io_kiocb, inflight_entry);
 		list_del(&req->inflight_entry);
-
-		io_fill_cqe_req(req, req->result, io_put_rw_kbuf(req));
+		cflags = io_put_rw_kbuf(req);
 		(*nr_events)++;
 
+		cqe = io_get_cqe(ctx);
+		if (cqe) {
+			WRITE_ONCE(cqe->user_data, req->user_data);
+			WRITE_ONCE(cqe->res, req->result);
+			WRITE_ONCE(cqe->flags, cflags);
+		} else {
+			spin_lock(&ctx->completion_lock);
+			io_cqring_event_overflow(ctx, req->user_data,
+							req->result, cflags);
+			spin_unlock(&ctx->completion_lock);
+		}
+
 		if (req_ref_put_and_test(req))
 			io_req_free_batch(&rb, req, &ctx->submit_state);
 	}

Signature · 1 line

2.39.0

Greg KH

author maintainer

2023-01-16 14:58 UTC #4 lore raw

Quoted 13 lines · Jens Axboe

Quoted 12 lines · Jens Axboe
Quoted 10 lines · Jens Axboe
Quoted 5 lines · On 1/14/23 2:51?AM, [email protected] wrote:
The patch below does not apply to the 5.15-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]>.
This has to be done a bit differently, but this one should work. I tested it on 5.10-stable, but should apply to 5.15-stable as well as they are the same base now.

Quoted 5 lines

Quoted 2 lines
Oops, missed accounting for overflow. Please use this one instead! Sorry about that.
Not batting 1000 on this one today. Wrote a test case for this specifically now and verified it, this one is good. For real.

Looks like Sasha grabbed all of these already, thanks for the backports!

greg k-h