FAILED: patch "[PATCH] io_uring/net: limit inline multishot retries" failed to apply to 6.7-stable tree
Subsystems (2)
[email protected]
maintainerThe patch below does not apply to the 6.7-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]>.
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.7.y
git checkout FETCH_HEAD
git cherry-pick -x 76b367a2d83163cf19173d5cb0b562acbabc8eac
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '2024021328-washboard-crevice-aaa0@gregkh' --subject-prefix 'PATCH 6.7.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 <[email protected]>
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: [email protected]
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")
io_uring/net.c +20 -3
@@ -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;
Jens Axboe
maintainerQuoted 15 lines · On 2/13/24 6:19 AM, [email protected] wrote:
The patch below does not apply to the 6.7-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]>.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.7.y
git checkoutFETCH_HEAD
git cherry-pick -x76b367a2d83163cf19173d5cb0b562acbabc8eac
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '2024021328-washboard-crevice-aaa0@gregkh' --subject-prefix 'PATCH 6.7.y' HEAD^..
Quoted 4 lines
Possible dependencies:
76b367a2d831("io_uring/net: limit inline multishot retries")91e5d765a82f("io_uring/net: un-indent mshot retry path inio_recv_finish()")
This has a number of dependencies, as listed in the commit. You need all three to make this both apply cleanly, and work. Here's the series for 6.7, I'll send the same for the other ones separately.
Signature · 1 line
Jens Axboe
From a88f5ddfeb6e2143786c7251124db95f2a5fc5e6 Mon Sep 17 00:00:00 2001
From: Jens Axboe <[email protected]>
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.
--- io_uring/net.c | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-)
io_uring/net.c +20 -16
@@ -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 )
Signature · 1 line
2.43.0
From 15677be5b84cb82383528b69f02c8a8829349e9b Mon Sep 17 00:00:00 2001
From: Jens Axboe <[email protected]>
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.
--- io_uring/poll.c | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-)
io_uring/poll.c +20 -20
@@ -228,6 +228,26 @@ enum { IOU_POLL_REISSUE = 3, }; +static void __io_poll_execute (struct io_kiocb *req,int mask) +{ +unsigned flags =0 ; + +io_req_set_res (req, mask,0 ); + req->io_task_work.func = io_poll_task_func; + +trace_io_uring_task_add (req, mask); + +if (!(req->flags &REQ_F_POLL_NO_LAZY )) + flags =IOU_F_TWQ_LAZY_WAKE ; +__io_req_task_work_add (req, flags); +} + +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,26 +384,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) -{ -unsigned flags =0 ; - -io_req_set_res (req, mask,0 ); - req->io_task_work.func = io_poll_task_func; - -trace_io_uring_task_add (req, mask); - -if (!(req->flags &REQ_F_POLL_NO_LAZY )) - flags =IOU_F_TWQ_LAZY_WAKE ; -__io_req_task_work_add (req, flags); -} - -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);
Signature · 1 line
2.43.0
From c299ba660bed768e3d1ecafe67c144a44ed21f2f Mon Sep 17 00:00:00 2001
From: Jens Axboe <[email protected]>
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: [email protected]
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")
--- io_uring/net.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-)
io_uring/net.c +20 -3
@@ -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;
Signature · 1 line
2.43.0
From 1a45c2b50f7f10e84a4ee57b0b0556d76f995866 Mon Sep 17 00:00:00 2001
From: Jens Axboe <[email protected]>
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.
--- io_uring/io_uring.h | 7 +++++++ io_uring/poll.c | 9 ++++++++- 2 files changed, 15 insertions(+), 1 deletion(-)
io_uring/io_uring.h +7 -0
@@ -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
io_uring/poll.c +8 -1
@@ -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)
@@ -329,6 +330,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; }
@@ -351,8 +354,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);
Signature · 1 line
2.43.0
Greg KH
maintainerQuoted 16 lines · Jens Axboe
Quoted 15 lines · On 2/13/24 6:19 AM, [email protected] wrote:
The patch below does not apply to the 6.7-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]>.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.7.y
git checkoutFETCH_HEAD
git cherry-pick -x76b367a2d83163cf19173d5cb0b562acbabc8eac
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '2024021328-washboard-crevice-aaa0@gregkh' --subject-prefix 'PATCH 6.7.y' HEAD^..
Quoted 8 lines
Quoted 4 lines
Possible dependencies:
76b367a2d831("io_uring/net: limit inline multishot retries")91e5d765a82f("io_uring/net: un-indent mshot retry path inio_recv_finish()")This has a number of dependencies, as listed in the commit. You need all three to make this both apply cleanly, and work. Here's the series for 6.7, I'll send the same for the other ones separately.
I tried applying the dependencies, but they didn't all apply either :(
Thanks for all the backports, I'll go queue them up.
greg k-h
Jens Axboe
maintainerQuoted 19 lines · Greg KH
Quoted 18 lines · Jens Axboe
Quoted 17 lines · On 2/13/24 6:19 AM, [email protected] wrote:
The patch below does not apply to the 6.7-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]>.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.7.y
git checkoutFETCH_HEAD
git cherry-pick -x76b367a2d83163cf19173d5cb0b562acbabc8eac
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '2024021328-washboard-crevice-aaa0@gregkh' --subject-prefix 'PATCH 6.7.y' HEAD^..Possible dependencies:
Quoted 8 lines
Quoted 6 lines
Quoted 2 lines
76b367a2d831("io_uring/net: limit inline multishot retries")91e5d765a82f("io_uring/net: un-indent mshot retry path inio_recv_finish()")This has a number of dependencies, as listed in the commit. You need all three to make this both apply cleanly, and work. Here's the series for 6.7, I'll send the same for the other ones separately.
I tried applying the dependencies, but they didn't all apply either :(
Only issue I had was the moving of code in the first dependency, so I had to do that manually. Rest was fine after that. But yeah, it would not have picked cleanly, required a bit of manual intervention. That's why I'm here :-)
Quoted 1 line
Thanks for all the backports, I'll go queue them up.
Thanks!
Signature · 1 line
Jens Axboe