* [PATCH 01/17] io_uring/net: switch io_send() and io_send_zc() to using io_async_msghdr
2024-03-20 22:55 [PATCHSET v2 0/17] Improve async state handling Jens Axboe
@ 2024-03-20 22:55 ` Jens Axboe
2024-04-06 20:58 ` Pavel Begunkov
2024-03-20 22:55 ` [PATCH 02/17] io_uring/net: switch io_recv() " Jens Axboe
` (15 subsequent siblings)
16 siblings, 1 reply; 30+ messages in thread
From: Jens Axboe @ 2024-03-20 22:55 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe
No functional changes in this patch, just in preparation for carrying
more state then we have now, if necessary. While unifying some of this
code, add a generic send setup prep handler that they can both use.
This gets rid of some manual msghdr and sockaddr on the stack, and makes
it look a bit more like the sendmsg/recvmsg variants. We can probably
unify a bit more on top of this going forward.
Signed-off-by: Jens Axboe <[email protected]>
---
io_uring/net.c | 196 ++++++++++++++++++++++++-----------------------
io_uring/opdef.c | 1 +
2 files changed, 103 insertions(+), 94 deletions(-)
diff --git a/io_uring/net.c b/io_uring/net.c
index ed798e185bbf..a16838c0c837 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -322,36 +322,25 @@ static int io_sendmsg_copy_hdr(struct io_kiocb *req,
int io_send_prep_async(struct io_kiocb *req)
{
- struct io_sr_msg *zc = io_kiocb_to_cmd(req, struct io_sr_msg);
+ struct io_sr_msg *sr = io_kiocb_to_cmd(req, struct io_sr_msg);
struct io_async_msghdr *io;
int ret;
if (req_has_async_data(req))
return 0;
- zc->done_io = 0;
- if (!zc->addr)
+ sr->done_io = 0;
+ if (!sr->addr)
return 0;
io = io_msg_alloc_async_prep(req);
if (!io)
return -ENOMEM;
- ret = move_addr_to_kernel(zc->addr, zc->addr_len, &io->addr);
- return ret;
-}
-
-static int io_setup_async_addr(struct io_kiocb *req,
- struct sockaddr_storage *addr_storage,
- unsigned int issue_flags)
-{
- struct io_sr_msg *sr = io_kiocb_to_cmd(req, struct io_sr_msg);
- struct io_async_msghdr *io;
-
- if (!sr->addr || req_has_async_data(req))
- return -EAGAIN;
- io = io_msg_alloc_async(req, issue_flags);
- if (!io)
- return -ENOMEM;
- memcpy(&io->addr, addr_storage, sizeof(io->addr));
- return -EAGAIN;
+ memset(&io->msg, 0, sizeof(io->msg));
+ ret = import_ubuf(ITER_SOURCE, sr->buf, sr->len, &io->msg.msg_iter);
+ if (unlikely(ret))
+ return ret;
+ io->msg.msg_name = &io->addr;
+ io->msg.msg_namelen = sr->addr_len;
+ return move_addr_to_kernel(sr->addr, sr->addr_len, &io->addr);
}
int io_sendmsg_prep_async(struct io_kiocb *req)
@@ -475,45 +464,68 @@ int io_sendmsg(struct io_kiocb *req, unsigned int issue_flags)
return IOU_OK;
}
-int io_send(struct io_kiocb *req, unsigned int issue_flags)
+static struct io_async_msghdr *io_send_setup(struct io_kiocb *req,
+ struct io_async_msghdr *stack_msg,
+ unsigned int issue_flags)
{
- struct sockaddr_storage __address;
struct io_sr_msg *sr = io_kiocb_to_cmd(req, struct io_sr_msg);
- struct msghdr msg;
- struct socket *sock;
- unsigned flags;
- int min_ret = 0;
+ struct io_async_msghdr *kmsg;
int ret;
- msg.msg_name = NULL;
- msg.msg_control = NULL;
- msg.msg_controllen = 0;
- msg.msg_namelen = 0;
- msg.msg_ubuf = NULL;
-
- if (sr->addr) {
- if (req_has_async_data(req)) {
- struct io_async_msghdr *io = req->async_data;
-
- msg.msg_name = &io->addr;
- } else {
- ret = move_addr_to_kernel(sr->addr, sr->addr_len, &__address);
+ if (req_has_async_data(req)) {
+ kmsg = req->async_data;
+ } else {
+ kmsg = stack_msg;
+ kmsg->free_iov = NULL;
+ kmsg->msg.msg_name = NULL;
+ kmsg->msg.msg_namelen = 0;
+ kmsg->msg.msg_control = NULL;
+ kmsg->msg.msg_controllen = 0;
+ kmsg->msg.msg_ubuf = NULL;
+
+ if (sr->addr) {
+ ret = move_addr_to_kernel(sr->addr, sr->addr_len,
+ &kmsg->addr);
if (unlikely(ret < 0))
- return ret;
- msg.msg_name = (struct sockaddr *)&__address;
+ return ERR_PTR(ret);
+ kmsg->msg.msg_name = &kmsg->addr;
+ kmsg->msg.msg_namelen = sr->addr_len;
+ }
+
+ if (!io_do_buffer_select(req)) {
+ ret = import_ubuf(ITER_SOURCE, sr->buf, sr->len,
+ &kmsg->msg.msg_iter);
+ if (unlikely(ret))
+ return ERR_PTR(ret);
}
- msg.msg_namelen = sr->addr_len;
}
if (!(req->flags & REQ_F_POLLED) &&
(sr->flags & IORING_RECVSEND_POLL_FIRST))
- return io_setup_async_addr(req, &__address, issue_flags);
+ return ERR_PTR(io_setup_async_msg(req, kmsg, issue_flags));
+
+ return kmsg;
+}
+
+int io_send(struct io_kiocb *req, unsigned int issue_flags)
+{
+ struct io_sr_msg *sr = io_kiocb_to_cmd(req, struct io_sr_msg);
+ struct io_async_msghdr iomsg, *kmsg;
+ size_t len = sr->len;
+ struct socket *sock;
+ unsigned flags;
+ int min_ret = 0;
+ int ret;
sock = sock_from_file(req->file);
if (unlikely(!sock))
return -ENOTSOCK;
- ret = import_ubuf(ITER_SOURCE, sr->buf, sr->len, &msg.msg_iter);
+ kmsg = io_send_setup(req, &iomsg, issue_flags);
+ if (IS_ERR(kmsg))
+ return PTR_ERR(kmsg);
+
+ ret = import_ubuf(ITER_SOURCE, sr->buf, len, &kmsg->msg.msg_iter);
if (unlikely(ret))
return ret;
@@ -521,21 +533,21 @@ int io_send(struct io_kiocb *req, unsigned int issue_flags)
if (issue_flags & IO_URING_F_NONBLOCK)
flags |= MSG_DONTWAIT;
if (flags & MSG_WAITALL)
- min_ret = iov_iter_count(&msg.msg_iter);
+ min_ret = iov_iter_count(&kmsg->msg.msg_iter);
flags &= ~MSG_INTERNAL_SENDMSG_FLAGS;
- msg.msg_flags = flags;
- ret = sock_sendmsg(sock, &msg);
+ kmsg->msg.msg_flags = flags;
+ ret = sock_sendmsg(sock, &kmsg->msg);
if (ret < min_ret) {
if (ret == -EAGAIN && (issue_flags & IO_URING_F_NONBLOCK))
- return io_setup_async_addr(req, &__address, issue_flags);
+ return io_setup_async_msg(req, kmsg, issue_flags);
if (ret > 0 && io_net_retry(sock, flags)) {
sr->len -= ret;
sr->buf += ret;
sr->done_io += ret;
req->flags |= REQ_F_BL_NO_RECYCLE;
- return io_setup_async_addr(req, &__address, issue_flags);
+ return io_setup_async_msg(req, kmsg, issue_flags);
}
if (ret == -ERESTARTSYS)
ret = -EINTR;
@@ -545,6 +557,7 @@ int io_send(struct io_kiocb *req, unsigned int issue_flags)
ret += sr->done_io;
else if (sr->done_io)
ret = sr->done_io;
+ io_req_msg_cleanup(req, kmsg, issue_flags);
io_req_set_res(req, ret, 0);
return IOU_OK;
}
@@ -1158,11 +1171,35 @@ static int io_sg_from_iter(struct sock *sk, struct sk_buff *skb,
return ret;
}
+static int io_send_zc_import(struct io_kiocb *req, struct io_async_msghdr *kmsg)
+{
+ struct io_sr_msg *sr = io_kiocb_to_cmd(req, struct io_sr_msg);
+ int ret;
+
+ if (sr->flags & IORING_RECVSEND_FIXED_BUF) {
+ ret = io_import_fixed(ITER_SOURCE, &kmsg->msg.msg_iter, req->imu,
+ (u64)(uintptr_t)sr->buf, sr->len);
+ if (unlikely(ret))
+ return ret;
+ kmsg->msg.sg_from_iter = io_sg_from_iter;
+ } else {
+ io_notif_set_extended(sr->notif);
+ ret = import_ubuf(ITER_SOURCE, sr->buf, sr->len, &kmsg->msg.msg_iter);
+ if (unlikely(ret))
+ return ret;
+ ret = io_notif_account_mem(sr->notif, sr->len);
+ if (unlikely(ret))
+ return ret;
+ kmsg->msg.sg_from_iter = io_sg_from_iter_iovec;
+ }
+
+ return ret;
+}
+
int io_send_zc(struct io_kiocb *req, unsigned int issue_flags)
{
- struct sockaddr_storage __address;
struct io_sr_msg *zc = io_kiocb_to_cmd(req, struct io_sr_msg);
- struct msghdr msg;
+ struct io_async_msghdr iomsg, *kmsg;
struct socket *sock;
unsigned msg_flags;
int ret, min_ret = 0;
@@ -1173,67 +1210,37 @@ int io_send_zc(struct io_kiocb *req, unsigned int issue_flags)
if (!test_bit(SOCK_SUPPORT_ZC, &sock->flags))
return -EOPNOTSUPP;
- msg.msg_name = NULL;
- msg.msg_control = NULL;
- msg.msg_controllen = 0;
- msg.msg_namelen = 0;
-
- if (zc->addr) {
- if (req_has_async_data(req)) {
- struct io_async_msghdr *io = req->async_data;
-
- msg.msg_name = &io->addr;
- } else {
- ret = move_addr_to_kernel(zc->addr, zc->addr_len, &__address);
- if (unlikely(ret < 0))
- return ret;
- msg.msg_name = (struct sockaddr *)&__address;
- }
- msg.msg_namelen = zc->addr_len;
- }
-
- if (!(req->flags & REQ_F_POLLED) &&
- (zc->flags & IORING_RECVSEND_POLL_FIRST))
- return io_setup_async_addr(req, &__address, issue_flags);
+ kmsg = io_send_setup(req, &iomsg, issue_flags);
+ if (IS_ERR(kmsg))
+ return PTR_ERR(kmsg);
- if (zc->flags & IORING_RECVSEND_FIXED_BUF) {
- ret = io_import_fixed(ITER_SOURCE, &msg.msg_iter, req->imu,
- (u64)(uintptr_t)zc->buf, zc->len);
- if (unlikely(ret))
- return ret;
- msg.sg_from_iter = io_sg_from_iter;
- } else {
- io_notif_set_extended(zc->notif);
- ret = import_ubuf(ITER_SOURCE, zc->buf, zc->len, &msg.msg_iter);
+ if (!zc->done_io) {
+ ret = io_send_zc_import(req, kmsg);
if (unlikely(ret))
return ret;
- ret = io_notif_account_mem(zc->notif, zc->len);
- if (unlikely(ret))
- return ret;
- msg.sg_from_iter = io_sg_from_iter_iovec;
}
msg_flags = zc->msg_flags | MSG_ZEROCOPY;
if (issue_flags & IO_URING_F_NONBLOCK)
msg_flags |= MSG_DONTWAIT;
if (msg_flags & MSG_WAITALL)
- min_ret = iov_iter_count(&msg.msg_iter);
+ min_ret = iov_iter_count(&kmsg->msg.msg_iter);
msg_flags &= ~MSG_INTERNAL_SENDMSG_FLAGS;
- msg.msg_flags = msg_flags;
- msg.msg_ubuf = &io_notif_to_data(zc->notif)->uarg;
- ret = sock_sendmsg(sock, &msg);
+ kmsg->msg.msg_flags = msg_flags;
+ kmsg->msg.msg_ubuf = &io_notif_to_data(zc->notif)->uarg;
+ ret = sock_sendmsg(sock, &kmsg->msg);
if (unlikely(ret < min_ret)) {
if (ret == -EAGAIN && (issue_flags & IO_URING_F_NONBLOCK))
- return io_setup_async_addr(req, &__address, issue_flags);
+ return io_setup_async_msg(req, kmsg, issue_flags);
- if (ret > 0 && io_net_retry(sock, msg.msg_flags)) {
+ if (ret > 0 && io_net_retry(sock, kmsg->msg.msg_flags)) {
zc->len -= ret;
zc->buf += ret;
zc->done_io += ret;
req->flags |= REQ_F_BL_NO_RECYCLE;
- return io_setup_async_addr(req, &__address, issue_flags);
+ return io_setup_async_msg(req, kmsg, issue_flags);
}
if (ret == -ERESTARTSYS)
ret = -EINTR;
@@ -1251,6 +1258,7 @@ int io_send_zc(struct io_kiocb *req, unsigned int issue_flags)
*/
if (!(issue_flags & IO_URING_F_UNLOCKED)) {
io_notif_flush(zc->notif);
+ io_netmsg_recycle(req, issue_flags);
req->flags &= ~REQ_F_NEED_CLEANUP;
}
io_req_set_res(req, ret, IORING_CQE_F_MORE);
diff --git a/io_uring/opdef.c b/io_uring/opdef.c
index 9c080aadc5a6..b0a990c6bbff 100644
--- a/io_uring/opdef.c
+++ b/io_uring/opdef.c
@@ -602,6 +602,7 @@ const struct io_cold_def io_cold_defs[] = {
.name = "SEND",
#if defined(CONFIG_NET)
.async_size = sizeof(struct io_async_msghdr),
+ .cleanup = io_sendmsg_recvmsg_cleanup,
.fail = io_sendrecv_fail,
.prep_async = io_send_prep_async,
#endif
--
2.43.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH 01/17] io_uring/net: switch io_send() and io_send_zc() to using io_async_msghdr
2024-03-20 22:55 ` [PATCH 01/17] io_uring/net: switch io_send() and io_send_zc() to using io_async_msghdr Jens Axboe
@ 2024-04-06 20:58 ` Pavel Begunkov
2024-04-07 21:47 ` Jens Axboe
0 siblings, 1 reply; 30+ messages in thread
From: Pavel Begunkov @ 2024-04-06 20:58 UTC (permalink / raw)
To: Jens Axboe, io-uring
On 3/20/24 22:55, Jens Axboe wrote:
> No functional changes in this patch, just in preparation for carrying
> more state then we have now, if necessary. While unifying some of this
> code, add a generic send setup prep handler that they can both use.
>
> This gets rid of some manual msghdr and sockaddr on the stack, and makes
> it look a bit more like the sendmsg/recvmsg variants. We can probably
> unify a bit more on top of this going forward.
>
> Signed-off-by: Jens Axboe <[email protected]>
> ---
> io_uring/net.c | 196 ++++++++++++++++++++++++-----------------------
> io_uring/opdef.c | 1 +
> 2 files changed, 103 insertions(+), 94 deletions(-)
>
> diff --git a/io_uring/net.c b/io_uring/net.c
> index ed798e185bbf..a16838c0c837 100644
> --- a/io_uring/net.c
> +++ b/io_uring/net.c
> @@ -322,36 +322,25 @@ static int io_sendmsg_copy_hdr(struct io_kiocb *req,
>
...
> -int io_send(struct io_kiocb *req, unsigned int issue_flags)
> +static struct io_async_msghdr *io_send_setup(struct io_kiocb *req,
> + struct io_async_msghdr *stack_msg,
> + unsigned int issue_flags)
> {
> - struct sockaddr_storage __address;
> struct io_sr_msg *sr = io_kiocb_to_cmd(req, struct io_sr_msg);
> - struct msghdr msg;
> - struct socket *sock;
> - unsigned flags;
> - int min_ret = 0;
> + struct io_async_msghdr *kmsg;
> int ret;
>
> - msg.msg_name = NULL;
> - msg.msg_control = NULL;
> - msg.msg_controllen = 0;
> - msg.msg_namelen = 0;
> - msg.msg_ubuf = NULL;
> -
> - if (sr->addr) {
> - if (req_has_async_data(req)) {
> - struct io_async_msghdr *io = req->async_data;
> -
> - msg.msg_name = &io->addr;
> - } else {
> - ret = move_addr_to_kernel(sr->addr, sr->addr_len, &__address);
> + if (req_has_async_data(req)) {
> + kmsg = req->async_data;
> + } else {
> + kmsg = stack_msg;
> + kmsg->free_iov = NULL;
> + kmsg->msg.msg_name = NULL;
> + kmsg->msg.msg_namelen = 0;
> + kmsg->msg.msg_control = NULL;
> + kmsg->msg.msg_controllen = 0;
> + kmsg->msg.msg_ubuf = NULL;
> +
> + if (sr->addr) {
> + ret = move_addr_to_kernel(sr->addr, sr->addr_len,
> + &kmsg->addr);
> if (unlikely(ret < 0))
> - return ret;
> - msg.msg_name = (struct sockaddr *)&__address;
> + return ERR_PTR(ret);
> + kmsg->msg.msg_name = &kmsg->addr;
> + kmsg->msg.msg_namelen = sr->addr_len;
> + }
> +
> + if (!io_do_buffer_select(req)) {
it seems, this chunk leaked from another series as well. fwiw,
it was moved in a later commit
"io_uring/net: get rid of ->prep_async() for send side"
> + ret = import_ubuf(ITER_SOURCE, sr->buf, sr->len,
> + &kmsg->msg.msg_iter);
> + if (unlikely(ret))
> + return ERR_PTR(ret);
> }
> - msg.msg_namelen = sr->addr_len;
> }
--
Pavel Begunkov
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH 01/17] io_uring/net: switch io_send() and io_send_zc() to using io_async_msghdr
2024-04-06 20:58 ` Pavel Begunkov
@ 2024-04-07 21:47 ` Jens Axboe
0 siblings, 0 replies; 30+ messages in thread
From: Jens Axboe @ 2024-04-07 21:47 UTC (permalink / raw)
To: Pavel Begunkov, io-uring
On 4/6/24 2:58 PM, Pavel Begunkov wrote:
> On 3/20/24 22:55, Jens Axboe wrote:
>> No functional changes in this patch, just in preparation for carrying
>> more state then we have now, if necessary. While unifying some of this
>> code, add a generic send setup prep handler that they can both use.
>>
>> This gets rid of some manual msghdr and sockaddr on the stack, and makes
>> it look a bit more like the sendmsg/recvmsg variants. We can probably
>> unify a bit more on top of this going forward.
>>
>> Signed-off-by: Jens Axboe <[email protected]>
>> ---
>> io_uring/net.c | 196 ++++++++++++++++++++++++-----------------------
>> io_uring/opdef.c | 1 +
>> 2 files changed, 103 insertions(+), 94 deletions(-)
>>
>> diff --git a/io_uring/net.c b/io_uring/net.c
>> index ed798e185bbf..a16838c0c837 100644
>> --- a/io_uring/net.c
>> +++ b/io_uring/net.c
>> @@ -322,36 +322,25 @@ static int io_sendmsg_copy_hdr(struct io_kiocb *req,
>>
> ...
>> -int io_send(struct io_kiocb *req, unsigned int issue_flags)
>> +static struct io_async_msghdr *io_send_setup(struct io_kiocb *req,
>> + struct io_async_msghdr *stack_msg,
>> + unsigned int issue_flags)
>> {
>> - struct sockaddr_storage __address;
>> struct io_sr_msg *sr = io_kiocb_to_cmd(req, struct io_sr_msg);
>> - struct msghdr msg;
>> - struct socket *sock;
>> - unsigned flags;
>> - int min_ret = 0;
>> + struct io_async_msghdr *kmsg;
>> int ret;
>> - msg.msg_name = NULL;
>> - msg.msg_control = NULL;
>> - msg.msg_controllen = 0;
>> - msg.msg_namelen = 0;
>> - msg.msg_ubuf = NULL;
>> -
>> - if (sr->addr) {
>> - if (req_has_async_data(req)) {
>> - struct io_async_msghdr *io = req->async_data;
>> -
>> - msg.msg_name = &io->addr;
>> - } else {
>> - ret = move_addr_to_kernel(sr->addr, sr->addr_len, &__address);
>> + if (req_has_async_data(req)) {
>> + kmsg = req->async_data;
>> + } else {
>> + kmsg = stack_msg;
>> + kmsg->free_iov = NULL;
>> + kmsg->msg.msg_name = NULL;
>> + kmsg->msg.msg_namelen = 0;
>> + kmsg->msg.msg_control = NULL;
>> + kmsg->msg.msg_controllen = 0;
>> + kmsg->msg.msg_ubuf = NULL;
>> +
>> + if (sr->addr) {
>> + ret = move_addr_to_kernel(sr->addr, sr->addr_len,
>> + &kmsg->addr);
>> if (unlikely(ret < 0))
>> - return ret;
>> - msg.msg_name = (struct sockaddr *)&__address;
>> + return ERR_PTR(ret);
>> + kmsg->msg.msg_name = &kmsg->addr;
>> + kmsg->msg.msg_namelen = sr->addr_len;
>> + }
>> +
>> + if (!io_do_buffer_select(req)) {
>
> it seems, this chunk leaked from another series as well. fwiw,
> it was moved in a later commit
> "io_uring/net: get rid of ->prep_async() for send side"
Thanks, yeah I think so too. Will fix, thanks!
--
Jens Axboe
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 02/17] io_uring/net: switch io_recv() to using io_async_msghdr
2024-03-20 22:55 [PATCHSET v2 0/17] Improve async state handling Jens Axboe
2024-03-20 22:55 ` [PATCH 01/17] io_uring/net: switch io_send() and io_send_zc() to using io_async_msghdr Jens Axboe
@ 2024-03-20 22:55 ` Jens Axboe
2024-03-20 22:55 ` [PATCH 03/17] io_uring/net: unify cleanup handling Jens Axboe
` (14 subsequent siblings)
16 siblings, 0 replies; 30+ messages in thread
From: Jens Axboe @ 2024-03-20 22:55 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe
No functional changes in this patch, just in preparation for carrying
more state then we have now, if necessary.
Signed-off-by: Jens Axboe <[email protected]>
---
io_uring/net.c | 75 ++++++++++++++++++++++++++++++------------------
io_uring/net.h | 2 +-
io_uring/opdef.c | 7 +++--
3 files changed, 53 insertions(+), 31 deletions(-)
diff --git a/io_uring/net.c b/io_uring/net.c
index a16838c0c837..d571115f4909 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -320,7 +320,7 @@ static int io_sendmsg_copy_hdr(struct io_kiocb *req,
return ret;
}
-int io_send_prep_async(struct io_kiocb *req)
+int io_sendrecv_prep_async(struct io_kiocb *req)
{
struct io_sr_msg *sr = io_kiocb_to_cmd(req, struct io_sr_msg);
struct io_async_msghdr *io;
@@ -705,13 +705,13 @@ static inline void io_recv_prep_retry(struct io_kiocb *req)
* again (for multishot).
*/
static inline bool io_recv_finish(struct io_kiocb *req, int *ret,
- struct msghdr *msg, bool mshot_finished,
- unsigned issue_flags)
+ struct io_async_msghdr *kmsg,
+ bool mshot_finished, unsigned issue_flags)
{
unsigned int cflags;
cflags = io_put_kbuf(req, issue_flags);
- if (msg->msg_inq > 0)
+ if (kmsg->msg.msg_inq > 0)
cflags |= IORING_CQE_F_SOCK_NONEMPTY;
/*
@@ -725,7 +725,7 @@ static inline bool io_recv_finish(struct io_kiocb *req, int *ret,
io_recv_prep_retry(req);
/* Known not-empty or unknown state, retry */
- if (cflags & IORING_CQE_F_SOCK_NONEMPTY || msg->msg_inq < 0) {
+ if (cflags & IORING_CQE_F_SOCK_NONEMPTY || kmsg->msg.msg_inq < 0) {
if (sr->nr_multishot_loops++ < MULTISHOT_MAX_RETRY)
return false;
/* mshot retries exceeded, force a requeue */
@@ -926,7 +926,7 @@ int io_recvmsg(struct io_kiocb *req, unsigned int issue_flags)
else
io_kbuf_recycle(req, issue_flags);
- if (!io_recv_finish(req, &ret, &kmsg->msg, mshot_finished, issue_flags))
+ if (!io_recv_finish(req, &ret, kmsg, mshot_finished, issue_flags))
goto retry_multishot;
if (mshot_finished)
@@ -940,29 +940,42 @@ int io_recvmsg(struct io_kiocb *req, unsigned int issue_flags)
int io_recv(struct io_kiocb *req, unsigned int issue_flags)
{
struct io_sr_msg *sr = io_kiocb_to_cmd(req, struct io_sr_msg);
- struct msghdr msg;
+ struct io_async_msghdr iomsg, *kmsg;
struct socket *sock;
unsigned flags;
int ret, min_ret = 0;
bool force_nonblock = issue_flags & IO_URING_F_NONBLOCK;
size_t len = sr->len;
+ if (req_has_async_data(req)) {
+ kmsg = req->async_data;
+ } else {
+ kmsg = &iomsg;
+ kmsg->free_iov = NULL;
+ kmsg->msg.msg_name = NULL;
+ kmsg->msg.msg_namelen = 0;
+ kmsg->msg.msg_control = NULL;
+ kmsg->msg.msg_get_inq = 1;
+ kmsg->msg.msg_controllen = 0;
+ kmsg->msg.msg_iocb = NULL;
+ kmsg->msg.msg_ubuf = NULL;
+
+ if (!io_do_buffer_select(req)) {
+ ret = import_ubuf(ITER_DEST, sr->buf, sr->len,
+ &kmsg->msg.msg_iter);
+ if (unlikely(ret))
+ return ret;
+ }
+ }
+
if (!(req->flags & REQ_F_POLLED) &&
(sr->flags & IORING_RECVSEND_POLL_FIRST))
- return -EAGAIN;
+ return io_setup_async_msg(req, kmsg, issue_flags);
sock = sock_from_file(req->file);
if (unlikely(!sock))
return -ENOTSOCK;
- msg.msg_name = NULL;
- msg.msg_namelen = 0;
- msg.msg_control = NULL;
- msg.msg_get_inq = 1;
- msg.msg_controllen = 0;
- msg.msg_iocb = NULL;
- msg.msg_ubuf = NULL;
-
flags = sr->msg_flags;
if (force_nonblock)
flags |= MSG_DONTWAIT;
@@ -976,22 +989,23 @@ int io_recv(struct io_kiocb *req, unsigned int issue_flags)
return -ENOBUFS;
sr->buf = buf;
sr->len = len;
+ ret = import_ubuf(ITER_DEST, sr->buf, sr->len,
+ &kmsg->msg.msg_iter);
+ if (unlikely(ret))
+ goto out_free;
}
- ret = import_ubuf(ITER_DEST, sr->buf, len, &msg.msg_iter);
- if (unlikely(ret))
- goto out_free;
-
- msg.msg_inq = -1;
- msg.msg_flags = 0;
+ kmsg->msg.msg_inq = -1;
+ kmsg->msg.msg_flags = 0;
if (flags & MSG_WAITALL)
- min_ret = iov_iter_count(&msg.msg_iter);
+ min_ret = iov_iter_count(&kmsg->msg.msg_iter);
- ret = sock_recvmsg(sock, &msg, flags);
+ ret = sock_recvmsg(sock, &kmsg->msg, flags);
if (ret < min_ret) {
if (ret == -EAGAIN && force_nonblock) {
- if (issue_flags & IO_URING_F_MULTISHOT) {
+ ret = io_setup_async_msg(req, kmsg, issue_flags);
+ if (ret == -EAGAIN && issue_flags & IO_URING_F_MULTISHOT) {
io_kbuf_recycle(req, issue_flags);
return IOU_ISSUE_SKIP_COMPLETE;
}
@@ -1003,12 +1017,12 @@ int io_recv(struct io_kiocb *req, unsigned int issue_flags)
sr->buf += ret;
sr->done_io += ret;
req->flags |= REQ_F_BL_NO_RECYCLE;
- return -EAGAIN;
+ return io_setup_async_msg(req, kmsg, issue_flags);
}
if (ret == -ERESTARTSYS)
ret = -EINTR;
req_set_fail(req);
- } else if ((flags & MSG_WAITALL) && (msg.msg_flags & (MSG_TRUNC | MSG_CTRUNC))) {
+ } else if ((flags & MSG_WAITALL) && (kmsg->msg.msg_flags & (MSG_TRUNC | MSG_CTRUNC))) {
out_free:
req_set_fail(req);
}
@@ -1020,9 +1034,14 @@ int io_recv(struct io_kiocb *req, unsigned int issue_flags)
else
io_kbuf_recycle(req, issue_flags);
- if (!io_recv_finish(req, &ret, &msg, ret <= 0, issue_flags))
+ if (!io_recv_finish(req, &ret, kmsg, ret <= 0, issue_flags))
goto retry_multishot;
+ if (ret == -EAGAIN)
+ return io_setup_async_msg(req, kmsg, issue_flags);
+ else if (ret != IOU_OK && ret != IOU_STOP_MULTISHOT)
+ io_req_msg_cleanup(req, kmsg, issue_flags);
+
return ret;
}
diff --git a/io_uring/net.h b/io_uring/net.h
index 191009979bcb..5c1230f1aaf9 100644
--- a/io_uring/net.h
+++ b/io_uring/net.h
@@ -40,7 +40,7 @@ int io_sendmsg_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
int io_sendmsg(struct io_kiocb *req, unsigned int issue_flags);
int io_send(struct io_kiocb *req, unsigned int issue_flags);
-int io_send_prep_async(struct io_kiocb *req);
+int io_sendrecv_prep_async(struct io_kiocb *req);
int io_recvmsg_prep_async(struct io_kiocb *req);
int io_recvmsg_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
diff --git a/io_uring/opdef.c b/io_uring/opdef.c
index b0a990c6bbff..77131826d603 100644
--- a/io_uring/opdef.c
+++ b/io_uring/opdef.c
@@ -604,13 +604,16 @@ const struct io_cold_def io_cold_defs[] = {
.async_size = sizeof(struct io_async_msghdr),
.cleanup = io_sendmsg_recvmsg_cleanup,
.fail = io_sendrecv_fail,
- .prep_async = io_send_prep_async,
+ .prep_async = io_sendrecv_prep_async,
#endif
},
[IORING_OP_RECV] = {
.name = "RECV",
#if defined(CONFIG_NET)
+ .async_size = sizeof(struct io_async_msghdr),
+ .cleanup = io_sendmsg_recvmsg_cleanup,
.fail = io_sendrecv_fail,
+ .prep_async = io_sendrecv_prep_async,
#endif
},
[IORING_OP_OPENAT2] = {
@@ -687,7 +690,7 @@ const struct io_cold_def io_cold_defs[] = {
.name = "SEND_ZC",
#if defined(CONFIG_NET)
.async_size = sizeof(struct io_async_msghdr),
- .prep_async = io_send_prep_async,
+ .prep_async = io_sendrecv_prep_async,
.cleanup = io_send_zc_cleanup,
.fail = io_sendrecv_fail,
#endif
--
2.43.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH 03/17] io_uring/net: unify cleanup handling
2024-03-20 22:55 [PATCHSET v2 0/17] Improve async state handling Jens Axboe
2024-03-20 22:55 ` [PATCH 01/17] io_uring/net: switch io_send() and io_send_zc() to using io_async_msghdr Jens Axboe
2024-03-20 22:55 ` [PATCH 02/17] io_uring/net: switch io_recv() " Jens Axboe
@ 2024-03-20 22:55 ` Jens Axboe
2024-03-20 22:55 ` [PATCH 04/17] io_uring/net: always setup an io_async_msghdr Jens Axboe
` (13 subsequent siblings)
16 siblings, 0 replies; 30+ messages in thread
From: Jens Axboe @ 2024-03-20 22:55 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe
Now that recv/recvmsg both do the same cleanup, put it in the retry and
finish handlers.
Signed-off-by: Jens Axboe <[email protected]>
---
io_uring/net.c | 26 +++++++++++---------------
1 file changed, 11 insertions(+), 15 deletions(-)
diff --git a/io_uring/net.c b/io_uring/net.c
index d571115f4909..2df59fb19a15 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -688,10 +688,16 @@ int io_recvmsg_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
return 0;
}
-static inline void io_recv_prep_retry(struct io_kiocb *req)
+static inline void io_recv_prep_retry(struct io_kiocb *req,
+ struct io_async_msghdr *kmsg)
{
struct io_sr_msg *sr = io_kiocb_to_cmd(req, struct io_sr_msg);
+ if (kmsg->free_iov) {
+ kfree(kmsg->free_iov);
+ kmsg->free_iov = NULL;
+ }
+
req->flags &= ~REQ_F_BL_EMPTY;
sr->done_io = 0;
sr->len = 0; /* get from the provided buffer */
@@ -723,7 +729,7 @@ static inline bool io_recv_finish(struct io_kiocb *req, int *ret,
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);
+ io_recv_prep_retry(req, kmsg);
/* Known not-empty or unknown state, retry */
if (cflags & IORING_CQE_F_SOCK_NONEMPTY || kmsg->msg.msg_inq < 0) {
if (sr->nr_multishot_loops++ < MULTISHOT_MAX_RETRY)
@@ -732,10 +738,9 @@ static inline bool io_recv_finish(struct io_kiocb *req, int *ret,
sr->nr_multishot_loops = 0;
mshot_retry_ret = IOU_REQUEUE;
}
- if (issue_flags & IO_URING_F_MULTISHOT)
+ *ret = io_setup_async_msg(req, kmsg, issue_flags);
+ if (*ret == -EAGAIN && issue_flags & IO_URING_F_MULTISHOT)
*ret = mshot_retry_ret;
- else
- *ret = -EAGAIN;
return true;
}
@@ -746,6 +751,7 @@ static inline bool io_recv_finish(struct io_kiocb *req, int *ret,
*ret = IOU_STOP_MULTISHOT;
else
*ret = IOU_OK;
+ io_req_msg_cleanup(req, kmsg, issue_flags);
return true;
}
@@ -929,11 +935,6 @@ int io_recvmsg(struct io_kiocb *req, unsigned int issue_flags)
if (!io_recv_finish(req, &ret, kmsg, mshot_finished, issue_flags))
goto retry_multishot;
- if (mshot_finished)
- io_req_msg_cleanup(req, kmsg, issue_flags);
- else if (ret == -EAGAIN)
- return io_setup_async_msg(req, kmsg, issue_flags);
-
return ret;
}
@@ -1037,11 +1038,6 @@ int io_recv(struct io_kiocb *req, unsigned int issue_flags)
if (!io_recv_finish(req, &ret, kmsg, ret <= 0, issue_flags))
goto retry_multishot;
- if (ret == -EAGAIN)
- return io_setup_async_msg(req, kmsg, issue_flags);
- else if (ret != IOU_OK && ret != IOU_STOP_MULTISHOT)
- io_req_msg_cleanup(req, kmsg, issue_flags);
-
return ret;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH 04/17] io_uring/net: always setup an io_async_msghdr
2024-03-20 22:55 [PATCHSET v2 0/17] Improve async state handling Jens Axboe
` (2 preceding siblings ...)
2024-03-20 22:55 ` [PATCH 03/17] io_uring/net: unify cleanup handling Jens Axboe
@ 2024-03-20 22:55 ` Jens Axboe
2024-03-20 22:55 ` [PATCH 05/17] io_uring/net: get rid of ->prep_async() for receive side Jens Axboe
` (12 subsequent siblings)
16 siblings, 0 replies; 30+ messages in thread
From: Jens Axboe @ 2024-03-20 22:55 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe
Rather than use an on-stack one and then need to allocate and copy if
we have to go async, always grab one upfront. This should be very
cheap, and potentially even have cache hotness benefits for back-to-back
send/recv requests.
For any recv type of request, this is probably a good choice in general,
as it's expected that no data is available initially. For send this is
not necessarily the case, as we expect space to be available. However,
getting a cached io_async_msghdr is very cheap, and as it should be
cache hot, probably the difference here is neglible, if any.
A nice side benefit is that we can kill io_setup_async_msg completely,
which has some nasty iovec manipulation code.
Signed-off-by: Jens Axboe <[email protected]>
---
io_uring/net.c | 117 ++++++++++++++++++++-----------------------------
1 file changed, 47 insertions(+), 70 deletions(-)
diff --git a/io_uring/net.c b/io_uring/net.c
index 2df59fb19a15..14491fab6d59 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -161,36 +161,6 @@ static inline struct io_async_msghdr *io_msg_alloc_async_prep(struct io_kiocb *r
return io_msg_alloc_async(req, 0);
}
-static int io_setup_async_msg(struct io_kiocb *req,
- struct io_async_msghdr *kmsg,
- unsigned int issue_flags)
-{
- struct io_async_msghdr *async_msg;
-
- if (req_has_async_data(req))
- return -EAGAIN;
- async_msg = io_msg_alloc_async(req, issue_flags);
- if (!async_msg) {
- kfree(kmsg->free_iov);
- return -ENOMEM;
- }
- req->flags |= REQ_F_NEED_CLEANUP;
- memcpy(async_msg, kmsg, sizeof(*kmsg));
- if (async_msg->msg.msg_name)
- async_msg->msg.msg_name = &async_msg->addr;
-
- if ((req->flags & REQ_F_BUFFER_SELECT) && !async_msg->msg.msg_iter.nr_segs)
- return -EAGAIN;
-
- /* if were using fast_iov, set it to the new one */
- if (iter_is_iovec(&kmsg->msg.msg_iter) && !kmsg->free_iov) {
- size_t fast_idx = iter_iov(&kmsg->msg.msg_iter) - kmsg->fast_iov;
- async_msg->msg.msg_iter.__iov = &async_msg->fast_iov[fast_idx];
- }
-
- return -EAGAIN;
-}
-
#ifdef CONFIG_COMPAT
static int io_compat_msg_copy_hdr(struct io_kiocb *req,
struct io_async_msghdr *iomsg,
@@ -409,7 +379,7 @@ static void io_req_msg_cleanup(struct io_kiocb *req,
int io_sendmsg(struct io_kiocb *req, unsigned int issue_flags)
{
struct io_sr_msg *sr = io_kiocb_to_cmd(req, struct io_sr_msg);
- struct io_async_msghdr iomsg, *kmsg;
+ struct io_async_msghdr *kmsg;
struct socket *sock;
unsigned flags;
int min_ret = 0;
@@ -423,15 +393,17 @@ int io_sendmsg(struct io_kiocb *req, unsigned int issue_flags)
kmsg = req->async_data;
kmsg->msg.msg_control_user = sr->msg_control;
} else {
- ret = io_sendmsg_copy_hdr(req, &iomsg);
+ kmsg = io_msg_alloc_async(req, issue_flags);
+ if (unlikely(!kmsg))
+ return -ENOMEM;
+ ret = io_sendmsg_copy_hdr(req, kmsg);
if (ret)
return ret;
- kmsg = &iomsg;
}
if (!(req->flags & REQ_F_POLLED) &&
(sr->flags & IORING_RECVSEND_POLL_FIRST))
- return io_setup_async_msg(req, kmsg, issue_flags);
+ return -EAGAIN;
flags = sr->msg_flags;
if (issue_flags & IO_URING_F_NONBLOCK)
@@ -443,13 +415,13 @@ int io_sendmsg(struct io_kiocb *req, unsigned int issue_flags)
if (ret < min_ret) {
if (ret == -EAGAIN && (issue_flags & IO_URING_F_NONBLOCK))
- return io_setup_async_msg(req, kmsg, issue_flags);
+ return -EAGAIN;
if (ret > 0 && io_net_retry(sock, flags)) {
kmsg->msg.msg_controllen = 0;
kmsg->msg.msg_control = NULL;
sr->done_io += ret;
req->flags |= REQ_F_BL_NO_RECYCLE;
- return io_setup_async_msg(req, kmsg, issue_flags);
+ return -EAGAIN;
}
if (ret == -ERESTARTSYS)
ret = -EINTR;
@@ -465,7 +437,6 @@ int io_sendmsg(struct io_kiocb *req, unsigned int issue_flags)
}
static struct io_async_msghdr *io_send_setup(struct io_kiocb *req,
- struct io_async_msghdr *stack_msg,
unsigned int issue_flags)
{
struct io_sr_msg *sr = io_kiocb_to_cmd(req, struct io_sr_msg);
@@ -475,8 +446,9 @@ static struct io_async_msghdr *io_send_setup(struct io_kiocb *req,
if (req_has_async_data(req)) {
kmsg = req->async_data;
} else {
- kmsg = stack_msg;
- kmsg->free_iov = NULL;
+ kmsg = io_msg_alloc_async(req, issue_flags);
+ if (unlikely(!kmsg))
+ return ERR_PTR(-ENOMEM);
kmsg->msg.msg_name = NULL;
kmsg->msg.msg_namelen = 0;
kmsg->msg.msg_control = NULL;
@@ -502,7 +474,7 @@ static struct io_async_msghdr *io_send_setup(struct io_kiocb *req,
if (!(req->flags & REQ_F_POLLED) &&
(sr->flags & IORING_RECVSEND_POLL_FIRST))
- return ERR_PTR(io_setup_async_msg(req, kmsg, issue_flags));
+ return ERR_PTR(-EAGAIN);
return kmsg;
}
@@ -510,7 +482,7 @@ static struct io_async_msghdr *io_send_setup(struct io_kiocb *req,
int io_send(struct io_kiocb *req, unsigned int issue_flags)
{
struct io_sr_msg *sr = io_kiocb_to_cmd(req, struct io_sr_msg);
- struct io_async_msghdr iomsg, *kmsg;
+ struct io_async_msghdr *kmsg;
size_t len = sr->len;
struct socket *sock;
unsigned flags;
@@ -521,7 +493,7 @@ int io_send(struct io_kiocb *req, unsigned int issue_flags)
if (unlikely(!sock))
return -ENOTSOCK;
- kmsg = io_send_setup(req, &iomsg, issue_flags);
+ kmsg = io_send_setup(req, issue_flags);
if (IS_ERR(kmsg))
return PTR_ERR(kmsg);
@@ -540,14 +512,14 @@ int io_send(struct io_kiocb *req, unsigned int issue_flags)
ret = sock_sendmsg(sock, &kmsg->msg);
if (ret < min_ret) {
if (ret == -EAGAIN && (issue_flags & IO_URING_F_NONBLOCK))
- return io_setup_async_msg(req, kmsg, issue_flags);
+ return -EAGAIN;
if (ret > 0 && io_net_retry(sock, flags)) {
sr->len -= ret;
sr->buf += ret;
sr->done_io += ret;
req->flags |= REQ_F_BL_NO_RECYCLE;
- return io_setup_async_msg(req, kmsg, issue_flags);
+ return -EAGAIN;
}
if (ret == -ERESTARTSYS)
ret = -EINTR;
@@ -738,9 +710,10 @@ static inline bool io_recv_finish(struct io_kiocb *req, int *ret,
sr->nr_multishot_loops = 0;
mshot_retry_ret = IOU_REQUEUE;
}
- *ret = io_setup_async_msg(req, kmsg, issue_flags);
- if (*ret == -EAGAIN && issue_flags & IO_URING_F_MULTISHOT)
+ if (issue_flags & IO_URING_F_MULTISHOT)
*ret = mshot_retry_ret;
+ else
+ *ret = -EAGAIN;
return true;
}
@@ -842,7 +815,7 @@ static int io_recvmsg_multishot(struct socket *sock, struct io_sr_msg *io,
int io_recvmsg(struct io_kiocb *req, unsigned int issue_flags)
{
struct io_sr_msg *sr = io_kiocb_to_cmd(req, struct io_sr_msg);
- struct io_async_msghdr iomsg, *kmsg;
+ struct io_async_msghdr *kmsg;
struct socket *sock;
unsigned flags;
int ret, min_ret = 0;
@@ -856,15 +829,17 @@ int io_recvmsg(struct io_kiocb *req, unsigned int issue_flags)
if (req_has_async_data(req)) {
kmsg = req->async_data;
} else {
- ret = io_recvmsg_copy_hdr(req, &iomsg);
+ kmsg = io_msg_alloc_async(req, issue_flags);
+ if (unlikely(!kmsg))
+ return -ENOMEM;
+ ret = io_recvmsg_copy_hdr(req, kmsg);
if (ret)
return ret;
- kmsg = &iomsg;
}
if (!(req->flags & REQ_F_POLLED) &&
(sr->flags & IORING_RECVSEND_POLL_FIRST))
- return io_setup_async_msg(req, kmsg, issue_flags);
+ return -EAGAIN;
flags = sr->msg_flags;
if (force_nonblock)
@@ -906,17 +881,16 @@ int io_recvmsg(struct io_kiocb *req, unsigned int issue_flags)
if (ret < min_ret) {
if (ret == -EAGAIN && force_nonblock) {
- ret = io_setup_async_msg(req, kmsg, issue_flags);
- if (ret == -EAGAIN && (issue_flags & IO_URING_F_MULTISHOT)) {
+ if (issue_flags & IO_URING_F_MULTISHOT) {
io_kbuf_recycle(req, issue_flags);
return IOU_ISSUE_SKIP_COMPLETE;
}
- return ret;
+ return -EAGAIN;
}
if (ret > 0 && io_net_retry(sock, flags)) {
sr->done_io += ret;
req->flags |= REQ_F_BL_NO_RECYCLE;
- return io_setup_async_msg(req, kmsg, issue_flags);
+ return -EAGAIN;
}
if (ret == -ERESTARTSYS)
ret = -EINTR;
@@ -941,7 +915,7 @@ int io_recvmsg(struct io_kiocb *req, unsigned int issue_flags)
int io_recv(struct io_kiocb *req, unsigned int issue_flags)
{
struct io_sr_msg *sr = io_kiocb_to_cmd(req, struct io_sr_msg);
- struct io_async_msghdr iomsg, *kmsg;
+ struct io_async_msghdr *kmsg;
struct socket *sock;
unsigned flags;
int ret, min_ret = 0;
@@ -951,7 +925,9 @@ int io_recv(struct io_kiocb *req, unsigned int issue_flags)
if (req_has_async_data(req)) {
kmsg = req->async_data;
} else {
- kmsg = &iomsg;
+ kmsg = io_msg_alloc_async(req, issue_flags);
+ if (unlikely(!kmsg))
+ return -ENOMEM;
kmsg->free_iov = NULL;
kmsg->msg.msg_name = NULL;
kmsg->msg.msg_namelen = 0;
@@ -971,7 +947,7 @@ int io_recv(struct io_kiocb *req, unsigned int issue_flags)
if (!(req->flags & REQ_F_POLLED) &&
(sr->flags & IORING_RECVSEND_POLL_FIRST))
- return io_setup_async_msg(req, kmsg, issue_flags);
+ return -EAGAIN;
sock = sock_from_file(req->file);
if (unlikely(!sock))
@@ -1005,8 +981,7 @@ int io_recv(struct io_kiocb *req, unsigned int issue_flags)
ret = sock_recvmsg(sock, &kmsg->msg, flags);
if (ret < min_ret) {
if (ret == -EAGAIN && force_nonblock) {
- ret = io_setup_async_msg(req, kmsg, issue_flags);
- if (ret == -EAGAIN && issue_flags & IO_URING_F_MULTISHOT) {
+ if (issue_flags & IO_URING_F_MULTISHOT) {
io_kbuf_recycle(req, issue_flags);
return IOU_ISSUE_SKIP_COMPLETE;
}
@@ -1018,7 +993,7 @@ int io_recv(struct io_kiocb *req, unsigned int issue_flags)
sr->buf += ret;
sr->done_io += ret;
req->flags |= REQ_F_BL_NO_RECYCLE;
- return io_setup_async_msg(req, kmsg, issue_flags);
+ return -EAGAIN;
}
if (ret == -ERESTARTSYS)
ret = -EINTR;
@@ -1214,7 +1189,7 @@ static int io_send_zc_import(struct io_kiocb *req, struct io_async_msghdr *kmsg)
int io_send_zc(struct io_kiocb *req, unsigned int issue_flags)
{
struct io_sr_msg *zc = io_kiocb_to_cmd(req, struct io_sr_msg);
- struct io_async_msghdr iomsg, *kmsg;
+ struct io_async_msghdr *kmsg;
struct socket *sock;
unsigned msg_flags;
int ret, min_ret = 0;
@@ -1225,7 +1200,7 @@ int io_send_zc(struct io_kiocb *req, unsigned int issue_flags)
if (!test_bit(SOCK_SUPPORT_ZC, &sock->flags))
return -EOPNOTSUPP;
- kmsg = io_send_setup(req, &iomsg, issue_flags);
+ kmsg = io_send_setup(req, issue_flags);
if (IS_ERR(kmsg))
return PTR_ERR(kmsg);
@@ -1248,14 +1223,14 @@ int io_send_zc(struct io_kiocb *req, unsigned int issue_flags)
if (unlikely(ret < min_ret)) {
if (ret == -EAGAIN && (issue_flags & IO_URING_F_NONBLOCK))
- return io_setup_async_msg(req, kmsg, issue_flags);
+ return -EAGAIN;
if (ret > 0 && io_net_retry(sock, kmsg->msg.msg_flags)) {
zc->len -= ret;
zc->buf += ret;
zc->done_io += ret;
req->flags |= REQ_F_BL_NO_RECYCLE;
- return io_setup_async_msg(req, kmsg, issue_flags);
+ return -EAGAIN;
}
if (ret == -ERESTARTSYS)
ret = -EINTR;
@@ -1283,7 +1258,7 @@ int io_send_zc(struct io_kiocb *req, unsigned int issue_flags)
int io_sendmsg_zc(struct io_kiocb *req, unsigned int issue_flags)
{
struct io_sr_msg *sr = io_kiocb_to_cmd(req, struct io_sr_msg);
- struct io_async_msghdr iomsg, *kmsg;
+ struct io_async_msghdr *kmsg;
struct socket *sock;
unsigned flags;
int ret, min_ret = 0;
@@ -1299,15 +1274,17 @@ int io_sendmsg_zc(struct io_kiocb *req, unsigned int issue_flags)
if (req_has_async_data(req)) {
kmsg = req->async_data;
} else {
- ret = io_sendmsg_copy_hdr(req, &iomsg);
+ kmsg = io_msg_alloc_async(req, issue_flags);
+ if (unlikely(!kmsg))
+ return -ENOMEM;
+ ret = io_sendmsg_copy_hdr(req, kmsg);
if (ret)
return ret;
- kmsg = &iomsg;
}
if (!(req->flags & REQ_F_POLLED) &&
(sr->flags & IORING_RECVSEND_POLL_FIRST))
- return io_setup_async_msg(req, kmsg, issue_flags);
+ return -EAGAIN;
flags = sr->msg_flags | MSG_ZEROCOPY;
if (issue_flags & IO_URING_F_NONBLOCK)
@@ -1321,12 +1298,12 @@ int io_sendmsg_zc(struct io_kiocb *req, unsigned int issue_flags)
if (unlikely(ret < min_ret)) {
if (ret == -EAGAIN && (issue_flags & IO_URING_F_NONBLOCK))
- return io_setup_async_msg(req, kmsg, issue_flags);
+ return -EAGAIN;
if (ret > 0 && io_net_retry(sock, flags)) {
sr->done_io += ret;
req->flags |= REQ_F_BL_NO_RECYCLE;
- return io_setup_async_msg(req, kmsg, issue_flags);
+ return -EAGAIN;
}
if (ret == -ERESTARTSYS)
ret = -EINTR;
--
2.43.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH 05/17] io_uring/net: get rid of ->prep_async() for receive side
2024-03-20 22:55 [PATCHSET v2 0/17] Improve async state handling Jens Axboe
` (3 preceding siblings ...)
2024-03-20 22:55 ` [PATCH 04/17] io_uring/net: always setup an io_async_msghdr Jens Axboe
@ 2024-03-20 22:55 ` Jens Axboe
2024-03-20 22:55 ` [PATCH 06/17] io_uring/net: get rid of ->prep_async() for send side Jens Axboe
` (11 subsequent siblings)
16 siblings, 0 replies; 30+ messages in thread
From: Jens Axboe @ 2024-03-20 22:55 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe
Move the io_async_msghdr out of the issue path and into prep handling,
since it's now done unconditionally and hence does not need to be part
of the issue path. This reduces the footprint of the multishot fast
path of multiple invocations of ->issue() per prep, and also means that
we can drop using ->prep_async() for recvmsg as we now do this setup on
the prep side.
Signed-off-by: Jens Axboe <[email protected]>
---
io_uring/net.c | 71 +++++++++++++++++++-----------------------------
io_uring/net.h | 1 -
io_uring/opdef.c | 2 --
3 files changed, 28 insertions(+), 46 deletions(-)
diff --git a/io_uring/net.c b/io_uring/net.c
index 14491fab6d59..e438b1ac2420 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -596,17 +596,36 @@ static int io_recvmsg_copy_hdr(struct io_kiocb *req,
msg.msg_controllen);
}
-int io_recvmsg_prep_async(struct io_kiocb *req)
+static int io_recvmsg_prep_setup(struct io_kiocb *req)
{
struct io_sr_msg *sr = io_kiocb_to_cmd(req, struct io_sr_msg);
- struct io_async_msghdr *iomsg;
+ struct io_async_msghdr *kmsg;
int ret;
- sr->done_io = 0;
- if (!io_msg_alloc_async_prep(req))
+ /* always locked for prep */
+ kmsg = io_msg_alloc_async(req, 0);
+ if (unlikely(!kmsg))
return -ENOMEM;
- iomsg = req->async_data;
- ret = io_recvmsg_copy_hdr(req, iomsg);
+
+ if (req->opcode == IORING_OP_RECV) {
+ kmsg->msg.msg_name = NULL;
+ kmsg->msg.msg_namelen = 0;
+ kmsg->msg.msg_control = NULL;
+ kmsg->msg.msg_get_inq = 1;
+ kmsg->msg.msg_controllen = 0;
+ kmsg->msg.msg_iocb = NULL;
+ kmsg->msg.msg_ubuf = NULL;
+
+ if (!io_do_buffer_select(req)) {
+ ret = import_ubuf(ITER_DEST, sr->buf, sr->len,
+ &kmsg->msg.msg_iter);
+ if (unlikely(ret))
+ return ret;
+ }
+ return 0;
+ }
+
+ ret = io_recvmsg_copy_hdr(req, kmsg);
if (!ret)
req->flags |= REQ_F_NEED_CLEANUP;
return ret;
@@ -657,7 +676,7 @@ int io_recvmsg_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
sr->msg_flags |= MSG_CMSG_COMPAT;
#endif
sr->nr_multishot_loops = 0;
- return 0;
+ return io_recvmsg_prep_setup(req);
}
static inline void io_recv_prep_retry(struct io_kiocb *req,
@@ -815,7 +834,7 @@ static int io_recvmsg_multishot(struct socket *sock, struct io_sr_msg *io,
int io_recvmsg(struct io_kiocb *req, unsigned int issue_flags)
{
struct io_sr_msg *sr = io_kiocb_to_cmd(req, struct io_sr_msg);
- struct io_async_msghdr *kmsg;
+ struct io_async_msghdr *kmsg = req->async_data;
struct socket *sock;
unsigned flags;
int ret, min_ret = 0;
@@ -826,17 +845,6 @@ int io_recvmsg(struct io_kiocb *req, unsigned int issue_flags)
if (unlikely(!sock))
return -ENOTSOCK;
- if (req_has_async_data(req)) {
- kmsg = req->async_data;
- } else {
- kmsg = io_msg_alloc_async(req, issue_flags);
- if (unlikely(!kmsg))
- return -ENOMEM;
- ret = io_recvmsg_copy_hdr(req, kmsg);
- if (ret)
- return ret;
- }
-
if (!(req->flags & REQ_F_POLLED) &&
(sr->flags & IORING_RECVSEND_POLL_FIRST))
return -EAGAIN;
@@ -915,36 +923,13 @@ int io_recvmsg(struct io_kiocb *req, unsigned int issue_flags)
int io_recv(struct io_kiocb *req, unsigned int issue_flags)
{
struct io_sr_msg *sr = io_kiocb_to_cmd(req, struct io_sr_msg);
- struct io_async_msghdr *kmsg;
+ struct io_async_msghdr *kmsg = req->async_data;
struct socket *sock;
unsigned flags;
int ret, min_ret = 0;
bool force_nonblock = issue_flags & IO_URING_F_NONBLOCK;
size_t len = sr->len;
- if (req_has_async_data(req)) {
- kmsg = req->async_data;
- } else {
- kmsg = io_msg_alloc_async(req, issue_flags);
- if (unlikely(!kmsg))
- return -ENOMEM;
- kmsg->free_iov = NULL;
- kmsg->msg.msg_name = NULL;
- kmsg->msg.msg_namelen = 0;
- kmsg->msg.msg_control = NULL;
- kmsg->msg.msg_get_inq = 1;
- kmsg->msg.msg_controllen = 0;
- kmsg->msg.msg_iocb = NULL;
- kmsg->msg.msg_ubuf = NULL;
-
- if (!io_do_buffer_select(req)) {
- ret = import_ubuf(ITER_DEST, sr->buf, sr->len,
- &kmsg->msg.msg_iter);
- if (unlikely(ret))
- return ret;
- }
- }
-
if (!(req->flags & REQ_F_POLLED) &&
(sr->flags & IORING_RECVSEND_POLL_FIRST))
return -EAGAIN;
diff --git a/io_uring/net.h b/io_uring/net.h
index 5c1230f1aaf9..4b4fd9b1b7b4 100644
--- a/io_uring/net.h
+++ b/io_uring/net.h
@@ -42,7 +42,6 @@ int io_sendmsg(struct io_kiocb *req, unsigned int issue_flags);
int io_send(struct io_kiocb *req, unsigned int issue_flags);
int io_sendrecv_prep_async(struct io_kiocb *req);
-int io_recvmsg_prep_async(struct io_kiocb *req);
int io_recvmsg_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
int io_recvmsg(struct io_kiocb *req, unsigned int issue_flags);
int io_recv(struct io_kiocb *req, unsigned int issue_flags);
diff --git a/io_uring/opdef.c b/io_uring/opdef.c
index 77131826d603..1368193edc57 100644
--- a/io_uring/opdef.c
+++ b/io_uring/opdef.c
@@ -536,7 +536,6 @@ const struct io_cold_def io_cold_defs[] = {
.name = "RECVMSG",
#if defined(CONFIG_NET)
.async_size = sizeof(struct io_async_msghdr),
- .prep_async = io_recvmsg_prep_async,
.cleanup = io_sendmsg_recvmsg_cleanup,
.fail = io_sendrecv_fail,
#endif
@@ -613,7 +612,6 @@ const struct io_cold_def io_cold_defs[] = {
.async_size = sizeof(struct io_async_msghdr),
.cleanup = io_sendmsg_recvmsg_cleanup,
.fail = io_sendrecv_fail,
- .prep_async = io_sendrecv_prep_async,
#endif
},
[IORING_OP_OPENAT2] = {
--
2.43.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH 06/17] io_uring/net: get rid of ->prep_async() for send side
2024-03-20 22:55 [PATCHSET v2 0/17] Improve async state handling Jens Axboe
` (4 preceding siblings ...)
2024-03-20 22:55 ` [PATCH 05/17] io_uring/net: get rid of ->prep_async() for receive side Jens Axboe
@ 2024-03-20 22:55 ` Jens Axboe
2024-03-20 22:55 ` [PATCH 07/17] io_uring: kill io_msg_alloc_async_prep() Jens Axboe
` (10 subsequent siblings)
16 siblings, 0 replies; 30+ messages in thread
From: Jens Axboe @ 2024-03-20 22:55 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe
Move the io_async_msghdr out of the issue path and into prep handling,
e it's now done unconditionally and hence does not need to be part
of the issue path. This means we can drop any usage of
io_sendrecv_prep_async() and io_sendmsg_prep_async(), and hence the
forced async setup path is now unified with the normal prep setup.
Signed-off-by: Jens Axboe <[email protected]>
---
io_uring/net.c | 162 +++++++++++++++--------------------------------
io_uring/net.h | 2 -
io_uring/opdef.c | 4 --
3 files changed, 50 insertions(+), 118 deletions(-)
diff --git a/io_uring/net.c b/io_uring/net.c
index e438b1ac2420..dc6cda076a93 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -290,50 +290,59 @@ static int io_sendmsg_copy_hdr(struct io_kiocb *req,
return ret;
}
-int io_sendrecv_prep_async(struct io_kiocb *req)
+void io_sendmsg_recvmsg_cleanup(struct io_kiocb *req)
+{
+ struct io_async_msghdr *io = req->async_data;
+
+ kfree(io->free_iov);
+}
+
+static int io_send_setup(struct io_kiocb *req)
{
struct io_sr_msg *sr = io_kiocb_to_cmd(req, struct io_sr_msg);
- struct io_async_msghdr *io;
+ struct io_async_msghdr *kmsg = req->async_data;
int ret;
- if (req_has_async_data(req))
- return 0;
- sr->done_io = 0;
- if (!sr->addr)
- return 0;
- io = io_msg_alloc_async_prep(req);
- if (!io)
- return -ENOMEM;
- memset(&io->msg, 0, sizeof(io->msg));
- ret = import_ubuf(ITER_SOURCE, sr->buf, sr->len, &io->msg.msg_iter);
- if (unlikely(ret))
- return ret;
- io->msg.msg_name = &io->addr;
- io->msg.msg_namelen = sr->addr_len;
- return move_addr_to_kernel(sr->addr, sr->addr_len, &io->addr);
+ kmsg->msg.msg_name = NULL;
+ kmsg->msg.msg_namelen = 0;
+ kmsg->msg.msg_control = NULL;
+ kmsg->msg.msg_controllen = 0;
+ kmsg->msg.msg_ubuf = NULL;
+
+ if (sr->addr) {
+ ret = move_addr_to_kernel(sr->addr, sr->addr_len, &kmsg->addr);
+ if (unlikely(ret < 0))
+ return ret;
+ kmsg->msg.msg_name = &kmsg->addr;
+ kmsg->msg.msg_namelen = sr->addr_len;
+ }
+ if (!io_do_buffer_select(req)) {
+ ret = import_ubuf(ITER_SOURCE, sr->buf, sr->len,
+ &kmsg->msg.msg_iter);
+ if (unlikely(ret < 0))
+ return ret;
+ }
+
+ return 0;
}
-int io_sendmsg_prep_async(struct io_kiocb *req)
+static int io_sendmsg_prep_setup(struct io_kiocb *req, int is_msg)
{
- struct io_sr_msg *sr = io_kiocb_to_cmd(req, struct io_sr_msg);
+ struct io_async_msghdr *kmsg;
int ret;
- sr->done_io = 0;
- if (!io_msg_alloc_async_prep(req))
+ /* always locked for prep */
+ kmsg = io_msg_alloc_async(req, 0);
+ if (unlikely(!kmsg))
return -ENOMEM;
- ret = io_sendmsg_copy_hdr(req, req->async_data);
+ if (!is_msg)
+ return io_send_setup(req);
+ ret = io_sendmsg_copy_hdr(req, kmsg);
if (!ret)
req->flags |= REQ_F_NEED_CLEANUP;
return ret;
}
-void io_sendmsg_recvmsg_cleanup(struct io_kiocb *req)
-{
- struct io_async_msghdr *io = req->async_data;
-
- kfree(io->free_iov);
-}
-
int io_sendmsg_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
{
struct io_sr_msg *sr = io_kiocb_to_cmd(req, struct io_sr_msg);
@@ -362,7 +371,7 @@ int io_sendmsg_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
if (req->ctx->compat)
sr->msg_flags |= MSG_CMSG_COMPAT;
#endif
- return 0;
+ return io_sendmsg_prep_setup(req, req->opcode == IORING_OP_SENDMSG);
}
static void io_req_msg_cleanup(struct io_kiocb *req,
@@ -379,7 +388,7 @@ static void io_req_msg_cleanup(struct io_kiocb *req,
int io_sendmsg(struct io_kiocb *req, unsigned int issue_flags)
{
struct io_sr_msg *sr = io_kiocb_to_cmd(req, struct io_sr_msg);
- struct io_async_msghdr *kmsg;
+ struct io_async_msghdr *kmsg = req->async_data;
struct socket *sock;
unsigned flags;
int min_ret = 0;
@@ -389,18 +398,6 @@ int io_sendmsg(struct io_kiocb *req, unsigned int issue_flags)
if (unlikely(!sock))
return -ENOTSOCK;
- if (req_has_async_data(req)) {
- kmsg = req->async_data;
- kmsg->msg.msg_control_user = sr->msg_control;
- } else {
- kmsg = io_msg_alloc_async(req, issue_flags);
- if (unlikely(!kmsg))
- return -ENOMEM;
- ret = io_sendmsg_copy_hdr(req, kmsg);
- if (ret)
- return ret;
- }
-
if (!(req->flags & REQ_F_POLLED) &&
(sr->flags & IORING_RECVSEND_POLL_FIRST))
return -EAGAIN;
@@ -436,54 +433,10 @@ int io_sendmsg(struct io_kiocb *req, unsigned int issue_flags)
return IOU_OK;
}
-static struct io_async_msghdr *io_send_setup(struct io_kiocb *req,
- unsigned int issue_flags)
-{
- struct io_sr_msg *sr = io_kiocb_to_cmd(req, struct io_sr_msg);
- struct io_async_msghdr *kmsg;
- int ret;
-
- if (req_has_async_data(req)) {
- kmsg = req->async_data;
- } else {
- kmsg = io_msg_alloc_async(req, issue_flags);
- if (unlikely(!kmsg))
- return ERR_PTR(-ENOMEM);
- kmsg->msg.msg_name = NULL;
- kmsg->msg.msg_namelen = 0;
- kmsg->msg.msg_control = NULL;
- kmsg->msg.msg_controllen = 0;
- kmsg->msg.msg_ubuf = NULL;
-
- if (sr->addr) {
- ret = move_addr_to_kernel(sr->addr, sr->addr_len,
- &kmsg->addr);
- if (unlikely(ret < 0))
- return ERR_PTR(ret);
- kmsg->msg.msg_name = &kmsg->addr;
- kmsg->msg.msg_namelen = sr->addr_len;
- }
-
- if (!io_do_buffer_select(req)) {
- ret = import_ubuf(ITER_SOURCE, sr->buf, sr->len,
- &kmsg->msg.msg_iter);
- if (unlikely(ret))
- return ERR_PTR(ret);
- }
- }
-
- if (!(req->flags & REQ_F_POLLED) &&
- (sr->flags & IORING_RECVSEND_POLL_FIRST))
- return ERR_PTR(-EAGAIN);
-
- return kmsg;
-}
-
int io_send(struct io_kiocb *req, unsigned int issue_flags)
{
struct io_sr_msg *sr = io_kiocb_to_cmd(req, struct io_sr_msg);
- struct io_async_msghdr *kmsg;
- size_t len = sr->len;
+ struct io_async_msghdr *kmsg = req->async_data;
struct socket *sock;
unsigned flags;
int min_ret = 0;
@@ -493,13 +446,9 @@ int io_send(struct io_kiocb *req, unsigned int issue_flags)
if (unlikely(!sock))
return -ENOTSOCK;
- kmsg = io_send_setup(req, issue_flags);
- if (IS_ERR(kmsg))
- return PTR_ERR(kmsg);
-
- ret = import_ubuf(ITER_SOURCE, sr->buf, len, &kmsg->msg.msg_iter);
- if (unlikely(ret))
- return ret;
+ if (!(req->flags & REQ_F_POLLED) &&
+ (sr->flags & IORING_RECVSEND_POLL_FIRST))
+ return -EAGAIN;
flags = sr->msg_flags;
if (issue_flags & IO_URING_F_NONBLOCK)
@@ -1085,7 +1034,7 @@ int io_send_zc_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
if (req->ctx->compat)
zc->msg_flags |= MSG_CMSG_COMPAT;
#endif
- return 0;
+ return io_sendmsg_prep_setup(req, req->opcode == IORING_OP_SENDMSG_ZC);
}
static int io_sg_from_iter_iovec(struct sock *sk, struct sk_buff *skb,
@@ -1174,7 +1123,7 @@ static int io_send_zc_import(struct io_kiocb *req, struct io_async_msghdr *kmsg)
int io_send_zc(struct io_kiocb *req, unsigned int issue_flags)
{
struct io_sr_msg *zc = io_kiocb_to_cmd(req, struct io_sr_msg);
- struct io_async_msghdr *kmsg;
+ struct io_async_msghdr *kmsg = req->async_data;
struct socket *sock;
unsigned msg_flags;
int ret, min_ret = 0;
@@ -1185,9 +1134,9 @@ int io_send_zc(struct io_kiocb *req, unsigned int issue_flags)
if (!test_bit(SOCK_SUPPORT_ZC, &sock->flags))
return -EOPNOTSUPP;
- kmsg = io_send_setup(req, issue_flags);
- if (IS_ERR(kmsg))
- return PTR_ERR(kmsg);
+ if (!(req->flags & REQ_F_POLLED) &&
+ (zc->flags & IORING_RECVSEND_POLL_FIRST))
+ return -EAGAIN;
if (!zc->done_io) {
ret = io_send_zc_import(req, kmsg);
@@ -1243,7 +1192,7 @@ int io_send_zc(struct io_kiocb *req, unsigned int issue_flags)
int io_sendmsg_zc(struct io_kiocb *req, unsigned int issue_flags)
{
struct io_sr_msg *sr = io_kiocb_to_cmd(req, struct io_sr_msg);
- struct io_async_msghdr *kmsg;
+ struct io_async_msghdr *kmsg = req->async_data;
struct socket *sock;
unsigned flags;
int ret, min_ret = 0;
@@ -1256,17 +1205,6 @@ int io_sendmsg_zc(struct io_kiocb *req, unsigned int issue_flags)
if (!test_bit(SOCK_SUPPORT_ZC, &sock->flags))
return -EOPNOTSUPP;
- if (req_has_async_data(req)) {
- kmsg = req->async_data;
- } else {
- kmsg = io_msg_alloc_async(req, issue_flags);
- if (unlikely(!kmsg))
- return -ENOMEM;
- ret = io_sendmsg_copy_hdr(req, kmsg);
- if (ret)
- return ret;
- }
-
if (!(req->flags & REQ_F_POLLED) &&
(sr->flags & IORING_RECVSEND_POLL_FIRST))
return -EAGAIN;
diff --git a/io_uring/net.h b/io_uring/net.h
index 4b4fd9b1b7b4..f99ebb9dc0bb 100644
--- a/io_uring/net.h
+++ b/io_uring/net.h
@@ -34,13 +34,11 @@ struct io_async_connect {
int io_shutdown_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
int io_shutdown(struct io_kiocb *req, unsigned int issue_flags);
-int io_sendmsg_prep_async(struct io_kiocb *req);
void io_sendmsg_recvmsg_cleanup(struct io_kiocb *req);
int io_sendmsg_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
int io_sendmsg(struct io_kiocb *req, unsigned int issue_flags);
int io_send(struct io_kiocb *req, unsigned int issue_flags);
-int io_sendrecv_prep_async(struct io_kiocb *req);
int io_recvmsg_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
int io_recvmsg(struct io_kiocb *req, unsigned int issue_flags);
diff --git a/io_uring/opdef.c b/io_uring/opdef.c
index 1368193edc57..dd4a1e1425e1 100644
--- a/io_uring/opdef.c
+++ b/io_uring/opdef.c
@@ -527,7 +527,6 @@ const struct io_cold_def io_cold_defs[] = {
.name = "SENDMSG",
#if defined(CONFIG_NET)
.async_size = sizeof(struct io_async_msghdr),
- .prep_async = io_sendmsg_prep_async,
.cleanup = io_sendmsg_recvmsg_cleanup,
.fail = io_sendrecv_fail,
#endif
@@ -603,7 +602,6 @@ const struct io_cold_def io_cold_defs[] = {
.async_size = sizeof(struct io_async_msghdr),
.cleanup = io_sendmsg_recvmsg_cleanup,
.fail = io_sendrecv_fail,
- .prep_async = io_sendrecv_prep_async,
#endif
},
[IORING_OP_RECV] = {
@@ -688,7 +686,6 @@ const struct io_cold_def io_cold_defs[] = {
.name = "SEND_ZC",
#if defined(CONFIG_NET)
.async_size = sizeof(struct io_async_msghdr),
- .prep_async = io_sendrecv_prep_async,
.cleanup = io_send_zc_cleanup,
.fail = io_sendrecv_fail,
#endif
@@ -697,7 +694,6 @@ const struct io_cold_def io_cold_defs[] = {
.name = "SENDMSG_ZC",
#if defined(CONFIG_NET)
.async_size = sizeof(struct io_async_msghdr),
- .prep_async = io_sendmsg_prep_async,
.cleanup = io_send_zc_cleanup,
.fail = io_sendrecv_fail,
#endif
--
2.43.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH 07/17] io_uring: kill io_msg_alloc_async_prep()
2024-03-20 22:55 [PATCHSET v2 0/17] Improve async state handling Jens Axboe
` (5 preceding siblings ...)
2024-03-20 22:55 ` [PATCH 06/17] io_uring/net: get rid of ->prep_async() for send side Jens Axboe
@ 2024-03-20 22:55 ` Jens Axboe
2024-03-20 22:55 ` [PATCH 08/17] io_uring/net: add iovec recycling Jens Axboe
` (9 subsequent siblings)
16 siblings, 0 replies; 30+ messages in thread
From: Jens Axboe @ 2024-03-20 22:55 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe
We now ONLY call io_msg_alloc_async() from inside prep handling, which
is always locked. No need for this helper anymore, or the check in
io_msg_alloc_async() on whether the ring is locked or not.
Signed-off-by: Jens Axboe <[email protected]>
---
io_uring/net.c | 31 ++++++++++---------------------
1 file changed, 10 insertions(+), 21 deletions(-)
diff --git a/io_uring/net.c b/io_uring/net.c
index dc6cda076a93..6b45311dcc08 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -129,22 +129,19 @@ static void io_netmsg_recycle(struct io_kiocb *req, unsigned int issue_flags)
}
}
-static struct io_async_msghdr *io_msg_alloc_async(struct io_kiocb *req,
- unsigned int issue_flags)
+static struct io_async_msghdr *io_msg_alloc_async(struct io_kiocb *req)
{
struct io_ring_ctx *ctx = req->ctx;
struct io_cache_entry *entry;
struct io_async_msghdr *hdr;
- if (!(issue_flags & IO_URING_F_UNLOCKED)) {
- entry = io_alloc_cache_get(&ctx->netmsg_cache);
- if (entry) {
- hdr = container_of(entry, struct io_async_msghdr, cache);
- hdr->free_iov = NULL;
- req->flags |= REQ_F_ASYNC_DATA;
- req->async_data = hdr;
- return hdr;
- }
+ entry = io_alloc_cache_get(&ctx->netmsg_cache);
+ if (entry) {
+ hdr = container_of(entry, struct io_async_msghdr, cache);
+ hdr->free_iov = NULL;
+ req->flags |= REQ_F_ASYNC_DATA;
+ req->async_data = hdr;
+ return hdr;
}
if (!io_alloc_async_data(req)) {
@@ -155,12 +152,6 @@ static struct io_async_msghdr *io_msg_alloc_async(struct io_kiocb *req,
return NULL;
}
-static inline struct io_async_msghdr *io_msg_alloc_async_prep(struct io_kiocb *req)
-{
- /* ->prep_async is always called from the submission context */
- return io_msg_alloc_async(req, 0);
-}
-
#ifdef CONFIG_COMPAT
static int io_compat_msg_copy_hdr(struct io_kiocb *req,
struct io_async_msghdr *iomsg,
@@ -331,8 +322,7 @@ static int io_sendmsg_prep_setup(struct io_kiocb *req, int is_msg)
struct io_async_msghdr *kmsg;
int ret;
- /* always locked for prep */
- kmsg = io_msg_alloc_async(req, 0);
+ kmsg = io_msg_alloc_async(req);
if (unlikely(!kmsg))
return -ENOMEM;
if (!is_msg)
@@ -551,8 +541,7 @@ static int io_recvmsg_prep_setup(struct io_kiocb *req)
struct io_async_msghdr *kmsg;
int ret;
- /* always locked for prep */
- kmsg = io_msg_alloc_async(req, 0);
+ kmsg = io_msg_alloc_async(req);
if (unlikely(!kmsg))
return -ENOMEM;
--
2.43.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH 08/17] io_uring/net: add iovec recycling
2024-03-20 22:55 [PATCHSET v2 0/17] Improve async state handling Jens Axboe
` (6 preceding siblings ...)
2024-03-20 22:55 ` [PATCH 07/17] io_uring: kill io_msg_alloc_async_prep() Jens Axboe
@ 2024-03-20 22:55 ` Jens Axboe
2024-03-20 22:55 ` [PATCH 09/17] io_uring/net: drop 'kmsg' parameter from io_req_msg_cleanup() Jens Axboe
` (8 subsequent siblings)
16 siblings, 0 replies; 30+ messages in thread
From: Jens Axboe @ 2024-03-20 22:55 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe
Right now the io_async_msghdr is recycled to avoid the overhead of
allocating+freeing it for every request. But the iovec is not included,
hence that will be allocated and freed for each transfer regardless.
This commit enables recyling of the iovec between io_async_msghdr
recycles. This avoids alloc+free for each one if we use it, and on top
of that, it extends the cache hot nature of msg to the iovec as well.
Also enables KASAN for the iovec entries, so that reuse can be detected
even while they are in the cache.
The io_async_msghdr also shrinks from 376 -> 288 bytes, an 88 byte
saving (or ~23% smaller), as the fast_iovec entry is dropped from 8
entries to a single entry. There's no point keeping a big fast iovec
entry, if we're not allocating and freeing new iovecs all the time.
Signed-off-by: Jens Axboe <[email protected]>
---
io_uring/net.c | 133 ++++++++++++++++++++++++++++++++-----------------
io_uring/net.h | 13 ++---
2 files changed, 93 insertions(+), 53 deletions(-)
diff --git a/io_uring/net.c b/io_uring/net.c
index 6b45311dcc08..20d6427f4250 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -115,15 +115,33 @@ static bool io_net_retry(struct socket *sock, int flags)
return sock->type == SOCK_STREAM || sock->type == SOCK_SEQPACKET;
}
+static void io_netmsg_iovec_free(struct io_async_msghdr *kmsg)
+{
+ if (kmsg->free_iov) {
+ kfree(kmsg->free_iov);
+ kmsg->free_iov_nr = 0;
+ kmsg->free_iov = NULL;
+ }
+}
+
static void io_netmsg_recycle(struct io_kiocb *req, unsigned int issue_flags)
{
struct io_async_msghdr *hdr = req->async_data;
+ struct iovec *iov;
- if (!req_has_async_data(req) || issue_flags & IO_URING_F_UNLOCKED)
+ if (!req_has_async_data(req))
+ return;
+ /* can't recycle, ensure we free the iovec if we have one */
+ if (issue_flags & IO_URING_F_UNLOCKED) {
+ io_netmsg_iovec_free(hdr);
return;
+ }
/* Let normal cleanup path reap it if we fail adding to the cache */
+ iov = hdr->free_iov;
if (io_alloc_cache_put(&req->ctx->netmsg_cache, &hdr->cache)) {
+ if (iov)
+ kasan_mempool_poison_object(iov);
req->async_data = NULL;
req->flags &= ~REQ_F_ASYNC_DATA;
}
@@ -138,7 +156,11 @@ static struct io_async_msghdr *io_msg_alloc_async(struct io_kiocb *req)
entry = io_alloc_cache_get(&ctx->netmsg_cache);
if (entry) {
hdr = container_of(entry, struct io_async_msghdr, cache);
- hdr->free_iov = NULL;
+ if (hdr->free_iov) {
+ kasan_mempool_unpoison_object(hdr->free_iov,
+ hdr->free_iov_nr * sizeof(struct iovec));
+ req->flags |= REQ_F_NEED_CLEANUP;
+ }
req->flags |= REQ_F_ASYNC_DATA;
req->async_data = hdr;
return hdr;
@@ -146,12 +168,27 @@ static struct io_async_msghdr *io_msg_alloc_async(struct io_kiocb *req)
if (!io_alloc_async_data(req)) {
hdr = req->async_data;
+ hdr->free_iov_nr = 0;
hdr->free_iov = NULL;
return hdr;
}
return NULL;
}
+/* assign new iovec to kmsg, if we need to */
+static int io_net_vec_assign(struct io_kiocb *req, struct io_async_msghdr *kmsg,
+ struct iovec *iov)
+{
+ if (iov) {
+ req->flags |= REQ_F_NEED_CLEANUP;
+ kmsg->free_iov_nr = kmsg->msg.msg_iter.nr_segs;
+ if (kmsg->free_iov)
+ kfree(kmsg->free_iov);
+ kmsg->free_iov = iov;
+ }
+ return 0;
+}
+
#ifdef CONFIG_COMPAT
static int io_compat_msg_copy_hdr(struct io_kiocb *req,
struct io_async_msghdr *iomsg,
@@ -159,7 +196,16 @@ static int io_compat_msg_copy_hdr(struct io_kiocb *req,
{
struct io_sr_msg *sr = io_kiocb_to_cmd(req, struct io_sr_msg);
struct compat_iovec __user *uiov;
- int ret;
+ struct iovec *iov;
+ int ret, nr_segs;
+
+ if (iomsg->free_iov) {
+ nr_segs = iomsg->free_iov_nr;
+ iov = iomsg->free_iov;
+ } else {
+ iov = &iomsg->fast_iov;
+ nr_segs = 1;
+ }
if (copy_from_user(msg, sr->umsg_compat, sizeof(*msg)))
return -EFAULT;
@@ -168,9 +214,9 @@ static int io_compat_msg_copy_hdr(struct io_kiocb *req,
if (req->flags & REQ_F_BUFFER_SELECT) {
compat_ssize_t clen;
- iomsg->free_iov = NULL;
if (msg->msg_iovlen == 0) {
- sr->len = 0;
+ sr->len = iov->iov_len = 0;
+ iov->iov_base = NULL;
} else if (msg->msg_iovlen > 1) {
return -EINVAL;
} else {
@@ -186,14 +232,12 @@ static int io_compat_msg_copy_hdr(struct io_kiocb *req,
return 0;
}
- iomsg->free_iov = iomsg->fast_iov;
ret = __import_iovec(ddir, (struct iovec __user *)uiov, msg->msg_iovlen,
- UIO_FASTIOV, &iomsg->free_iov,
- &iomsg->msg.msg_iter, true);
+ nr_segs, &iov, &iomsg->msg.msg_iter, true);
if (unlikely(ret < 0))
return ret;
- return 0;
+ return io_net_vec_assign(req, iomsg, iov);
}
#endif
@@ -201,7 +245,16 @@ static int io_msg_copy_hdr(struct io_kiocb *req, struct io_async_msghdr *iomsg,
struct user_msghdr *msg, int ddir)
{
struct io_sr_msg *sr = io_kiocb_to_cmd(req, struct io_sr_msg);
- int ret;
+ struct iovec *iov;
+ int ret, nr_segs;
+
+ if (iomsg->free_iov) {
+ nr_segs = iomsg->free_iov_nr;
+ iov = iomsg->free_iov;
+ } else {
+ iov = &iomsg->fast_iov;
+ nr_segs = 1;
+ }
if (!user_access_begin(sr->umsg, sizeof(*sr->umsg)))
return -EFAULT;
@@ -217,9 +270,8 @@ static int io_msg_copy_hdr(struct io_kiocb *req, struct io_async_msghdr *iomsg,
if (req->flags & REQ_F_BUFFER_SELECT) {
if (msg->msg_iovlen == 0) {
- sr->len = iomsg->fast_iov[0].iov_len = 0;
- iomsg->fast_iov[0].iov_base = NULL;
- iomsg->free_iov = NULL;
+ sr->len = iov->iov_len = 0;
+ iov->iov_base = NULL;
} else if (msg->msg_iovlen > 1) {
ret = -EINVAL;
goto ua_end;
@@ -227,10 +279,9 @@ static int io_msg_copy_hdr(struct io_kiocb *req, struct io_async_msghdr *iomsg,
/* we only need the length for provided buffers */
if (!access_ok(&msg->msg_iov[0].iov_len, sizeof(__kernel_size_t)))
goto ua_end;
- unsafe_get_user(iomsg->fast_iov[0].iov_len,
- &msg->msg_iov[0].iov_len, ua_end);
- sr->len = iomsg->fast_iov[0].iov_len;
- iomsg->free_iov = NULL;
+ unsafe_get_user(iov->iov_len, &msg->msg_iov[0].iov_len,
+ ua_end);
+ sr->len = iov->iov_len;
}
ret = 0;
ua_end:
@@ -239,13 +290,12 @@ static int io_msg_copy_hdr(struct io_kiocb *req, struct io_async_msghdr *iomsg,
}
user_access_end();
- iomsg->free_iov = iomsg->fast_iov;
- ret = __import_iovec(ddir, msg->msg_iov, msg->msg_iovlen, UIO_FASTIOV,
- &iomsg->free_iov, &iomsg->msg.msg_iter, false);
+ ret = __import_iovec(ddir, msg->msg_iov, msg->msg_iovlen, nr_segs,
+ &iov, &iomsg->msg.msg_iter, false);
if (unlikely(ret < 0))
return ret;
- return 0;
+ return io_net_vec_assign(req, iomsg, iov);
}
static int io_sendmsg_copy_hdr(struct io_kiocb *req,
@@ -285,7 +335,7 @@ void io_sendmsg_recvmsg_cleanup(struct io_kiocb *req)
{
struct io_async_msghdr *io = req->async_data;
- kfree(io->free_iov);
+ io_netmsg_iovec_free(io);
}
static int io_send_setup(struct io_kiocb *req)
@@ -369,9 +419,6 @@ static void io_req_msg_cleanup(struct io_kiocb *req,
unsigned int issue_flags)
{
req->flags &= ~REQ_F_NEED_CLEANUP;
- /* fast path, check for non-NULL to avoid function call */
- if (kmsg->free_iov)
- kfree(kmsg->free_iov);
io_netmsg_recycle(req, issue_flags);
}
@@ -622,11 +669,6 @@ static inline void io_recv_prep_retry(struct io_kiocb *req,
{
struct io_sr_msg *sr = io_kiocb_to_cmd(req, struct io_sr_msg);
- if (kmsg->free_iov) {
- kfree(kmsg->free_iov);
- kmsg->free_iov = NULL;
- }
-
req->flags &= ~REQ_F_BL_EMPTY;
sr->done_io = 0;
sr->len = 0; /* get from the provided buffer */
@@ -942,14 +984,10 @@ int io_recv(struct io_kiocb *req, unsigned int issue_flags)
void io_send_zc_cleanup(struct io_kiocb *req)
{
struct io_sr_msg *zc = io_kiocb_to_cmd(req, struct io_sr_msg);
- struct io_async_msghdr *io;
+ struct io_async_msghdr *io = req->async_data;
- if (req_has_async_data(req)) {
- io = req->async_data;
- /* might be ->fast_iov if *msg_copy_hdr failed */
- if (io->free_iov != io->fast_iov)
- kfree(io->free_iov);
- }
+ if (req_has_async_data(req))
+ io_netmsg_iovec_free(io);
if (zc->notif) {
io_notif_flush(zc->notif);
zc->notif = NULL;
@@ -1171,8 +1209,7 @@ int io_send_zc(struct io_kiocb *req, unsigned int issue_flags)
*/
if (!(issue_flags & IO_URING_F_UNLOCKED)) {
io_notif_flush(zc->notif);
- io_netmsg_recycle(req, issue_flags);
- req->flags &= ~REQ_F_NEED_CLEANUP;
+ io_req_msg_cleanup(req, kmsg, 0);
}
io_req_set_res(req, ret, IORING_CQE_F_MORE);
return IOU_OK;
@@ -1221,13 +1258,7 @@ int io_sendmsg_zc(struct io_kiocb *req, unsigned int issue_flags)
ret = -EINTR;
req_set_fail(req);
}
- /* fast path, check for non-NULL to avoid function call */
- if (kmsg->free_iov) {
- kfree(kmsg->free_iov);
- kmsg->free_iov = NULL;
- }
- io_netmsg_recycle(req, issue_flags);
if (ret >= 0)
ret += sr->done_io;
else if (sr->done_io)
@@ -1239,7 +1270,7 @@ int io_sendmsg_zc(struct io_kiocb *req, unsigned int issue_flags)
*/
if (!(issue_flags & IO_URING_F_UNLOCKED)) {
io_notif_flush(sr->notif);
- req->flags &= ~REQ_F_NEED_CLEANUP;
+ io_req_msg_cleanup(req, kmsg, 0);
}
io_req_set_res(req, ret, IORING_CQE_F_MORE);
return IOU_OK;
@@ -1483,6 +1514,14 @@ int io_connect(struct io_kiocb *req, unsigned int issue_flags)
void io_netmsg_cache_free(struct io_cache_entry *entry)
{
- kfree(container_of(entry, struct io_async_msghdr, cache));
+ struct io_async_msghdr *kmsg;
+
+ kmsg = container_of(entry, struct io_async_msghdr, cache);
+ if (kmsg->free_iov) {
+ kasan_mempool_unpoison_object(kmsg->free_iov,
+ kmsg->free_iov_nr * sizeof(struct iovec));
+ io_netmsg_iovec_free(kmsg);
+ }
+ kfree(kmsg);
}
#endif
diff --git a/io_uring/net.h b/io_uring/net.h
index f99ebb9dc0bb..0aef1c992aee 100644
--- a/io_uring/net.h
+++ b/io_uring/net.h
@@ -8,17 +8,18 @@
struct io_async_msghdr {
#if defined(CONFIG_NET)
union {
- struct iovec fast_iov[UIO_FASTIOV];
+ struct iovec fast_iov;
struct {
- struct iovec fast_iov_one;
- __kernel_size_t controllen;
- int namelen;
- __kernel_size_t payloadlen;
+ struct io_cache_entry cache;
+ /* entry size of ->free_iov, if valid */
+ int free_iov_nr;
};
- struct io_cache_entry cache;
};
/* points to an allocated iov, if NULL we use fast_iov instead */
struct iovec *free_iov;
+ __kernel_size_t controllen;
+ __kernel_size_t payloadlen;
+ int namelen;
struct sockaddr __user *uaddr;
struct msghdr msg;
struct sockaddr_storage addr;
--
2.43.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH 09/17] io_uring/net: drop 'kmsg' parameter from io_req_msg_cleanup()
2024-03-20 22:55 [PATCHSET v2 0/17] Improve async state handling Jens Axboe
` (7 preceding siblings ...)
2024-03-20 22:55 ` [PATCH 08/17] io_uring/net: add iovec recycling Jens Axboe
@ 2024-03-20 22:55 ` Jens Axboe
2024-03-20 22:55 ` [PATCH 10/17] io_uring/rw: always setup io_async_rw for read/write requests Jens Axboe
` (7 subsequent siblings)
16 siblings, 0 replies; 30+ messages in thread
From: Jens Axboe @ 2024-03-20 22:55 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe
Now that iovec recycling is being done, the iovec is no longer being
freed in there. Hence the kmsg parameter is now useless.
Signed-off-by: Jens Axboe <[email protected]>
---
io_uring/net.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/io_uring/net.c b/io_uring/net.c
index 20d6427f4250..9472a66e035c 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -415,7 +415,6 @@ int io_sendmsg_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
}
static void io_req_msg_cleanup(struct io_kiocb *req,
- struct io_async_msghdr *kmsg,
unsigned int issue_flags)
{
req->flags &= ~REQ_F_NEED_CLEANUP;
@@ -461,7 +460,7 @@ int io_sendmsg(struct io_kiocb *req, unsigned int issue_flags)
ret = -EINTR;
req_set_fail(req);
}
- io_req_msg_cleanup(req, kmsg, issue_flags);
+ io_req_msg_cleanup(req, issue_flags);
if (ret >= 0)
ret += sr->done_io;
else if (sr->done_io)
@@ -515,7 +514,7 @@ int io_send(struct io_kiocb *req, unsigned int issue_flags)
ret += sr->done_io;
else if (sr->done_io)
ret = sr->done_io;
- io_req_msg_cleanup(req, kmsg, issue_flags);
+ io_req_msg_cleanup(req, issue_flags);
io_req_set_res(req, ret, 0);
return IOU_OK;
}
@@ -723,7 +722,7 @@ static inline bool io_recv_finish(struct io_kiocb *req, int *ret,
*ret = IOU_STOP_MULTISHOT;
else
*ret = IOU_OK;
- io_req_msg_cleanup(req, kmsg, issue_flags);
+ io_req_msg_cleanup(req, issue_flags);
return true;
}
@@ -1209,7 +1208,7 @@ int io_send_zc(struct io_kiocb *req, unsigned int issue_flags)
*/
if (!(issue_flags & IO_URING_F_UNLOCKED)) {
io_notif_flush(zc->notif);
- io_req_msg_cleanup(req, kmsg, 0);
+ io_req_msg_cleanup(req, 0);
}
io_req_set_res(req, ret, IORING_CQE_F_MORE);
return IOU_OK;
@@ -1270,7 +1269,7 @@ int io_sendmsg_zc(struct io_kiocb *req, unsigned int issue_flags)
*/
if (!(issue_flags & IO_URING_F_UNLOCKED)) {
io_notif_flush(sr->notif);
- io_req_msg_cleanup(req, kmsg, 0);
+ io_req_msg_cleanup(req, 0);
}
io_req_set_res(req, ret, IORING_CQE_F_MORE);
return IOU_OK;
--
2.43.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH 10/17] io_uring/rw: always setup io_async_rw for read/write requests
2024-03-20 22:55 [PATCHSET v2 0/17] Improve async state handling Jens Axboe
` (8 preceding siblings ...)
2024-03-20 22:55 ` [PATCH 09/17] io_uring/net: drop 'kmsg' parameter from io_req_msg_cleanup() Jens Axboe
@ 2024-03-20 22:55 ` Jens Axboe
2024-03-25 12:03 ` Anuj gupta
2024-03-20 22:55 ` [PATCH 11/17] io_uring: get rid of struct io_rw_state Jens Axboe
` (6 subsequent siblings)
16 siblings, 1 reply; 30+ messages in thread
From: Jens Axboe @ 2024-03-20 22:55 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe
read/write requests try to put everything on the stack, and then alloc
and copy if we need to retry. This necessitates a bunch of nasty code
that deals with intermediate state.
Get rid of this, and have the prep side setup everything we need
upfront, which greatly simplifies the opcode handlers.
This includes adding an alloc cache for io_async_rw, to make it cheap
to handle.
In terms of cost, this should be basically free and transparent. For
the worst case of {READ,WRITE}_FIXED which didn't need it before,
performance is unaffected in the normal peak workload that is being
used to test that. Still runs at 122M IOPS.
Signed-off-by: Jens Axboe <[email protected]>
---
include/linux/io_uring_types.h | 1 +
io_uring/io_uring.c | 3 +
io_uring/opdef.c | 15 +-
io_uring/rw.c | 538 ++++++++++++++++-----------------
io_uring/rw.h | 19 +-
5 files changed, 278 insertions(+), 298 deletions(-)
diff --git a/include/linux/io_uring_types.h b/include/linux/io_uring_types.h
index f37caff64d05..2ba8676f83cc 100644
--- a/include/linux/io_uring_types.h
+++ b/include/linux/io_uring_types.h
@@ -300,6 +300,7 @@ struct io_ring_ctx {
struct io_hash_table cancel_table_locked;
struct io_alloc_cache apoll_cache;
struct io_alloc_cache netmsg_cache;
+ struct io_alloc_cache rw_cache;
/*
* Any cancelable uring_cmd is added to this list in
diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index ff0e233ce3c9..cc8ce830ff4b 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -308,6 +308,8 @@ static __cold struct io_ring_ctx *io_ring_ctx_alloc(struct io_uring_params *p)
sizeof(struct async_poll));
io_alloc_cache_init(&ctx->netmsg_cache, IO_ALLOC_CACHE_MAX,
sizeof(struct io_async_msghdr));
+ io_alloc_cache_init(&ctx->rw_cache, IO_ALLOC_CACHE_MAX,
+ sizeof(struct io_async_rw));
io_futex_cache_init(ctx);
init_completion(&ctx->ref_comp);
xa_init_flags(&ctx->personalities, XA_FLAGS_ALLOC1);
@@ -2898,6 +2900,7 @@ static __cold void io_ring_ctx_free(struct io_ring_ctx *ctx)
io_eventfd_unregister(ctx);
io_alloc_cache_free(&ctx->apoll_cache, io_apoll_cache_free);
io_alloc_cache_free(&ctx->netmsg_cache, io_netmsg_cache_free);
+ io_alloc_cache_free(&ctx->rw_cache, io_rw_cache_free);
io_futex_cache_free(ctx);
io_destroy_buffers(ctx);
mutex_unlock(&ctx->uring_lock);
diff --git a/io_uring/opdef.c b/io_uring/opdef.c
index dd4a1e1425e1..fcae75a08f2c 100644
--- a/io_uring/opdef.c
+++ b/io_uring/opdef.c
@@ -67,7 +67,7 @@ const struct io_issue_def io_issue_defs[] = {
.iopoll = 1,
.iopoll_queue = 1,
.vectored = 1,
- .prep = io_prep_rwv,
+ .prep = io_prep_readv,
.issue = io_read,
},
[IORING_OP_WRITEV] = {
@@ -81,7 +81,7 @@ const struct io_issue_def io_issue_defs[] = {
.iopoll = 1,
.iopoll_queue = 1,
.vectored = 1,
- .prep = io_prep_rwv,
+ .prep = io_prep_writev,
.issue = io_write,
},
[IORING_OP_FSYNC] = {
@@ -99,7 +99,7 @@ const struct io_issue_def io_issue_defs[] = {
.ioprio = 1,
.iopoll = 1,
.iopoll_queue = 1,
- .prep = io_prep_rw_fixed,
+ .prep = io_prep_read_fixed,
.issue = io_read,
},
[IORING_OP_WRITE_FIXED] = {
@@ -112,7 +112,7 @@ const struct io_issue_def io_issue_defs[] = {
.ioprio = 1,
.iopoll = 1,
.iopoll_queue = 1,
- .prep = io_prep_rw_fixed,
+ .prep = io_prep_write_fixed,
.issue = io_write,
},
[IORING_OP_POLL_ADD] = {
@@ -239,7 +239,7 @@ const struct io_issue_def io_issue_defs[] = {
.ioprio = 1,
.iopoll = 1,
.iopoll_queue = 1,
- .prep = io_prep_rw,
+ .prep = io_prep_read,
.issue = io_read,
},
[IORING_OP_WRITE] = {
@@ -252,7 +252,7 @@ const struct io_issue_def io_issue_defs[] = {
.ioprio = 1,
.iopoll = 1,
.iopoll_queue = 1,
- .prep = io_prep_rw,
+ .prep = io_prep_write,
.issue = io_write,
},
[IORING_OP_FADVISE] = {
@@ -490,14 +490,12 @@ const struct io_cold_def io_cold_defs[] = {
[IORING_OP_READV] = {
.async_size = sizeof(struct io_async_rw),
.name = "READV",
- .prep_async = io_readv_prep_async,
.cleanup = io_readv_writev_cleanup,
.fail = io_rw_fail,
},
[IORING_OP_WRITEV] = {
.async_size = sizeof(struct io_async_rw),
.name = "WRITEV",
- .prep_async = io_writev_prep_async,
.cleanup = io_readv_writev_cleanup,
.fail = io_rw_fail,
},
@@ -699,6 +697,7 @@ const struct io_cold_def io_cold_defs[] = {
#endif
},
[IORING_OP_READ_MULTISHOT] = {
+ .async_size = sizeof(struct io_async_rw),
.name = "READ_MULTISHOT",
},
[IORING_OP_WAITID] = {
diff --git a/io_uring/rw.c b/io_uring/rw.c
index 35216e8adc29..583fe61a0acb 100644
--- a/io_uring/rw.c
+++ b/io_uring/rw.c
@@ -75,7 +75,153 @@ static int io_iov_buffer_select_prep(struct io_kiocb *req)
return 0;
}
-int io_prep_rw(struct io_kiocb *req, const struct io_uring_sqe *sqe)
+static int __io_import_iovec(int ddir, struct io_kiocb *req,
+ struct io_async_rw *io,
+ unsigned int issue_flags)
+{
+ const struct io_issue_def *def = &io_issue_defs[req->opcode];
+ struct io_rw *rw = io_kiocb_to_cmd(req, struct io_rw);
+ void __user *buf;
+ size_t sqe_len;
+
+ buf = u64_to_user_ptr(rw->addr);
+ sqe_len = rw->len;
+
+ if (!def->vectored || req->flags & REQ_F_BUFFER_SELECT) {
+ if (io_do_buffer_select(req)) {
+ buf = io_buffer_select(req, &sqe_len, issue_flags);
+ if (!buf)
+ return -ENOBUFS;
+ rw->addr = (unsigned long) buf;
+ rw->len = sqe_len;
+ }
+
+ return import_ubuf(ddir, buf, sqe_len, &io->s.iter);
+ }
+
+ io->free_iovec = io->s.fast_iov;
+ return __import_iovec(ddir, buf, sqe_len, UIO_FASTIOV, &io->free_iovec,
+ &io->s.iter, req->ctx->compat);
+}
+
+static inline int io_import_iovec(int rw, struct io_kiocb *req,
+ struct io_async_rw *io,
+ unsigned int issue_flags)
+{
+ int ret;
+
+ ret = __io_import_iovec(rw, req, io, issue_flags);
+ if (unlikely(ret < 0))
+ return ret;
+
+ iov_iter_save_state(&io->s.iter, &io->s.iter_state);
+ return 0;
+}
+
+static void io_rw_iovec_free(struct io_async_rw *rw)
+{
+ if (rw->free_iovec) {
+ kfree(rw->free_iovec);
+ rw->free_iovec = NULL;
+ }
+}
+
+static void io_rw_recycle(struct io_kiocb *req, unsigned int issue_flags)
+{
+ struct io_async_rw *rw = req->async_data;
+
+ if (unlikely(issue_flags & IO_URING_F_UNLOCKED)) {
+ io_rw_iovec_free(rw);
+ return;
+ }
+ if (io_alloc_cache_put(&req->ctx->rw_cache, &rw->cache)) {
+ req->async_data = NULL;
+ req->flags &= ~REQ_F_ASYNC_DATA;
+ }
+}
+
+static void io_req_rw_cleanup(struct io_kiocb *req, unsigned int issue_flags)
+{
+ /*
+ * Disable quick recycling for anything that's gone through io-wq.
+ * In theory, this should be fine to cleanup. However, some read or
+ * write iter handling touches the iovec AFTER having called into the
+ * handler, eg to reexpand or revert. This means we can have:
+ *
+ * task io-wq
+ * issue
+ * punt to io-wq
+ * issue
+ * blkdev_write_iter()
+ * ->ki_complete()
+ * io_complete_rw()
+ * queue tw complete
+ * run tw
+ * req_rw_cleanup
+ * iov_iter_count() <- look at iov_iter again
+ *
+ * which can lead to a UAF. This is only possible for io-wq offload
+ * as the cleanup can run in parallel. As io-wq is not the fast path,
+ * just leave cleanup to the end.
+ *
+ * This is really a bug in the core code that does this, any issue
+ * path should assume that a successful (or -EIOCBQUEUED) return can
+ * mean that the underlying data can be gone at any time. But that
+ * should be fixed seperately, and then this check could be killed.
+ */
+ if (!(req->flags & REQ_F_REFCOUNT)) {
+ req->flags &= ~REQ_F_NEED_CLEANUP;
+ io_rw_recycle(req, issue_flags);
+ }
+}
+
+static int io_rw_alloc_async(struct io_kiocb *req)
+{
+ struct io_ring_ctx *ctx = req->ctx;
+ struct io_cache_entry *entry;
+ struct io_async_rw *rw;
+
+ entry = io_alloc_cache_get(&ctx->rw_cache);
+ if (entry) {
+ rw = container_of(entry, struct io_async_rw, cache);
+ req->flags |= REQ_F_ASYNC_DATA;
+ req->async_data = rw;
+ goto done;
+ }
+
+ if (!io_alloc_async_data(req)) {
+ rw = req->async_data;
+done:
+ rw->free_iovec = NULL;
+ rw->bytes_done = 0;
+ return 0;
+ }
+
+ return -ENOMEM;
+}
+
+static int io_prep_rw_setup(struct io_kiocb *req, int ddir, bool do_import)
+{
+ struct io_async_rw *rw;
+ int ret;
+
+ if (io_rw_alloc_async(req))
+ return -ENOMEM;
+
+ if (!do_import || io_do_buffer_select(req))
+ return 0;
+
+ rw = req->async_data;
+ ret = io_import_iovec(ddir, req, rw, 0);
+ if (unlikely(ret < 0))
+ return ret;
+
+ iov_iter_save_state(&rw->s.iter, &rw->s.iter_state);
+ return 0;
+}
+
+static int io_prep_rw(struct io_kiocb *req, const struct io_uring_sqe *sqe,
+ int ddir, bool do_import)
{
struct io_rw *rw = io_kiocb_to_cmd(req, struct io_rw);
unsigned ioprio;
@@ -100,34 +246,58 @@ int io_prep_rw(struct io_kiocb *req, const struct io_uring_sqe *sqe)
rw->addr = READ_ONCE(sqe->addr);
rw->len = READ_ONCE(sqe->len);
rw->flags = READ_ONCE(sqe->rw_flags);
- return 0;
+ return io_prep_rw_setup(req, ddir, do_import);
}
-int io_prep_rwv(struct io_kiocb *req, const struct io_uring_sqe *sqe)
+int io_prep_read(struct io_kiocb *req, const struct io_uring_sqe *sqe)
{
+ return io_prep_rw(req, sqe, ITER_DEST, true);
+}
+
+int io_prep_write(struct io_kiocb *req, const struct io_uring_sqe *sqe)
+{
+ return io_prep_rw(req, sqe, ITER_SOURCE, true);
+}
+
+static int io_prep_rwv(struct io_kiocb *req, const struct io_uring_sqe *sqe,
+ int ddir)
+{
+ const bool do_import = !(req->flags & REQ_F_BUFFER_SELECT);
int ret;
- ret = io_prep_rw(req, sqe);
+ ret = io_prep_rw(req, sqe, ddir, do_import);
if (unlikely(ret))
return ret;
+ if (do_import)
+ return 0;
/*
* Have to do this validation here, as this is in io_read() rw->len
* might have chanaged due to buffer selection
*/
- if (req->flags & REQ_F_BUFFER_SELECT)
- return io_iov_buffer_select_prep(req);
+ return io_iov_buffer_select_prep(req);
+}
- return 0;
+int io_prep_readv(struct io_kiocb *req, const struct io_uring_sqe *sqe)
+{
+ return io_prep_rwv(req, sqe, ITER_DEST);
+}
+
+int io_prep_writev(struct io_kiocb *req, const struct io_uring_sqe *sqe)
+{
+ return io_prep_rwv(req, sqe, ITER_SOURCE);
}
-int io_prep_rw_fixed(struct io_kiocb *req, const struct io_uring_sqe *sqe)
+static int io_prep_rw_fixed(struct io_kiocb *req, const struct io_uring_sqe *sqe,
+ int ddir)
{
+ struct io_rw *rw = io_kiocb_to_cmd(req, struct io_rw);
struct io_ring_ctx *ctx = req->ctx;
+ struct io_async_rw *io;
u16 index;
int ret;
- ret = io_prep_rw(req, sqe);
+ ret = io_prep_rw(req, sqe, ddir, false);
if (unlikely(ret))
return ret;
@@ -136,7 +306,21 @@ int io_prep_rw_fixed(struct io_kiocb *req, const struct io_uring_sqe *sqe)
index = array_index_nospec(req->buf_index, ctx->nr_user_bufs);
req->imu = ctx->user_bufs[index];
io_req_set_rsrc_node(req, ctx, 0);
- return 0;
+
+ io = req->async_data;
+ ret = io_import_fixed(ddir, &io->s.iter, req->imu, rw->addr, rw->len);
+ iov_iter_save_state(&io->s.iter, &io->s.iter_state);
+ return ret;
+}
+
+int io_prep_read_fixed(struct io_kiocb *req, const struct io_uring_sqe *sqe)
+{
+ return io_prep_rw_fixed(req, sqe, ITER_DEST);
+}
+
+int io_prep_write_fixed(struct io_kiocb *req, const struct io_uring_sqe *sqe)
+{
+ return io_prep_rw_fixed(req, sqe, ITER_SOURCE);
}
/*
@@ -152,7 +336,7 @@ int io_read_mshot_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
if (!(req->flags & REQ_F_BUFFER_SELECT))
return -EINVAL;
- ret = io_prep_rw(req, sqe);
+ ret = io_prep_rw(req, sqe, ITER_DEST, false);
if (unlikely(ret))
return ret;
@@ -165,9 +349,7 @@ int io_read_mshot_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
void io_readv_writev_cleanup(struct io_kiocb *req)
{
- struct io_async_rw *io = req->async_data;
-
- kfree(io->free_iovec);
+ io_rw_iovec_free(req->async_data);
}
static inline loff_t *io_kiocb_update_pos(struct io_kiocb *req)
@@ -188,14 +370,11 @@ static inline loff_t *io_kiocb_update_pos(struct io_kiocb *req)
}
#ifdef CONFIG_BLOCK
-static bool io_resubmit_prep(struct io_kiocb *req)
+static void io_resubmit_prep(struct io_kiocb *req)
{
struct io_async_rw *io = req->async_data;
- if (!req_has_async_data(req))
- return !io_req_prep_async(req);
iov_iter_restore(&io->s.iter, &io->s.iter_state);
- return true;
}
static bool io_rw_should_reissue(struct io_kiocb *req)
@@ -224,9 +403,8 @@ static bool io_rw_should_reissue(struct io_kiocb *req)
return true;
}
#else
-static bool io_resubmit_prep(struct io_kiocb *req)
+static void io_resubmit_prep(struct io_kiocb *req)
{
- return false;
}
static bool io_rw_should_reissue(struct io_kiocb *req)
{
@@ -308,6 +486,7 @@ void io_req_rw_complete(struct io_kiocb *req, struct io_tw_state *ts)
if (req->flags & (REQ_F_BUFFER_SELECTED|REQ_F_BUFFER_RING))
req->cqe.flags |= io_put_kbuf(req, 0);
+ io_req_rw_cleanup(req, 0);
io_req_task_complete(req, ts);
}
@@ -388,6 +567,7 @@ static int kiocb_done(struct io_kiocb *req, ssize_t ret,
io_req_io_end(req);
io_req_set_res(req, final_ret,
io_put_kbuf(req, issue_flags));
+ io_req_rw_cleanup(req, issue_flags);
return IOU_OK;
}
} else {
@@ -396,71 +576,12 @@ static int kiocb_done(struct io_kiocb *req, ssize_t ret,
if (req->flags & REQ_F_REISSUE) {
req->flags &= ~REQ_F_REISSUE;
- if (io_resubmit_prep(req))
- return -EAGAIN;
- else
- io_req_task_queue_fail(req, final_ret);
+ io_resubmit_prep(req);
+ return -EAGAIN;
}
return IOU_ISSUE_SKIP_COMPLETE;
}
-static struct iovec *__io_import_iovec(int ddir, struct io_kiocb *req,
- struct io_rw_state *s,
- unsigned int issue_flags)
-{
- struct io_rw *rw = io_kiocb_to_cmd(req, struct io_rw);
- struct iov_iter *iter = &s->iter;
- u8 opcode = req->opcode;
- struct iovec *iovec;
- void __user *buf;
- size_t sqe_len;
- ssize_t ret;
-
- if (opcode == IORING_OP_READ_FIXED || opcode == IORING_OP_WRITE_FIXED) {
- ret = io_import_fixed(ddir, iter, req->imu, rw->addr, rw->len);
- if (ret)
- return ERR_PTR(ret);
- return NULL;
- }
-
- buf = u64_to_user_ptr(rw->addr);
- sqe_len = rw->len;
-
- if (!io_issue_defs[opcode].vectored || req->flags & REQ_F_BUFFER_SELECT) {
- if (io_do_buffer_select(req)) {
- buf = io_buffer_select(req, &sqe_len, issue_flags);
- if (!buf)
- return ERR_PTR(-ENOBUFS);
- rw->addr = (unsigned long) buf;
- rw->len = sqe_len;
- }
-
- ret = import_ubuf(ddir, buf, sqe_len, iter);
- if (ret)
- return ERR_PTR(ret);
- return NULL;
- }
-
- iovec = s->fast_iov;
- ret = __import_iovec(ddir, buf, sqe_len, UIO_FASTIOV, &iovec, iter,
- req->ctx->compat);
- if (unlikely(ret < 0))
- return ERR_PTR(ret);
- return iovec;
-}
-
-static inline int io_import_iovec(int rw, struct io_kiocb *req,
- struct iovec **iovec, struct io_rw_state *s,
- unsigned int issue_flags)
-{
- *iovec = __io_import_iovec(rw, req, s, issue_flags);
- if (IS_ERR(*iovec))
- return PTR_ERR(*iovec);
-
- iov_iter_save_state(&s->iter, &s->iter_state);
- return 0;
-}
-
static inline loff_t *io_kiocb_ppos(struct kiocb *kiocb)
{
return (kiocb->ki_filp->f_mode & FMODE_STREAM) ? NULL : &kiocb->ki_pos;
@@ -532,89 +653,6 @@ static ssize_t loop_rw_iter(int ddir, struct io_rw *rw, struct iov_iter *iter)
return ret;
}
-static void io_req_map_rw(struct io_kiocb *req, const struct iovec *iovec,
- const struct iovec *fast_iov, struct iov_iter *iter)
-{
- struct io_async_rw *io = req->async_data;
-
- memcpy(&io->s.iter, iter, sizeof(*iter));
- io->free_iovec = iovec;
- io->bytes_done = 0;
- /* can only be fixed buffers, no need to do anything */
- if (iov_iter_is_bvec(iter) || iter_is_ubuf(iter))
- return;
- if (!iovec) {
- unsigned iov_off = 0;
-
- io->s.iter.__iov = io->s.fast_iov;
- if (iter->__iov != fast_iov) {
- iov_off = iter_iov(iter) - fast_iov;
- io->s.iter.__iov += iov_off;
- }
- if (io->s.fast_iov != fast_iov)
- memcpy(io->s.fast_iov + iov_off, fast_iov + iov_off,
- sizeof(struct iovec) * iter->nr_segs);
- } else {
- req->flags |= REQ_F_NEED_CLEANUP;
- }
-}
-
-static int io_setup_async_rw(struct io_kiocb *req, const struct iovec *iovec,
- struct io_rw_state *s, bool force)
-{
- if (!force && !io_cold_defs[req->opcode].prep_async)
- return 0;
- /* opcode type doesn't need async data */
- if (!io_cold_defs[req->opcode].async_size)
- return 0;
- if (!req_has_async_data(req)) {
- struct io_async_rw *iorw;
-
- if (io_alloc_async_data(req)) {
- kfree(iovec);
- return -ENOMEM;
- }
-
- io_req_map_rw(req, iovec, s->fast_iov, &s->iter);
- iorw = req->async_data;
- /* we've copied and mapped the iter, ensure state is saved */
- iov_iter_save_state(&iorw->s.iter, &iorw->s.iter_state);
- }
- return 0;
-}
-
-static inline int io_rw_prep_async(struct io_kiocb *req, int rw)
-{
- struct io_async_rw *iorw = req->async_data;
- struct iovec *iov;
- int ret;
-
- iorw->bytes_done = 0;
- iorw->free_iovec = NULL;
-
- /* submission path, ->uring_lock should already be taken */
- ret = io_import_iovec(rw, req, &iov, &iorw->s, 0);
- if (unlikely(ret < 0))
- return ret;
-
- if (iov) {
- iorw->free_iovec = iov;
- req->flags |= REQ_F_NEED_CLEANUP;
- }
-
- return 0;
-}
-
-int io_readv_prep_async(struct io_kiocb *req)
-{
- return io_rw_prep_async(req, ITER_DEST);
-}
-
-int io_writev_prep_async(struct io_kiocb *req)
-{
- return io_rw_prep_async(req, ITER_SOURCE);
-}
-
/*
* This is our waitqueue callback handler, registered through __folio_lock_async()
* when we initially tried to do the IO with the iocb armed our waitqueue.
@@ -754,54 +792,28 @@ static int io_rw_init_file(struct io_kiocb *req, fmode_t mode)
static int __io_read(struct io_kiocb *req, unsigned int issue_flags)
{
+ bool force_nonblock = issue_flags & IO_URING_F_NONBLOCK;
struct io_rw *rw = io_kiocb_to_cmd(req, struct io_rw);
- struct io_rw_state __s, *s = &__s;
- struct iovec *iovec;
+ struct io_async_rw *io = req->async_data;
struct kiocb *kiocb = &rw->kiocb;
- bool force_nonblock = issue_flags & IO_URING_F_NONBLOCK;
- struct io_async_rw *io;
- ssize_t ret, ret2;
+ ssize_t ret;
loff_t *ppos;
- if (!req_has_async_data(req)) {
- ret = io_import_iovec(ITER_DEST, req, &iovec, s, issue_flags);
+ if (io_do_buffer_select(req)) {
+ ret = io_import_iovec(ITER_DEST, req, io, issue_flags);
if (unlikely(ret < 0))
return ret;
- } else {
- io = req->async_data;
- s = &io->s;
-
- /*
- * Safe and required to re-import if we're using provided
- * buffers, as we dropped the selected one before retry.
- */
- if (io_do_buffer_select(req)) {
- ret = io_import_iovec(ITER_DEST, req, &iovec, s, issue_flags);
- if (unlikely(ret < 0))
- return ret;
- }
-
- /*
- * We come here from an earlier attempt, restore our state to
- * match in case it doesn't. It's cheap enough that we don't
- * need to make this conditional.
- */
- iov_iter_restore(&s->iter, &s->iter_state);
- iovec = NULL;
}
+
ret = io_rw_init_file(req, FMODE_READ);
- if (unlikely(ret)) {
- kfree(iovec);
+ if (unlikely(ret))
return ret;
- }
- req->cqe.res = iov_iter_count(&s->iter);
+ req->cqe.res = iov_iter_count(&io->s.iter);
if (force_nonblock) {
/* If the file doesn't support async, just async punt */
- if (unlikely(!io_file_supports_nowait(req))) {
- ret = io_setup_async_rw(req, iovec, s, true);
- return ret ?: -EAGAIN;
- }
+ if (unlikely(!io_file_supports_nowait(req)))
+ return -EAGAIN;
kiocb->ki_flags |= IOCB_NOWAIT;
} else {
/* Ensure we clear previously set non-block flag */
@@ -811,20 +823,15 @@ static int __io_read(struct io_kiocb *req, unsigned int issue_flags)
ppos = io_kiocb_update_pos(req);
ret = rw_verify_area(READ, req->file, ppos, req->cqe.res);
- if (unlikely(ret)) {
- kfree(iovec);
+ if (unlikely(ret))
return ret;
- }
- ret = io_iter_do_read(rw, &s->iter);
+ ret = io_iter_do_read(rw, &io->s.iter);
if (ret == -EAGAIN || (req->flags & REQ_F_REISSUE)) {
req->flags &= ~REQ_F_REISSUE;
- /*
- * If we can poll, just do that. For a vectored read, we'll
- * need to copy state first.
- */
- if (io_file_can_poll(req) && !io_issue_defs[req->opcode].vectored)
+ /* If we can poll, just do that. */
+ if (io_file_can_poll(req))
return -EAGAIN;
/* IOPOLL retry should happen for io-wq threads */
if (!force_nonblock && !(req->ctx->flags & IORING_SETUP_IOPOLL))
@@ -834,8 +841,6 @@ static int __io_read(struct io_kiocb *req, unsigned int issue_flags)
goto done;
ret = 0;
} else if (ret == -EIOCBQUEUED) {
- if (iovec)
- kfree(iovec);
return IOU_ISSUE_SKIP_COMPLETE;
} else if (ret == req->cqe.res || ret <= 0 || !force_nonblock ||
(req->flags & REQ_F_NOWAIT) || !need_complete_io(req)) {
@@ -848,21 +853,7 @@ static int __io_read(struct io_kiocb *req, unsigned int issue_flags)
* untouched in case of error. Restore it and we'll advance it
* manually if we need to.
*/
- iov_iter_restore(&s->iter, &s->iter_state);
-
- ret2 = io_setup_async_rw(req, iovec, s, true);
- iovec = NULL;
- if (ret2) {
- ret = ret > 0 ? ret : ret2;
- goto done;
- }
-
- io = req->async_data;
- s = &io->s;
- /*
- * Now use our persistent iterator and state, if we aren't already.
- * We've restored and mapped the iter to match.
- */
+ iov_iter_restore(&io->s.iter, &io->s.iter_state);
do {
/*
@@ -870,11 +861,11 @@ static int __io_read(struct io_kiocb *req, unsigned int issue_flags)
* above or inside this loop. Advance the iter by the bytes
* that were consumed.
*/
- iov_iter_advance(&s->iter, ret);
- if (!iov_iter_count(&s->iter))
+ iov_iter_advance(&io->s.iter, ret);
+ if (!iov_iter_count(&io->s.iter))
break;
io->bytes_done += ret;
- iov_iter_save_state(&s->iter, &s->iter_state);
+ iov_iter_save_state(&io->s.iter, &io->s.iter_state);
/* if we can retry, do so with the callbacks armed */
if (!io_rw_should_retry(req)) {
@@ -882,24 +873,22 @@ static int __io_read(struct io_kiocb *req, unsigned int issue_flags)
return -EAGAIN;
}
- req->cqe.res = iov_iter_count(&s->iter);
+ req->cqe.res = iov_iter_count(&io->s.iter);
/*
* Now retry read with the IOCB_WAITQ parts set in the iocb. If
* we get -EIOCBQUEUED, then we'll get a notification when the
* desired page gets unlocked. We can also get a partial read
* here, and if we do, then just retry at the new offset.
*/
- ret = io_iter_do_read(rw, &s->iter);
+ ret = io_iter_do_read(rw, &io->s.iter);
if (ret == -EIOCBQUEUED)
return IOU_ISSUE_SKIP_COMPLETE;
/* we got some bytes, but not all. retry. */
kiocb->ki_flags &= ~IOCB_WAITQ;
- iov_iter_restore(&s->iter, &s->iter_state);
+ iov_iter_restore(&io->s.iter, &io->s.iter_state);
} while (ret > 0);
done:
/* it's faster to check here then delegate to kfree */
- if (iovec)
- kfree(iovec);
return ret;
}
@@ -908,8 +897,9 @@ int io_read(struct io_kiocb *req, unsigned int issue_flags)
int ret;
ret = __io_read(req, issue_flags);
- if (ret >= 0)
- return kiocb_done(req, ret, issue_flags);
+ if (ret >= 0) {
+ ret = kiocb_done(req, ret, issue_flags);
+ }
return ret;
}
@@ -974,6 +964,7 @@ int io_read_mshot(struct io_kiocb *req, unsigned int issue_flags)
* multishot request, hitting overflow will terminate it.
*/
io_req_set_res(req, ret, cflags);
+ io_req_rw_cleanup(req, issue_flags);
if (issue_flags & IO_URING_F_MULTISHOT)
return IOU_STOP_MULTISHOT;
return IOU_OK;
@@ -981,42 +972,28 @@ int io_read_mshot(struct io_kiocb *req, unsigned int issue_flags)
int io_write(struct io_kiocb *req, unsigned int issue_flags)
{
+ bool force_nonblock = issue_flags & IO_URING_F_NONBLOCK;
struct io_rw *rw = io_kiocb_to_cmd(req, struct io_rw);
- struct io_rw_state __s, *s = &__s;
- struct iovec *iovec;
+ struct io_async_rw *io = req->async_data;
struct kiocb *kiocb = &rw->kiocb;
- bool force_nonblock = issue_flags & IO_URING_F_NONBLOCK;
ssize_t ret, ret2;
loff_t *ppos;
- if (!req_has_async_data(req)) {
- ret = io_import_iovec(ITER_SOURCE, req, &iovec, s, issue_flags);
- if (unlikely(ret < 0))
- return ret;
- } else {
- struct io_async_rw *io = req->async_data;
-
- s = &io->s;
- iov_iter_restore(&s->iter, &s->iter_state);
- iovec = NULL;
- }
ret = io_rw_init_file(req, FMODE_WRITE);
- if (unlikely(ret)) {
- kfree(iovec);
+ if (unlikely(ret))
return ret;
- }
- req->cqe.res = iov_iter_count(&s->iter);
+ req->cqe.res = iov_iter_count(&io->s.iter);
if (force_nonblock) {
/* If the file doesn't support async, just async punt */
if (unlikely(!io_file_supports_nowait(req)))
- goto copy_iov;
+ goto ret_eagain;
/* File path supports NOWAIT for non-direct_IO only for block devices. */
if (!(kiocb->ki_flags & IOCB_DIRECT) &&
!(kiocb->ki_filp->f_mode & FMODE_BUF_WASYNC) &&
(req->flags & REQ_F_ISREG))
- goto copy_iov;
+ goto ret_eagain;
kiocb->ki_flags |= IOCB_NOWAIT;
} else {
@@ -1027,19 +1004,17 @@ int io_write(struct io_kiocb *req, unsigned int issue_flags)
ppos = io_kiocb_update_pos(req);
ret = rw_verify_area(WRITE, req->file, ppos, req->cqe.res);
- if (unlikely(ret)) {
- kfree(iovec);
+ if (unlikely(ret))
return ret;
- }
if (req->flags & REQ_F_ISREG)
kiocb_start_write(kiocb);
kiocb->ki_flags |= IOCB_WRITE;
if (likely(req->file->f_op->write_iter))
- ret2 = call_write_iter(req->file, kiocb, &s->iter);
+ ret2 = call_write_iter(req->file, kiocb, &io->s.iter);
else if (req->file->f_op->write)
- ret2 = loop_rw_iter(WRITE, rw, &s->iter);
+ ret2 = loop_rw_iter(WRITE, rw, &io->s.iter);
else
ret2 = -EINVAL;
@@ -1060,11 +1035,9 @@ int io_write(struct io_kiocb *req, unsigned int issue_flags)
if (!force_nonblock || ret2 != -EAGAIN) {
/* IOPOLL retry should happen for io-wq threads */
if (ret2 == -EAGAIN && (req->ctx->flags & IORING_SETUP_IOPOLL))
- goto copy_iov;
+ goto ret_eagain;
if (ret2 != req->cqe.res && ret2 >= 0 && need_complete_io(req)) {
- struct io_async_rw *io;
-
trace_io_uring_short_write(req->ctx, kiocb->ki_pos - ret2,
req->cqe.res, ret2);
@@ -1073,33 +1046,22 @@ int io_write(struct io_kiocb *req, unsigned int issue_flags)
* in the worker. Also update bytes_done to account for
* the bytes already written.
*/
- iov_iter_save_state(&s->iter, &s->iter_state);
- ret = io_setup_async_rw(req, iovec, s, true);
-
- io = req->async_data;
- if (io)
- io->bytes_done += ret2;
+ iov_iter_save_state(&io->s.iter, &io->s.iter_state);
+ io->bytes_done += ret2;
if (kiocb->ki_flags & IOCB_WRITE)
io_req_end_write(req);
- return ret ? ret : -EAGAIN;
+ return -EAGAIN;
}
done:
ret = kiocb_done(req, ret2, issue_flags);
} else {
-copy_iov:
- iov_iter_restore(&s->iter, &s->iter_state);
- ret = io_setup_async_rw(req, iovec, s, false);
- if (!ret) {
- if (kiocb->ki_flags & IOCB_WRITE)
- io_req_end_write(req);
- return -EAGAIN;
- }
- return ret;
+ret_eagain:
+ iov_iter_restore(&io->s.iter, &io->s.iter_state);
+ if (kiocb->ki_flags & IOCB_WRITE)
+ io_req_end_write(req);
+ return -EAGAIN;
}
- /* it's reportedly faster than delegating the null check to kfree() */
- if (iovec)
- kfree(iovec);
return ret;
}
@@ -1174,6 +1136,8 @@ int io_do_iopoll(struct io_ring_ctx *ctx, bool force_nonspin)
break;
nr_events++;
req->cqe.flags = io_put_kbuf(req, 0);
+ if (req->opcode != IORING_OP_URING_CMD)
+ io_req_rw_cleanup(req, 0);
}
if (unlikely(!nr_events))
return 0;
@@ -1187,3 +1151,11 @@ int io_do_iopoll(struct io_ring_ctx *ctx, bool force_nonspin)
__io_submit_flush_completions(ctx);
return nr_events;
}
+
+void io_rw_cache_free(struct io_cache_entry *entry)
+{
+ struct io_async_rw *rw;
+
+ rw = container_of(entry, struct io_async_rw, cache);
+ kfree(rw);
+}
diff --git a/io_uring/rw.h b/io_uring/rw.h
index f9e89b4fe4da..f7905070d10b 100644
--- a/io_uring/rw.h
+++ b/io_uring/rw.h
@@ -9,21 +9,26 @@ struct io_rw_state {
};
struct io_async_rw {
+ union {
+ size_t bytes_done;
+ struct io_cache_entry cache;
+ };
struct io_rw_state s;
- const struct iovec *free_iovec;
- size_t bytes_done;
+ struct iovec *free_iovec;
struct wait_page_queue wpq;
};
-int io_prep_rw(struct io_kiocb *req, const struct io_uring_sqe *sqe);
-int io_prep_rwv(struct io_kiocb *req, const struct io_uring_sqe *sqe);
-int io_prep_rw_fixed(struct io_kiocb *req, const struct io_uring_sqe *sqe);
+int io_prep_read_fixed(struct io_kiocb *req, const struct io_uring_sqe *sqe);
+int io_prep_write_fixed(struct io_kiocb *req, const struct io_uring_sqe *sqe);
+int io_prep_readv(struct io_kiocb *req, const struct io_uring_sqe *sqe);
+int io_prep_writev(struct io_kiocb *req, const struct io_uring_sqe *sqe);
+int io_prep_read(struct io_kiocb *req, const struct io_uring_sqe *sqe);
+int io_prep_write(struct io_kiocb *req, const struct io_uring_sqe *sqe);
int io_read(struct io_kiocb *req, unsigned int issue_flags);
-int io_readv_prep_async(struct io_kiocb *req);
int io_write(struct io_kiocb *req, unsigned int issue_flags);
-int io_writev_prep_async(struct io_kiocb *req);
void io_readv_writev_cleanup(struct io_kiocb *req);
void io_rw_fail(struct io_kiocb *req);
void io_req_rw_complete(struct io_kiocb *req, struct io_tw_state *ts);
int io_read_mshot_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
int io_read_mshot(struct io_kiocb *req, unsigned int issue_flags);
+void io_rw_cache_free(struct io_cache_entry *entry);
--
2.43.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH 10/17] io_uring/rw: always setup io_async_rw for read/write requests
2024-03-20 22:55 ` [PATCH 10/17] io_uring/rw: always setup io_async_rw for read/write requests Jens Axboe
@ 2024-03-25 12:03 ` Anuj gupta
2024-03-25 14:54 ` Jens Axboe
0 siblings, 1 reply; 30+ messages in thread
From: Anuj gupta @ 2024-03-25 12:03 UTC (permalink / raw)
To: Jens Axboe; +Cc: io-uring
On Thu, Mar 21, 2024 at 4:28 AM Jens Axboe <[email protected]> wrote:
>
> read/write requests try to put everything on the stack, and then alloc
> and copy if we need to retry. This necessitates a bunch of nasty code
> that deals with intermediate state.
>
> Get rid of this, and have the prep side setup everything we need
> upfront, which greatly simplifies the opcode handlers.
>
> This includes adding an alloc cache for io_async_rw, to make it cheap
> to handle.
>
> In terms of cost, this should be basically free and transparent. For
> the worst case of {READ,WRITE}_FIXED which didn't need it before,
> performance is unaffected in the normal peak workload that is being
> used to test that. Still runs at 122M IOPS.
>
> Signed-off-by: Jens Axboe <[email protected]>
> ---
> include/linux/io_uring_types.h | 1 +
> io_uring/io_uring.c | 3 +
> io_uring/opdef.c | 15 +-
> io_uring/rw.c | 538 ++++++++++++++++-----------------
> io_uring/rw.h | 19 +-
> 5 files changed, 278 insertions(+), 298 deletions(-)
>
> diff --git a/include/linux/io_uring_types.h b/include/linux/io_uring_types.h
> index f37caff64d05..2ba8676f83cc 100644
> --- a/include/linux/io_uring_types.h
> +++ b/include/linux/io_uring_types.h
> @@ -300,6 +300,7 @@ struct io_ring_ctx {
> struct io_hash_table cancel_table_locked;
> struct io_alloc_cache apoll_cache;
> struct io_alloc_cache netmsg_cache;
> + struct io_alloc_cache rw_cache;
>
> /*
> * Any cancelable uring_cmd is added to this list in
> diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
> index ff0e233ce3c9..cc8ce830ff4b 100644
> --- a/io_uring/io_uring.c
> +++ b/io_uring/io_uring.c
> @@ -308,6 +308,8 @@ static __cold struct io_ring_ctx *io_ring_ctx_alloc(struct io_uring_params *p)
> sizeof(struct async_poll));
> io_alloc_cache_init(&ctx->netmsg_cache, IO_ALLOC_CACHE_MAX,
> sizeof(struct io_async_msghdr));
> + io_alloc_cache_init(&ctx->rw_cache, IO_ALLOC_CACHE_MAX,
> + sizeof(struct io_async_rw));
> io_futex_cache_init(ctx);
> init_completion(&ctx->ref_comp);
> xa_init_flags(&ctx->personalities, XA_FLAGS_ALLOC1);
> @@ -2898,6 +2900,7 @@ static __cold void io_ring_ctx_free(struct io_ring_ctx *ctx)
> io_eventfd_unregister(ctx);
> io_alloc_cache_free(&ctx->apoll_cache, io_apoll_cache_free);
> io_alloc_cache_free(&ctx->netmsg_cache, io_netmsg_cache_free);
> + io_alloc_cache_free(&ctx->rw_cache, io_rw_cache_free);
> io_futex_cache_free(ctx);
> io_destroy_buffers(ctx);
> mutex_unlock(&ctx->uring_lock);
> diff --git a/io_uring/opdef.c b/io_uring/opdef.c
> index dd4a1e1425e1..fcae75a08f2c 100644
> --- a/io_uring/opdef.c
> +++ b/io_uring/opdef.c
> @@ -67,7 +67,7 @@ const struct io_issue_def io_issue_defs[] = {
> .iopoll = 1,
> .iopoll_queue = 1,
> .vectored = 1,
> - .prep = io_prep_rwv,
> + .prep = io_prep_readv,
> .issue = io_read,
> },
> [IORING_OP_WRITEV] = {
> @@ -81,7 +81,7 @@ const struct io_issue_def io_issue_defs[] = {
> .iopoll = 1,
> .iopoll_queue = 1,
> .vectored = 1,
> - .prep = io_prep_rwv,
> + .prep = io_prep_writev,
> .issue = io_write,
> },
> [IORING_OP_FSYNC] = {
> @@ -99,7 +99,7 @@ const struct io_issue_def io_issue_defs[] = {
> .ioprio = 1,
> .iopoll = 1,
> .iopoll_queue = 1,
> - .prep = io_prep_rw_fixed,
> + .prep = io_prep_read_fixed,
> .issue = io_read,
> },
> [IORING_OP_WRITE_FIXED] = {
> @@ -112,7 +112,7 @@ const struct io_issue_def io_issue_defs[] = {
> .ioprio = 1,
> .iopoll = 1,
> .iopoll_queue = 1,
> - .prep = io_prep_rw_fixed,
> + .prep = io_prep_write_fixed,
> .issue = io_write,
> },
> [IORING_OP_POLL_ADD] = {
> @@ -239,7 +239,7 @@ const struct io_issue_def io_issue_defs[] = {
> .ioprio = 1,
> .iopoll = 1,
> .iopoll_queue = 1,
> - .prep = io_prep_rw,
> + .prep = io_prep_read,
> .issue = io_read,
> },
> [IORING_OP_WRITE] = {
> @@ -252,7 +252,7 @@ const struct io_issue_def io_issue_defs[] = {
> .ioprio = 1,
> .iopoll = 1,
> .iopoll_queue = 1,
> - .prep = io_prep_rw,
> + .prep = io_prep_write,
> .issue = io_write,
> },
> [IORING_OP_FADVISE] = {
> @@ -490,14 +490,12 @@ const struct io_cold_def io_cold_defs[] = {
> [IORING_OP_READV] = {
> .async_size = sizeof(struct io_async_rw),
> .name = "READV",
> - .prep_async = io_readv_prep_async,
> .cleanup = io_readv_writev_cleanup,
> .fail = io_rw_fail,
> },
> [IORING_OP_WRITEV] = {
> .async_size = sizeof(struct io_async_rw),
> .name = "WRITEV",
> - .prep_async = io_writev_prep_async,
> .cleanup = io_readv_writev_cleanup,
> .fail = io_rw_fail,
> },
> @@ -699,6 +697,7 @@ const struct io_cold_def io_cold_defs[] = {
> #endif
> },
> [IORING_OP_READ_MULTISHOT] = {
> + .async_size = sizeof(struct io_async_rw),
> .name = "READ_MULTISHOT",
> },
> [IORING_OP_WAITID] = {
> diff --git a/io_uring/rw.c b/io_uring/rw.c
> index 35216e8adc29..583fe61a0acb 100644
> --- a/io_uring/rw.c
> +++ b/io_uring/rw.c
> @@ -75,7 +75,153 @@ static int io_iov_buffer_select_prep(struct io_kiocb *req)
> return 0;
> }
>
> -int io_prep_rw(struct io_kiocb *req, const struct io_uring_sqe *sqe)
> +static int __io_import_iovec(int ddir, struct io_kiocb *req,
> + struct io_async_rw *io,
> + unsigned int issue_flags)
> +{
> + const struct io_issue_def *def = &io_issue_defs[req->opcode];
> + struct io_rw *rw = io_kiocb_to_cmd(req, struct io_rw);
> + void __user *buf;
> + size_t sqe_len;
> +
> + buf = u64_to_user_ptr(rw->addr);
> + sqe_len = rw->len;
> +
> + if (!def->vectored || req->flags & REQ_F_BUFFER_SELECT) {
> + if (io_do_buffer_select(req)) {
> + buf = io_buffer_select(req, &sqe_len, issue_flags);
> + if (!buf)
> + return -ENOBUFS;
> + rw->addr = (unsigned long) buf;
> + rw->len = sqe_len;
> + }
> +
> + return import_ubuf(ddir, buf, sqe_len, &io->s.iter);
> + }
> +
> + io->free_iovec = io->s.fast_iov;
> + return __import_iovec(ddir, buf, sqe_len, UIO_FASTIOV, &io->free_iovec,
> + &io->s.iter, req->ctx->compat);
> +}
> +
> +static inline int io_import_iovec(int rw, struct io_kiocb *req,
> + struct io_async_rw *io,
> + unsigned int issue_flags)
> +{
> + int ret;
> +
> + ret = __io_import_iovec(rw, req, io, issue_flags);
> + if (unlikely(ret < 0))
> + return ret;
> +
> + iov_iter_save_state(&io->s.iter, &io->s.iter_state);
> + return 0;
> +}
> +
> +static void io_rw_iovec_free(struct io_async_rw *rw)
> +{
> + if (rw->free_iovec) {
> + kfree(rw->free_iovec);
> + rw->free_iovec = NULL;
> + }
> +}
> +
> +static void io_rw_recycle(struct io_kiocb *req, unsigned int issue_flags)
> +{
> + struct io_async_rw *rw = req->async_data;
> +
> + if (unlikely(issue_flags & IO_URING_F_UNLOCKED)) {
> + io_rw_iovec_free(rw);
> + return;
> + }
> + if (io_alloc_cache_put(&req->ctx->rw_cache, &rw->cache)) {
> + req->async_data = NULL;
> + req->flags &= ~REQ_F_ASYNC_DATA;
> + }
> +}
> +
> +static void io_req_rw_cleanup(struct io_kiocb *req, unsigned int issue_flags)
> +{
> + /*
> + * Disable quick recycling for anything that's gone through io-wq.
> + * In theory, this should be fine to cleanup. However, some read or
> + * write iter handling touches the iovec AFTER having called into the
> + * handler, eg to reexpand or revert. This means we can have:
> + *
> + * task io-wq
> + * issue
> + * punt to io-wq
> + * issue
> + * blkdev_write_iter()
> + * ->ki_complete()
> + * io_complete_rw()
> + * queue tw complete
> + * run tw
> + * req_rw_cleanup
> + * iov_iter_count() <- look at iov_iter again
> + *
> + * which can lead to a UAF. This is only possible for io-wq offload
> + * as the cleanup can run in parallel. As io-wq is not the fast path,
> + * just leave cleanup to the end.
> + *
> + * This is really a bug in the core code that does this, any issue
> + * path should assume that a successful (or -EIOCBQUEUED) return can
> + * mean that the underlying data can be gone at any time. But that
> + * should be fixed seperately, and then this check could be killed.
> + */
> + if (!(req->flags & REQ_F_REFCOUNT)) {
> + req->flags &= ~REQ_F_NEED_CLEANUP;
> + io_rw_recycle(req, issue_flags);
> + }
> +}
> +
> +static int io_rw_alloc_async(struct io_kiocb *req)
> +{
> + struct io_ring_ctx *ctx = req->ctx;
> + struct io_cache_entry *entry;
> + struct io_async_rw *rw;
> +
> + entry = io_alloc_cache_get(&ctx->rw_cache);
> + if (entry) {
> + rw = container_of(entry, struct io_async_rw, cache);
> + req->flags |= REQ_F_ASYNC_DATA;
> + req->async_data = rw;
> + goto done;
> + }
> +
> + if (!io_alloc_async_data(req)) {
> + rw = req->async_data;
> +done:
> + rw->free_iovec = NULL;
> + rw->bytes_done = 0;
> + return 0;
> + }
> +
> + return -ENOMEM;
> +}
> +
> +static int io_prep_rw_setup(struct io_kiocb *req, int ddir, bool do_import)
> +{
> + struct io_async_rw *rw;
> + int ret;
> +
> + if (io_rw_alloc_async(req))
> + return -ENOMEM;
> +
> + if (!do_import || io_do_buffer_select(req))
> + return 0;
> +
> + rw = req->async_data;
> + ret = io_import_iovec(ddir, req, rw, 0);
> + if (unlikely(ret < 0))
> + return ret;
> +
> + iov_iter_save_state(&rw->s.iter, &rw->s.iter_state);
It seems that the state of iov_iter gets saved in the caller io_import_iovec
as well. Do we need to save it again here?
--
Anuj Gupta
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH 10/17] io_uring/rw: always setup io_async_rw for read/write requests
2024-03-25 12:03 ` Anuj gupta
@ 2024-03-25 14:54 ` Jens Axboe
0 siblings, 0 replies; 30+ messages in thread
From: Jens Axboe @ 2024-03-25 14:54 UTC (permalink / raw)
To: Anuj gupta; +Cc: io-uring
On 3/25/24 6:03 AM, Anuj gupta wrote:
>> +static int io_prep_rw_setup(struct io_kiocb *req, int ddir, bool do_import)
>> +{
>> + struct io_async_rw *rw;
>> + int ret;
>> +
>> + if (io_rw_alloc_async(req))
>> + return -ENOMEM;
>> +
>> + if (!do_import || io_do_buffer_select(req))
>> + return 0;
>> +
>> + rw = req->async_data;
>> + ret = io_import_iovec(ddir, req, rw, 0);
>> + if (unlikely(ret < 0))
>> + return ret;
>> +
>> + iov_iter_save_state(&rw->s.iter, &rw->s.iter_state);
>
> It seems that the state of iov_iter gets saved in the caller io_import_iovec
> as well. Do we need to save it again here?
There's probably room left for further cleaning this up, as now the
state is always going to be known. But it isn't true right now - for
example, __io_read() will import and not save, yet do a restore if we
need to ensure the state is sane again.
--
Jens Axboe
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 11/17] io_uring: get rid of struct io_rw_state
2024-03-20 22:55 [PATCHSET v2 0/17] Improve async state handling Jens Axboe
` (9 preceding siblings ...)
2024-03-20 22:55 ` [PATCH 10/17] io_uring/rw: always setup io_async_rw for read/write requests Jens Axboe
@ 2024-03-20 22:55 ` Jens Axboe
2024-03-20 22:55 ` [PATCH 12/17] io_uring/rw: add iovec recycling Jens Axboe
` (5 subsequent siblings)
16 siblings, 0 replies; 30+ messages in thread
From: Jens Axboe @ 2024-03-20 22:55 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe
A separate state struct is not needed anymore, just fold it in with
io_async_rw.
Signed-off-by: Jens Axboe <[email protected]>
---
io_uring/rw.c | 45 +++++++++++++++++++++++----------------------
io_uring/rw.h | 10 +++-------
2 files changed, 26 insertions(+), 29 deletions(-)
diff --git a/io_uring/rw.c b/io_uring/rw.c
index 583fe61a0acb..19e866929cd3 100644
--- a/io_uring/rw.c
+++ b/io_uring/rw.c
@@ -96,12 +96,12 @@ static int __io_import_iovec(int ddir, struct io_kiocb *req,
rw->len = sqe_len;
}
- return import_ubuf(ddir, buf, sqe_len, &io->s.iter);
+ return import_ubuf(ddir, buf, sqe_len, &io->iter);
}
- io->free_iovec = io->s.fast_iov;
+ io->free_iovec = io->fast_iov;
return __import_iovec(ddir, buf, sqe_len, UIO_FASTIOV, &io->free_iovec,
- &io->s.iter, req->ctx->compat);
+ &io->iter, req->ctx->compat);
}
static inline int io_import_iovec(int rw, struct io_kiocb *req,
@@ -114,7 +114,7 @@ static inline int io_import_iovec(int rw, struct io_kiocb *req,
if (unlikely(ret < 0))
return ret;
- iov_iter_save_state(&io->s.iter, &io->s.iter_state);
+ iov_iter_save_state(&io->iter, &io->iter_state);
return 0;
}
@@ -216,7 +216,7 @@ static int io_prep_rw_setup(struct io_kiocb *req, int ddir, bool do_import)
if (unlikely(ret < 0))
return ret;
- iov_iter_save_state(&rw->s.iter, &rw->s.iter_state);
+ iov_iter_save_state(&rw->iter, &rw->iter_state);
return 0;
}
@@ -308,8 +308,8 @@ static int io_prep_rw_fixed(struct io_kiocb *req, const struct io_uring_sqe *sqe
io_req_set_rsrc_node(req, ctx, 0);
io = req->async_data;
- ret = io_import_fixed(ddir, &io->s.iter, req->imu, rw->addr, rw->len);
- iov_iter_save_state(&io->s.iter, &io->s.iter_state);
+ ret = io_import_fixed(ddir, &io->iter, req->imu, rw->addr, rw->len);
+ iov_iter_save_state(&io->iter, &io->iter_state);
return ret;
}
@@ -374,7 +374,7 @@ static void io_resubmit_prep(struct io_kiocb *req)
{
struct io_async_rw *io = req->async_data;
- iov_iter_restore(&io->s.iter, &io->s.iter_state);
+ iov_iter_restore(&io->iter, &io->iter_state);
}
static bool io_rw_should_reissue(struct io_kiocb *req)
@@ -808,7 +808,7 @@ static int __io_read(struct io_kiocb *req, unsigned int issue_flags)
ret = io_rw_init_file(req, FMODE_READ);
if (unlikely(ret))
return ret;
- req->cqe.res = iov_iter_count(&io->s.iter);
+ req->cqe.res = iov_iter_count(&io->iter);
if (force_nonblock) {
/* If the file doesn't support async, just async punt */
@@ -826,7 +826,7 @@ static int __io_read(struct io_kiocb *req, unsigned int issue_flags)
if (unlikely(ret))
return ret;
- ret = io_iter_do_read(rw, &io->s.iter);
+ ret = io_iter_do_read(rw, &io->iter);
if (ret == -EAGAIN || (req->flags & REQ_F_REISSUE)) {
req->flags &= ~REQ_F_REISSUE;
@@ -853,7 +853,7 @@ static int __io_read(struct io_kiocb *req, unsigned int issue_flags)
* untouched in case of error. Restore it and we'll advance it
* manually if we need to.
*/
- iov_iter_restore(&io->s.iter, &io->s.iter_state);
+ iov_iter_restore(&io->iter, &io->iter_state);
do {
/*
@@ -861,11 +861,11 @@ static int __io_read(struct io_kiocb *req, unsigned int issue_flags)
* above or inside this loop. Advance the iter by the bytes
* that were consumed.
*/
- iov_iter_advance(&io->s.iter, ret);
- if (!iov_iter_count(&io->s.iter))
+ iov_iter_advance(&io->iter, ret);
+ if (!iov_iter_count(&io->iter))
break;
io->bytes_done += ret;
- iov_iter_save_state(&io->s.iter, &io->s.iter_state);
+ iov_iter_save_state(&io->iter, &io->iter_state);
/* if we can retry, do so with the callbacks armed */
if (!io_rw_should_retry(req)) {
@@ -873,19 +873,19 @@ static int __io_read(struct io_kiocb *req, unsigned int issue_flags)
return -EAGAIN;
}
- req->cqe.res = iov_iter_count(&io->s.iter);
+ req->cqe.res = iov_iter_count(&io->iter);
/*
* Now retry read with the IOCB_WAITQ parts set in the iocb. If
* we get -EIOCBQUEUED, then we'll get a notification when the
* desired page gets unlocked. We can also get a partial read
* here, and if we do, then just retry at the new offset.
*/
- ret = io_iter_do_read(rw, &io->s.iter);
+ ret = io_iter_do_read(rw, &io->iter);
if (ret == -EIOCBQUEUED)
return IOU_ISSUE_SKIP_COMPLETE;
/* we got some bytes, but not all. retry. */
kiocb->ki_flags &= ~IOCB_WAITQ;
- iov_iter_restore(&io->s.iter, &io->s.iter_state);
+ iov_iter_restore(&io->iter, &io->iter_state);
} while (ret > 0);
done:
/* it's faster to check here then delegate to kfree */
@@ -982,7 +982,7 @@ int io_write(struct io_kiocb *req, unsigned int issue_flags)
ret = io_rw_init_file(req, FMODE_WRITE);
if (unlikely(ret))
return ret;
- req->cqe.res = iov_iter_count(&io->s.iter);
+ req->cqe.res = iov_iter_count(&io->iter);
if (force_nonblock) {
/* If the file doesn't support async, just async punt */
@@ -1012,9 +1012,9 @@ int io_write(struct io_kiocb *req, unsigned int issue_flags)
kiocb->ki_flags |= IOCB_WRITE;
if (likely(req->file->f_op->write_iter))
- ret2 = call_write_iter(req->file, kiocb, &io->s.iter);
+ ret2 = call_write_iter(req->file, kiocb, &io->iter);
else if (req->file->f_op->write)
- ret2 = loop_rw_iter(WRITE, rw, &io->s.iter);
+ ret2 = loop_rw_iter(WRITE, rw, &io->iter);
else
ret2 = -EINVAL;
@@ -1046,7 +1046,7 @@ int io_write(struct io_kiocb *req, unsigned int issue_flags)
* in the worker. Also update bytes_done to account for
* the bytes already written.
*/
- iov_iter_save_state(&io->s.iter, &io->s.iter_state);
+ iov_iter_save_state(&io->iter, &io->iter_state);
io->bytes_done += ret2;
if (kiocb->ki_flags & IOCB_WRITE)
@@ -1057,7 +1057,7 @@ int io_write(struct io_kiocb *req, unsigned int issue_flags)
ret = kiocb_done(req, ret2, issue_flags);
} else {
ret_eagain:
- iov_iter_restore(&io->s.iter, &io->s.iter_state);
+ iov_iter_restore(&io->iter, &io->iter_state);
if (kiocb->ki_flags & IOCB_WRITE)
io_req_end_write(req);
return -EAGAIN;
@@ -1157,5 +1157,6 @@ void io_rw_cache_free(struct io_cache_entry *entry)
struct io_async_rw *rw;
rw = container_of(entry, struct io_async_rw, cache);
+ kfree(rw->free_iovec);
kfree(rw);
}
diff --git a/io_uring/rw.h b/io_uring/rw.h
index f7905070d10b..7824896dc52d 100644
--- a/io_uring/rw.h
+++ b/io_uring/rw.h
@@ -2,18 +2,14 @@
#include <linux/pagemap.h>
-struct io_rw_state {
- struct iov_iter iter;
- struct iov_iter_state iter_state;
- struct iovec fast_iov[UIO_FASTIOV];
-};
-
struct io_async_rw {
union {
size_t bytes_done;
struct io_cache_entry cache;
};
- struct io_rw_state s;
+ struct iov_iter iter;
+ struct iov_iter_state iter_state;
+ struct iovec fast_iov[UIO_FASTIOV];
struct iovec *free_iovec;
struct wait_page_queue wpq;
};
--
2.43.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH 12/17] io_uring/rw: add iovec recycling
2024-03-20 22:55 [PATCHSET v2 0/17] Improve async state handling Jens Axboe
` (10 preceding siblings ...)
2024-03-20 22:55 ` [PATCH 11/17] io_uring: get rid of struct io_rw_state Jens Axboe
@ 2024-03-20 22:55 ` Jens Axboe
2024-03-20 22:55 ` [PATCH 13/17] io_uring/net: move connect to always using async data Jens Axboe
` (4 subsequent siblings)
16 siblings, 0 replies; 30+ messages in thread
From: Jens Axboe @ 2024-03-20 22:55 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe
Let the io_async_rw hold on to the iovec and reuse it, rather than always
allocate and free them.
Also enables KASAN for the iovec entries, so that reuse can be detected
even while they are in the cache.
While doing so, shrink io_async_rw by getting rid of the bigger embedded
fast iovec. Since iovecs are being recycled now, shrink it from 8 to 1.
This reduces the io_async_rw size from 264 to 160 bytes, a 40% reduction.
Signed-off-by: Jens Axboe <[email protected]>
---
io_uring/rw.c | 42 +++++++++++++++++++++++++++++++++++++-----
io_uring/rw.h | 3 ++-
2 files changed, 39 insertions(+), 6 deletions(-)
diff --git a/io_uring/rw.c b/io_uring/rw.c
index 19e866929cd3..57f2d315a620 100644
--- a/io_uring/rw.c
+++ b/io_uring/rw.c
@@ -81,7 +81,9 @@ static int __io_import_iovec(int ddir, struct io_kiocb *req,
{
const struct io_issue_def *def = &io_issue_defs[req->opcode];
struct io_rw *rw = io_kiocb_to_cmd(req, struct io_rw);
+ struct iovec *iov;
void __user *buf;
+ int nr_segs, ret;
size_t sqe_len;
buf = u64_to_user_ptr(rw->addr);
@@ -99,9 +101,24 @@ static int __io_import_iovec(int ddir, struct io_kiocb *req,
return import_ubuf(ddir, buf, sqe_len, &io->iter);
}
- io->free_iovec = io->fast_iov;
- return __import_iovec(ddir, buf, sqe_len, UIO_FASTIOV, &io->free_iovec,
- &io->iter, req->ctx->compat);
+ if (io->free_iovec) {
+ nr_segs = io->free_iov_nr;
+ iov = io->free_iovec;
+ } else {
+ iov = &io->fast_iov;
+ nr_segs = 1;
+ }
+ ret = __import_iovec(ddir, buf, sqe_len, nr_segs, &iov, &io->iter,
+ req->ctx->compat);
+ if (unlikely(ret < 0))
+ return ret;
+ if (iov) {
+ req->flags |= REQ_F_NEED_CLEANUP;
+ io->free_iov_nr = io->iter.nr_segs;
+ kfree(io->free_iovec);
+ io->free_iovec = iov;
+ }
+ return 0;
}
static inline int io_import_iovec(int rw, struct io_kiocb *req,
@@ -122,6 +139,7 @@ static void io_rw_iovec_free(struct io_async_rw *rw)
{
if (rw->free_iovec) {
kfree(rw->free_iovec);
+ rw->free_iov_nr = 0;
rw->free_iovec = NULL;
}
}
@@ -129,12 +147,16 @@ static void io_rw_iovec_free(struct io_async_rw *rw)
static void io_rw_recycle(struct io_kiocb *req, unsigned int issue_flags)
{
struct io_async_rw *rw = req->async_data;
+ struct iovec *iov;
if (unlikely(issue_flags & IO_URING_F_UNLOCKED)) {
io_rw_iovec_free(rw);
return;
}
+ iov = rw->free_iovec;
if (io_alloc_cache_put(&req->ctx->rw_cache, &rw->cache)) {
+ if (iov)
+ kasan_mempool_poison_object(iov);
req->async_data = NULL;
req->flags &= ~REQ_F_ASYNC_DATA;
}
@@ -184,6 +206,11 @@ static int io_rw_alloc_async(struct io_kiocb *req)
entry = io_alloc_cache_get(&ctx->rw_cache);
if (entry) {
rw = container_of(entry, struct io_async_rw, cache);
+ if (rw->free_iovec) {
+ kasan_mempool_unpoison_object(rw->free_iovec,
+ rw->free_iov_nr * sizeof(struct iovec));
+ req->flags |= REQ_F_NEED_CLEANUP;
+ }
req->flags |= REQ_F_ASYNC_DATA;
req->async_data = rw;
goto done;
@@ -191,8 +218,9 @@ static int io_rw_alloc_async(struct io_kiocb *req)
if (!io_alloc_async_data(req)) {
rw = req->async_data;
-done:
rw->free_iovec = NULL;
+ rw->free_iov_nr = 0;
+done:
rw->bytes_done = 0;
return 0;
}
@@ -1157,6 +1185,10 @@ void io_rw_cache_free(struct io_cache_entry *entry)
struct io_async_rw *rw;
rw = container_of(entry, struct io_async_rw, cache);
- kfree(rw->free_iovec);
+ if (rw->free_iovec) {
+ kasan_mempool_unpoison_object(rw->free_iovec,
+ rw->free_iov_nr * sizeof(struct iovec));
+ io_rw_iovec_free(rw);
+ }
kfree(rw);
}
diff --git a/io_uring/rw.h b/io_uring/rw.h
index 7824896dc52d..cf51d0eb407a 100644
--- a/io_uring/rw.h
+++ b/io_uring/rw.h
@@ -9,8 +9,9 @@ struct io_async_rw {
};
struct iov_iter iter;
struct iov_iter_state iter_state;
- struct iovec fast_iov[UIO_FASTIOV];
+ struct iovec fast_iov;
struct iovec *free_iovec;
+ int free_iov_nr;
struct wait_page_queue wpq;
};
--
2.43.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH 13/17] io_uring/net: move connect to always using async data
2024-03-20 22:55 [PATCHSET v2 0/17] Improve async state handling Jens Axboe
` (11 preceding siblings ...)
2024-03-20 22:55 ` [PATCH 12/17] io_uring/rw: add iovec recycling Jens Axboe
@ 2024-03-20 22:55 ` Jens Axboe
2024-03-20 22:55 ` [PATCH 14/17] io_uring/uring_cmd: switch to always allocating " Jens Axboe
` (3 subsequent siblings)
16 siblings, 0 replies; 30+ messages in thread
From: Jens Axboe @ 2024-03-20 22:55 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe
While doing that, get rid of io_async_connect and just use the generic
io_async_msghdr. Both of them have a struct sockaddr_storage in there,
and while io_async_msghdr is bigger, if the same type can be used then
we get recycling for free.
Signed-off-by: Jens Axboe <[email protected]>
---
io_uring/net.c | 41 +++++++++++------------------------------
io_uring/net.h | 5 -----
io_uring/opdef.c | 3 +--
3 files changed, 12 insertions(+), 37 deletions(-)
diff --git a/io_uring/net.c b/io_uring/net.c
index 9472a66e035c..5794b941254c 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -1430,17 +1430,10 @@ int io_socket(struct io_kiocb *req, unsigned int issue_flags)
return IOU_OK;
}
-int io_connect_prep_async(struct io_kiocb *req)
-{
- struct io_async_connect *io = req->async_data;
- struct io_connect *conn = io_kiocb_to_cmd(req, struct io_connect);
-
- return move_addr_to_kernel(conn->addr, conn->addr_len, &io->address);
-}
-
int io_connect_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
{
struct io_connect *conn = io_kiocb_to_cmd(req, struct io_connect);
+ struct io_async_msghdr *io;
if (sqe->len || sqe->buf_index || sqe->rw_flags || sqe->splice_fd_in)
return -EINVAL;
@@ -1448,32 +1441,26 @@ int io_connect_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
conn->addr = u64_to_user_ptr(READ_ONCE(sqe->addr));
conn->addr_len = READ_ONCE(sqe->addr2);
conn->in_progress = conn->seen_econnaborted = false;
- return 0;
+
+ io = io_msg_alloc_async(req);
+ if (unlikely(!io))
+ return -ENOMEM;
+
+ return move_addr_to_kernel(conn->addr, conn->addr_len, &io->addr);
}
int io_connect(struct io_kiocb *req, unsigned int issue_flags)
{
struct io_connect *connect = io_kiocb_to_cmd(req, struct io_connect);
- struct io_async_connect __io, *io;
+ struct io_async_msghdr *io = req->async_data;
unsigned file_flags;
int ret;
bool force_nonblock = issue_flags & IO_URING_F_NONBLOCK;
- if (req_has_async_data(req)) {
- io = req->async_data;
- } else {
- ret = move_addr_to_kernel(connect->addr,
- connect->addr_len,
- &__io.address);
- if (ret)
- goto out;
- io = &__io;
- }
-
file_flags = force_nonblock ? O_NONBLOCK : 0;
- ret = __sys_connect_file(req->file, &io->address,
- connect->addr_len, file_flags);
+ ret = __sys_connect_file(req->file, &io->addr, connect->addr_len,
+ file_flags);
if ((ret == -EAGAIN || ret == -EINPROGRESS || ret == -ECONNABORTED)
&& force_nonblock) {
if (ret == -EINPROGRESS) {
@@ -1483,13 +1470,6 @@ int io_connect(struct io_kiocb *req, unsigned int issue_flags)
goto out;
connect->seen_econnaborted = true;
}
- if (req_has_async_data(req))
- return -EAGAIN;
- if (io_alloc_async_data(req)) {
- ret = -ENOMEM;
- goto out;
- }
- memcpy(req->async_data, &__io, sizeof(__io));
return -EAGAIN;
}
if (connect->in_progress) {
@@ -1507,6 +1487,7 @@ int io_connect(struct io_kiocb *req, unsigned int issue_flags)
out:
if (ret < 0)
req_set_fail(req);
+ io_req_msg_cleanup(req, issue_flags);
io_req_set_res(req, ret, 0);
return IOU_OK;
}
diff --git a/io_uring/net.h b/io_uring/net.h
index 0aef1c992aee..b47b43ec6459 100644
--- a/io_uring/net.h
+++ b/io_uring/net.h
@@ -28,10 +28,6 @@ struct io_async_msghdr {
#if defined(CONFIG_NET)
-struct io_async_connect {
- struct sockaddr_storage address;
-};
-
int io_shutdown_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
int io_shutdown(struct io_kiocb *req, unsigned int issue_flags);
@@ -53,7 +49,6 @@ int io_accept(struct io_kiocb *req, unsigned int issue_flags);
int io_socket_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
int io_socket(struct io_kiocb *req, unsigned int issue_flags);
-int io_connect_prep_async(struct io_kiocb *req);
int io_connect_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
int io_connect(struct io_kiocb *req, unsigned int issue_flags);
diff --git a/io_uring/opdef.c b/io_uring/opdef.c
index fcae75a08f2c..1951107210d4 100644
--- a/io_uring/opdef.c
+++ b/io_uring/opdef.c
@@ -557,8 +557,7 @@ const struct io_cold_def io_cold_defs[] = {
[IORING_OP_CONNECT] = {
.name = "CONNECT",
#if defined(CONFIG_NET)
- .async_size = sizeof(struct io_async_connect),
- .prep_async = io_connect_prep_async,
+ .async_size = sizeof(struct io_async_msghdr),
#endif
},
[IORING_OP_FALLOCATE] = {
--
2.43.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH 14/17] io_uring/uring_cmd: switch to always allocating async data
2024-03-20 22:55 [PATCHSET v2 0/17] Improve async state handling Jens Axboe
` (12 preceding siblings ...)
2024-03-20 22:55 ` [PATCH 13/17] io_uring/net: move connect to always using async data Jens Axboe
@ 2024-03-20 22:55 ` Jens Axboe
2024-03-20 22:55 ` [PATCH 15/17] io_uring/uring_cmd: defer SQE copying until we need it Jens Axboe
` (2 subsequent siblings)
16 siblings, 0 replies; 30+ messages in thread
From: Jens Axboe @ 2024-03-20 22:55 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe
Basic conversion ensuring async_data is allocated off the prep path. Adds
a basic alloc cache as well, as passthrough IO can be quite high in rate.
Signed-off-by: Jens Axboe <[email protected]>
---
include/linux/io_uring_types.h | 1 +
io_uring/io_uring.c | 3 ++
io_uring/opdef.c | 1 -
io_uring/uring_cmd.c | 77 ++++++++++++++++++++++++----------
io_uring/uring_cmd.h | 10 ++++-
5 files changed, 69 insertions(+), 23 deletions(-)
diff --git a/include/linux/io_uring_types.h b/include/linux/io_uring_types.h
index 2ba8676f83cc..e3ec84c43f1a 100644
--- a/include/linux/io_uring_types.h
+++ b/include/linux/io_uring_types.h
@@ -301,6 +301,7 @@ struct io_ring_ctx {
struct io_alloc_cache apoll_cache;
struct io_alloc_cache netmsg_cache;
struct io_alloc_cache rw_cache;
+ struct io_alloc_cache uring_cache;
/*
* Any cancelable uring_cmd is added to this list in
diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index cc8ce830ff4b..e2b9b00eedef 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -310,6 +310,8 @@ static __cold struct io_ring_ctx *io_ring_ctx_alloc(struct io_uring_params *p)
sizeof(struct io_async_msghdr));
io_alloc_cache_init(&ctx->rw_cache, IO_ALLOC_CACHE_MAX,
sizeof(struct io_async_rw));
+ io_alloc_cache_init(&ctx->uring_cache, IO_ALLOC_CACHE_MAX,
+ sizeof(struct uring_cache));
io_futex_cache_init(ctx);
init_completion(&ctx->ref_comp);
xa_init_flags(&ctx->personalities, XA_FLAGS_ALLOC1);
@@ -2901,6 +2903,7 @@ static __cold void io_ring_ctx_free(struct io_ring_ctx *ctx)
io_alloc_cache_free(&ctx->apoll_cache, io_apoll_cache_free);
io_alloc_cache_free(&ctx->netmsg_cache, io_netmsg_cache_free);
io_alloc_cache_free(&ctx->rw_cache, io_rw_cache_free);
+ io_alloc_cache_free(&ctx->uring_cache, io_uring_cache_free);
io_futex_cache_free(ctx);
io_destroy_buffers(ctx);
mutex_unlock(&ctx->uring_lock);
diff --git a/io_uring/opdef.c b/io_uring/opdef.c
index 1951107210d4..745246086c23 100644
--- a/io_uring/opdef.c
+++ b/io_uring/opdef.c
@@ -677,7 +677,6 @@ const struct io_cold_def io_cold_defs[] = {
[IORING_OP_URING_CMD] = {
.name = "URING_CMD",
.async_size = 2 * sizeof(struct io_uring_sqe),
- .prep_async = io_uring_cmd_prep_async,
},
[IORING_OP_SEND_ZC] = {
.name = "SEND_ZC",
diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c
index 4614ce734fee..9bd0ba87553f 100644
--- a/io_uring/uring_cmd.c
+++ b/io_uring/uring_cmd.c
@@ -14,6 +14,38 @@
#include "rsrc.h"
#include "uring_cmd.h"
+static struct uring_cache *io_uring_async_get(struct io_kiocb *req)
+{
+ struct io_ring_ctx *ctx = req->ctx;
+ struct io_cache_entry *entry;
+ struct uring_cache *cache;
+
+ entry = io_alloc_cache_get(&ctx->uring_cache);
+ if (entry) {
+ cache = container_of(entry, struct uring_cache, cache);
+ req->flags |= REQ_F_ASYNC_DATA;
+ req->async_data = cache;
+ return cache;
+ }
+ if (!io_alloc_async_data(req))
+ return req->async_data;
+ return NULL;
+}
+
+static void io_req_uring_cleanup(struct io_kiocb *req, unsigned int issue_flags)
+{
+ struct io_uring_cmd *ioucmd = io_kiocb_to_cmd(req, struct io_uring_cmd);
+ struct uring_cache *cache = req->async_data;
+
+ if (issue_flags & IO_URING_F_UNLOCKED)
+ return;
+ if (io_alloc_cache_put(&req->ctx->uring_cache, &cache->cache)) {
+ ioucmd->sqe = NULL;
+ req->async_data = NULL;
+ req->flags &= ~REQ_F_ASYNC_DATA;
+ }
+}
+
bool io_uring_try_cancel_uring_cmd(struct io_ring_ctx *ctx,
struct task_struct *task, bool cancel_all)
{
@@ -128,6 +160,7 @@ void io_uring_cmd_done(struct io_uring_cmd *ioucmd, ssize_t ret, ssize_t res2,
io_req_set_res(req, ret, 0);
if (req->ctx->flags & IORING_SETUP_CQE32)
io_req_set_cqe32_extra(req, res2, 0);
+ io_req_uring_cleanup(req, issue_flags);
if (req->ctx->flags & IORING_SETUP_IOPOLL) {
/* order with io_iopoll_req_issued() checking ->iopoll_complete */
smp_store_release(&req->iopoll_completed, 1);
@@ -142,13 +175,19 @@ void io_uring_cmd_done(struct io_uring_cmd *ioucmd, ssize_t ret, ssize_t res2,
}
EXPORT_SYMBOL_GPL(io_uring_cmd_done);
-int io_uring_cmd_prep_async(struct io_kiocb *req)
+static int io_uring_cmd_prep_setup(struct io_kiocb *req,
+ const struct io_uring_sqe *sqe)
{
struct io_uring_cmd *ioucmd = io_kiocb_to_cmd(req, struct io_uring_cmd);
+ struct uring_cache *cache;
- memcpy(req->async_data, ioucmd->sqe, uring_sqe_size(req->ctx));
- ioucmd->sqe = req->async_data;
- return 0;
+ cache = io_uring_async_get(req);
+ if (cache) {
+ memcpy(cache->sqes, sqe, uring_sqe_size(req->ctx));
+ ioucmd->sqe = req->async_data;
+ return 0;
+ }
+ return -ENOMEM;
}
int io_uring_cmd_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
@@ -173,9 +212,9 @@ int io_uring_cmd_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
req->imu = ctx->user_bufs[index];
io_req_set_rsrc_node(req, ctx, 0);
}
- ioucmd->sqe = sqe;
ioucmd->cmd_op = READ_ONCE(sqe->cmd_op);
- return 0;
+
+ return io_uring_cmd_prep_setup(req, sqe);
}
int io_uring_cmd(struct io_kiocb *req, unsigned int issue_flags)
@@ -206,23 +245,14 @@ int io_uring_cmd(struct io_kiocb *req, unsigned int issue_flags)
}
ret = file->f_op->uring_cmd(ioucmd, issue_flags);
- if (ret == -EAGAIN) {
- if (!req_has_async_data(req)) {
- if (io_alloc_async_data(req))
- return -ENOMEM;
- io_uring_cmd_prep_async(req);
- }
- return -EAGAIN;
- }
-
- if (ret != -EIOCBQUEUED) {
- if (ret < 0)
- req_set_fail(req);
- io_req_set_res(req, ret, 0);
+ if (ret == -EAGAIN || ret == -EIOCBQUEUED)
return ret;
- }
- return IOU_ISSUE_SKIP_COMPLETE;
+ if (ret < 0)
+ req_set_fail(req);
+ io_req_uring_cleanup(req, issue_flags);
+ io_req_set_res(req, ret, 0);
+ return ret;
}
int io_uring_cmd_import_fixed(u64 ubuf, unsigned long len, int rw,
@@ -311,3 +341,8 @@ int io_uring_cmd_sock(struct io_uring_cmd *cmd, unsigned int issue_flags)
}
EXPORT_SYMBOL_GPL(io_uring_cmd_sock);
#endif
+
+void io_uring_cache_free(struct io_cache_entry *entry)
+{
+ kfree(container_of(entry, struct uring_cache, cache));
+}
diff --git a/io_uring/uring_cmd.h b/io_uring/uring_cmd.h
index 7356bf9aa655..b0ccff7091ee 100644
--- a/io_uring/uring_cmd.h
+++ b/io_uring/uring_cmd.h
@@ -1,8 +1,16 @@
// SPDX-License-Identifier: GPL-2.0
+struct uring_cache {
+ union {
+ struct io_cache_entry cache;
+ struct io_uring_sqe sqes[2];
+ };
+};
+
int io_uring_cmd(struct io_kiocb *req, unsigned int issue_flags);
int io_uring_cmd_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
int io_uring_cmd_prep_async(struct io_kiocb *req);
+void io_uring_cache_free(struct io_cache_entry *entry);
bool io_uring_try_cancel_uring_cmd(struct io_ring_ctx *ctx,
- struct task_struct *task, bool cancel_all);
\ No newline at end of file
+ struct task_struct *task, bool cancel_all);
--
2.43.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* [PATCH 15/17] io_uring/uring_cmd: defer SQE copying until we need it
2024-03-20 22:55 [PATCHSET v2 0/17] Improve async state handling Jens Axboe
` (13 preceding siblings ...)
2024-03-20 22:55 ` [PATCH 14/17] io_uring/uring_cmd: switch to always allocating " Jens Axboe
@ 2024-03-20 22:55 ` Jens Axboe
2024-03-25 12:41 ` Anuj gupta
2024-03-20 22:55 ` [PATCH 16/17] io_uring: drop ->prep_async() Jens Axboe
2024-03-20 22:55 ` [PATCH 17/17] io_uring/alloc_cache: switch to array based caching Jens Axboe
16 siblings, 1 reply; 30+ messages in thread
From: Jens Axboe @ 2024-03-20 22:55 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe
The previous commit turned on async data for uring_cmd, and did the
basic conversion of setting everything up on the prep side. However, for
a lot of use cases, we'll get -EIOCBQUEUED on issue, which means we do
not need a persistent big SQE copied.
Unless we're going async immediately, defer copying the double SQE until
we know we have to.
This greatly reduces the overhead of such commands, as evidenced by
a perf diff from before and after this change:
10.60% -8.58% [kernel.vmlinux] [k] io_uring_cmd_prep
where the prep side drops from 10.60% to ~2%, which is more expected.
Performance also rises from ~113M IOPS to ~122M IOPS, bringing us back
to where it was before the async command prep.
Signed-off-by: Jens Axboe <[email protected]>
~# Last command done (1 command done):
---
io_uring/uring_cmd.c | 25 +++++++++++++++++++------
1 file changed, 19 insertions(+), 6 deletions(-)
diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c
index 9bd0ba87553f..92346b5d9f5b 100644
--- a/io_uring/uring_cmd.c
+++ b/io_uring/uring_cmd.c
@@ -182,12 +182,18 @@ static int io_uring_cmd_prep_setup(struct io_kiocb *req,
struct uring_cache *cache;
cache = io_uring_async_get(req);
- if (cache) {
- memcpy(cache->sqes, sqe, uring_sqe_size(req->ctx));
- ioucmd->sqe = req->async_data;
+ if (unlikely(!cache))
+ return -ENOMEM;
+
+ if (!(req->flags & REQ_F_FORCE_ASYNC)) {
+ /* defer memcpy until we need it */
+ ioucmd->sqe = sqe;
return 0;
}
- return -ENOMEM;
+
+ memcpy(req->async_data, sqe, uring_sqe_size(req->ctx));
+ ioucmd->sqe = req->async_data;
+ return 0;
}
int io_uring_cmd_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
@@ -245,8 +251,15 @@ int io_uring_cmd(struct io_kiocb *req, unsigned int issue_flags)
}
ret = file->f_op->uring_cmd(ioucmd, issue_flags);
- if (ret == -EAGAIN || ret == -EIOCBQUEUED)
- return ret;
+ if (ret == -EAGAIN) {
+ struct uring_cache *cache = req->async_data;
+
+ if (ioucmd->sqe != (void *) cache)
+ memcpy(cache, ioucmd->sqe, uring_sqe_size(req->ctx));
+ return -EAGAIN;
+ } else if (ret == -EIOCBQUEUED) {
+ return -EIOCBQUEUED;
+ }
if (ret < 0)
req_set_fail(req);
--
2.43.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH 15/17] io_uring/uring_cmd: defer SQE copying until we need it
2024-03-20 22:55 ` [PATCH 15/17] io_uring/uring_cmd: defer SQE copying until we need it Jens Axboe
@ 2024-03-25 12:41 ` Anuj gupta
2024-03-25 14:55 ` Jens Axboe
0 siblings, 1 reply; 30+ messages in thread
From: Anuj gupta @ 2024-03-25 12:41 UTC (permalink / raw)
To: Jens Axboe; +Cc: io-uring
On Thu, Mar 21, 2024 at 4:28 AM Jens Axboe <[email protected]> wrote:
>
> The previous commit turned on async data for uring_cmd, and did the
> basic conversion of setting everything up on the prep side. However, for
> a lot of use cases, we'll get -EIOCBQUEUED on issue, which means we do
> not need a persistent big SQE copied.
>
> Unless we're going async immediately, defer copying the double SQE until
> we know we have to.
>
> This greatly reduces the overhead of such commands, as evidenced by
> a perf diff from before and after this change:
>
> 10.60% -8.58% [kernel.vmlinux] [k] io_uring_cmd_prep
>
> where the prep side drops from 10.60% to ~2%, which is more expected.
> Performance also rises from ~113M IOPS to ~122M IOPS, bringing us back
> to where it was before the async command prep.
>
> Signed-off-by: Jens Axboe <[email protected]>
>
> ~# Last command done (1 command done):
> ---
> io_uring/uring_cmd.c | 25 +++++++++++++++++++------
> 1 file changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c
> index 9bd0ba87553f..92346b5d9f5b 100644
> --- a/io_uring/uring_cmd.c
> +++ b/io_uring/uring_cmd.c
> @@ -182,12 +182,18 @@ static int io_uring_cmd_prep_setup(struct io_kiocb *req,
> struct uring_cache *cache;
>
> cache = io_uring_async_get(req);
> - if (cache) {
> - memcpy(cache->sqes, sqe, uring_sqe_size(req->ctx));
> - ioucmd->sqe = req->async_data;
> + if (unlikely(!cache))
> + return -ENOMEM;
> +
> + if (!(req->flags & REQ_F_FORCE_ASYNC)) {
> + /* defer memcpy until we need it */
> + ioucmd->sqe = sqe;
> return 0;
> }
> - return -ENOMEM;
> +
> + memcpy(req->async_data, sqe, uring_sqe_size(req->ctx));
> + ioucmd->sqe = req->async_data;
> + return 0;
> }
>
> int io_uring_cmd_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
> @@ -245,8 +251,15 @@ int io_uring_cmd(struct io_kiocb *req, unsigned int issue_flags)
> }
>
> ret = file->f_op->uring_cmd(ioucmd, issue_flags);
> - if (ret == -EAGAIN || ret == -EIOCBQUEUED)
> - return ret;
> + if (ret == -EAGAIN) {
> + struct uring_cache *cache = req->async_data;
> +
> + if (ioucmd->sqe != (void *) cache)
> + memcpy(cache, ioucmd->sqe, uring_sqe_size(req->ctx));
> + return -EAGAIN;
> + } else if (ret == -EIOCBQUEUED) {
> + return -EIOCBQUEUED;
> + }
>
> if (ret < 0)
> req_set_fail(req);
> --
> 2.43.0
>
>
The io_uring_cmd plumbing part of this series looks good to me.
I tested it with io_uring nvme-passthrough on my setup with two
optanes and there is no drop in performance as well [1].
For this and the previous patch,
Tested-by: Anuj Gupta <[email protected]>
Reviewed-by: Anuj Gupta <[email protected]>
[1]
# taskset -c 2,5 t/io_uring -b512 -d128 -c32 -s32 -p1 -O0 -F1 -B1 -u1
-n2 -r4 /dev/ng0n1 /dev/ng2n1
submitter=1, tid=7166, file=/dev/ng2n1, nfiles=1, node=-1
submitter=0, tid=7165, file=/dev/ng0n1, nfiles=1, node=-1
polled=1, fixedbufs=1, register_files=1, buffered=1, QD=128
Engine=io_uring, sq_ring=128, cq_ring=128
IOPS=10.02M, BW=4.89GiB/s, IOS/call=31/31
IOPS=10.04M, BW=4.90GiB/s, IOS/call=31/31
IOPS=10.04M, BW=4.90GiB/s, IOS/call=31/31
Exiting on timeout
Maximum IOPS=10.04M
--
Anuj Gupta
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH 15/17] io_uring/uring_cmd: defer SQE copying until we need it
2024-03-25 12:41 ` Anuj gupta
@ 2024-03-25 14:55 ` Jens Axboe
0 siblings, 0 replies; 30+ messages in thread
From: Jens Axboe @ 2024-03-25 14:55 UTC (permalink / raw)
To: Anuj gupta; +Cc: io-uring
On 3/25/24 6:41 AM, Anuj gupta wrote:
> On Thu, Mar 21, 2024 at 4:28?AM Jens Axboe <[email protected]> wrote:
>>
>> The previous commit turned on async data for uring_cmd, and did the
>> basic conversion of setting everything up on the prep side. However, for
>> a lot of use cases, we'll get -EIOCBQUEUED on issue, which means we do
>> not need a persistent big SQE copied.
>>
>> Unless we're going async immediately, defer copying the double SQE until
>> we know we have to.
>>
>> This greatly reduces the overhead of such commands, as evidenced by
>> a perf diff from before and after this change:
>>
>> 10.60% -8.58% [kernel.vmlinux] [k] io_uring_cmd_prep
>>
>> where the prep side drops from 10.60% to ~2%, which is more expected.
>> Performance also rises from ~113M IOPS to ~122M IOPS, bringing us back
>> to where it was before the async command prep.
>>
>> Signed-off-by: Jens Axboe <[email protected]>
>>
>> ~# Last command done (1 command done):
>> ---
>> io_uring/uring_cmd.c | 25 +++++++++++++++++++------
>> 1 file changed, 19 insertions(+), 6 deletions(-)
>>
>> diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c
>> index 9bd0ba87553f..92346b5d9f5b 100644
>> --- a/io_uring/uring_cmd.c
>> +++ b/io_uring/uring_cmd.c
>> @@ -182,12 +182,18 @@ static int io_uring_cmd_prep_setup(struct io_kiocb *req,
>> struct uring_cache *cache;
>>
>> cache = io_uring_async_get(req);
>> - if (cache) {
>> - memcpy(cache->sqes, sqe, uring_sqe_size(req->ctx));
>> - ioucmd->sqe = req->async_data;
>> + if (unlikely(!cache))
>> + return -ENOMEM;
>> +
>> + if (!(req->flags & REQ_F_FORCE_ASYNC)) {
>> + /* defer memcpy until we need it */
>> + ioucmd->sqe = sqe;
>> return 0;
>> }
>> - return -ENOMEM;
>> +
>> + memcpy(req->async_data, sqe, uring_sqe_size(req->ctx));
>> + ioucmd->sqe = req->async_data;
>> + return 0;
>> }
>>
>> int io_uring_cmd_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>> @@ -245,8 +251,15 @@ int io_uring_cmd(struct io_kiocb *req, unsigned int issue_flags)
>> }
>>
>> ret = file->f_op->uring_cmd(ioucmd, issue_flags);
>> - if (ret == -EAGAIN || ret == -EIOCBQUEUED)
>> - return ret;
>> + if (ret == -EAGAIN) {
>> + struct uring_cache *cache = req->async_data;
>> +
>> + if (ioucmd->sqe != (void *) cache)
>> + memcpy(cache, ioucmd->sqe, uring_sqe_size(req->ctx));
>> + return -EAGAIN;
>> + } else if (ret == -EIOCBQUEUED) {
>> + return -EIOCBQUEUED;
>> + }
>>
>> if (ret < 0)
>> req_set_fail(req);
>> --
>> 2.43.0
>>
>>
>
> The io_uring_cmd plumbing part of this series looks good to me.
> I tested it with io_uring nvme-passthrough on my setup with two
> optanes and there is no drop in performance as well [1].
> For this and the previous patch,
>
> Tested-by: Anuj Gupta <[email protected]>
> Reviewed-by: Anuj Gupta <[email protected]>
Thanks for reviewing and testing, will add.
--
Jens Axboe
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 16/17] io_uring: drop ->prep_async()
2024-03-20 22:55 [PATCHSET v2 0/17] Improve async state handling Jens Axboe
` (14 preceding siblings ...)
2024-03-20 22:55 ` [PATCH 15/17] io_uring/uring_cmd: defer SQE copying until we need it Jens Axboe
@ 2024-03-20 22:55 ` Jens Axboe
2024-04-06 20:54 ` Pavel Begunkov
2024-03-20 22:55 ` [PATCH 17/17] io_uring/alloc_cache: switch to array based caching Jens Axboe
16 siblings, 1 reply; 30+ messages in thread
From: Jens Axboe @ 2024-03-20 22:55 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe
It's now unused, drop the code related to it. This includes the
io_issue_defs->manual alloc field.
While in there, and since ->async_size is now being used a bit more
frequently and in the issue path, move it to io_issue_defs[].
Signed-off-by: Jens Axboe <[email protected]>
---
io_uring/io_uring.c | 36 ++++--------------------------------
io_uring/io_uring.h | 1 -
io_uring/opdef.c | 44 +++++++++++++++++++-------------------------
io_uring/opdef.h | 9 +++------
io_uring/uring_cmd.h | 1 -
5 files changed, 26 insertions(+), 65 deletions(-)
diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index e2b9b00eedef..5eee07563079 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -1709,8 +1709,10 @@ io_req_flags_t io_file_get_flags(struct file *file)
bool io_alloc_async_data(struct io_kiocb *req)
{
- WARN_ON_ONCE(!io_cold_defs[req->opcode].async_size);
- req->async_data = kmalloc(io_cold_defs[req->opcode].async_size, GFP_KERNEL);
+ const struct io_issue_def *def = &io_issue_defs[req->opcode];
+
+ WARN_ON_ONCE(!def->async_size);
+ req->async_data = kmalloc(def->async_size, GFP_KERNEL);
if (req->async_data) {
req->flags |= REQ_F_ASYNC_DATA;
return false;
@@ -1718,25 +1720,6 @@ bool io_alloc_async_data(struct io_kiocb *req)
return true;
}
-int io_req_prep_async(struct io_kiocb *req)
-{
- const struct io_cold_def *cdef = &io_cold_defs[req->opcode];
- const struct io_issue_def *def = &io_issue_defs[req->opcode];
-
- /* assign early for deferred execution for non-fixed file */
- if (def->needs_file && !(req->flags & REQ_F_FIXED_FILE) && !req->file)
- req->file = io_file_get_normal(req, req->cqe.fd);
- if (!cdef->prep_async)
- return 0;
- if (WARN_ON_ONCE(req_has_async_data(req)))
- return -EFAULT;
- if (!def->manual_alloc) {
- if (io_alloc_async_data(req))
- return -EAGAIN;
- }
- return cdef->prep_async(req);
-}
-
static u32 io_get_sequence(struct io_kiocb *req)
{
u32 seq = req->ctx->cached_sq_head;
@@ -2049,13 +2032,6 @@ static void io_queue_sqe_fallback(struct io_kiocb *req)
req->flags |= REQ_F_LINK;
io_req_defer_failed(req, req->cqe.res);
} else {
- int ret = io_req_prep_async(req);
-
- if (unlikely(ret)) {
- io_req_defer_failed(req, ret);
- return;
- }
-
if (unlikely(req->ctx->drain_active))
io_drain_req(req);
else
@@ -2265,10 +2241,6 @@ static inline int io_submit_sqe(struct io_ring_ctx *ctx, struct io_kiocb *req,
* conditions are true (normal request), then just queue it.
*/
if (unlikely(link->head)) {
- ret = io_req_prep_async(req);
- if (unlikely(ret))
- return io_submit_fail_init(sqe, req, ret);
-
trace_io_uring_link(req, link->head);
link->last->link = req;
link->last = req;
diff --git a/io_uring/io_uring.h b/io_uring/io_uring.h
index ef9bf610734c..caf1f573bb87 100644
--- a/io_uring/io_uring.h
+++ b/io_uring/io_uring.h
@@ -101,7 +101,6 @@ int io_poll_issue(struct io_kiocb *req, struct io_tw_state *ts);
int io_submit_sqes(struct io_ring_ctx *ctx, unsigned int nr);
int io_do_iopoll(struct io_ring_ctx *ctx, bool force_nonspin);
void __io_submit_flush_completions(struct io_ring_ctx *ctx);
-int io_req_prep_async(struct io_kiocb *req);
struct io_wq_work *io_wq_free_work(struct io_wq_work *work);
void io_wq_submit_work(struct io_wq_work *work);
diff --git a/io_uring/opdef.c b/io_uring/opdef.c
index 745246086c23..2de5cca9504e 100644
--- a/io_uring/opdef.c
+++ b/io_uring/opdef.c
@@ -67,6 +67,7 @@ const struct io_issue_def io_issue_defs[] = {
.iopoll = 1,
.iopoll_queue = 1,
.vectored = 1,
+ .async_size = sizeof(struct io_async_rw),
.prep = io_prep_readv,
.issue = io_read,
},
@@ -81,6 +82,7 @@ const struct io_issue_def io_issue_defs[] = {
.iopoll = 1,
.iopoll_queue = 1,
.vectored = 1,
+ .async_size = sizeof(struct io_async_rw),
.prep = io_prep_writev,
.issue = io_write,
},
@@ -99,6 +101,7 @@ const struct io_issue_def io_issue_defs[] = {
.ioprio = 1,
.iopoll = 1,
.iopoll_queue = 1,
+ .async_size = sizeof(struct io_async_rw),
.prep = io_prep_read_fixed,
.issue = io_read,
},
@@ -112,6 +115,7 @@ const struct io_issue_def io_issue_defs[] = {
.ioprio = 1,
.iopoll = 1,
.iopoll_queue = 1,
+ .async_size = sizeof(struct io_async_rw),
.prep = io_prep_write_fixed,
.issue = io_write,
},
@@ -138,8 +142,8 @@ const struct io_issue_def io_issue_defs[] = {
.unbound_nonreg_file = 1,
.pollout = 1,
.ioprio = 1,
- .manual_alloc = 1,
#if defined(CONFIG_NET)
+ .async_size = sizeof(struct io_async_msghdr),
.prep = io_sendmsg_prep,
.issue = io_sendmsg,
#else
@@ -152,8 +156,8 @@ const struct io_issue_def io_issue_defs[] = {
.pollin = 1,
.buffer_select = 1,
.ioprio = 1,
- .manual_alloc = 1,
#if defined(CONFIG_NET)
+ .async_size = sizeof(struct io_async_msghdr),
.prep = io_recvmsg_prep,
.issue = io_recvmsg,
#else
@@ -162,6 +166,7 @@ const struct io_issue_def io_issue_defs[] = {
},
[IORING_OP_TIMEOUT] = {
.audit_skip = 1,
+ .async_size = sizeof(struct io_timeout_data),
.prep = io_timeout_prep,
.issue = io_timeout,
},
@@ -191,6 +196,7 @@ const struct io_issue_def io_issue_defs[] = {
},
[IORING_OP_LINK_TIMEOUT] = {
.audit_skip = 1,
+ .async_size = sizeof(struct io_timeout_data),
.prep = io_link_timeout_prep,
.issue = io_no_issue,
},
@@ -199,6 +205,7 @@ const struct io_issue_def io_issue_defs[] = {
.unbound_nonreg_file = 1,
.pollout = 1,
#if defined(CONFIG_NET)
+ .async_size = sizeof(struct io_async_msghdr),
.prep = io_connect_prep,
.issue = io_connect,
#else
@@ -239,6 +246,7 @@ const struct io_issue_def io_issue_defs[] = {
.ioprio = 1,
.iopoll = 1,
.iopoll_queue = 1,
+ .async_size = sizeof(struct io_async_rw),
.prep = io_prep_read,
.issue = io_read,
},
@@ -252,6 +260,7 @@ const struct io_issue_def io_issue_defs[] = {
.ioprio = 1,
.iopoll = 1,
.iopoll_queue = 1,
+ .async_size = sizeof(struct io_async_rw),
.prep = io_prep_write,
.issue = io_write,
},
@@ -272,8 +281,9 @@ const struct io_issue_def io_issue_defs[] = {
.pollout = 1,
.audit_skip = 1,
.ioprio = 1,
- .manual_alloc = 1,
+ .buffer_select = 1,
#if defined(CONFIG_NET)
+ .async_size = sizeof(struct io_async_msghdr),
.prep = io_sendmsg_prep,
.issue = io_send,
#else
@@ -288,6 +298,7 @@ const struct io_issue_def io_issue_defs[] = {
.audit_skip = 1,
.ioprio = 1,
#if defined(CONFIG_NET)
+ .async_size = sizeof(struct io_async_msghdr),
.prep = io_recvmsg_prep,
.issue = io_recv,
#else
@@ -403,6 +414,7 @@ const struct io_issue_def io_issue_defs[] = {
.plug = 1,
.iopoll = 1,
.iopoll_queue = 1,
+ .async_size = 2 * sizeof(struct io_uring_sqe),
.prep = io_uring_cmd_prep,
.issue = io_uring_cmd,
},
@@ -412,8 +424,8 @@ const struct io_issue_def io_issue_defs[] = {
.pollout = 1,
.audit_skip = 1,
.ioprio = 1,
- .manual_alloc = 1,
#if defined(CONFIG_NET)
+ .async_size = sizeof(struct io_async_msghdr),
.prep = io_send_zc_prep,
.issue = io_send_zc,
#else
@@ -425,8 +437,8 @@ const struct io_issue_def io_issue_defs[] = {
.unbound_nonreg_file = 1,
.pollout = 1,
.ioprio = 1,
- .manual_alloc = 1,
#if defined(CONFIG_NET)
+ .async_size = sizeof(struct io_async_msghdr),
.prep = io_send_zc_prep,
.issue = io_sendmsg_zc,
#else
@@ -439,10 +451,12 @@ const struct io_issue_def io_issue_defs[] = {
.pollin = 1,
.buffer_select = 1,
.audit_skip = 1,
+ .async_size = sizeof(struct io_async_rw),
.prep = io_read_mshot_prep,
.issue = io_read_mshot,
},
[IORING_OP_WAITID] = {
+ .async_size = sizeof(struct io_waitid_async),
.prep = io_waitid_prep,
.issue = io_waitid,
},
@@ -488,13 +502,11 @@ const struct io_cold_def io_cold_defs[] = {
.name = "NOP",
},
[IORING_OP_READV] = {
- .async_size = sizeof(struct io_async_rw),
.name = "READV",
.cleanup = io_readv_writev_cleanup,
.fail = io_rw_fail,
},
[IORING_OP_WRITEV] = {
- .async_size = sizeof(struct io_async_rw),
.name = "WRITEV",
.cleanup = io_readv_writev_cleanup,
.fail = io_rw_fail,
@@ -503,12 +515,10 @@ const struct io_cold_def io_cold_defs[] = {
.name = "FSYNC",
},
[IORING_OP_READ_FIXED] = {
- .async_size = sizeof(struct io_async_rw),
.name = "READ_FIXED",
.fail = io_rw_fail,
},
[IORING_OP_WRITE_FIXED] = {
- .async_size = sizeof(struct io_async_rw),
.name = "WRITE_FIXED",
.fail = io_rw_fail,
},
@@ -524,7 +534,6 @@ const struct io_cold_def io_cold_defs[] = {
[IORING_OP_SENDMSG] = {
.name = "SENDMSG",
#if defined(CONFIG_NET)
- .async_size = sizeof(struct io_async_msghdr),
.cleanup = io_sendmsg_recvmsg_cleanup,
.fail = io_sendrecv_fail,
#endif
@@ -532,13 +541,11 @@ const struct io_cold_def io_cold_defs[] = {
[IORING_OP_RECVMSG] = {
.name = "RECVMSG",
#if defined(CONFIG_NET)
- .async_size = sizeof(struct io_async_msghdr),
.cleanup = io_sendmsg_recvmsg_cleanup,
.fail = io_sendrecv_fail,
#endif
},
[IORING_OP_TIMEOUT] = {
- .async_size = sizeof(struct io_timeout_data),
.name = "TIMEOUT",
},
[IORING_OP_TIMEOUT_REMOVE] = {
@@ -551,14 +558,10 @@ const struct io_cold_def io_cold_defs[] = {
.name = "ASYNC_CANCEL",
},
[IORING_OP_LINK_TIMEOUT] = {
- .async_size = sizeof(struct io_timeout_data),
.name = "LINK_TIMEOUT",
},
[IORING_OP_CONNECT] = {
.name = "CONNECT",
-#if defined(CONFIG_NET)
- .async_size = sizeof(struct io_async_msghdr),
-#endif
},
[IORING_OP_FALLOCATE] = {
.name = "FALLOCATE",
@@ -578,12 +581,10 @@ const struct io_cold_def io_cold_defs[] = {
.cleanup = io_statx_cleanup,
},
[IORING_OP_READ] = {
- .async_size = sizeof(struct io_async_rw),
.name = "READ",
.fail = io_rw_fail,
},
[IORING_OP_WRITE] = {
- .async_size = sizeof(struct io_async_rw),
.name = "WRITE",
.fail = io_rw_fail,
},
@@ -596,7 +597,6 @@ const struct io_cold_def io_cold_defs[] = {
[IORING_OP_SEND] = {
.name = "SEND",
#if defined(CONFIG_NET)
- .async_size = sizeof(struct io_async_msghdr),
.cleanup = io_sendmsg_recvmsg_cleanup,
.fail = io_sendrecv_fail,
#endif
@@ -604,7 +604,6 @@ const struct io_cold_def io_cold_defs[] = {
[IORING_OP_RECV] = {
.name = "RECV",
#if defined(CONFIG_NET)
- .async_size = sizeof(struct io_async_msghdr),
.cleanup = io_sendmsg_recvmsg_cleanup,
.fail = io_sendrecv_fail,
#endif
@@ -676,12 +675,10 @@ const struct io_cold_def io_cold_defs[] = {
},
[IORING_OP_URING_CMD] = {
.name = "URING_CMD",
- .async_size = 2 * sizeof(struct io_uring_sqe),
},
[IORING_OP_SEND_ZC] = {
.name = "SEND_ZC",
#if defined(CONFIG_NET)
- .async_size = sizeof(struct io_async_msghdr),
.cleanup = io_send_zc_cleanup,
.fail = io_sendrecv_fail,
#endif
@@ -689,18 +686,15 @@ const struct io_cold_def io_cold_defs[] = {
[IORING_OP_SENDMSG_ZC] = {
.name = "SENDMSG_ZC",
#if defined(CONFIG_NET)
- .async_size = sizeof(struct io_async_msghdr),
.cleanup = io_send_zc_cleanup,
.fail = io_sendrecv_fail,
#endif
},
[IORING_OP_READ_MULTISHOT] = {
- .async_size = sizeof(struct io_async_rw),
.name = "READ_MULTISHOT",
},
[IORING_OP_WAITID] = {
.name = "WAITID",
- .async_size = sizeof(struct io_waitid_async),
},
[IORING_OP_FUTEX_WAIT] = {
.name = "FUTEX_WAIT",
diff --git a/io_uring/opdef.h b/io_uring/opdef.h
index 9e5435ec27d0..7ee6f5aa90aa 100644
--- a/io_uring/opdef.h
+++ b/io_uring/opdef.h
@@ -27,22 +27,19 @@ struct io_issue_def {
unsigned iopoll : 1;
/* have to be put into the iopoll list */
unsigned iopoll_queue : 1;
- /* opcode specific path will handle ->async_data allocation if needed */
- unsigned manual_alloc : 1;
/* vectored opcode, set if 1) vectored, and 2) handler needs to know */
unsigned vectored : 1;
+ /* size of async data needed, if any */
+ unsigned short async_size;
+
int (*issue)(struct io_kiocb *, unsigned int);
int (*prep)(struct io_kiocb *, const struct io_uring_sqe *);
};
struct io_cold_def {
- /* size of async data needed, if any */
- unsigned short async_size;
-
const char *name;
- int (*prep_async)(struct io_kiocb *);
void (*cleanup)(struct io_kiocb *);
void (*fail)(struct io_kiocb *);
};
diff --git a/io_uring/uring_cmd.h b/io_uring/uring_cmd.h
index b0ccff7091ee..477ea8865639 100644
--- a/io_uring/uring_cmd.h
+++ b/io_uring/uring_cmd.h
@@ -9,7 +9,6 @@ struct uring_cache {
int io_uring_cmd(struct io_kiocb *req, unsigned int issue_flags);
int io_uring_cmd_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
-int io_uring_cmd_prep_async(struct io_kiocb *req);
void io_uring_cache_free(struct io_cache_entry *entry);
bool io_uring_try_cancel_uring_cmd(struct io_ring_ctx *ctx,
--
2.43.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH 16/17] io_uring: drop ->prep_async()
2024-03-20 22:55 ` [PATCH 16/17] io_uring: drop ->prep_async() Jens Axboe
@ 2024-04-06 20:54 ` Pavel Begunkov
2024-04-07 21:46 ` Jens Axboe
0 siblings, 1 reply; 30+ messages in thread
From: Pavel Begunkov @ 2024-04-06 20:54 UTC (permalink / raw)
To: Jens Axboe, io-uring
On 3/20/24 22:55, Jens Axboe wrote:
> It's now unused, drop the code related to it. This includes the
> io_issue_defs->manual alloc field.
>
> While in there, and since ->async_size is now being used a bit more
> frequently and in the issue path, move it to io_issue_defs[].
>
> Signed-off-by: Jens Axboe <[email protected]>
> ---
...
> @@ -252,6 +260,7 @@ const struct io_issue_def io_issue_defs[] = {
> .ioprio = 1,
> .iopoll = 1,
> .iopoll_queue = 1,
> + .async_size = sizeof(struct io_async_rw),
> .prep = io_prep_write,
> .issue = io_write,
> },
> @@ -272,8 +281,9 @@ const struct io_issue_def io_issue_defs[] = {
> .pollout = 1,
> .audit_skip = 1,
> .ioprio = 1,
> - .manual_alloc = 1,
> + .buffer_select = 1,
This does not belong to this series
> #if defined(CONFIG_NET)
> + .async_size = sizeof(struct io_async_msghdr),
> .prep = io_sendmsg_prep,
> .issue = io_send,
> #else
--
Pavel Begunkov
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH 16/17] io_uring: drop ->prep_async()
2024-04-06 20:54 ` Pavel Begunkov
@ 2024-04-07 21:46 ` Jens Axboe
0 siblings, 0 replies; 30+ messages in thread
From: Jens Axboe @ 2024-04-07 21:46 UTC (permalink / raw)
To: Pavel Begunkov, io-uring
On 4/6/24 2:54 PM, Pavel Begunkov wrote:
> On 3/20/24 22:55, Jens Axboe wrote:
>> It's now unused, drop the code related to it. This includes the
>> io_issue_defs->manual alloc field.
>>
>> While in there, and since ->async_size is now being used a bit more
>> frequently and in the issue path, move it to io_issue_defs[].
>>
>> Signed-off-by: Jens Axboe <[email protected]>
>> ---
> ...
>> @@ -252,6 +260,7 @@ const struct io_issue_def io_issue_defs[] = {
>> .ioprio = 1,
>> .iopoll = 1,
>> .iopoll_queue = 1,
>> + .async_size = sizeof(struct io_async_rw),
>> .prep = io_prep_write,
>> .issue = io_write,
>> },
>> @@ -272,8 +281,9 @@ const struct io_issue_def io_issue_defs[] = {
>> .pollout = 1,
>> .audit_skip = 1,
>> .ioprio = 1,
>> - .manual_alloc = 1,
>> + .buffer_select = 1,
>
> This does not belong to this series
Indeed, thanks for catching that. I'll reshuffle the 6.10 branch once
the next -rc is tagged.
--
Jens Axboe
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 17/17] io_uring/alloc_cache: switch to array based caching
2024-03-20 22:55 [PATCHSET v2 0/17] Improve async state handling Jens Axboe
` (15 preceding siblings ...)
2024-03-20 22:55 ` [PATCH 16/17] io_uring: drop ->prep_async() Jens Axboe
@ 2024-03-20 22:55 ` Jens Axboe
2024-03-21 15:59 ` Gabriel Krisman Bertazi
16 siblings, 1 reply; 30+ messages in thread
From: Jens Axboe @ 2024-03-20 22:55 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe
Currently lists are being used to manage this, but lists isn't a very
good choice for as extracting the current entry necessitates touching
the next entry as well, to update the list head.
Outside of that detail, games are also played with KASAN as the list
is inside the cached entry itself.
Finally, all users of this need a struct io_cache_entry embedded in
their struct, which is union'ized with something else in there that
isn't used across the free -> realloc cycle.
Get rid of all of that, and simply have it be an array. This will not
change the memory used, as we're just trading an 8-byte member entry
for the per-elem array size.
This reduces the overhead of the recycled allocations, and it reduces
the code we have to support recycling.
Signed-off-by: Jens Axboe <[email protected]>
---
include/linux/io_uring_types.h | 2 +-
io_uring/alloc_cache.h | 51 +++++++++++++++-------------------
io_uring/futex.c | 26 ++++++-----------
io_uring/futex.h | 5 ++--
io_uring/io_uring.c | 35 ++++++++++++-----------
io_uring/net.c | 13 ++++-----
io_uring/net.h | 16 ++++-------
io_uring/poll.c | 11 ++------
io_uring/poll.h | 7 +----
io_uring/rsrc.c | 9 ++----
io_uring/rsrc.h | 5 +---
io_uring/rw.c | 13 ++++-----
io_uring/rw.h | 7 ++---
io_uring/uring_cmd.c | 13 ++-------
io_uring/uring_cmd.h | 6 +---
15 files changed, 82 insertions(+), 137 deletions(-)
diff --git a/include/linux/io_uring_types.h b/include/linux/io_uring_types.h
index e3ec84c43f1a..aeb4639785b5 100644
--- a/include/linux/io_uring_types.h
+++ b/include/linux/io_uring_types.h
@@ -220,7 +220,7 @@ struct io_ev_fd {
};
struct io_alloc_cache {
- struct io_wq_work_node list;
+ void **entries;
unsigned int nr_cached;
unsigned int max_cached;
size_t elem_size;
diff --git a/io_uring/alloc_cache.h b/io_uring/alloc_cache.h
index 138ad14b0b12..4349d3519563 100644
--- a/io_uring/alloc_cache.h
+++ b/io_uring/alloc_cache.h
@@ -6,61 +6,54 @@
*/
#define IO_ALLOC_CACHE_MAX 128
-struct io_cache_entry {
- struct io_wq_work_node node;
-};
-
static inline bool io_alloc_cache_put(struct io_alloc_cache *cache,
- struct io_cache_entry *entry)
+ void *entry)
{
if (cache->nr_cached < cache->max_cached) {
- cache->nr_cached++;
- wq_stack_add_head(&entry->node, &cache->list);
- kasan_mempool_poison_object(entry);
+ if (!kasan_mempool_poison_object(entry))
+ return false;
+ cache->entries[cache->nr_cached++] = entry;
return true;
}
return false;
}
-static inline bool io_alloc_cache_empty(struct io_alloc_cache *cache)
-{
- return !cache->list.next;
-}
-
-static inline struct io_cache_entry *io_alloc_cache_get(struct io_alloc_cache *cache)
+static inline void *io_alloc_cache_get(struct io_alloc_cache *cache)
{
- if (cache->list.next) {
- struct io_cache_entry *entry;
+ if (cache->nr_cached) {
+ void *entry = cache->entries[--cache->nr_cached];
- entry = container_of(cache->list.next, struct io_cache_entry, node);
kasan_mempool_unpoison_object(entry, cache->elem_size);
- cache->list.next = cache->list.next->next;
- cache->nr_cached--;
return entry;
}
return NULL;
}
-static inline void io_alloc_cache_init(struct io_alloc_cache *cache,
- unsigned max_nr, size_t size)
+static inline int io_alloc_cache_init(struct io_alloc_cache *cache,
+ unsigned max_nr, size_t size)
{
- cache->list.next = NULL;
+ cache->entries = kvmalloc_array(max_nr, sizeof(void *), GFP_KERNEL);
+ if (!cache->entries)
+ return -ENOMEM;
cache->nr_cached = 0;
cache->max_cached = max_nr;
cache->elem_size = size;
+ return 0;
}
static inline void io_alloc_cache_free(struct io_alloc_cache *cache,
- void (*free)(struct io_cache_entry *))
+ void (*free)(const void *))
{
- while (1) {
- struct io_cache_entry *entry = io_alloc_cache_get(cache);
+ void *entry;
- if (!entry)
- break;
+ if (!cache->entries)
+ return;
+
+ while ((entry = io_alloc_cache_get(cache)) != NULL)
free(entry);
- }
- cache->nr_cached = 0;
+
+ kvfree(cache->entries);
+ cache->entries = NULL;
}
#endif
diff --git a/io_uring/futex.c b/io_uring/futex.c
index 792a03df58de..3dd6d394ca88 100644
--- a/io_uring/futex.c
+++ b/io_uring/futex.c
@@ -27,27 +27,19 @@ struct io_futex {
};
struct io_futex_data {
- union {
- struct futex_q q;
- struct io_cache_entry cache;
- };
+ struct futex_q q;
struct io_kiocb *req;
};
-void io_futex_cache_init(struct io_ring_ctx *ctx)
+int io_futex_cache_init(struct io_ring_ctx *ctx)
{
- io_alloc_cache_init(&ctx->futex_cache, IO_NODE_ALLOC_CACHE_MAX,
+ return io_alloc_cache_init(&ctx->futex_cache, IO_NODE_ALLOC_CACHE_MAX,
sizeof(struct io_futex_data));
}
-static void io_futex_cache_entry_free(struct io_cache_entry *entry)
-{
- kfree(container_of(entry, struct io_futex_data, cache));
-}
-
void io_futex_cache_free(struct io_ring_ctx *ctx)
{
- io_alloc_cache_free(&ctx->futex_cache, io_futex_cache_entry_free);
+ io_alloc_cache_free(&ctx->futex_cache, kfree);
}
static void __io_futex_complete(struct io_kiocb *req, struct io_tw_state *ts)
@@ -63,7 +55,7 @@ static void io_futex_complete(struct io_kiocb *req, struct io_tw_state *ts)
struct io_ring_ctx *ctx = req->ctx;
io_tw_lock(ctx, ts);
- if (!io_alloc_cache_put(&ctx->futex_cache, &ifd->cache))
+ if (!io_alloc_cache_put(&ctx->futex_cache, ifd))
kfree(ifd);
__io_futex_complete(req, ts);
}
@@ -259,11 +251,11 @@ static void io_futex_wake_fn(struct wake_q_head *wake_q, struct futex_q *q)
static struct io_futex_data *io_alloc_ifd(struct io_ring_ctx *ctx)
{
- struct io_cache_entry *entry;
+ struct io_futex_data *ifd;
- entry = io_alloc_cache_get(&ctx->futex_cache);
- if (entry)
- return container_of(entry, struct io_futex_data, cache);
+ ifd = io_alloc_cache_get(&ctx->futex_cache);
+ if (ifd)
+ return ifd;
return kmalloc(sizeof(struct io_futex_data), GFP_NOWAIT);
}
diff --git a/io_uring/futex.h b/io_uring/futex.h
index 0847e9e8a127..75ea753240ba 100644
--- a/io_uring/futex.h
+++ b/io_uring/futex.h
@@ -13,7 +13,7 @@ int io_futex_cancel(struct io_ring_ctx *ctx, struct io_cancel_data *cd,
unsigned int issue_flags);
bool io_futex_remove_all(struct io_ring_ctx *ctx, struct task_struct *task,
bool cancel_all);
-void io_futex_cache_init(struct io_ring_ctx *ctx);
+int io_futex_cache_init(struct io_ring_ctx *ctx);
void io_futex_cache_free(struct io_ring_ctx *ctx);
#else
static inline int io_futex_cancel(struct io_ring_ctx *ctx,
@@ -27,8 +27,9 @@ static inline bool io_futex_remove_all(struct io_ring_ctx *ctx,
{
return false;
}
-static inline void io_futex_cache_init(struct io_ring_ctx *ctx)
+static inline int io_futex_cache_init(struct io_ring_ctx *ctx)
{
+ return 0;
}
static inline void io_futex_cache_free(struct io_ring_ctx *ctx)
{
diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 5eee07563079..2aa3f223739a 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -273,7 +273,7 @@ static int io_alloc_hash_table(struct io_hash_table *table, unsigned bits)
static __cold struct io_ring_ctx *io_ring_ctx_alloc(struct io_uring_params *p)
{
struct io_ring_ctx *ctx;
- int hash_bits;
+ int ret, hash_bits;
ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
if (!ctx)
@@ -302,17 +302,19 @@ static __cold struct io_ring_ctx *io_ring_ctx_alloc(struct io_uring_params *p)
INIT_LIST_HEAD(&ctx->sqd_list);
INIT_LIST_HEAD(&ctx->cq_overflow_list);
INIT_LIST_HEAD(&ctx->io_buffers_cache);
- io_alloc_cache_init(&ctx->rsrc_node_cache, IO_NODE_ALLOC_CACHE_MAX,
+ ret = io_alloc_cache_init(&ctx->rsrc_node_cache, IO_NODE_ALLOC_CACHE_MAX,
sizeof(struct io_rsrc_node));
- io_alloc_cache_init(&ctx->apoll_cache, IO_ALLOC_CACHE_MAX,
+ ret |= io_alloc_cache_init(&ctx->apoll_cache, IO_ALLOC_CACHE_MAX,
sizeof(struct async_poll));
- io_alloc_cache_init(&ctx->netmsg_cache, IO_ALLOC_CACHE_MAX,
+ ret |= io_alloc_cache_init(&ctx->netmsg_cache, IO_ALLOC_CACHE_MAX,
sizeof(struct io_async_msghdr));
- io_alloc_cache_init(&ctx->rw_cache, IO_ALLOC_CACHE_MAX,
+ ret |= io_alloc_cache_init(&ctx->rw_cache, IO_ALLOC_CACHE_MAX,
sizeof(struct io_async_rw));
- io_alloc_cache_init(&ctx->uring_cache, IO_ALLOC_CACHE_MAX,
+ ret |= io_alloc_cache_init(&ctx->uring_cache, IO_ALLOC_CACHE_MAX,
sizeof(struct uring_cache));
- io_futex_cache_init(ctx);
+ ret |= io_futex_cache_init(ctx);
+ if (ret)
+ goto err;
init_completion(&ctx->ref_comp);
xa_init_flags(&ctx->personalities, XA_FLAGS_ALLOC1);
mutex_init(&ctx->uring_lock);
@@ -342,6 +344,12 @@ static __cold struct io_ring_ctx *io_ring_ctx_alloc(struct io_uring_params *p)
return ctx;
err:
+ io_alloc_cache_free(&ctx->rsrc_node_cache, kfree);
+ io_alloc_cache_free(&ctx->apoll_cache, kfree);
+ io_alloc_cache_free(&ctx->netmsg_cache, io_netmsg_cache_free);
+ io_alloc_cache_free(&ctx->rw_cache, io_rw_cache_free);
+ io_alloc_cache_free(&ctx->uring_cache, kfree);
+ io_futex_cache_free(ctx);
kfree(ctx->cancel_table.hbs);
kfree(ctx->cancel_table_locked.hbs);
xa_destroy(&ctx->io_bl_xa);
@@ -1479,7 +1487,7 @@ static void io_free_batch_list(struct io_ring_ctx *ctx,
if (apoll->double_poll)
kfree(apoll->double_poll);
- if (!io_alloc_cache_put(&ctx->apoll_cache, &apoll->cache))
+ if (!io_alloc_cache_put(&ctx->apoll_cache, apoll))
kfree(apoll);
req->flags &= ~REQ_F_POLLED;
}
@@ -2853,11 +2861,6 @@ static void io_req_caches_free(struct io_ring_ctx *ctx)
mutex_unlock(&ctx->uring_lock);
}
-static void io_rsrc_node_cache_free(struct io_cache_entry *entry)
-{
- kfree(container_of(entry, struct io_rsrc_node, cache));
-}
-
static __cold void io_ring_ctx_free(struct io_ring_ctx *ctx)
{
io_sq_thread_finish(ctx);
@@ -2872,10 +2875,10 @@ static __cold void io_ring_ctx_free(struct io_ring_ctx *ctx)
__io_sqe_files_unregister(ctx);
io_cqring_overflow_kill(ctx);
io_eventfd_unregister(ctx);
- io_alloc_cache_free(&ctx->apoll_cache, io_apoll_cache_free);
+ io_alloc_cache_free(&ctx->apoll_cache, kfree);
io_alloc_cache_free(&ctx->netmsg_cache, io_netmsg_cache_free);
io_alloc_cache_free(&ctx->rw_cache, io_rw_cache_free);
- io_alloc_cache_free(&ctx->uring_cache, io_uring_cache_free);
+ io_alloc_cache_free(&ctx->uring_cache, kfree);
io_futex_cache_free(ctx);
io_destroy_buffers(ctx);
mutex_unlock(&ctx->uring_lock);
@@ -2891,7 +2894,7 @@ static __cold void io_ring_ctx_free(struct io_ring_ctx *ctx)
WARN_ON_ONCE(!list_empty(&ctx->rsrc_ref_list));
WARN_ON_ONCE(!list_empty(&ctx->ltimeout_list));
- io_alloc_cache_free(&ctx->rsrc_node_cache, io_rsrc_node_cache_free);
+ io_alloc_cache_free(&ctx->rsrc_node_cache, kfree);
if (ctx->mm_account) {
mmdrop(ctx->mm_account);
ctx->mm_account = NULL;
diff --git a/io_uring/net.c b/io_uring/net.c
index 5794b941254c..6485c50493ac 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -139,7 +139,7 @@ static void io_netmsg_recycle(struct io_kiocb *req, unsigned int issue_flags)
/* Let normal cleanup path reap it if we fail adding to the cache */
iov = hdr->free_iov;
- if (io_alloc_cache_put(&req->ctx->netmsg_cache, &hdr->cache)) {
+ if (io_alloc_cache_put(&req->ctx->netmsg_cache, hdr)) {
if (iov)
kasan_mempool_poison_object(iov);
req->async_data = NULL;
@@ -150,12 +150,10 @@ static void io_netmsg_recycle(struct io_kiocb *req, unsigned int issue_flags)
static struct io_async_msghdr *io_msg_alloc_async(struct io_kiocb *req)
{
struct io_ring_ctx *ctx = req->ctx;
- struct io_cache_entry *entry;
struct io_async_msghdr *hdr;
- entry = io_alloc_cache_get(&ctx->netmsg_cache);
- if (entry) {
- hdr = container_of(entry, struct io_async_msghdr, cache);
+ hdr = io_alloc_cache_get(&ctx->netmsg_cache);
+ if (hdr) {
if (hdr->free_iov) {
kasan_mempool_unpoison_object(hdr->free_iov,
hdr->free_iov_nr * sizeof(struct iovec));
@@ -1492,11 +1490,10 @@ int io_connect(struct io_kiocb *req, unsigned int issue_flags)
return IOU_OK;
}
-void io_netmsg_cache_free(struct io_cache_entry *entry)
+void io_netmsg_cache_free(const void *entry)
{
- struct io_async_msghdr *kmsg;
+ struct io_async_msghdr *kmsg = (struct io_async_msghdr *) entry;
- kmsg = container_of(entry, struct io_async_msghdr, cache);
if (kmsg->free_iov) {
kasan_mempool_unpoison_object(kmsg->free_iov,
kmsg->free_iov_nr * sizeof(struct iovec));
diff --git a/io_uring/net.h b/io_uring/net.h
index b47b43ec6459..c48c44a81850 100644
--- a/io_uring/net.h
+++ b/io_uring/net.h
@@ -7,19 +7,13 @@
struct io_async_msghdr {
#if defined(CONFIG_NET)
- union {
- struct iovec fast_iov;
- struct {
- struct io_cache_entry cache;
- /* entry size of ->free_iov, if valid */
- int free_iov_nr;
- };
- };
+ struct iovec fast_iov;
/* points to an allocated iov, if NULL we use fast_iov instead */
struct iovec *free_iov;
+ int free_iov_nr;
+ int namelen;
__kernel_size_t controllen;
__kernel_size_t payloadlen;
- int namelen;
struct sockaddr __user *uaddr;
struct msghdr msg;
struct sockaddr_storage addr;
@@ -57,9 +51,9 @@ int io_sendmsg_zc(struct io_kiocb *req, unsigned int issue_flags);
int io_send_zc_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
void io_send_zc_cleanup(struct io_kiocb *req);
-void io_netmsg_cache_free(struct io_cache_entry *entry);
+void io_netmsg_cache_free(const void *entry);
#else
-static inline void io_netmsg_cache_free(struct io_cache_entry *entry)
+static inline void io_netmsg_cache_free(const void *entry)
{
}
#endif
diff --git a/io_uring/poll.c b/io_uring/poll.c
index 5d55bbf1de15..536c4eda7c26 100644
--- a/io_uring/poll.c
+++ b/io_uring/poll.c
@@ -686,17 +686,15 @@ static struct async_poll *io_req_alloc_apoll(struct io_kiocb *req,
unsigned issue_flags)
{
struct io_ring_ctx *ctx = req->ctx;
- struct io_cache_entry *entry;
struct async_poll *apoll;
if (req->flags & REQ_F_POLLED) {
apoll = req->apoll;
kfree(apoll->double_poll);
} else if (!(issue_flags & IO_URING_F_UNLOCKED)) {
- entry = io_alloc_cache_get(&ctx->apoll_cache);
- if (entry == NULL)
+ apoll = io_alloc_cache_get(&ctx->apoll_cache);
+ if (!apoll)
goto alloc_apoll;
- apoll = container_of(entry, struct async_poll, cache);
apoll->poll.retries = APOLL_MAX_RETRY;
} else {
alloc_apoll:
@@ -1055,8 +1053,3 @@ int io_poll_remove(struct io_kiocb *req, unsigned int issue_flags)
io_req_set_res(req, ret, 0);
return IOU_OK;
}
-
-void io_apoll_cache_free(struct io_cache_entry *entry)
-{
- kfree(container_of(entry, struct async_poll, cache));
-}
diff --git a/io_uring/poll.h b/io_uring/poll.h
index 1dacae9e816c..f67c5aeabb63 100644
--- a/io_uring/poll.h
+++ b/io_uring/poll.h
@@ -17,10 +17,7 @@ struct io_poll {
};
struct async_poll {
- union {
- struct io_poll poll;
- struct io_cache_entry cache;
- };
+ struct io_poll poll;
struct io_poll *double_poll;
};
@@ -46,6 +43,4 @@ int io_arm_poll_handler(struct io_kiocb *req, unsigned issue_flags);
bool io_poll_remove_all(struct io_ring_ctx *ctx, struct task_struct *tsk,
bool cancel_all);
-void io_apoll_cache_free(struct io_cache_entry *entry);
-
void io_poll_task_func(struct io_kiocb *req, struct io_tw_state *ts);
diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c
index 7195c01e675a..2def86427a5e 100644
--- a/io_uring/rsrc.c
+++ b/io_uring/rsrc.c
@@ -169,7 +169,7 @@ static void io_rsrc_put_work(struct io_rsrc_node *node)
void io_rsrc_node_destroy(struct io_ring_ctx *ctx, struct io_rsrc_node *node)
{
- if (!io_alloc_cache_put(&ctx->rsrc_node_cache, &node->cache))
+ if (!io_alloc_cache_put(&ctx->rsrc_node_cache, node))
kfree(node);
}
@@ -197,12 +197,9 @@ void io_rsrc_node_ref_zero(struct io_rsrc_node *node)
struct io_rsrc_node *io_rsrc_node_alloc(struct io_ring_ctx *ctx)
{
struct io_rsrc_node *ref_node;
- struct io_cache_entry *entry;
- entry = io_alloc_cache_get(&ctx->rsrc_node_cache);
- if (entry) {
- ref_node = container_of(entry, struct io_rsrc_node, cache);
- } else {
+ ref_node = io_alloc_cache_get(&ctx->rsrc_node_cache);
+ if (!ref_node) {
ref_node = kzalloc(sizeof(*ref_node), GFP_KERNEL);
if (!ref_node)
return NULL;
diff --git a/io_uring/rsrc.h b/io_uring/rsrc.h
index e21000238954..b4cec653100d 100644
--- a/io_uring/rsrc.h
+++ b/io_uring/rsrc.h
@@ -36,10 +36,7 @@ struct io_rsrc_data {
};
struct io_rsrc_node {
- union {
- struct io_cache_entry cache;
- struct io_ring_ctx *ctx;
- };
+ struct io_ring_ctx *ctx;
int refs;
bool empty;
u16 type;
diff --git a/io_uring/rw.c b/io_uring/rw.c
index 57f2d315a620..6849795532ab 100644
--- a/io_uring/rw.c
+++ b/io_uring/rw.c
@@ -154,7 +154,7 @@ static void io_rw_recycle(struct io_kiocb *req, unsigned int issue_flags)
return;
}
iov = rw->free_iovec;
- if (io_alloc_cache_put(&req->ctx->rw_cache, &rw->cache)) {
+ if (io_alloc_cache_put(&req->ctx->rw_cache, rw)) {
if (iov)
kasan_mempool_poison_object(iov);
req->async_data = NULL;
@@ -200,12 +200,10 @@ static void io_req_rw_cleanup(struct io_kiocb *req, unsigned int issue_flags)
static int io_rw_alloc_async(struct io_kiocb *req)
{
struct io_ring_ctx *ctx = req->ctx;
- struct io_cache_entry *entry;
struct io_async_rw *rw;
- entry = io_alloc_cache_get(&ctx->rw_cache);
- if (entry) {
- rw = container_of(entry, struct io_async_rw, cache);
+ rw = io_alloc_cache_get(&ctx->rw_cache);
+ if (rw) {
if (rw->free_iovec) {
kasan_mempool_unpoison_object(rw->free_iovec,
rw->free_iov_nr * sizeof(struct iovec));
@@ -1180,11 +1178,10 @@ int io_do_iopoll(struct io_ring_ctx *ctx, bool force_nonspin)
return nr_events;
}
-void io_rw_cache_free(struct io_cache_entry *entry)
+void io_rw_cache_free(const void *entry)
{
- struct io_async_rw *rw;
+ struct io_async_rw *rw = (struct io_async_rw *) entry;
- rw = container_of(entry, struct io_async_rw, cache);
if (rw->free_iovec) {
kasan_mempool_unpoison_object(rw->free_iovec,
rw->free_iov_nr * sizeof(struct iovec));
diff --git a/io_uring/rw.h b/io_uring/rw.h
index cf51d0eb407a..3f432dc75441 100644
--- a/io_uring/rw.h
+++ b/io_uring/rw.h
@@ -3,10 +3,7 @@
#include <linux/pagemap.h>
struct io_async_rw {
- union {
- size_t bytes_done;
- struct io_cache_entry cache;
- };
+ size_t bytes_done;
struct iov_iter iter;
struct iov_iter_state iter_state;
struct iovec fast_iov;
@@ -28,4 +25,4 @@ void io_rw_fail(struct io_kiocb *req);
void io_req_rw_complete(struct io_kiocb *req, struct io_tw_state *ts);
int io_read_mshot_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
int io_read_mshot(struct io_kiocb *req, unsigned int issue_flags);
-void io_rw_cache_free(struct io_cache_entry *entry);
+void io_rw_cache_free(const void *entry);
diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c
index 92346b5d9f5b..509cfd56726c 100644
--- a/io_uring/uring_cmd.c
+++ b/io_uring/uring_cmd.c
@@ -17,12 +17,10 @@
static struct uring_cache *io_uring_async_get(struct io_kiocb *req)
{
struct io_ring_ctx *ctx = req->ctx;
- struct io_cache_entry *entry;
struct uring_cache *cache;
- entry = io_alloc_cache_get(&ctx->uring_cache);
- if (entry) {
- cache = container_of(entry, struct uring_cache, cache);
+ cache = io_alloc_cache_get(&ctx->uring_cache);
+ if (cache) {
req->flags |= REQ_F_ASYNC_DATA;
req->async_data = cache;
return cache;
@@ -39,7 +37,7 @@ static void io_req_uring_cleanup(struct io_kiocb *req, unsigned int issue_flags)
if (issue_flags & IO_URING_F_UNLOCKED)
return;
- if (io_alloc_cache_put(&req->ctx->uring_cache, &cache->cache)) {
+ if (io_alloc_cache_put(&req->ctx->uring_cache, cache)) {
ioucmd->sqe = NULL;
req->async_data = NULL;
req->flags &= ~REQ_F_ASYNC_DATA;
@@ -354,8 +352,3 @@ int io_uring_cmd_sock(struct io_uring_cmd *cmd, unsigned int issue_flags)
}
EXPORT_SYMBOL_GPL(io_uring_cmd_sock);
#endif
-
-void io_uring_cache_free(struct io_cache_entry *entry)
-{
- kfree(container_of(entry, struct uring_cache, cache));
-}
diff --git a/io_uring/uring_cmd.h b/io_uring/uring_cmd.h
index 477ea8865639..a361f98664d2 100644
--- a/io_uring/uring_cmd.h
+++ b/io_uring/uring_cmd.h
@@ -1,15 +1,11 @@
// SPDX-License-Identifier: GPL-2.0
struct uring_cache {
- union {
- struct io_cache_entry cache;
- struct io_uring_sqe sqes[2];
- };
+ struct io_uring_sqe sqes[2];
};
int io_uring_cmd(struct io_kiocb *req, unsigned int issue_flags);
int io_uring_cmd_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
-void io_uring_cache_free(struct io_cache_entry *entry);
bool io_uring_try_cancel_uring_cmd(struct io_ring_ctx *ctx,
struct task_struct *task, bool cancel_all);
--
2.43.0
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH 17/17] io_uring/alloc_cache: switch to array based caching
2024-03-20 22:55 ` [PATCH 17/17] io_uring/alloc_cache: switch to array based caching Jens Axboe
@ 2024-03-21 15:59 ` Gabriel Krisman Bertazi
2024-03-21 16:38 ` Jens Axboe
0 siblings, 1 reply; 30+ messages in thread
From: Gabriel Krisman Bertazi @ 2024-03-21 15:59 UTC (permalink / raw)
To: Jens Axboe; +Cc: io-uring
Jens Axboe <[email protected]> writes:
> Currently lists are being used to manage this, but lists isn't a very
> good choice for as extracting the current entry necessitates touching
> the next entry as well, to update the list head.
>
> Outside of that detail, games are also played with KASAN as the list
> is inside the cached entry itself.
>
> Finally, all users of this need a struct io_cache_entry embedded in
> their struct, which is union'ized with something else in there that
> isn't used across the free -> realloc cycle.
>
> Get rid of all of that, and simply have it be an array. This will not
> change the memory used, as we're just trading an 8-byte member entry
> for the per-elem array size.
>
> This reduces the overhead of the recycled allocations, and it reduces
> the code we have to support recycling.
Hi Jens,
I tried applying the entire to your for-6.10/io_uring branch to test it
and only this last patch failed to apply. The tip of the branch I have
is 22261e73e8d2 ("io_uring/alloc_cache: shrink default max entries from
512 to 128").
> -static inline struct io_cache_entry *io_alloc_cache_get(struct io_alloc_cache *cache)
> +static inline void *io_alloc_cache_get(struct io_alloc_cache *cache)
> {
> - if (cache->list.next) {
> - struct io_cache_entry *entry;
> + if (cache->nr_cached) {
> + void *entry = cache->entries[--cache->nr_cached];
>
> - entry = container_of(cache->list.next, struct io_cache_entry, node);
> kasan_mempool_unpoison_object(entry, cache->elem_size);
> - cache->list.next = cache->list.next->next;
> - cache->nr_cached--;
> return entry;
> }
>
> return NULL;
> }
>
> -static inline void io_alloc_cache_init(struct io_alloc_cache *cache,
> - unsigned max_nr, size_t size)
> +static inline int io_alloc_cache_init(struct io_alloc_cache *cache,
> + unsigned max_nr, size_t size)
> {
> - cache->list.next = NULL;
> + cache->entries = kvmalloc_array(max_nr, sizeof(void *), GFP_KERNEL);
> + if (!cache->entries)
> + return -ENOMEM;
> cache->nr_cached = 0;
> cache->max_cached = max_nr;
> cache->elem_size = size;
> + return 0;
> }
>
> static inline void io_alloc_cache_free(struct io_alloc_cache *cache,
> - void (*free)(struct io_cache_entry *))
> + void (*free)(const void *))
Minor, but since free is supposed to free the entry, const doesn't make
sense here. Also, you actually just cast it away immediately in every usage.
--
Gabriel Krisman Bertazi
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH 17/17] io_uring/alloc_cache: switch to array based caching
2024-03-21 15:59 ` Gabriel Krisman Bertazi
@ 2024-03-21 16:38 ` Jens Axboe
2024-03-21 17:20 ` Gabriel Krisman Bertazi
0 siblings, 1 reply; 30+ messages in thread
From: Jens Axboe @ 2024-03-21 16:38 UTC (permalink / raw)
To: Gabriel Krisman Bertazi; +Cc: io-uring
On 3/21/24 9:59 AM, Gabriel Krisman Bertazi wrote:
> Jens Axboe <[email protected]> writes:
>
>> Currently lists are being used to manage this, but lists isn't a very
>> good choice for as extracting the current entry necessitates touching
>> the next entry as well, to update the list head.
>>
>> Outside of that detail, games are also played with KASAN as the list
>> is inside the cached entry itself.
>>
>> Finally, all users of this need a struct io_cache_entry embedded in
>> their struct, which is union'ized with something else in there that
>> isn't used across the free -> realloc cycle.
>>
>> Get rid of all of that, and simply have it be an array. This will not
>> change the memory used, as we're just trading an 8-byte member entry
>> for the per-elem array size.
>>
>> This reduces the overhead of the recycled allocations, and it reduces
>> the code we have to support recycling.
>
> Hi Jens,
>
> I tried applying the entire to your for-6.10/io_uring branch to test it
> and only this last patch failed to apply. The tip of the branch I have
> is 22261e73e8d2 ("io_uring/alloc_cache: shrink default max entries from
> 512 to 128").
Yeah it has some dependencies that need unraveling. The easiest is if
you just pull:
git://git.kernel.dk/linux io_uring-recvsend-bundle
into current -git master, and then just test that. That gets you pretty
much everything that's being tested and played with.
Top of tree is d5653d2fcf1383c0fbe8b64545664aea36c7aca2 right now.
>> -static inline struct io_cache_entry *io_alloc_cache_get(struct io_alloc_cache *cache)
>> +static inline void *io_alloc_cache_get(struct io_alloc_cache *cache)
>> {
>> - if (cache->list.next) {
>> - struct io_cache_entry *entry;
>> + if (cache->nr_cached) {
>> + void *entry = cache->entries[--cache->nr_cached];
>>
>> - entry = container_of(cache->list.next, struct io_cache_entry, node);
>> kasan_mempool_unpoison_object(entry, cache->elem_size);
>> - cache->list.next = cache->list.next->next;
>> - cache->nr_cached--;
>> return entry;
>> }
>>
>> return NULL;
>> }
>>
>> -static inline void io_alloc_cache_init(struct io_alloc_cache *cache,
>> - unsigned max_nr, size_t size)
>> +static inline int io_alloc_cache_init(struct io_alloc_cache *cache,
>> + unsigned max_nr, size_t size)
>> {
>> - cache->list.next = NULL;
>> + cache->entries = kvmalloc_array(max_nr, sizeof(void *), GFP_KERNEL);
>> + if (!cache->entries)
>> + return -ENOMEM;
>> cache->nr_cached = 0;
>> cache->max_cached = max_nr;
>> cache->elem_size = size;
>> + return 0;
>> }
>>
>> static inline void io_alloc_cache_free(struct io_alloc_cache *cache,
>> - void (*free)(struct io_cache_entry *))
>> + void (*free)(const void *))
>
> Minor, but since free is supposed to free the entry, const doesn't
> make sense here. Also, you actually just cast it away immediately in
> every usage.
It's because then I can use kfree() directly for most cases, only two of
them have special freeing functions. And kfree takes a const void *. I
should add a comment about that.
--
Jens Axboe
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH 17/17] io_uring/alloc_cache: switch to array based caching
2024-03-21 16:38 ` Jens Axboe
@ 2024-03-21 17:20 ` Gabriel Krisman Bertazi
2024-03-21 17:22 ` Jens Axboe
0 siblings, 1 reply; 30+ messages in thread
From: Gabriel Krisman Bertazi @ 2024-03-21 17:20 UTC (permalink / raw)
To: Jens Axboe; +Cc: io-uring
Jens Axboe <[email protected]> writes:
> On 3/21/24 9:59 AM, Gabriel Krisman Bertazi wrote:
>> Jens Axboe <[email protected]> writes:
>>
>>> Currently lists are being used to manage this, but lists isn't a very
>>> good choice for as extracting the current entry necessitates touching
>>> the next entry as well, to update the list head.
>>>
>>> Outside of that detail, games are also played with KASAN as the list
>>> is inside the cached entry itself.
>>>
>>> Finally, all users of this need a struct io_cache_entry embedded in
>>> their struct, which is union'ized with something else in there that
>>> isn't used across the free -> realloc cycle.
>>>
>>> Get rid of all of that, and simply have it be an array. This will not
>>> change the memory used, as we're just trading an 8-byte member entry
>>> for the per-elem array size.
>>>
>>> This reduces the overhead of the recycled allocations, and it reduces
>>> the code we have to support recycling.
>>
>> Hi Jens,
>>
>> I tried applying the entire to your for-6.10/io_uring branch to test it
>> and only this last patch failed to apply. The tip of the branch I have
>> is 22261e73e8d2 ("io_uring/alloc_cache: shrink default max entries from
>> 512 to 128").
>
> Yeah it has some dependencies that need unraveling. The easiest is if
> you just pull:
>
> git://git.kernel.dk/linux io_uring-recvsend-bundle
>
> into current -git master, and then just test that. That gets you pretty
> much everything that's being tested and played with.
>
> Top of tree is d5653d2fcf1383c0fbe8b64545664aea36c7aca2 right now.
thanks, I'll test with that.
>
>>> -static inline struct io_cache_entry *io_alloc_cache_get(struct io_alloc_cache *cache)
>>> +static inline void *io_alloc_cache_get(struct io_alloc_cache *cache)
>>> {
>>> - if (cache->list.next) {
>>> - struct io_cache_entry *entry;
>>> + if (cache->nr_cached) {
>>> + void *entry = cache->entries[--cache->nr_cached];
>>>
>>> - entry = container_of(cache->list.next, struct io_cache_entry, node);
>>> kasan_mempool_unpoison_object(entry, cache->elem_size);
>>> - cache->list.next = cache->list.next->next;
>>> - cache->nr_cached--;
>>> return entry;
>>> }
>>>
>>> return NULL;
>>> }
>>>
>>> -static inline void io_alloc_cache_init(struct io_alloc_cache *cache,
>>> - unsigned max_nr, size_t size)
>>> +static inline int io_alloc_cache_init(struct io_alloc_cache *cache,
>>> + unsigned max_nr, size_t size)
>>> {
>>> - cache->list.next = NULL;
>>> + cache->entries = kvmalloc_array(max_nr, sizeof(void *), GFP_KERNEL);
>>> + if (!cache->entries)
>>> + return -ENOMEM;
>>> cache->nr_cached = 0;
>>> cache->max_cached = max_nr;
>>> cache->elem_size = size;
>>> + return 0;
>>> }
>>>
>>> static inline void io_alloc_cache_free(struct io_alloc_cache *cache,
>>> - void (*free)(struct io_cache_entry *))
>>> + void (*free)(const void *))
>>
>> Minor, but since free is supposed to free the entry, const doesn't
>> make sense here. Also, you actually just cast it away immediately in
>> every usage.
>
> It's because then I can use kfree() directly for most cases, only two of
> them have special freeing functions. And kfree takes a const void *. I
> should add a comment about that.
TIL. For the record, I was very puzzled on why kfree receives a const
pointer just to cast it away immediately too. Then I found Linus
discussing it at https://yarchive.net/comp/const.html
Anyway, in this case, we are actually modifying it in io_rw_cache_free,
and we don't need to explicitly cast from non-const to const , so I still
think you can avoid the comment and drop the const. But that is just
a nitpick that i won't insist.
Thanks!
--
Gabriel Krisman Bertazi
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH 17/17] io_uring/alloc_cache: switch to array based caching
2024-03-21 17:20 ` Gabriel Krisman Bertazi
@ 2024-03-21 17:22 ` Jens Axboe
0 siblings, 0 replies; 30+ messages in thread
From: Jens Axboe @ 2024-03-21 17:22 UTC (permalink / raw)
To: Gabriel Krisman Bertazi; +Cc: io-uring
On 3/21/24 11:20 AM, Gabriel Krisman Bertazi wrote:
> Jens Axboe <[email protected]> writes:
>
>> On 3/21/24 9:59 AM, Gabriel Krisman Bertazi wrote:
>>> Jens Axboe <[email protected]> writes:
>>>
>>>> Currently lists are being used to manage this, but lists isn't a very
>>>> good choice for as extracting the current entry necessitates touching
>>>> the next entry as well, to update the list head.
>>>>
>>>> Outside of that detail, games are also played with KASAN as the list
>>>> is inside the cached entry itself.
>>>>
>>>> Finally, all users of this need a struct io_cache_entry embedded in
>>>> their struct, which is union'ized with something else in there that
>>>> isn't used across the free -> realloc cycle.
>>>>
>>>> Get rid of all of that, and simply have it be an array. This will not
>>>> change the memory used, as we're just trading an 8-byte member entry
>>>> for the per-elem array size.
>>>>
>>>> This reduces the overhead of the recycled allocations, and it reduces
>>>> the code we have to support recycling.
>>>
>>> Hi Jens,
>>>
>>> I tried applying the entire to your for-6.10/io_uring branch to test it
>>> and only this last patch failed to apply. The tip of the branch I have
>>> is 22261e73e8d2 ("io_uring/alloc_cache: shrink default max entries from
>>> 512 to 128").
>>
>> Yeah it has some dependencies that need unraveling. The easiest is if
>> you just pull:
>>
>> git://git.kernel.dk/linux io_uring-recvsend-bundle
>>
>> into current -git master, and then just test that. That gets you pretty
>> much everything that's being tested and played with.
>>
>> Top of tree is d5653d2fcf1383c0fbe8b64545664aea36c7aca2 right now.
>
> thanks, I'll test with that.
Thanks!
>>>> -static inline struct io_cache_entry *io_alloc_cache_get(struct io_alloc_cache *cache)
>>>> +static inline void *io_alloc_cache_get(struct io_alloc_cache *cache)
>>>> {
>>>> - if (cache->list.next) {
>>>> - struct io_cache_entry *entry;
>>>> + if (cache->nr_cached) {
>>>> + void *entry = cache->entries[--cache->nr_cached];
>>>>
>>>> - entry = container_of(cache->list.next, struct io_cache_entry, node);
>>>> kasan_mempool_unpoison_object(entry, cache->elem_size);
>>>> - cache->list.next = cache->list.next->next;
>>>> - cache->nr_cached--;
>>>> return entry;
>>>> }
>>>>
>>>> return NULL;
>>>> }
>>>>
>>>> -static inline void io_alloc_cache_init(struct io_alloc_cache *cache,
>>>> - unsigned max_nr, size_t size)
>>>> +static inline int io_alloc_cache_init(struct io_alloc_cache *cache,
>>>> + unsigned max_nr, size_t size)
>>>> {
>>>> - cache->list.next = NULL;
>>>> + cache->entries = kvmalloc_array(max_nr, sizeof(void *), GFP_KERNEL);
>>>> + if (!cache->entries)
>>>> + return -ENOMEM;
>>>> cache->nr_cached = 0;
>>>> cache->max_cached = max_nr;
>>>> cache->elem_size = size;
>>>> + return 0;
>>>> }
>>>>
>>>> static inline void io_alloc_cache_free(struct io_alloc_cache *cache,
>>>> - void (*free)(struct io_cache_entry *))
>>>> + void (*free)(const void *))
>>>
>>> Minor, but since free is supposed to free the entry, const doesn't
>>> make sense here. Also, you actually just cast it away immediately in
>>> every usage.
>>
>> It's because then I can use kfree() directly for most cases, only two of
>> them have special freeing functions. And kfree takes a const void *. I
>> should add a comment about that.
>
> TIL. For the record, I was very puzzled on why kfree receives a const
> pointer just to cast it away immediately too. Then I found Linus
> discussing it at https://yarchive.net/comp/const.html
>
> Anyway, in this case, we are actually modifying it in io_rw_cache_free,
> and we don't need to explicitly cast from non-const to const , so I still
> think you can avoid the comment and drop the const. But that is just
> a nitpick that i won't insist.
Right, but then I need a wrapper ala:
io_uring_kfree(void *entry)
{
kfree(entry);
}
which obviously isn't the end of the world, but the cast is just fine
as-is imho.
--
Jens Axboe
^ permalink raw reply [flat|nested] 30+ messages in thread