All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 5.10-stable 0/3] backports for 5.10
@ 2021-01-12 21:17 Pavel Begunkov
  2021-01-12 21:17 ` [PATCH 5.10-stable 1/3] io_uring: synchronise IOPOLL on task_submit fail Pavel Begunkov
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Pavel Begunkov @ 2021-01-12 21:17 UTC (permalink / raw)
  To: gregkh; +Cc: stable, Jens Axboe

5.10-stable, 2/3 is a dependency, the other two failed to apply.

Pavel Begunkov (3):
  io_uring: synchronise IOPOLL on task_submit fail
  io_uring: limit {io|sq}poll submit locking scope
  io_uring: patch up IOPOLL overflow_flush sync

 fs/io_uring.c | 88 ++++++++++++++++++++++++++++-----------------------
 1 file changed, 48 insertions(+), 40 deletions(-)

-- 
2.24.0


^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH 5.10-stable 1/3] io_uring: synchronise IOPOLL on task_submit fail
  2021-01-12 21:17 [PATCH 5.10-stable 0/3] backports for 5.10 Pavel Begunkov
@ 2021-01-12 21:17 ` Pavel Begunkov
  2021-01-13 16:54   ` Sasha Levin
  2021-01-12 21:17 ` [PATCH 5.10-stable 2/3] io_uring: limit {io|sq}poll submit locking scope Pavel Begunkov
  2021-01-12 21:17 ` [PATCH 5.10-stable 3/3] io_uring: patch up IOPOLL overflow_flush sync Pavel Begunkov
  2 siblings, 1 reply; 6+ messages in thread
From: Pavel Begunkov @ 2021-01-12 21:17 UTC (permalink / raw)
  To: gregkh; +Cc: stable, Jens Axboe

commit 81b6d05ccad4f3d8a9dfb091fb46ad6978ee40e4 upstream

io_req_task_submit() might be called for IOPOLL, do the fail path under
uring_lock to comply with IOPOLL synchronisation based solely on it.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 fs/io_uring.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 1f798c5c4213..3974b4f124b6 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -2047,14 +2047,15 @@ static void io_req_task_cancel(struct callback_head *cb)
 static void __io_req_task_submit(struct io_kiocb *req)
 {
 	struct io_ring_ctx *ctx = req->ctx;
+	bool fail;
 
-	if (!__io_sq_thread_acquire_mm(ctx)) {
-		mutex_lock(&ctx->uring_lock);
+	fail = __io_sq_thread_acquire_mm(ctx);
+	mutex_lock(&ctx->uring_lock);
+	if (!fail)
 		__io_queue_sqe(req, NULL);
-		mutex_unlock(&ctx->uring_lock);
-	} else {
+	else
 		__io_req_task_cancel(req, -EFAULT);
-	}
+	mutex_unlock(&ctx->uring_lock);
 }
 
 static void io_req_task_submit(struct callback_head *cb)
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 5.10-stable 2/3] io_uring: limit {io|sq}poll submit locking scope
  2021-01-12 21:17 [PATCH 5.10-stable 0/3] backports for 5.10 Pavel Begunkov
  2021-01-12 21:17 ` [PATCH 5.10-stable 1/3] io_uring: synchronise IOPOLL on task_submit fail Pavel Begunkov
@ 2021-01-12 21:17 ` Pavel Begunkov
  2021-01-12 21:17 ` [PATCH 5.10-stable 3/3] io_uring: patch up IOPOLL overflow_flush sync Pavel Begunkov
  2 siblings, 0 replies; 6+ messages in thread
From: Pavel Begunkov @ 2021-01-12 21:17 UTC (permalink / raw)
  To: gregkh; +Cc: stable, Jens Axboe

commit 89448c47b8452b67c146dc6cad6f737e004c5caf upstream

We don't need to take uring_lock for SQPOLL|IOPOLL to do
io_cqring_overflow_flush() when cq_overflow_list is empty, remove it
from the hot path.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 fs/io_uring.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 3974b4f124b6..5ba312ab9978 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -9024,10 +9024,13 @@ SYSCALL_DEFINE6(io_uring_enter, unsigned int, fd, u32, to_submit,
 	 */
 	ret = 0;
 	if (ctx->flags & IORING_SETUP_SQPOLL) {
-		io_ring_submit_lock(ctx, (ctx->flags & IORING_SETUP_IOPOLL));
-		if (!list_empty_careful(&ctx->cq_overflow_list))
+		if (!list_empty_careful(&ctx->cq_overflow_list)) {
+			bool needs_lock = ctx->flags & IORING_SETUP_IOPOLL;
+
+			io_ring_submit_lock(ctx, needs_lock);
 			io_cqring_overflow_flush(ctx, false, NULL, NULL);
-		io_ring_submit_unlock(ctx, (ctx->flags & IORING_SETUP_IOPOLL));
+			io_ring_submit_unlock(ctx, needs_lock);
+		}
 		if (flags & IORING_ENTER_SQ_WAKEUP)
 			wake_up(&ctx->sq_data->wait);
 		if (flags & IORING_ENTER_SQ_WAIT)
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 5.10-stable 3/3] io_uring: patch up IOPOLL overflow_flush sync
  2021-01-12 21:17 [PATCH 5.10-stable 0/3] backports for 5.10 Pavel Begunkov
  2021-01-12 21:17 ` [PATCH 5.10-stable 1/3] io_uring: synchronise IOPOLL on task_submit fail Pavel Begunkov
  2021-01-12 21:17 ` [PATCH 5.10-stable 2/3] io_uring: limit {io|sq}poll submit locking scope Pavel Begunkov
@ 2021-01-12 21:17 ` Pavel Begunkov
  2 siblings, 0 replies; 6+ messages in thread
From: Pavel Begunkov @ 2021-01-12 21:17 UTC (permalink / raw)
  To: gregkh; +Cc: stable, Jens Axboe

commit 6c503150ae33ee19036255cfda0998463613352c upstream

IOPOLL skips completion locking but keeps it under uring_lock, thus
io_cqring_overflow_flush() and so io_cqring_events() need additional
locking with uring_lock in some cases for IOPOLL.

Remove __io_cqring_overflow_flush() from io_cqring_events(), introduce a
wrapper around flush doing needed synchronisation and call it by hand.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 fs/io_uring.c | 78 +++++++++++++++++++++++++++------------------------
 1 file changed, 41 insertions(+), 37 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 5ba312ab9978..492492a010a2 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -1625,9 +1625,9 @@ static bool io_match_files(struct io_kiocb *req,
 }
 
 /* Returns true if there are no backlogged entries after the flush */
-static bool io_cqring_overflow_flush(struct io_ring_ctx *ctx, bool force,
-				     struct task_struct *tsk,
-				     struct files_struct *files)
+static bool __io_cqring_overflow_flush(struct io_ring_ctx *ctx, bool force,
+				       struct task_struct *tsk,
+				       struct files_struct *files)
 {
 	struct io_rings *rings = ctx->rings;
 	struct io_kiocb *req, *tmp;
@@ -1681,6 +1681,20 @@ static bool io_cqring_overflow_flush(struct io_ring_ctx *ctx, bool force,
 	return cqe != NULL;
 }
 
+static void io_cqring_overflow_flush(struct io_ring_ctx *ctx, bool force,
+				     struct task_struct *tsk,
+				     struct files_struct *files)
+{
+	if (test_bit(0, &ctx->cq_check_overflow)) {
+		/* iopoll syncs against uring_lock, not completion_lock */
+		if (ctx->flags & IORING_SETUP_IOPOLL)
+			mutex_lock(&ctx->uring_lock);
+		__io_cqring_overflow_flush(ctx, force, tsk, files);
+		if (ctx->flags & IORING_SETUP_IOPOLL)
+			mutex_unlock(&ctx->uring_lock);
+	}
+}
+
 static void __io_cqring_fill_event(struct io_kiocb *req, long res, long cflags)
 {
 	struct io_ring_ctx *ctx = req->ctx;
@@ -2235,22 +2249,10 @@ static void io_double_put_req(struct io_kiocb *req)
 		io_free_req(req);
 }
 
-static unsigned io_cqring_events(struct io_ring_ctx *ctx, bool noflush)
+static unsigned io_cqring_events(struct io_ring_ctx *ctx)
 {
 	struct io_rings *rings = ctx->rings;
 
-	if (test_bit(0, &ctx->cq_check_overflow)) {
-		/*
-		 * noflush == true is from the waitqueue handler, just ensure
-		 * we wake up the task, and the next invocation will flush the
-		 * entries. We cannot safely to it from here.
-		 */
-		if (noflush)
-			return -1U;
-
-		io_cqring_overflow_flush(ctx, false, NULL, NULL);
-	}
-
 	/* See comment at the top of this file */
 	smp_rmb();
 	return ctx->cached_cq_tail - READ_ONCE(rings->cq.head);
@@ -2475,7 +2477,9 @@ static int io_iopoll_check(struct io_ring_ctx *ctx, long min)
 		 * If we do, we can potentially be spinning for commands that
 		 * already triggered a CQE (eg in error).
 		 */
-		if (io_cqring_events(ctx, false))
+		if (test_bit(0, &ctx->cq_check_overflow))
+			__io_cqring_overflow_flush(ctx, false, NULL, NULL);
+		if (io_cqring_events(ctx))
 			break;
 
 		/*
@@ -6578,7 +6582,7 @@ static int io_submit_sqes(struct io_ring_ctx *ctx, unsigned int nr)
 
 	/* if we have a backlog and couldn't flush it all, return BUSY */
 	if (test_bit(0, &ctx->sq_check_overflow)) {
-		if (!io_cqring_overflow_flush(ctx, false, NULL, NULL))
+		if (!__io_cqring_overflow_flush(ctx, false, NULL, NULL))
 			return -EBUSY;
 	}
 
@@ -6867,7 +6871,7 @@ struct io_wait_queue {
 	unsigned nr_timeouts;
 };
 
-static inline bool io_should_wake(struct io_wait_queue *iowq, bool noflush)
+static inline bool io_should_wake(struct io_wait_queue *iowq)
 {
 	struct io_ring_ctx *ctx = iowq->ctx;
 
@@ -6876,7 +6880,7 @@ static inline bool io_should_wake(struct io_wait_queue *iowq, bool noflush)
 	 * started waiting. For timeouts, we always want to return to userspace,
 	 * regardless of event count.
 	 */
-	return io_cqring_events(ctx, noflush) >= iowq->to_wait ||
+	return io_cqring_events(ctx) >= iowq->to_wait ||
 			atomic_read(&ctx->cq_timeouts) != iowq->nr_timeouts;
 }
 
@@ -6886,11 +6890,13 @@ static int io_wake_function(struct wait_queue_entry *curr, unsigned int mode,
 	struct io_wait_queue *iowq = container_of(curr, struct io_wait_queue,
 							wq);
 
-	/* use noflush == true, as we can't safely rely on locking context */
-	if (!io_should_wake(iowq, true))
-		return -1;
-
-	return autoremove_wake_function(curr, mode, wake_flags, key);
+	/*
+	 * Cannot safely flush overflowed CQEs from here, ensure we wake up
+	 * the task, and the next invocation will do it.
+	 */
+	if (io_should_wake(iowq) || test_bit(0, &iowq->ctx->cq_check_overflow))
+		return autoremove_wake_function(curr, mode, wake_flags, key);
+	return -1;
 }
 
 static int io_run_task_work_sig(void)
@@ -6929,7 +6935,8 @@ static int io_cqring_wait(struct io_ring_ctx *ctx, int min_events,
 	int ret = 0;
 
 	do {
-		if (io_cqring_events(ctx, false) >= min_events)
+		io_cqring_overflow_flush(ctx, false, NULL, NULL);
+		if (io_cqring_events(ctx) >= min_events)
 			return 0;
 		if (!io_run_task_work())
 			break;
@@ -6951,6 +6958,7 @@ static int io_cqring_wait(struct io_ring_ctx *ctx, int min_events,
 	iowq.nr_timeouts = atomic_read(&ctx->cq_timeouts);
 	trace_io_uring_cqring_wait(ctx, min_events);
 	do {
+		io_cqring_overflow_flush(ctx, false, NULL, NULL);
 		prepare_to_wait_exclusive(&ctx->wait, &iowq.wq,
 						TASK_INTERRUPTIBLE);
 		/* make sure we run task_work before checking for signals */
@@ -6959,8 +6967,10 @@ static int io_cqring_wait(struct io_ring_ctx *ctx, int min_events,
 			continue;
 		else if (ret < 0)
 			break;
-		if (io_should_wake(&iowq, false))
+		if (io_should_wake(&iowq))
 			break;
+		if (test_bit(0, &ctx->cq_check_overflow))
+			continue;
 		schedule();
 	} while (1);
 	finish_wait(&ctx->wait, &iowq.wq);
@@ -8385,7 +8395,8 @@ static __poll_t io_uring_poll(struct file *file, poll_table *wait)
 	smp_rmb();
 	if (!io_sqring_full(ctx))
 		mask |= EPOLLOUT | EPOLLWRNORM;
-	if (io_cqring_events(ctx, false))
+	io_cqring_overflow_flush(ctx, false, NULL, NULL);
+	if (io_cqring_events(ctx))
 		mask |= EPOLLIN | EPOLLRDNORM;
 
 	return mask;
@@ -8443,7 +8454,7 @@ static void io_ring_ctx_wait_and_kill(struct io_ring_ctx *ctx)
 	/* if force is set, the ring is going away. always drop after that */
 	ctx->cq_overflow_flushed = 1;
 	if (ctx->rings)
-		io_cqring_overflow_flush(ctx, true, NULL, NULL);
+		__io_cqring_overflow_flush(ctx, true, NULL, NULL);
 	mutex_unlock(&ctx->uring_lock);
 
 	io_kill_timeouts(ctx, NULL);
@@ -8716,9 +8727,7 @@ static void io_uring_cancel_task_requests(struct io_ring_ctx *ctx,
 	}
 
 	io_cancel_defer_files(ctx, task, files);
-	io_ring_submit_lock(ctx, (ctx->flags & IORING_SETUP_IOPOLL));
 	io_cqring_overflow_flush(ctx, true, task, files);
-	io_ring_submit_unlock(ctx, (ctx->flags & IORING_SETUP_IOPOLL));
 
 	while (__io_uring_cancel_task_requests(ctx, task, files)) {
 		io_run_task_work();
@@ -9024,13 +9033,8 @@ SYSCALL_DEFINE6(io_uring_enter, unsigned int, fd, u32, to_submit,
 	 */
 	ret = 0;
 	if (ctx->flags & IORING_SETUP_SQPOLL) {
-		if (!list_empty_careful(&ctx->cq_overflow_list)) {
-			bool needs_lock = ctx->flags & IORING_SETUP_IOPOLL;
+		io_cqring_overflow_flush(ctx, false, NULL, NULL);
 
-			io_ring_submit_lock(ctx, needs_lock);
-			io_cqring_overflow_flush(ctx, false, NULL, NULL);
-			io_ring_submit_unlock(ctx, needs_lock);
-		}
 		if (flags & IORING_ENTER_SQ_WAKEUP)
 			wake_up(&ctx->sq_data->wait);
 		if (flags & IORING_ENTER_SQ_WAIT)
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH 5.10-stable 1/3] io_uring: synchronise IOPOLL on task_submit fail
  2021-01-12 21:17 ` [PATCH 5.10-stable 1/3] io_uring: synchronise IOPOLL on task_submit fail Pavel Begunkov
@ 2021-01-13 16:54   ` Sasha Levin
  2021-01-13 16:58     ` Pavel Begunkov
  0 siblings, 1 reply; 6+ messages in thread
From: Sasha Levin @ 2021-01-13 16:54 UTC (permalink / raw)
  To: Pavel Begunkov; +Cc: gregkh, stable, Jens Axboe

On Tue, Jan 12, 2021 at 09:17:24PM +0000, Pavel Begunkov wrote:
>commit 81b6d05ccad4f3d8a9dfb091fb46ad6978ee40e4 upstream
>
>io_req_task_submit() might be called for IOPOLL, do the fail path under
>uring_lock to comply with IOPOLL synchronisation based solely on it.

Just curious, why did the stable tag disappear from this backport?

Otherwise, I've queued up the series, thanks!

-- 
Thanks,
Sasha

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 5.10-stable 1/3] io_uring: synchronise IOPOLL on task_submit fail
  2021-01-13 16:54   ` Sasha Levin
@ 2021-01-13 16:58     ` Pavel Begunkov
  0 siblings, 0 replies; 6+ messages in thread
From: Pavel Begunkov @ 2021-01-13 16:58 UTC (permalink / raw)
  To: Sasha Levin; +Cc: gregkh, stable, Jens Axboe

On 13/01/2021 16:54, Sasha Levin wrote:
> On Tue, Jan 12, 2021 at 09:17:24PM +0000, Pavel Begunkov wrote:
>> commit 81b6d05ccad4f3d8a9dfb091fb46ad6978ee40e4 upstream
>>
>> io_req_task_submit() might be called for IOPOLL, do the fail path under
>> uring_lock to comply with IOPOLL synchronisation based solely on it.
> 
> Just curious, why did the stable tag disappear from this backport?

Probably I just didn't pay attention and erased while adding "upstream
commit" and so. Didn't know that it's better to leave it though.

> Otherwise, I've queued up the series, thanks!

Thanks!

-- 
Pavel Begunkov

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-01-13 17:02 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-12 21:17 [PATCH 5.10-stable 0/3] backports for 5.10 Pavel Begunkov
2021-01-12 21:17 ` [PATCH 5.10-stable 1/3] io_uring: synchronise IOPOLL on task_submit fail Pavel Begunkov
2021-01-13 16:54   ` Sasha Levin
2021-01-13 16:58     ` Pavel Begunkov
2021-01-12 21:17 ` [PATCH 5.10-stable 2/3] io_uring: limit {io|sq}poll submit locking scope Pavel Begunkov
2021-01-12 21:17 ` [PATCH 5.10-stable 3/3] io_uring: patch up IOPOLL overflow_flush sync Pavel Begunkov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.