From c2fa700c5f7667800b6cfedcb9994eb5eb69e6cc Mon Sep 17 00:00:00 2001 From: Ammar Faizi Date: Tue, 23 Aug 2022 18:45:48 +0700 Subject: [PATCH 01/10] MAINTAINERS: Add `include/linux/io_uring_types.h` File include/linux/io_uring_types.h doesn't have a maintainer, add it to the io_uring section. Signed-off-by: Ammar Faizi Signed-off-by: Jens Axboe --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 8a5012ba6ff9..6dc55be41420 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -10657,6 +10657,7 @@ T: git git://git.kernel.dk/linux-block T: git git://git.kernel.dk/liburing F: io_uring/ F: include/linux/io_uring.h +F: include/linux/io_uring_types.h F: include/uapi/linux/io_uring.h F: tools/io_uring/ From e1d0c6d05afdcff01ace698edb3b8808db1dc066 Mon Sep 17 00:00:00 2001 From: Ammar Faizi Date: Tue, 23 Aug 2022 18:45:49 +0700 Subject: [PATCH 02/10] io_uring: uapi: Add `extern "C"` in io_uring.h for liburing Make it easy for liburing to integrate uapi header with the kernel. Previously, when this header changes, the liburing side can't directly copy this header file due to some small differences. Sync them. Link: https://lore.kernel.org/io-uring/f1feef16-6ea2-0653-238f-4aaee35060b6@kernel.dk Cc: Bart Van Assche Cc: Dylan Yudaken Cc: Facebook Kernel Team Signed-off-by: Ammar Faizi Signed-off-by: Jens Axboe --- include/uapi/linux/io_uring.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h index 1463cfecb56b..9e0b5c8d92ce 100644 --- a/include/uapi/linux/io_uring.h +++ b/include/uapi/linux/io_uring.h @@ -12,6 +12,10 @@ #include #include +#ifdef __cplusplus +extern "C" { +#endif + /* * IO submission data structure (Submission Queue Entry) */ @@ -661,4 +665,8 @@ struct io_uring_recvmsg_out { __u32 flags; }; +#ifdef __cplusplus +} +#endif + #endif From 47abea041f897d64dbd5777f0cf7745148f85d75 Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Tue, 23 Aug 2022 07:26:08 -0600 Subject: [PATCH 03/10] io_uring: fix off-by-one in sync cancelation file check The passed in index should be validated against the number of registered files we have, it needs to be smaller than the index value to avoid going one beyond the end. Fixes: 78a861b94959 ("io_uring: add sync cancelation API through io_uring_register()") Reported-by: Luo Likang Signed-off-by: Jens Axboe --- io_uring/cancel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/io_uring/cancel.c b/io_uring/cancel.c index e4e1dc0325f0..5fc5d3e80fcb 100644 --- a/io_uring/cancel.c +++ b/io_uring/cancel.c @@ -218,7 +218,7 @@ static int __io_sync_cancel(struct io_uring_task *tctx, (cd->flags & IORING_ASYNC_CANCEL_FD_FIXED)) { unsigned long file_ptr; - if (unlikely(fd > ctx->nr_user_files)) + if (unlikely(fd >= ctx->nr_user_files)) return -EBADF; fd = array_index_nospec(fd, ctx->nr_user_files); file_ptr = io_fixed_file_slot(&ctx->file_table, fd)->file_ptr; From a9c3eda7eada94e8cf29cb102aa80e1370d8fa2e Mon Sep 17 00:00:00 2001 From: Kanchan Joshi Date: Tue, 23 Aug 2022 20:40:22 +0530 Subject: [PATCH 04/10] io_uring: fix submission-failure handling for uring-cmd If ->uring_cmd returned an error value different from -EAGAIN or -EIOCBQUEUED, it gets overridden with IOU_OK. This invites trouble as caller (io_uring core code) handles IOU_OK differently than other error codes. Fix this by returning the actual error code. Signed-off-by: Kanchan Joshi Signed-off-by: Jens Axboe --- io_uring/uring_cmd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c index 8e0cc2d9205e..b9989ae7b957 100644 --- a/io_uring/uring_cmd.c +++ b/io_uring/uring_cmd.c @@ -112,7 +112,7 @@ int io_uring_cmd(struct io_kiocb *req, unsigned int issue_flags) if (ret < 0) req_set_fail(req); io_req_set_res(req, ret, 0); - return IOU_OK; + return ret; } return IOU_ISSUE_SKIP_COMPLETE; From 2cacedc873ab5f5945d8f1b71804b0bcea0383ff Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Wed, 24 Aug 2022 13:07:38 +0100 Subject: [PATCH 05/10] io_uring/net: fix must_hold annotation Fix up the io_alloc_notif()'s __must_hold as we don't have a ctx argument there but should get it from the slot instead. Reported-by: Stefan Metzmacher Signed-off-by: Pavel Begunkov Link: https://lore.kernel.org/r/cbb0a920f18e0aed590bf58300af817b9befb8a3.1661342812.git.asml.silence@gmail.com Signed-off-by: Jens Axboe --- io_uring/notif.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/io_uring/notif.c b/io_uring/notif.c index 977736e82c1a..568ff17dc552 100644 --- a/io_uring/notif.c +++ b/io_uring/notif.c @@ -73,7 +73,7 @@ struct io_kiocb *io_alloc_notif(struct io_ring_ctx *ctx, } void io_notif_slot_flush(struct io_notif_slot *slot) - __must_hold(&ctx->uring_lock) + __must_hold(&slot->notif->ctx->uring_lock) { struct io_kiocb *notif = slot->notif; struct io_notif_data *nd = io_notif_to_data(notif); From 5a848b7c9e5e4d94390fbc391ccb81d40f3ccfb5 Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Wed, 24 Aug 2022 13:07:39 +0100 Subject: [PATCH 06/10] io_uring/net: fix zc send link failing Failed requests should be marked with req_set_fail(), so links and cqe skipping work correctly, which is missing in io_sendzc(). Note, io_sendzc() return IOU_OK on failure, so the core code won't do the cleanup for us. Fixes: 06a5464be84e4 ("io_uring: wire send zc request type") Signed-off-by: Pavel Begunkov Link: https://lore.kernel.org/r/e47d46fda9db30154ce66a549bb0d3380b780520.1661342812.git.asml.silence@gmail.com Signed-off-by: Jens Axboe --- io_uring/net.c | 1 + 1 file changed, 1 insertion(+) diff --git a/io_uring/net.c b/io_uring/net.c index f8cdf1dc3863..d6310c655a0f 100644 --- a/io_uring/net.c +++ b/io_uring/net.c @@ -1023,6 +1023,7 @@ int io_sendzc(struct io_kiocb *req, unsigned int issue_flags) } if (ret == -ERESTARTSYS) ret = -EINTR; + req_set_fail(req); } else if (zc->flags & IORING_RECVSEND_NOTIF_FLUSH) { io_notif_slot_flush_submit(notif_slot, 0); } From 986e263def32eec89153babf469859d837507d34 Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Wed, 24 Aug 2022 13:07:40 +0100 Subject: [PATCH 07/10] io_uring/net: fix indentation Fix up indentation before we get complaints from tooling. Signed-off-by: Pavel Begunkov Link: https://lore.kernel.org/r/bd5754e3764215ccd7fb04cd636ea9167aaa275d.1661342812.git.asml.silence@gmail.com Signed-off-by: Jens Axboe --- io_uring/net.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/io_uring/net.c b/io_uring/net.c index d6310c655a0f..3adcb09ae264 100644 --- a/io_uring/net.c +++ b/io_uring/net.c @@ -989,7 +989,7 @@ int io_sendzc(struct io_kiocb *req, unsigned int issue_flags) ret = io_import_fixed(WRITE, &msg.msg_iter, req->imu, (u64)(uintptr_t)zc->buf, zc->len); if (unlikely(ret)) - return ret; + return ret; } else { ret = import_single_range(WRITE, zc->buf, zc->len, &iov, &msg.msg_iter); From 53bdc88aac9a21aae937452724fa4738cd843795 Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Wed, 24 Aug 2022 13:07:41 +0100 Subject: [PATCH 08/10] io_uring/notif: order notif vs send CQEs Currently, there is no ordering between notification CQEs and completions of the send flushing it, this quite complicates the userspace, especially since we don't flush notification when the send(+flush) request fails, i.e. there will be only one CQE. What we can do is to make sure that notification completions come only after sends. The easiest way to achieve this is to not try to complete a notification inline from io_sendzc() but defer it to task_work, considering that io-wq sendzc is disallowed CQEs will be naturally ordered because task_works will only be executed after we're done with submission and so inline completion. Signed-off-by: Pavel Begunkov Link: https://lore.kernel.org/r/cddfd1c2bf91f22b9fe08e13b7dffdd8f858a151.1661342812.git.asml.silence@gmail.com Signed-off-by: Jens Axboe --- io_uring/notif.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/io_uring/notif.c b/io_uring/notif.c index 568ff17dc552..96f076b175e0 100644 --- a/io_uring/notif.c +++ b/io_uring/notif.c @@ -81,8 +81,10 @@ void io_notif_slot_flush(struct io_notif_slot *slot) slot->notif = NULL; /* drop slot's master ref */ - if (refcount_dec_and_test(&nd->uarg.refcnt)) - io_notif_complete(notif); + if (refcount_dec_and_test(&nd->uarg.refcnt)) { + notif->io_task_work.func = __io_notif_complete_tw; + io_req_task_work_add(notif); + } } __cold int io_notif_unregister(struct io_ring_ctx *ctx) From 5916943943d19a854238d50d1fe2047467cbeb3c Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Wed, 24 Aug 2022 13:07:42 +0100 Subject: [PATCH 09/10] io_uring: conditional ->async_data allocation There are opcodes that need ->async_data only in some cases and allocation it unconditionally may hurt performance. Add an option to opdef to make move the allocation part from the core io_uring to opcode specific code. Note, we can't just set opdef->async_size to zero because there are other helpers that rely on it, e.g. io_alloc_async_data(). Signed-off-by: Pavel Begunkov Link: https://lore.kernel.org/r/9dc62be9e88dd0ed63c48365340e8922d2498293.1661342812.git.asml.silence@gmail.com Signed-off-by: Jens Axboe --- io_uring/io_uring.c | 7 ++++--- io_uring/opdef.h | 2 ++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c index ebfdb2212ec2..77616279000b 100644 --- a/io_uring/io_uring.c +++ b/io_uring/io_uring.c @@ -1450,9 +1450,10 @@ int io_req_prep_async(struct io_kiocb *req) return 0; if (WARN_ON_ONCE(req_has_async_data(req))) return -EFAULT; - if (io_alloc_async_data(req)) - return -EAGAIN; - + if (!io_op_defs[req->opcode].manual_alloc) { + if (io_alloc_async_data(req)) + return -EAGAIN; + } return def->prep_async(req); } diff --git a/io_uring/opdef.h b/io_uring/opdef.h index ece8ed4f96c4..763c6e54e2ee 100644 --- a/io_uring/opdef.h +++ b/io_uring/opdef.h @@ -25,6 +25,8 @@ struct io_op_def { unsigned ioprio : 1; /* supports iopoll */ unsigned iopoll : 1; + /* opcode specific path will handle ->async_data allocation if needed */ + unsigned manual_alloc : 1; /* size of async data needed, if any */ unsigned short async_size; From 581711c46612c1fd7f98960f9ad53f04fdb89853 Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Wed, 24 Aug 2022 13:07:43 +0100 Subject: [PATCH 10/10] io_uring/net: save address for sendzc async execution We usually copy all bits that a request needs from the userspace for async execution, so the userspace can keep them on the stack. However, send zerocopy violates this pattern for addresses and may reloads it e.g. from io-wq. Save the address if any in ->async_data as usual. Reported-by: Stefan Metzmacher Signed-off-by: Pavel Begunkov Link: https://lore.kernel.org/r/d7512d7aa9abcd36e9afe1a4d292a24cb2d157e5.1661342812.git.asml.silence@gmail.com [axboe: fold in incremental fix] Signed-off-by: Jens Axboe --- io_uring/net.c | 53 +++++++++++++++++++++++++++++++++++++++++------- io_uring/net.h | 1 + io_uring/opdef.c | 4 +++- 3 files changed, 50 insertions(+), 8 deletions(-) diff --git a/io_uring/net.c b/io_uring/net.c index 3adcb09ae264..0af8a02df580 100644 --- a/io_uring/net.c +++ b/io_uring/net.c @@ -182,6 +182,37 @@ static int io_sendmsg_copy_hdr(struct io_kiocb *req, &iomsg->free_iov); } +int io_sendzc_prep_async(struct io_kiocb *req) +{ + struct io_sendzc *zc = io_kiocb_to_cmd(req, struct io_sendzc); + struct io_async_msghdr *io; + int ret; + + if (!zc->addr || req_has_async_data(req)) + return 0; + if (io_alloc_async_data(req)) + return -ENOMEM; + + io = req->async_data; + 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, + unsigned int issue_flags) +{ + struct io_async_msghdr *io; + + if (!addr || req_has_async_data(req)) + return -EAGAIN; + if (io_alloc_async_data(req)) + return -ENOMEM; + io = req->async_data; + memcpy(&io->addr, addr, sizeof(io->addr)); + return -EAGAIN; +} + int io_sendmsg_prep_async(struct io_kiocb *req) { int ret; @@ -944,7 +975,7 @@ static int io_sg_from_iter(struct sock *sk, struct sk_buff *skb, int io_sendzc(struct io_kiocb *req, unsigned int issue_flags) { - struct sockaddr_storage address; + struct sockaddr_storage __address, *addr = NULL; struct io_ring_ctx *ctx = req->ctx; struct io_sendzc *zc = io_kiocb_to_cmd(req, struct io_sendzc); struct io_notif_slot *notif_slot; @@ -978,10 +1009,17 @@ int io_sendzc(struct io_kiocb *req, unsigned int issue_flags) msg.msg_namelen = 0; if (zc->addr) { - ret = move_addr_to_kernel(zc->addr, zc->addr_len, &address); - if (unlikely(ret < 0)) - return ret; - msg.msg_name = (struct sockaddr *)&address; + if (req_has_async_data(req)) { + struct io_async_msghdr *io = req->async_data; + + msg.msg_name = addr = &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; + addr = &__address; + } msg.msg_namelen = zc->addr_len; } @@ -1013,13 +1051,14 @@ int io_sendzc(struct io_kiocb *req, unsigned int issue_flags) if (unlikely(ret < min_ret)) { if (ret == -EAGAIN && (issue_flags & IO_URING_F_NONBLOCK)) - return -EAGAIN; + return io_setup_async_addr(req, addr, issue_flags); + if (ret > 0 && io_net_retry(sock, msg.msg_flags)) { zc->len -= ret; zc->buf += ret; zc->done_io += ret; req->flags |= REQ_F_PARTIAL_IO; - return -EAGAIN; + return io_setup_async_addr(req, addr, issue_flags); } if (ret == -ERESTARTSYS) ret = -EINTR; diff --git a/io_uring/net.h b/io_uring/net.h index 7c438d39c089..f91f56c6eeac 100644 --- a/io_uring/net.h +++ b/io_uring/net.h @@ -31,6 +31,7 @@ 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_sendzc_prep_async(struct io_kiocb *req); 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); diff --git a/io_uring/opdef.c b/io_uring/opdef.c index 72dd2b2d8a9d..41410126c1c6 100644 --- a/io_uring/opdef.c +++ b/io_uring/opdef.c @@ -478,13 +478,15 @@ const struct io_op_def io_op_defs[] = { .pollout = 1, .audit_skip = 1, .ioprio = 1, + .manual_alloc = 1, #if defined(CONFIG_NET) + .async_size = sizeof(struct io_async_msghdr), .prep = io_sendzc_prep, .issue = io_sendzc, + .prep_async = io_sendzc_prep_async, #else .prep = io_eopnotsupp_prep, #endif - }, };