All of lore.kernel.org
 help / color / mirror / Atom feed
* FAILED: patch "[PATCH] io_uring/net: limit inline multishot retries" failed to apply to 6.6-stable tree
@ 2024-02-13 13:19 gregkh
  2024-02-13 14:52 ` Jens Axboe
  0 siblings, 1 reply; 6+ messages in thread
From: gregkh @ 2024-02-13 13:19 UTC (permalink / raw)
  To: axboe; +Cc: stable


The patch below does not apply to the 6.6-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 <stable@vger.kernel.org>.

To reproduce the conflict and resubmit, you may use the following commands:

git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.6.y
git checkout FETCH_HEAD
git cherry-pick -x 76b367a2d83163cf19173d5cb0b562acbabc8eac
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<stable@vger.kernel.org>' --in-reply-to '2024021330-twice-pacify-2be5@gregkh' --subject-prefix 'PATCH 6.6.y' HEAD^..

Possible dependencies:

76b367a2d831 ("io_uring/net: limit inline multishot retries")
91e5d765a82f ("io_uring/net: un-indent mshot retry path in io_recv_finish()")

thanks,

greg k-h

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

From 76b367a2d83163cf19173d5cb0b562acbabc8eac Mon Sep 17 00:00:00 2001
From: Jens Axboe <axboe@kernel.dk>
Date: Mon, 29 Jan 2024 12:00:58 -0700
Subject: [PATCH] io_uring/net: limit inline multishot retries

If we have multiple clients and some/all are flooding the receives to
such an extent that we can retry a LOT handling multishot receives, then
we can be starving some clients and hence serving traffic in an
imbalanced fashion.

Limit multishot retry attempts to some arbitrary value, whose only
purpose serves to ensure that we don't keep serving a single connection
for way too long. We default to 32 retries, which should be more than
enough to provide fairness, yet not so small that we'll spend too much
time requeuing rather than handling traffic.

Cc: stable@vger.kernel.org
Depends-on: 704ea888d646 ("io_uring/poll: add requeue return code from poll multishot handling")
Depends-on: 1e5d765a82f ("io_uring/net: un-indent mshot retry path in io_recv_finish()")
Depends-on: e84b01a880f6 ("io_uring/poll: move poll execution helpers higher up")
Fixes: b3fdea6ecb55 ("io_uring: multishot recv")
Fixes: 9bb66906f23e ("io_uring: support multishot in recvmsg")
Link: https://github.com/axboe/liburing/issues/1043
Signed-off-by: Jens Axboe <axboe@kernel.dk>

diff --git a/io_uring/net.c b/io_uring/net.c
index 740c6bfa5b59..a12ff69e6843 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -60,6 +60,7 @@ struct io_sr_msg {
 	unsigned			len;
 	unsigned			done_io;
 	unsigned			msg_flags;
+	unsigned			nr_multishot_loops;
 	u16				flags;
 	/* initialised and used only by !msg send variants */
 	u16				addr_len;
@@ -70,6 +71,13 @@ struct io_sr_msg {
 	struct io_kiocb 		*notif;
 };
 
+/*
+ * Number of times we'll try and do receives if there's more data. If we
+ * exceed this limit, then add us to the back of the queue and retry from
+ * there. This helps fairness between flooding clients.
+ */
+#define MULTISHOT_MAX_RETRY	32
+
 static inline bool io_check_multishot(struct io_kiocb *req,
 				      unsigned int issue_flags)
 {
@@ -611,6 +619,7 @@ int io_recvmsg_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 		sr->msg_flags |= MSG_CMSG_COMPAT;
 #endif
 	sr->done_io = 0;
+	sr->nr_multishot_loops = 0;
 	return 0;
 }
 
@@ -654,12 +663,20 @@ static inline bool io_recv_finish(struct io_kiocb *req, int *ret,
 	 */
 	if (io_fill_cqe_req_aux(req, issue_flags & IO_URING_F_COMPLETE_DEFER,
 				*ret, cflags | IORING_CQE_F_MORE)) {
+		struct io_sr_msg *sr = io_kiocb_to_cmd(req, struct io_sr_msg);
+		int mshot_retry_ret = IOU_ISSUE_SKIP_COMPLETE;
+
 		io_recv_prep_retry(req);
 		/* Known not-empty or unknown state, retry */
-		if (cflags & IORING_CQE_F_SOCK_NONEMPTY || msg->msg_inq == -1)
-			return false;
+		if (cflags & IORING_CQE_F_SOCK_NONEMPTY || msg->msg_inq == -1) {
+			if (sr->nr_multishot_loops++ < MULTISHOT_MAX_RETRY)
+				return false;
+			/* mshot retries exceeded, force a requeue */
+			sr->nr_multishot_loops = 0;
+			mshot_retry_ret = IOU_REQUEUE;
+		}
 		if (issue_flags & IO_URING_F_MULTISHOT)
-			*ret = IOU_ISSUE_SKIP_COMPLETE;
+			*ret = mshot_retry_ret;
 		else
 			*ret = -EAGAIN;
 		return true;


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

* Re: FAILED: patch "[PATCH] io_uring/net: limit inline multishot retries" failed to apply to 6.6-stable tree
  2024-02-13 13:19 FAILED: patch "[PATCH] io_uring/net: limit inline multishot retries" failed to apply to 6.6-stable tree gregkh
@ 2024-02-13 14:52 ` Jens Axboe
  2024-02-13 16:15   ` Greg KH
  0 siblings, 1 reply; 6+ messages in thread
From: Jens Axboe @ 2024-02-13 14:52 UTC (permalink / raw)
  To: gregkh; +Cc: stable

[-- Attachment #1: Type: text/plain, Size: 807 bytes --]

On 2/13/24 6:19 AM, gregkh@linuxfoundation.org wrote:
> 
> The patch below does not apply to the 6.6-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 <stable@vger.kernel.org>.
> 
> To reproduce the conflict and resubmit, you may use the following commands:
> 
> git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.6.y
> git checkout FETCH_HEAD
> git cherry-pick -x 76b367a2d83163cf19173d5cb0b562acbabc8eac
> # <resolve conflicts, build, test, etc.>
> git commit -s
> git send-email --to '<stable@vger.kernel.org>' --in-reply-to '2024021330-twice-pacify-2be5@gregkh' --subject-prefix 'PATCH 6.6.y' HEAD^..

Here's the series for 6.6-stable.

-- 
Jens Axboe


[-- Attachment #2: 0001-io_uring-poll-move-poll-execution-helpers-higher-up.patch --]
[-- Type: text/x-patch, Size: 2439 bytes --]

From 582cc8795c22337041abc7ee06f9de34f1592922 Mon Sep 17 00:00:00 2001
From: Jens Axboe <axboe@kernel.dk>
Date: Mon, 29 Jan 2024 11:52:54 -0700
Subject: [PATCH 1/4] io_uring/poll: move poll execution helpers higher up

Commit e84b01a880f635e3084a361afba41f95ff500d12 upstream.

In preparation for calling __io_poll_execute() higher up, move the
functions to avoid forward declarations.

No functional changes in this patch.

Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 io_uring/poll.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/io_uring/poll.c b/io_uring/poll.c
index a4084acaff91..a2f21ae093dc 100644
--- a/io_uring/poll.c
+++ b/io_uring/poll.c
@@ -226,6 +226,30 @@ enum {
 	IOU_POLL_REISSUE = 3,
 };
 
+static void __io_poll_execute(struct io_kiocb *req, int mask)
+{
+	io_req_set_res(req, mask, 0);
+	/*
+	 * This is useful for poll that is armed on behalf of another
+	 * request, and where the wakeup path could be on a different
+	 * CPU. We want to avoid pulling in req->apoll->events for that
+	 * case.
+	 */
+	if (req->opcode == IORING_OP_POLL_ADD)
+		req->io_task_work.func = io_poll_task_func;
+	else
+		req->io_task_work.func = io_apoll_task_func;
+
+	trace_io_uring_task_add(req, mask);
+	io_req_task_work_add(req);
+}
+
+static inline void io_poll_execute(struct io_kiocb *req, int res)
+{
+	if (io_poll_get_ownership(req))
+		__io_poll_execute(req, res);
+}
+
 /*
  * All poll tw should go through this. Checks for poll events, manages
  * references, does rewait, etc.
@@ -372,30 +396,6 @@ static void io_apoll_task_func(struct io_kiocb *req, bool *locked)
 		io_req_complete_failed(req, ret);
 }
 
-static void __io_poll_execute(struct io_kiocb *req, int mask)
-{
-	io_req_set_res(req, mask, 0);
-	/*
-	 * This is useful for poll that is armed on behalf of another
-	 * request, and where the wakeup path could be on a different
-	 * CPU. We want to avoid pulling in req->apoll->events for that
-	 * case.
-	 */
-	if (req->opcode == IORING_OP_POLL_ADD)
-		req->io_task_work.func = io_poll_task_func;
-	else
-		req->io_task_work.func = io_apoll_task_func;
-
-	trace_io_uring_task_add(req, mask);
-	io_req_task_work_add(req);
-}
-
-static inline void io_poll_execute(struct io_kiocb *req, int res)
-{
-	if (io_poll_get_ownership(req))
-		__io_poll_execute(req, res);
-}
-
 static void io_poll_cancel_req(struct io_kiocb *req)
 {
 	io_poll_mark_cancelled(req);

[-- Attachment #3: 0003-io_uring-poll-add-requeue-return-code-from-poll-mult.patch --]
[-- Type: text/x-patch, Size: 2606 bytes --]

From 330e3bbfe6e21b715b02362d2a6e427907efb6c8 Mon Sep 17 00:00:00 2001
From: Jens Axboe <axboe@kernel.dk>
Date: Mon, 29 Jan 2024 11:57:11 -0700
Subject: [PATCH 3/4] io_uring/poll: add requeue return code from poll
 multishot handling

Commit 704ea888d646cb9d715662944cf389c823252ee0 upstream.

Since our poll handling is edge triggered, multishot handlers retry
internally until they know that no more data is available. In
preparation for limiting these retries, add an internal return code,
IOU_REQUEUE, which can be used to inform the poll backend about the
handler wanting to retry, but that this should happen through a normal
task_work requeue rather than keep hammering on the issue side for this
one request.

No functional changes in this patch, nobody is using this return code
just yet.

Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 io_uring/io_uring.h | 7 +++++++
 io_uring/poll.c     | 9 ++++++++-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/io_uring/io_uring.h b/io_uring/io_uring.h
index d2bad1df347d..c8cba7831083 100644
--- a/io_uring/io_uring.h
+++ b/io_uring/io_uring.h
@@ -30,6 +30,13 @@ enum {
 	IOU_OK			= 0,
 	IOU_ISSUE_SKIP_COMPLETE	= -EIOCBQUEUED,
 
+	/*
+	 * Requeue the task_work to restart operations on this request. The
+	 * actual value isn't important, should just be not an otherwise
+	 * valid error code, yet less than -MAX_ERRNO and valid internally.
+	 */
+	IOU_REQUEUE		= -3072,
+
 	/*
 	 * Intended only when both IO_URING_F_MULTISHOT is passed
 	 * to indicate to the poll runner that multishot should be
diff --git a/io_uring/poll.c b/io_uring/poll.c
index b0bc78526394..48ca0810a54a 100644
--- a/io_uring/poll.c
+++ b/io_uring/poll.c
@@ -226,6 +226,7 @@ enum {
 	IOU_POLL_NO_ACTION = 1,
 	IOU_POLL_REMOVE_POLL_USE_RES = 2,
 	IOU_POLL_REISSUE = 3,
+	IOU_POLL_REQUEUE = 4,
 };
 
 static void __io_poll_execute(struct io_kiocb *req, int mask)
@@ -324,6 +325,8 @@ static int io_poll_check_events(struct io_kiocb *req, struct io_tw_state *ts)
 			int ret = io_poll_issue(req, ts);
 			if (ret == IOU_STOP_MULTISHOT)
 				return IOU_POLL_REMOVE_POLL_USE_RES;
+			else if (ret == IOU_REQUEUE)
+				return IOU_POLL_REQUEUE;
 			if (ret < 0)
 				return ret;
 		}
@@ -346,8 +349,12 @@ void io_poll_task_func(struct io_kiocb *req, struct io_tw_state *ts)
 	int ret;
 
 	ret = io_poll_check_events(req, ts);
-	if (ret == IOU_POLL_NO_ACTION)
+	if (ret == IOU_POLL_NO_ACTION) {
 		return;
+	} else if (ret == IOU_POLL_REQUEUE) {
+		__io_poll_execute(req, 0);
+		return;
+	}
 	io_poll_remove_entries(req);
 	io_poll_tw_hash_eject(req, ts);
 
-- 
2.43.0


[-- Attachment #4: 0002-io_uring-net-un-indent-mshot-retry-path-in-io_recv_f.patch --]
[-- Type: text/x-patch, Size: 2094 bytes --]

From 37dbf0056a58703f815ae076b08dfb9da86c610d Mon Sep 17 00:00:00 2001
From: Jens Axboe <axboe@kernel.dk>
Date: Mon, 29 Jan 2024 11:54:18 -0700
Subject: [PATCH 2/4] io_uring/net: un-indent mshot retry path in
 io_recv_finish()

Commit 91e5d765a82fb2c9d0b7ad930d8953208081ddf1 upstream.

In preparation for putting some retry logic in there, have the done
path just skip straight to the end rather than have too much nesting
in here.

No functional changes in this patch.

Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 io_uring/net.c | 36 ++++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/io_uring/net.c b/io_uring/net.c
index 75d494dad7e2..740c6bfa5b59 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -645,23 +645,27 @@ static inline bool io_recv_finish(struct io_kiocb *req, int *ret,
 		return true;
 	}
 
-	if (!mshot_finished) {
-		if (io_fill_cqe_req_aux(req, issue_flags & IO_URING_F_COMPLETE_DEFER,
-					*ret, cflags | IORING_CQE_F_MORE)) {
-			io_recv_prep_retry(req);
-			/* Known not-empty or unknown state, retry */
-			if (cflags & IORING_CQE_F_SOCK_NONEMPTY ||
-			    msg->msg_inq == -1)
-				return false;
-			if (issue_flags & IO_URING_F_MULTISHOT)
-				*ret = IOU_ISSUE_SKIP_COMPLETE;
-			else
-				*ret = -EAGAIN;
-			return true;
-		}
-		/* Otherwise stop multishot but use the current result. */
-	}
+	if (mshot_finished)
+		goto finish;
 
+	/*
+	 * Fill CQE for this receive and see if we should keep trying to
+	 * receive from this socket.
+	 */
+	if (io_fill_cqe_req_aux(req, issue_flags & IO_URING_F_COMPLETE_DEFER,
+				*ret, cflags | IORING_CQE_F_MORE)) {
+		io_recv_prep_retry(req);
+		/* Known not-empty or unknown state, retry */
+		if (cflags & IORING_CQE_F_SOCK_NONEMPTY || msg->msg_inq == -1)
+			return false;
+		if (issue_flags & IO_URING_F_MULTISHOT)
+			*ret = IOU_ISSUE_SKIP_COMPLETE;
+		else
+			*ret = -EAGAIN;
+		return true;
+	}
+	/* Otherwise stop multishot but use the current result. */
+finish:
 	io_req_set_res(req, *ret, cflags);
 
 	if (issue_flags & IO_URING_F_MULTISHOT)
-- 
2.43.0


[-- Attachment #5: 0004-io_uring-net-limit-inline-multishot-retries.patch --]
[-- Type: text/x-patch, Size: 3321 bytes --]

From aeec8884c1607a73b379e610cc8de27e3f6244ce Mon Sep 17 00:00:00 2001
From: Jens Axboe <axboe@kernel.dk>
Date: Mon, 29 Jan 2024 12:00:58 -0700
Subject: [PATCH 4/4] io_uring/net: limit inline multishot retries

Commit 76b367a2d83163cf19173d5cb0b562acbabc8eac upstream.

If we have multiple clients and some/all are flooding the receives to
such an extent that we can retry a LOT handling multishot receives, then
we can be starving some clients and hence serving traffic in an
imbalanced fashion.

Limit multishot retry attempts to some arbitrary value, whose only
purpose serves to ensure that we don't keep serving a single connection
for way too long. We default to 32 retries, which should be more than
enough to provide fairness, yet not so small that we'll spend too much
time requeuing rather than handling traffic.

Cc: stable@vger.kernel.org
Depends-on: 704ea888d646 ("io_uring/poll: add requeue return code from poll multishot handling")
Depends-on: 1e5d765a82f ("io_uring/net: un-indent mshot retry path in io_recv_finish()")
Depends-on: e84b01a880f6 ("io_uring/poll: move poll execution helpers higher up")
Fixes: b3fdea6ecb55 ("io_uring: multishot recv")
Fixes: 9bb66906f23e ("io_uring: support multishot in recvmsg")
Link: https://github.com/axboe/liburing/issues/1043
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 io_uring/net.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/io_uring/net.c b/io_uring/net.c
index 740c6bfa5b59..a12ff69e6843 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -60,6 +60,7 @@ struct io_sr_msg {
 	unsigned			len;
 	unsigned			done_io;
 	unsigned			msg_flags;
+	unsigned			nr_multishot_loops;
 	u16				flags;
 	/* initialised and used only by !msg send variants */
 	u16				addr_len;
@@ -70,6 +71,13 @@ struct io_sr_msg {
 	struct io_kiocb 		*notif;
 };
 
+/*
+ * Number of times we'll try and do receives if there's more data. If we
+ * exceed this limit, then add us to the back of the queue and retry from
+ * there. This helps fairness between flooding clients.
+ */
+#define MULTISHOT_MAX_RETRY	32
+
 static inline bool io_check_multishot(struct io_kiocb *req,
 				      unsigned int issue_flags)
 {
@@ -611,6 +619,7 @@ int io_recvmsg_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 		sr->msg_flags |= MSG_CMSG_COMPAT;
 #endif
 	sr->done_io = 0;
+	sr->nr_multishot_loops = 0;
 	return 0;
 }
 
@@ -654,12 +663,20 @@ static inline bool io_recv_finish(struct io_kiocb *req, int *ret,
 	 */
 	if (io_fill_cqe_req_aux(req, issue_flags & IO_URING_F_COMPLETE_DEFER,
 				*ret, cflags | IORING_CQE_F_MORE)) {
+		struct io_sr_msg *sr = io_kiocb_to_cmd(req, struct io_sr_msg);
+		int mshot_retry_ret = IOU_ISSUE_SKIP_COMPLETE;
+
 		io_recv_prep_retry(req);
 		/* Known not-empty or unknown state, retry */
-		if (cflags & IORING_CQE_F_SOCK_NONEMPTY || msg->msg_inq == -1)
-			return false;
+		if (cflags & IORING_CQE_F_SOCK_NONEMPTY || msg->msg_inq == -1) {
+			if (sr->nr_multishot_loops++ < MULTISHOT_MAX_RETRY)
+				return false;
+			/* mshot retries exceeded, force a requeue */
+			sr->nr_multishot_loops = 0;
+			mshot_retry_ret = IOU_REQUEUE;
+		}
 		if (issue_flags & IO_URING_F_MULTISHOT)
-			*ret = IOU_ISSUE_SKIP_COMPLETE;
+			*ret = mshot_retry_ret;
 		else
 			*ret = -EAGAIN;
 		return true;
-- 
2.43.0


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

* Re: FAILED: patch "[PATCH] io_uring/net: limit inline multishot retries" failed to apply to 6.6-stable tree
  2024-02-13 14:52 ` Jens Axboe
@ 2024-02-13 16:15   ` Greg KH
  2024-02-13 16:18     ` Jens Axboe
  0 siblings, 1 reply; 6+ messages in thread
From: Greg KH @ 2024-02-13 16:15 UTC (permalink / raw)
  To: Jens Axboe; +Cc: stable

On Tue, Feb 13, 2024 at 07:52:53AM -0700, Jens Axboe wrote:
> On 2/13/24 6:19 AM, gregkh@linuxfoundation.org wrote:
> > 
> > The patch below does not apply to the 6.6-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 <stable@vger.kernel.org>.
> > 
> > To reproduce the conflict and resubmit, you may use the following commands:
> > 
> > git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.6.y
> > git checkout FETCH_HEAD
> > git cherry-pick -x 76b367a2d83163cf19173d5cb0b562acbabc8eac
> > # <resolve conflicts, build, test, etc.>
> > git commit -s
> > git send-email --to '<stable@vger.kernel.org>' --in-reply-to '2024021330-twice-pacify-2be5@gregkh' --subject-prefix 'PATCH 6.6.y' HEAD^..
> 
> Here's the series for 6.6-stable.
> 
> -- 
> Jens Axboe
> 

> From 582cc8795c22337041abc7ee06f9de34f1592922 Mon Sep 17 00:00:00 2001
> From: Jens Axboe <axboe@kernel.dk>
> Date: Mon, 29 Jan 2024 11:52:54 -0700
> Subject: [PATCH 1/4] io_uring/poll: move poll execution helpers higher up
> 
> Commit e84b01a880f635e3084a361afba41f95ff500d12 upstream.
> 
> In preparation for calling __io_poll_execute() higher up, move the
> functions to avoid forward declarations.
> 
> No functional changes in this patch.
> 
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> ---
>  io_uring/poll.c | 30 +++++++++++++++---------------
>  1 file changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/io_uring/poll.c b/io_uring/poll.c
> index a4084acaff91..a2f21ae093dc 100644
> --- a/io_uring/poll.c
> +++ b/io_uring/poll.c
> @@ -226,6 +226,30 @@ enum {
>  	IOU_POLL_REISSUE = 3,
>  };
>  
> +static void __io_poll_execute(struct io_kiocb *req, int mask)
> +{
> +	io_req_set_res(req, mask, 0);
> +	/*
> +	 * This is useful for poll that is armed on behalf of another
> +	 * request, and where the wakeup path could be on a different
> +	 * CPU. We want to avoid pulling in req->apoll->events for that
> +	 * case.
> +	 */
> +	if (req->opcode == IORING_OP_POLL_ADD)
> +		req->io_task_work.func = io_poll_task_func;
> +	else
> +		req->io_task_work.func = io_apoll_task_func;
> +
> +	trace_io_uring_task_add(req, mask);
> +	io_req_task_work_add(req);
> +}
> +
> +static inline void io_poll_execute(struct io_kiocb *req, int res)
> +{
> +	if (io_poll_get_ownership(req))
> +		__io_poll_execute(req, res);
> +}
> +
>  /*
>   * All poll tw should go through this. Checks for poll events, manages
>   * references, does rewait, etc.
> @@ -372,30 +396,6 @@ static void io_apoll_task_func(struct io_kiocb *req, bool *locked)
>  		io_req_complete_failed(req, ret);
>  }
>  
> -static void __io_poll_execute(struct io_kiocb *req, int mask)
> -{
> -	io_req_set_res(req, mask, 0);
> -	/*
> -	 * This is useful for poll that is armed on behalf of another
> -	 * request, and where the wakeup path could be on a different
> -	 * CPU. We want to avoid pulling in req->apoll->events for that
> -	 * case.
> -	 */
> -	if (req->opcode == IORING_OP_POLL_ADD)
> -		req->io_task_work.func = io_poll_task_func;
> -	else
> -		req->io_task_work.func = io_apoll_task_func;
> -
> -	trace_io_uring_task_add(req, mask);
> -	io_req_task_work_add(req);
> -}
> -
> -static inline void io_poll_execute(struct io_kiocb *req, int res)
> -{
> -	if (io_poll_get_ownership(req))
> -		__io_poll_execute(req, res);
> -}
> -

This first patch fails to apply to the 6.6.y tree, are you sure you made
it against the correct one?  These functions do not look like this to
me.

confused,

greg k-h

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

* Re: FAILED: patch "[PATCH] io_uring/net: limit inline multishot retries" failed to apply to 6.6-stable tree
  2024-02-13 16:15   ` Greg KH
@ 2024-02-13 16:18     ` Jens Axboe
  2024-02-13 16:27       ` Greg KH
  0 siblings, 1 reply; 6+ messages in thread
From: Jens Axboe @ 2024-02-13 16:18 UTC (permalink / raw)
  To: Greg KH; +Cc: stable

[-- Attachment #1: Type: text/plain, Size: 3894 bytes --]

On 2/13/24 9:15 AM, Greg KH wrote:
> On Tue, Feb 13, 2024 at 07:52:53AM -0700, Jens Axboe wrote:
>> On 2/13/24 6:19 AM, gregkh@linuxfoundation.org wrote:
>>>
>>> The patch below does not apply to the 6.6-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 <stable@vger.kernel.org>.
>>>
>>> To reproduce the conflict and resubmit, you may use the following commands:
>>>
>>> git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.6.y
>>> git checkout FETCH_HEAD
>>> git cherry-pick -x 76b367a2d83163cf19173d5cb0b562acbabc8eac
>>> # <resolve conflicts, build, test, etc.>
>>> git commit -s
>>> git send-email --to '<stable@vger.kernel.org>' --in-reply-to '2024021330-twice-pacify-2be5@gregkh' --subject-prefix 'PATCH 6.6.y' HEAD^..
>>
>> Here's the series for 6.6-stable.
>>
>> -- 
>> Jens Axboe
>>
> 
>> From 582cc8795c22337041abc7ee06f9de34f1592922 Mon Sep 17 00:00:00 2001
>> From: Jens Axboe <axboe@kernel.dk>
>> Date: Mon, 29 Jan 2024 11:52:54 -0700
>> Subject: [PATCH 1/4] io_uring/poll: move poll execution helpers higher up
>>
>> Commit e84b01a880f635e3084a361afba41f95ff500d12 upstream.
>>
>> In preparation for calling __io_poll_execute() higher up, move the
>> functions to avoid forward declarations.
>>
>> No functional changes in this patch.
>>
>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>> ---
>>  io_uring/poll.c | 30 +++++++++++++++---------------
>>  1 file changed, 15 insertions(+), 15 deletions(-)
>>
>> diff --git a/io_uring/poll.c b/io_uring/poll.c
>> index a4084acaff91..a2f21ae093dc 100644
>> --- a/io_uring/poll.c
>> +++ b/io_uring/poll.c
>> @@ -226,6 +226,30 @@ enum {
>>  	IOU_POLL_REISSUE = 3,
>>  };
>>  
>> +static void __io_poll_execute(struct io_kiocb *req, int mask)
>> +{
>> +	io_req_set_res(req, mask, 0);
>> +	/*
>> +	 * This is useful for poll that is armed on behalf of another
>> +	 * request, and where the wakeup path could be on a different
>> +	 * CPU. We want to avoid pulling in req->apoll->events for that
>> +	 * case.
>> +	 */
>> +	if (req->opcode == IORING_OP_POLL_ADD)
>> +		req->io_task_work.func = io_poll_task_func;
>> +	else
>> +		req->io_task_work.func = io_apoll_task_func;
>> +
>> +	trace_io_uring_task_add(req, mask);
>> +	io_req_task_work_add(req);
>> +}
>> +
>> +static inline void io_poll_execute(struct io_kiocb *req, int res)
>> +{
>> +	if (io_poll_get_ownership(req))
>> +		__io_poll_execute(req, res);
>> +}
>> +
>>  /*
>>   * All poll tw should go through this. Checks for poll events, manages
>>   * references, does rewait, etc.
>> @@ -372,30 +396,6 @@ static void io_apoll_task_func(struct io_kiocb *req, bool *locked)
>>  		io_req_complete_failed(req, ret);
>>  }
>>  
>> -static void __io_poll_execute(struct io_kiocb *req, int mask)
>> -{
>> -	io_req_set_res(req, mask, 0);
>> -	/*
>> -	 * This is useful for poll that is armed on behalf of another
>> -	 * request, and where the wakeup path could be on a different
>> -	 * CPU. We want to avoid pulling in req->apoll->events for that
>> -	 * case.
>> -	 */
>> -	if (req->opcode == IORING_OP_POLL_ADD)
>> -		req->io_task_work.func = io_poll_task_func;
>> -	else
>> -		req->io_task_work.func = io_apoll_task_func;
>> -
>> -	trace_io_uring_task_add(req, mask);
>> -	io_req_task_work_add(req);
>> -}
>> -
>> -static inline void io_poll_execute(struct io_kiocb *req, int res)
>> -{
>> -	if (io_poll_get_ownership(req))
>> -		__io_poll_execute(req, res);
>> -}
>> -
> 
> This first patch fails to apply to the 6.6.y tree, are you sure you made
> it against the correct one?  These functions do not look like this to
> me.

Sorry my bad, refreshing them for 6.1-stable and I guess I did that
before I sent them out. Hence the mua used the new copy...

Here are the ones I have in my local tree, from testing.

-- 
Jens Axboe

[-- Attachment #2: 0003-io_uring-poll-add-requeue-return-code-from-poll-mult.patch --]
[-- Type: text/x-patch, Size: 2606 bytes --]

From 330e3bbfe6e21b715b02362d2a6e427907efb6c8 Mon Sep 17 00:00:00 2001
From: Jens Axboe <axboe@kernel.dk>
Date: Mon, 29 Jan 2024 11:57:11 -0700
Subject: [PATCH 3/4] io_uring/poll: add requeue return code from poll
 multishot handling

Commit 704ea888d646cb9d715662944cf389c823252ee0 upstream.

Since our poll handling is edge triggered, multishot handlers retry
internally until they know that no more data is available. In
preparation for limiting these retries, add an internal return code,
IOU_REQUEUE, which can be used to inform the poll backend about the
handler wanting to retry, but that this should happen through a normal
task_work requeue rather than keep hammering on the issue side for this
one request.

No functional changes in this patch, nobody is using this return code
just yet.

Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 io_uring/io_uring.h | 7 +++++++
 io_uring/poll.c     | 9 ++++++++-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/io_uring/io_uring.h b/io_uring/io_uring.h
index d2bad1df347d..c8cba7831083 100644
--- a/io_uring/io_uring.h
+++ b/io_uring/io_uring.h
@@ -30,6 +30,13 @@ enum {
 	IOU_OK			= 0,
 	IOU_ISSUE_SKIP_COMPLETE	= -EIOCBQUEUED,
 
+	/*
+	 * Requeue the task_work to restart operations on this request. The
+	 * actual value isn't important, should just be not an otherwise
+	 * valid error code, yet less than -MAX_ERRNO and valid internally.
+	 */
+	IOU_REQUEUE		= -3072,
+
 	/*
 	 * Intended only when both IO_URING_F_MULTISHOT is passed
 	 * to indicate to the poll runner that multishot should be
diff --git a/io_uring/poll.c b/io_uring/poll.c
index b0bc78526394..48ca0810a54a 100644
--- a/io_uring/poll.c
+++ b/io_uring/poll.c
@@ -226,6 +226,7 @@ enum {
 	IOU_POLL_NO_ACTION = 1,
 	IOU_POLL_REMOVE_POLL_USE_RES = 2,
 	IOU_POLL_REISSUE = 3,
+	IOU_POLL_REQUEUE = 4,
 };
 
 static void __io_poll_execute(struct io_kiocb *req, int mask)
@@ -324,6 +325,8 @@ static int io_poll_check_events(struct io_kiocb *req, struct io_tw_state *ts)
 			int ret = io_poll_issue(req, ts);
 			if (ret == IOU_STOP_MULTISHOT)
 				return IOU_POLL_REMOVE_POLL_USE_RES;
+			else if (ret == IOU_REQUEUE)
+				return IOU_POLL_REQUEUE;
 			if (ret < 0)
 				return ret;
 		}
@@ -346,8 +349,12 @@ void io_poll_task_func(struct io_kiocb *req, struct io_tw_state *ts)
 	int ret;
 
 	ret = io_poll_check_events(req, ts);
-	if (ret == IOU_POLL_NO_ACTION)
+	if (ret == IOU_POLL_NO_ACTION) {
 		return;
+	} else if (ret == IOU_POLL_REQUEUE) {
+		__io_poll_execute(req, 0);
+		return;
+	}
 	io_poll_remove_entries(req);
 	io_poll_tw_hash_eject(req, ts);
 
-- 
2.43.0


[-- Attachment #3: 0002-io_uring-net-un-indent-mshot-retry-path-in-io_recv_f.patch --]
[-- Type: text/x-patch, Size: 2094 bytes --]

From 37dbf0056a58703f815ae076b08dfb9da86c610d Mon Sep 17 00:00:00 2001
From: Jens Axboe <axboe@kernel.dk>
Date: Mon, 29 Jan 2024 11:54:18 -0700
Subject: [PATCH 2/4] io_uring/net: un-indent mshot retry path in
 io_recv_finish()

Commit 91e5d765a82fb2c9d0b7ad930d8953208081ddf1 upstream.

In preparation for putting some retry logic in there, have the done
path just skip straight to the end rather than have too much nesting
in here.

No functional changes in this patch.

Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 io_uring/net.c | 36 ++++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/io_uring/net.c b/io_uring/net.c
index 75d494dad7e2..740c6bfa5b59 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -645,23 +645,27 @@ static inline bool io_recv_finish(struct io_kiocb *req, int *ret,
 		return true;
 	}
 
-	if (!mshot_finished) {
-		if (io_fill_cqe_req_aux(req, issue_flags & IO_URING_F_COMPLETE_DEFER,
-					*ret, cflags | IORING_CQE_F_MORE)) {
-			io_recv_prep_retry(req);
-			/* Known not-empty or unknown state, retry */
-			if (cflags & IORING_CQE_F_SOCK_NONEMPTY ||
-			    msg->msg_inq == -1)
-				return false;
-			if (issue_flags & IO_URING_F_MULTISHOT)
-				*ret = IOU_ISSUE_SKIP_COMPLETE;
-			else
-				*ret = -EAGAIN;
-			return true;
-		}
-		/* Otherwise stop multishot but use the current result. */
-	}
+	if (mshot_finished)
+		goto finish;
 
+	/*
+	 * Fill CQE for this receive and see if we should keep trying to
+	 * receive from this socket.
+	 */
+	if (io_fill_cqe_req_aux(req, issue_flags & IO_URING_F_COMPLETE_DEFER,
+				*ret, cflags | IORING_CQE_F_MORE)) {
+		io_recv_prep_retry(req);
+		/* Known not-empty or unknown state, retry */
+		if (cflags & IORING_CQE_F_SOCK_NONEMPTY || msg->msg_inq == -1)
+			return false;
+		if (issue_flags & IO_URING_F_MULTISHOT)
+			*ret = IOU_ISSUE_SKIP_COMPLETE;
+		else
+			*ret = -EAGAIN;
+		return true;
+	}
+	/* Otherwise stop multishot but use the current result. */
+finish:
 	io_req_set_res(req, *ret, cflags);
 
 	if (issue_flags & IO_URING_F_MULTISHOT)
-- 
2.43.0


[-- Attachment #4: 0001-io_uring-poll-move-poll-execution-helpers-higher-up.patch --]
[-- Type: text/x-patch, Size: 1794 bytes --]

From 582cc8795c22337041abc7ee06f9de34f1592922 Mon Sep 17 00:00:00 2001
From: Jens Axboe <axboe@kernel.dk>
Date: Mon, 29 Jan 2024 11:52:54 -0700
Subject: [PATCH 1/4] io_uring/poll: move poll execution helpers higher up

Commit e84b01a880f635e3084a361afba41f95ff500d12 upstream.

In preparation for calling __io_poll_execute() higher up, move the
functions to avoid forward declarations.

No functional changes in this patch.

Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 io_uring/poll.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/io_uring/poll.c b/io_uring/poll.c
index 4c360ba8793a..b0bc78526394 100644
--- a/io_uring/poll.c
+++ b/io_uring/poll.c
@@ -228,6 +228,21 @@ enum {
 	IOU_POLL_REISSUE = 3,
 };
 
+static void __io_poll_execute(struct io_kiocb *req, int mask)
+{
+	io_req_set_res(req, mask, 0);
+	req->io_task_work.func = io_poll_task_func;
+
+	trace_io_uring_task_add(req, mask);
+	io_req_task_work_add(req);
+}
+
+static inline void io_poll_execute(struct io_kiocb *req, int res)
+{
+	if (io_poll_get_ownership(req))
+		__io_poll_execute(req, res);
+}
+
 /*
  * All poll tw should go through this. Checks for poll events, manages
  * references, does rewait, etc.
@@ -364,21 +379,6 @@ void io_poll_task_func(struct io_kiocb *req, struct io_tw_state *ts)
 	}
 }
 
-static void __io_poll_execute(struct io_kiocb *req, int mask)
-{
-	io_req_set_res(req, mask, 0);
-	req->io_task_work.func = io_poll_task_func;
-
-	trace_io_uring_task_add(req, mask);
-	io_req_task_work_add(req);
-}
-
-static inline void io_poll_execute(struct io_kiocb *req, int res)
-{
-	if (io_poll_get_ownership(req))
-		__io_poll_execute(req, res);
-}
-
 static void io_poll_cancel_req(struct io_kiocb *req)
 {
 	io_poll_mark_cancelled(req);
-- 
2.43.0


[-- Attachment #5: 0004-io_uring-net-limit-inline-multishot-retries.patch --]
[-- Type: text/x-patch, Size: 3321 bytes --]

From aeec8884c1607a73b379e610cc8de27e3f6244ce Mon Sep 17 00:00:00 2001
From: Jens Axboe <axboe@kernel.dk>
Date: Mon, 29 Jan 2024 12:00:58 -0700
Subject: [PATCH 4/4] io_uring/net: limit inline multishot retries

Commit 76b367a2d83163cf19173d5cb0b562acbabc8eac upstream.

If we have multiple clients and some/all are flooding the receives to
such an extent that we can retry a LOT handling multishot receives, then
we can be starving some clients and hence serving traffic in an
imbalanced fashion.

Limit multishot retry attempts to some arbitrary value, whose only
purpose serves to ensure that we don't keep serving a single connection
for way too long. We default to 32 retries, which should be more than
enough to provide fairness, yet not so small that we'll spend too much
time requeuing rather than handling traffic.

Cc: stable@vger.kernel.org
Depends-on: 704ea888d646 ("io_uring/poll: add requeue return code from poll multishot handling")
Depends-on: 1e5d765a82f ("io_uring/net: un-indent mshot retry path in io_recv_finish()")
Depends-on: e84b01a880f6 ("io_uring/poll: move poll execution helpers higher up")
Fixes: b3fdea6ecb55 ("io_uring: multishot recv")
Fixes: 9bb66906f23e ("io_uring: support multishot in recvmsg")
Link: https://github.com/axboe/liburing/issues/1043
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 io_uring/net.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/io_uring/net.c b/io_uring/net.c
index 740c6bfa5b59..a12ff69e6843 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -60,6 +60,7 @@ struct io_sr_msg {
 	unsigned			len;
 	unsigned			done_io;
 	unsigned			msg_flags;
+	unsigned			nr_multishot_loops;
 	u16				flags;
 	/* initialised and used only by !msg send variants */
 	u16				addr_len;
@@ -70,6 +71,13 @@ struct io_sr_msg {
 	struct io_kiocb 		*notif;
 };
 
+/*
+ * Number of times we'll try and do receives if there's more data. If we
+ * exceed this limit, then add us to the back of the queue and retry from
+ * there. This helps fairness between flooding clients.
+ */
+#define MULTISHOT_MAX_RETRY	32
+
 static inline bool io_check_multishot(struct io_kiocb *req,
 				      unsigned int issue_flags)
 {
@@ -611,6 +619,7 @@ int io_recvmsg_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 		sr->msg_flags |= MSG_CMSG_COMPAT;
 #endif
 	sr->done_io = 0;
+	sr->nr_multishot_loops = 0;
 	return 0;
 }
 
@@ -654,12 +663,20 @@ static inline bool io_recv_finish(struct io_kiocb *req, int *ret,
 	 */
 	if (io_fill_cqe_req_aux(req, issue_flags & IO_URING_F_COMPLETE_DEFER,
 				*ret, cflags | IORING_CQE_F_MORE)) {
+		struct io_sr_msg *sr = io_kiocb_to_cmd(req, struct io_sr_msg);
+		int mshot_retry_ret = IOU_ISSUE_SKIP_COMPLETE;
+
 		io_recv_prep_retry(req);
 		/* Known not-empty or unknown state, retry */
-		if (cflags & IORING_CQE_F_SOCK_NONEMPTY || msg->msg_inq == -1)
-			return false;
+		if (cflags & IORING_CQE_F_SOCK_NONEMPTY || msg->msg_inq == -1) {
+			if (sr->nr_multishot_loops++ < MULTISHOT_MAX_RETRY)
+				return false;
+			/* mshot retries exceeded, force a requeue */
+			sr->nr_multishot_loops = 0;
+			mshot_retry_ret = IOU_REQUEUE;
+		}
 		if (issue_flags & IO_URING_F_MULTISHOT)
-			*ret = IOU_ISSUE_SKIP_COMPLETE;
+			*ret = mshot_retry_ret;
 		else
 			*ret = -EAGAIN;
 		return true;
-- 
2.43.0


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

* Re: FAILED: patch "[PATCH] io_uring/net: limit inline multishot retries" failed to apply to 6.6-stable tree
  2024-02-13 16:18     ` Jens Axboe
@ 2024-02-13 16:27       ` Greg KH
  2024-02-13 16:32         ` Jens Axboe
  0 siblings, 1 reply; 6+ messages in thread
From: Greg KH @ 2024-02-13 16:27 UTC (permalink / raw)
  To: Jens Axboe; +Cc: stable

On Tue, Feb 13, 2024 at 09:18:43AM -0700, Jens Axboe wrote:
> > This first patch fails to apply to the 6.6.y tree, are you sure you made
> > it against the correct one?  These functions do not look like this to
> > me.
> 
> Sorry my bad, refreshing them for 6.1-stable and I guess I did that
> before I sent them out. Hence the mua used the new copy...
> 
> Here are the ones I have in my local tree, from testing.

Now queued up, but to confirm, 6.1.y did NOT need these, right?

thanks,

greg k-h

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

* Re: FAILED: patch "[PATCH] io_uring/net: limit inline multishot retries" failed to apply to 6.6-stable tree
  2024-02-13 16:27       ` Greg KH
@ 2024-02-13 16:32         ` Jens Axboe
  0 siblings, 0 replies; 6+ messages in thread
From: Jens Axboe @ 2024-02-13 16:32 UTC (permalink / raw)
  To: Greg KH; +Cc: stable

On 2/13/24 9:27 AM, Greg KH wrote:
> On Tue, Feb 13, 2024 at 09:18:43AM -0700, Jens Axboe wrote:
>>> This first patch fails to apply to the 6.6.y tree, are you sure you made
>>> it against the correct one?  These functions do not look like this to
>>> me.
>>
>> Sorry my bad, refreshing them for 6.1-stable and I guess I did that
>> before I sent them out. Hence the mua used the new copy...
>>
>> Here are the ones I have in my local tree, from testing.
> 
> Now queued up, but to confirm, 6.1.y did NOT need these, right?

Correct, it's not required. Would be nice to have, but I'd need to
backport more things. So I think it's better if we leave 6.1-stable
as-is for now.

-- 
Jens Axboe


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

end of thread, other threads:[~2024-02-13 16:32 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-13 13:19 FAILED: patch "[PATCH] io_uring/net: limit inline multishot retries" failed to apply to 6.6-stable tree gregkh
2024-02-13 14:52 ` Jens Axboe
2024-02-13 16:15   ` Greg KH
2024-02-13 16:18     ` Jens Axboe
2024-02-13 16:27       ` Greg KH
2024-02-13 16:32         ` Jens Axboe

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.