From 06bdea20c1076471f7ab7d3ad7f35cbcbd59a8e3 Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Tue, 23 Nov 2021 00:07:46 +0000 Subject: io_uring: simplify reissue in kiocb_done Simplify failed resubmission prep in kiocb_done(), it's a bit ugly with conditional logic and hand handling cflags / select buffers. Instead, punt to tw and use io_req_task_complete() already handling all the cases. Signed-off-by: Pavel Begunkov Link: https://lore.kernel.org/r/667c33484b05b612e9420e1b1d5f4dc46d0ee9ce.1637524285.git.asml.silence@gmail.com Signed-off-by: Jens Axboe --- fs/io_uring.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index b07196b4511c..8b7b30835c72 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -2948,17 +2948,10 @@ static void kiocb_done(struct kiocb *kiocb, ssize_t ret, if (io_resubmit_prep(req)) { io_req_task_queue_reissue(req); } else { - unsigned int cflags = io_put_rw_kbuf(req); - struct io_ring_ctx *ctx = req->ctx; - req_set_fail(req); - if (issue_flags & IO_URING_F_UNLOCKED) { - mutex_lock(&ctx->uring_lock); - __io_req_complete(req, issue_flags, ret, cflags); - mutex_unlock(&ctx->uring_lock); - } else { - __io_req_complete(req, issue_flags, ret, cflags); - } + req->result = ret; + req->io_task_work.func = io_req_task_complete; + io_req_task_work_add(req); } } } -- cgit From 7297ce3d59449de49d3c9e1f64ae25488750a1fc Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Tue, 23 Nov 2021 00:07:47 +0000 Subject: io_uring: improve send/recv error handling Hide all error handling under common if block, removes two extra ifs on the success path and keeps the handling more condensed. Signed-off-by: Pavel Begunkov Link: https://lore.kernel.org/r/5761545158a12968f3caf30f747eea65ed75dfc1.1637524285.git.asml.silence@gmail.com Signed-off-by: Jens Axboe --- fs/io_uring.c | 55 +++++++++++++++++++++++++++++++------------------------ 1 file changed, 31 insertions(+), 24 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 8b7b30835c72..7ef2d0c1296f 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -4788,17 +4788,18 @@ static int io_sendmsg(struct io_kiocb *req, unsigned int issue_flags) min_ret = iov_iter_count(&kmsg->msg.msg_iter); ret = __sys_sendmsg_sock(sock, &kmsg->msg, flags); - if ((issue_flags & IO_URING_F_NONBLOCK) && ret == -EAGAIN) - return io_setup_async_msg(req, kmsg); - if (ret == -ERESTARTSYS) - ret = -EINTR; + if (ret < min_ret) { + if (ret == -EAGAIN && (issue_flags & IO_URING_F_NONBLOCK)) + return io_setup_async_msg(req, kmsg); + if (ret == -ERESTARTSYS) + ret = -EINTR; + req_set_fail(req); + } /* fast path, check for non-NULL to avoid function call */ if (kmsg->free_iov) kfree(kmsg->free_iov); req->flags &= ~REQ_F_NEED_CLEANUP; - if (ret < min_ret) - req_set_fail(req); __io_req_complete(req, issue_flags, ret, 0); return 0; } @@ -4834,13 +4835,13 @@ static int io_send(struct io_kiocb *req, unsigned int issue_flags) msg.msg_flags = flags; ret = sock_sendmsg(sock, &msg); - if ((issue_flags & IO_URING_F_NONBLOCK) && ret == -EAGAIN) - return -EAGAIN; - if (ret == -ERESTARTSYS) - ret = -EINTR; - - if (ret < min_ret) + if (ret < min_ret) { + if (ret == -EAGAIN && (issue_flags & IO_URING_F_NONBLOCK)) + return -EAGAIN; + if (ret == -ERESTARTSYS) + ret = -EINTR; req_set_fail(req); + } __io_req_complete(req, issue_flags, ret, 0); return 0; } @@ -5017,10 +5018,15 @@ static int io_recvmsg(struct io_kiocb *req, unsigned int issue_flags) ret = __sys_recvmsg_sock(sock, &kmsg->msg, req->sr_msg.umsg, kmsg->uaddr, flags); - if (force_nonblock && ret == -EAGAIN) - return io_setup_async_msg(req, kmsg); - if (ret == -ERESTARTSYS) - ret = -EINTR; + if (ret < min_ret) { + if (ret == -EAGAIN && force_nonblock) + return io_setup_async_msg(req, kmsg); + if (ret == -ERESTARTSYS) + ret = -EINTR; + req_set_fail(req); + } else if ((flags & MSG_WAITALL) && (kmsg->msg.msg_flags & (MSG_TRUNC | MSG_CTRUNC))) { + req_set_fail(req); + } if (req->flags & REQ_F_BUFFER_SELECTED) cflags = io_put_recv_kbuf(req); @@ -5028,8 +5034,6 @@ static int io_recvmsg(struct io_kiocb *req, unsigned int issue_flags) if (kmsg->free_iov) kfree(kmsg->free_iov); req->flags &= ~REQ_F_NEED_CLEANUP; - if (ret < min_ret || ((flags & MSG_WAITALL) && (kmsg->msg.msg_flags & (MSG_TRUNC | MSG_CTRUNC)))) - req_set_fail(req); __io_req_complete(req, issue_flags, ret, cflags); return 0; } @@ -5076,15 +5080,18 @@ static int io_recv(struct io_kiocb *req, unsigned int issue_flags) min_ret = iov_iter_count(&msg.msg_iter); ret = sock_recvmsg(sock, &msg, flags); - if (force_nonblock && ret == -EAGAIN) - return -EAGAIN; - if (ret == -ERESTARTSYS) - ret = -EINTR; out_free: + if (ret < min_ret) { + if (ret == -EAGAIN && force_nonblock) + return -EAGAIN; + if (ret == -ERESTARTSYS) + ret = -EINTR; + req_set_fail(req); + } else if ((flags & MSG_WAITALL) && (msg.msg_flags & (MSG_TRUNC | MSG_CTRUNC))) { + req_set_fail(req); + } if (req->flags & REQ_F_BUFFER_SELECTED) cflags = io_put_recv_kbuf(req); - if (ret < min_ret || ((flags & MSG_WAITALL) && (msg.msg_flags & (MSG_TRUNC | MSG_CTRUNC)))) - req_set_fail(req); __io_req_complete(req, issue_flags, ret, cflags); return 0; } -- cgit From f3251183b298912e09297cb22614361c63122e82 Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Tue, 23 Nov 2021 00:07:48 +0000 Subject: io_uring: clean __io_import_iovec() Apparently, implicit 0 to NULL conversion with ERR_PTR is not recommended and makes some tooling like Smatch to complain. Handle it explicitly, compilers are perfectly capable to optimise it out. Link: https://lore.kernel.org/all/20211108134937.GA2863@kili/ Reported-by: Dan Carpenter Signed-off-by: Pavel Begunkov Link: https://lore.kernel.org/r/5c6ed369ad95075dab345df679f8677b8fe66656.1637524285.git.asml.silence@gmail.com Signed-off-by: Jens Axboe --- fs/io_uring.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 7ef2d0c1296f..a65fb9cd9db7 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -3179,10 +3179,12 @@ static struct iovec *__io_import_iovec(int rw, struct io_kiocb *req, size_t sqe_len; ssize_t ret; - BUILD_BUG_ON(ERR_PTR(0) != NULL); - - if (opcode == IORING_OP_READ_FIXED || opcode == IORING_OP_WRITE_FIXED) - return ERR_PTR(io_import_fixed(req, rw, iter)); + if (opcode == IORING_OP_READ_FIXED || opcode == IORING_OP_WRITE_FIXED) { + ret = io_import_fixed(req, rw, iter); + if (ret) + return ERR_PTR(ret); + return NULL; + } /* buffer index only valid with fixed read/write, or buffer select */ if (unlikely(req->buf_index && !(req->flags & REQ_F_BUFFER_SELECT))) @@ -3200,15 +3202,18 @@ static struct iovec *__io_import_iovec(int rw, struct io_kiocb *req, } ret = import_single_range(rw, buf, sqe_len, s->fast_iov, iter); - return ERR_PTR(ret); + if (ret) + return ERR_PTR(ret); + return NULL; } iovec = s->fast_iov; if (req->flags & REQ_F_BUFFER_SELECT) { ret = io_iov_buffer_select(req, iovec, issue_flags); - if (!ret) - iov_iter_init(iter, rw, iovec, 1, iovec->iov_len); - return ERR_PTR(ret); + if (ret) + return ERR_PTR(ret); + iov_iter_init(iter, rw, iovec, 1, iovec->iov_len); + return NULL; } ret = __import_iovec(rw, buf, sqe_len, UIO_FASTIOV, &iovec, iter, -- cgit From 2ea537ca02b12e6e03dfcac82013ff289a75eed8 Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Tue, 23 Nov 2021 00:07:49 +0000 Subject: io_uring: improve argument types of kiocb_done() kiocb_done() accepts a pointer to struct kiocb, pass struct io_kiocb (i.e. io_uring's request) instead so we can get rid of useless container_of(). Signed-off-by: Pavel Begunkov Link: https://lore.kernel.org/r/252016eed77806f58b48251a85cd8c645f900433.1637524285.git.asml.silence@gmail.com Signed-off-by: Jens Axboe --- fs/io_uring.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index a65fb9cd9db7..86847eac3a99 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -2922,10 +2922,9 @@ static inline void io_rw_done(struct kiocb *kiocb, ssize_t ret) } } -static void kiocb_done(struct kiocb *kiocb, ssize_t ret, +static void kiocb_done(struct io_kiocb *req, ssize_t ret, unsigned int issue_flags) { - struct io_kiocb *req = container_of(kiocb, struct io_kiocb, rw.kiocb); struct io_async_rw *io = req->async_data; /* add previously done IO, if any */ @@ -2937,11 +2936,11 @@ static void kiocb_done(struct kiocb *kiocb, ssize_t ret, } if (req->flags & REQ_F_CUR_POS) - req->file->f_pos = kiocb->ki_pos; - if (ret >= 0 && (kiocb->ki_complete == io_complete_rw)) + req->file->f_pos = req->rw.kiocb.ki_pos; + if (ret >= 0 && (req->rw.kiocb.ki_complete == io_complete_rw)) __io_complete_rw(req, ret, 0, issue_flags); else - io_rw_done(kiocb, ret); + io_rw_done(&req->rw.kiocb, ret); if (req->flags & REQ_F_REISSUE) { req->flags &= ~REQ_F_REISSUE; @@ -3584,7 +3583,7 @@ static int io_read(struct io_kiocb *req, unsigned int issue_flags) iov_iter_restore(&s->iter, &s->iter_state); } while (ret > 0); done: - kiocb_done(kiocb, ret, issue_flags); + kiocb_done(req, ret, issue_flags); out_free: /* it's faster to check here then delegate to kfree */ if (iovec) @@ -3681,7 +3680,7 @@ static int io_write(struct io_kiocb *req, unsigned int issue_flags) if (ret2 == -EAGAIN && (req->ctx->flags & IORING_SETUP_IOPOLL)) goto copy_iov; done: - kiocb_done(kiocb, ret2, issue_flags); + kiocb_done(req, ret2, issue_flags); } else { copy_iov: iov_iter_restore(&s->iter, &s->iter_state); -- cgit From 913a571affedd17239c4d4ea90c8874b32fc2191 Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Wed, 10 Nov 2021 15:49:31 +0000 Subject: io_uring: clean cqe filling functions Split io_cqring_fill_event() into a couple of more targeted functions. The first on is io_fill_cqe_aux() for completions that are not associated with request completions and doing the ->cq_extra accounting. Examples are additional CQEs from multishot poll and rsrc notifications. The second is io_fill_cqe_req(), should be called when it's a normal request completion. Nothing more to it at the moment, will be used in later patches. The last one is inlined __io_fill_cqe() for a finer grained control, should be used with caution and in hottest places. Signed-off-by: Pavel Begunkov Link: https://lore.kernel.org/r/59a9117a4a44fc9efcf04b3afa51e0d080f5943c.1636559119.git.asml.silence@gmail.com Signed-off-by: Jens Axboe --- fs/io_uring.c | 58 ++++++++++++++++++++++++++++++---------------------------- 1 file changed, 30 insertions(+), 28 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 86847eac3a99..2a89928ac1ba 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -1108,8 +1108,8 @@ static void io_uring_try_cancel_requests(struct io_ring_ctx *ctx, bool cancel_all); static void io_uring_cancel_generic(bool cancel_all, struct io_sq_data *sqd); -static bool io_cqring_fill_event(struct io_ring_ctx *ctx, u64 user_data, - s32 res, u32 cflags); +static void io_fill_cqe_req(struct io_kiocb *req, s32 res, u32 cflags); + static void io_put_req(struct io_kiocb *req); static void io_put_req_deferred(struct io_kiocb *req); static void io_dismantle_req(struct io_kiocb *req); @@ -1560,7 +1560,7 @@ static void io_kill_timeout(struct io_kiocb *req, int status) atomic_set(&req->ctx->cq_timeouts, atomic_read(&req->ctx->cq_timeouts) + 1); list_del_init(&req->timeout.list); - io_cqring_fill_event(req->ctx, req->user_data, status, 0); + io_fill_cqe_req(req, status, 0); io_put_req_deferred(req); } } @@ -1819,8 +1819,8 @@ static bool io_cqring_event_overflow(struct io_ring_ctx *ctx, u64 user_data, return true; } -static inline bool __io_cqring_fill_event(struct io_ring_ctx *ctx, u64 user_data, - s32 res, u32 cflags) +static inline bool __io_fill_cqe(struct io_ring_ctx *ctx, u64 user_data, + s32 res, u32 cflags) { struct io_uring_cqe *cqe; @@ -1841,11 +1841,16 @@ static inline bool __io_cqring_fill_event(struct io_ring_ctx *ctx, u64 user_data return io_cqring_event_overflow(ctx, user_data, res, cflags); } -/* not as hot to bloat with inlining */ -static noinline bool io_cqring_fill_event(struct io_ring_ctx *ctx, u64 user_data, - s32 res, u32 cflags) +static noinline void io_fill_cqe_req(struct io_kiocb *req, s32 res, u32 cflags) +{ + __io_fill_cqe(req->ctx, req->user_data, res, cflags); +} + +static noinline bool io_fill_cqe_aux(struct io_ring_ctx *ctx, u64 user_data, + s32 res, u32 cflags) { - return __io_cqring_fill_event(ctx, user_data, res, cflags); + ctx->cq_extra++; + return __io_fill_cqe(ctx, user_data, res, cflags); } static void io_req_complete_post(struct io_kiocb *req, s32 res, @@ -1854,7 +1859,7 @@ static void io_req_complete_post(struct io_kiocb *req, s32 res, struct io_ring_ctx *ctx = req->ctx; spin_lock(&ctx->completion_lock); - __io_cqring_fill_event(ctx, req->user_data, res, cflags); + __io_fill_cqe(ctx, req->user_data, res, cflags); /* * If we're the last reference to this request, add to our locked * free_list cache. @@ -2062,8 +2067,7 @@ static bool io_kill_linked_timeout(struct io_kiocb *req) link->timeout.head = NULL; if (hrtimer_try_to_cancel(&io->timer) != -1) { list_del(&link->timeout.list); - io_cqring_fill_event(link->ctx, link->user_data, - -ECANCELED, 0); + io_fill_cqe_req(link, -ECANCELED, 0); io_put_req_deferred(link); return true; } @@ -2087,7 +2091,7 @@ static void io_fail_links(struct io_kiocb *req) link->link = NULL; trace_io_uring_fail_link(req, link); - io_cqring_fill_event(link->ctx, link->user_data, res, 0); + io_fill_cqe_req(link, res, 0); io_put_req_deferred(link); link = nxt; } @@ -2104,8 +2108,7 @@ static bool io_disarm_next(struct io_kiocb *req) req->flags &= ~REQ_F_ARM_LTIMEOUT; if (link && link->opcode == IORING_OP_LINK_TIMEOUT) { io_remove_next_linked(req); - io_cqring_fill_event(link->ctx, link->user_data, - -ECANCELED, 0); + io_fill_cqe_req(link, -ECANCELED, 0); io_put_req_deferred(link); posted = true; } @@ -2369,8 +2372,8 @@ static void __io_submit_flush_completions(struct io_ring_ctx *ctx) struct io_kiocb *req = container_of(node, struct io_kiocb, comp_list); - __io_cqring_fill_event(ctx, req->user_data, req->result, - req->cflags); + __io_fill_cqe(ctx, req->user_data, req->result, + req->cflags); } io_commit_cqring(ctx); spin_unlock(&ctx->completion_lock); @@ -2504,8 +2507,8 @@ static int io_do_iopoll(struct io_ring_ctx *ctx, bool force_nonspin) /* order with io_complete_rw_iopoll(), e.g. ->result updates */ if (!smp_load_acquire(&req->iopoll_completed)) break; - __io_cqring_fill_event(ctx, req->user_data, req->result, - io_put_rw_kbuf(req)); + __io_fill_cqe(ctx, req->user_data, req->result, + io_put_rw_kbuf(req)); nr_events++; } @@ -5360,13 +5363,13 @@ static bool __io_poll_complete(struct io_kiocb *req, __poll_t mask) } if (req->poll.events & EPOLLONESHOT) flags = 0; - if (!io_cqring_fill_event(ctx, req->user_data, error, flags)) { + + if (!(flags & IORING_CQE_F_MORE)) { + io_fill_cqe_req(req, error, flags); + } else if (!io_fill_cqe_aux(ctx, req->user_data, error, flags)) { req->poll.events |= EPOLLONESHOT; flags = 0; } - if (flags & IORING_CQE_F_MORE) - ctx->cq_extra++; - return !(flags & IORING_CQE_F_MORE); } @@ -5684,9 +5687,9 @@ static bool io_poll_remove_one(struct io_kiocb *req) do_complete = __io_poll_remove_one(req, io_poll_get_single(req), true); if (do_complete) { - io_cqring_fill_event(req->ctx, req->user_data, -ECANCELED, 0); - io_commit_cqring(req->ctx); req_set_fail(req); + io_fill_cqe_req(req, -ECANCELED, 0); + io_commit_cqring(req->ctx); io_put_req_deferred(req); } return do_complete; @@ -5986,7 +5989,7 @@ static int io_timeout_cancel(struct io_ring_ctx *ctx, __u64 user_data) return PTR_ERR(req); req_set_fail(req); - io_cqring_fill_event(ctx, req->user_data, -ECANCELED, 0); + io_fill_cqe_req(req, -ECANCELED, 0); io_put_req_deferred(req); return 0; } @@ -8219,8 +8222,7 @@ static void __io_rsrc_put_work(struct io_rsrc_node *ref_node) io_ring_submit_lock(ctx, lock_ring); spin_lock(&ctx->completion_lock); - io_cqring_fill_event(ctx, prsrc->tag, 0, 0); - ctx->cq_extra++; + io_fill_cqe_aux(ctx, prsrc->tag, 0, 0); io_commit_cqring(ctx); spin_unlock(&ctx->completion_lock); io_cqring_ev_posted(ctx); -- cgit From 04c76b41ca974b508522831441dd7e5b1b59cbb0 Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Wed, 10 Nov 2021 15:49:32 +0000 Subject: io_uring: add option to skip CQE posting Emitting a CQE is expensive from the kernel perspective. Often, it's also not convenient for the userspace, spends some cycles on processing and just complicates the logic. A similar problems goes for linked requests, where we post an CQE for each request in the link. Introduce a new flags, IOSQE_CQE_SKIP_SUCCESS, trying to help with it. When set and a request completed successfully, it won't generate a CQE. When fails, it produces an CQE, but all following linked requests will be CQE-less, regardless whether they have IOSQE_CQE_SKIP_SUCCESS or not. The notion of "fail" is the same as for link failing-cancellation, where it's opcode dependent, and _usually_ result >= 0 is a success, but not always. Linked timeouts are a bit special. When the requests it's linked to was not attempted to be executed, e.g. failing linked requests, it follows the description above. Otherwise, whether a linked timeout will post a completion or not solely depends on IOSQE_CQE_SKIP_SUCCESS of that linked timeout request. Linked timeout never "fail" during execution, so for them it's unconditional. It's expected for users to not really care about the result of it but rely solely on the result of the master request. Another reason for such a treatment is that it's racy, and the timeout callback may be running awhile the master request posts its completion. use case 1: If one doesn't care about results of some requests, e.g. normal timeouts, just set IOSQE_CQE_SKIP_SUCCESS. Error result will still be posted and need to be handled. use case 2: Set IOSQE_CQE_SKIP_SUCCESS for all requests of a link but the last, and it'll post a completion only for the last one if everything goes right, otherwise there will be one only one CQE for the first failed request. Signed-off-by: Pavel Begunkov Link: https://lore.kernel.org/r/0220fbe06f7cf99e6fc71b4297bb1cb6c0e89c2c.1636559119.git.asml.silence@gmail.com Signed-off-by: Jens Axboe --- fs/io_uring.c | 42 +++++++++++++++++++++++++++++++++--------- include/uapi/linux/io_uring.h | 4 ++++ 2 files changed, 37 insertions(+), 9 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 2a89928ac1ba..7d3589e3a277 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -106,7 +106,8 @@ #define IORING_MAX_REG_BUFFERS (1U << 14) #define SQE_COMMON_FLAGS (IOSQE_FIXED_FILE | IOSQE_IO_LINK | \ - IOSQE_IO_HARDLINK | IOSQE_ASYNC) + IOSQE_IO_HARDLINK | IOSQE_ASYNC | \ + IOSQE_CQE_SKIP_SUCCESS) #define SQE_VALID_FLAGS (SQE_COMMON_FLAGS|IOSQE_BUFFER_SELECT|IOSQE_IO_DRAIN) @@ -721,6 +722,7 @@ enum { REQ_F_HARDLINK_BIT = IOSQE_IO_HARDLINK_BIT, REQ_F_FORCE_ASYNC_BIT = IOSQE_ASYNC_BIT, REQ_F_BUFFER_SELECT_BIT = IOSQE_BUFFER_SELECT_BIT, + REQ_F_CQE_SKIP_BIT = IOSQE_CQE_SKIP_SUCCESS_BIT, /* first byte is taken by user flags, shift it to not overlap */ REQ_F_FAIL_BIT = 8, @@ -737,6 +739,7 @@ enum { REQ_F_REFCOUNT_BIT, REQ_F_ARM_LTIMEOUT_BIT, REQ_F_ASYNC_DATA_BIT, + REQ_F_SKIP_LINK_CQES_BIT, /* keep async read/write and isreg together and in order */ REQ_F_SUPPORT_NOWAIT_BIT, REQ_F_ISREG_BIT, @@ -758,6 +761,8 @@ enum { REQ_F_FORCE_ASYNC = BIT(REQ_F_FORCE_ASYNC_BIT), /* IOSQE_BUFFER_SELECT */ REQ_F_BUFFER_SELECT = BIT(REQ_F_BUFFER_SELECT_BIT), + /* IOSQE_CQE_SKIP_SUCCESS */ + REQ_F_CQE_SKIP = BIT(REQ_F_CQE_SKIP_BIT), /* fail rest of links */ REQ_F_FAIL = BIT(REQ_F_FAIL_BIT), @@ -791,6 +796,8 @@ enum { REQ_F_ARM_LTIMEOUT = BIT(REQ_F_ARM_LTIMEOUT_BIT), /* ->async_data allocated */ REQ_F_ASYNC_DATA = BIT(REQ_F_ASYNC_DATA_BIT), + /* don't post CQEs while failing linked requests */ + REQ_F_SKIP_LINK_CQES = BIT(REQ_F_SKIP_LINK_CQES_BIT), }; struct async_poll { @@ -1301,6 +1308,10 @@ static inline bool req_has_async_data(struct io_kiocb *req) static inline void req_set_fail(struct io_kiocb *req) { req->flags |= REQ_F_FAIL; + if (req->flags & REQ_F_CQE_SKIP) { + req->flags &= ~REQ_F_CQE_SKIP; + req->flags |= REQ_F_SKIP_LINK_CQES; + } } static inline void req_fail_link_node(struct io_kiocb *req, int res) @@ -1843,7 +1854,8 @@ static inline bool __io_fill_cqe(struct io_ring_ctx *ctx, u64 user_data, static noinline void io_fill_cqe_req(struct io_kiocb *req, s32 res, u32 cflags) { - __io_fill_cqe(req->ctx, req->user_data, res, cflags); + if (!(req->flags & REQ_F_CQE_SKIP)) + __io_fill_cqe(req->ctx, req->user_data, res, cflags); } static noinline bool io_fill_cqe_aux(struct io_ring_ctx *ctx, u64 user_data, @@ -1859,7 +1871,8 @@ static void io_req_complete_post(struct io_kiocb *req, s32 res, struct io_ring_ctx *ctx = req->ctx; spin_lock(&ctx->completion_lock); - __io_fill_cqe(ctx, req->user_data, res, cflags); + if (!(req->flags & REQ_F_CQE_SKIP)) + __io_fill_cqe(ctx, req->user_data, res, cflags); /* * If we're the last reference to this request, add to our locked * free_list cache. @@ -2067,6 +2080,7 @@ static bool io_kill_linked_timeout(struct io_kiocb *req) link->timeout.head = NULL; if (hrtimer_try_to_cancel(&io->timer) != -1) { list_del(&link->timeout.list); + /* leave REQ_F_CQE_SKIP to io_fill_cqe_req */ io_fill_cqe_req(link, -ECANCELED, 0); io_put_req_deferred(link); return true; @@ -2079,6 +2093,7 @@ static void io_fail_links(struct io_kiocb *req) __must_hold(&req->ctx->completion_lock) { struct io_kiocb *nxt, *link = req->link; + bool ignore_cqes = req->flags & REQ_F_SKIP_LINK_CQES; req->link = NULL; while (link) { @@ -2091,7 +2106,10 @@ static void io_fail_links(struct io_kiocb *req) link->link = NULL; trace_io_uring_fail_link(req, link); - io_fill_cqe_req(link, res, 0); + if (!ignore_cqes) { + link->flags &= ~REQ_F_CQE_SKIP; + io_fill_cqe_req(link, res, 0); + } io_put_req_deferred(link); link = nxt; } @@ -2108,6 +2126,7 @@ static bool io_disarm_next(struct io_kiocb *req) req->flags &= ~REQ_F_ARM_LTIMEOUT; if (link && link->opcode == IORING_OP_LINK_TIMEOUT) { io_remove_next_linked(req); + /* leave REQ_F_CQE_SKIP to io_fill_cqe_req */ io_fill_cqe_req(link, -ECANCELED, 0); io_put_req_deferred(link); posted = true; @@ -2372,8 +2391,9 @@ static void __io_submit_flush_completions(struct io_ring_ctx *ctx) struct io_kiocb *req = container_of(node, struct io_kiocb, comp_list); - __io_fill_cqe(ctx, req->user_data, req->result, - req->cflags); + if (!(req->flags & REQ_F_CQE_SKIP)) + __io_fill_cqe(ctx, req->user_data, req->result, + req->cflags); } io_commit_cqring(ctx); spin_unlock(&ctx->completion_lock); @@ -2503,12 +2523,14 @@ static int io_do_iopoll(struct io_ring_ctx *ctx, bool force_nonspin) prev = start; wq_list_for_each_resume(pos, prev) { struct io_kiocb *req = container_of(pos, struct io_kiocb, comp_list); + u32 cflags; /* order with io_complete_rw_iopoll(), e.g. ->result updates */ if (!smp_load_acquire(&req->iopoll_completed)) break; - __io_fill_cqe(ctx, req->user_data, req->result, - io_put_rw_kbuf(req)); + cflags = io_put_rw_kbuf(req); + if (!(req->flags & REQ_F_CQE_SKIP)) + __io_fill_cqe(ctx, req->user_data, req->result, cflags); nr_events++; } @@ -5832,6 +5854,8 @@ static int io_poll_add_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe flags = READ_ONCE(sqe->len); if (flags & ~IORING_POLL_ADD_MULTI) return -EINVAL; + if ((flags & IORING_POLL_ADD_MULTI) && (req->flags & REQ_F_CQE_SKIP)) + return -EINVAL; io_req_set_refcount(req); poll->events = io_poll_parse_events(sqe, flags); @@ -10442,7 +10466,7 @@ static __cold int io_uring_create(unsigned entries, struct io_uring_params *p, IORING_FEAT_CUR_PERSONALITY | IORING_FEAT_FAST_POLL | IORING_FEAT_POLL_32BITS | IORING_FEAT_SQPOLL_NONFIXED | IORING_FEAT_EXT_ARG | IORING_FEAT_NATIVE_WORKERS | - IORING_FEAT_RSRC_TAGS; + IORING_FEAT_RSRC_TAGS | IORING_FEAT_CQE_SKIP; if (copy_to_user(params, p, sizeof(*p))) { ret = -EFAULT; diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h index c45b5e9a9387..787f491f0d2a 100644 --- a/include/uapi/linux/io_uring.h +++ b/include/uapi/linux/io_uring.h @@ -70,6 +70,7 @@ enum { IOSQE_IO_HARDLINK_BIT, IOSQE_ASYNC_BIT, IOSQE_BUFFER_SELECT_BIT, + IOSQE_CQE_SKIP_SUCCESS_BIT, }; /* @@ -87,6 +88,8 @@ enum { #define IOSQE_ASYNC (1U << IOSQE_ASYNC_BIT) /* select buffer from sqe->buf_group */ #define IOSQE_BUFFER_SELECT (1U << IOSQE_BUFFER_SELECT_BIT) +/* don't post CQE if request succeeded */ +#define IOSQE_CQE_SKIP_SUCCESS (1U << IOSQE_CQE_SKIP_SUCCESS_BIT) /* * io_uring_setup() flags @@ -289,6 +292,7 @@ struct io_uring_params { #define IORING_FEAT_EXT_ARG (1U << 8) #define IORING_FEAT_NATIVE_WORKERS (1U << 9) #define IORING_FEAT_RSRC_TAGS (1U << 10) +#define IORING_FEAT_CQE_SKIP (1U << 11) /* * io_uring_register(2) opcodes and arguments -- cgit From 3d4aeb9f98058c3bdfef5286e240cf18c50fee89 Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Wed, 10 Nov 2021 15:49:33 +0000 Subject: io_uring: don't spinlock when not posting CQEs When no of queued for the batch completion requests need to post an CQE, see IOSQE_CQE_SKIP_SUCCESS, avoid grabbing ->completion_lock and other commit/post. Signed-off-by: Pavel Begunkov Link: https://lore.kernel.org/r/8d4b4a08bca022cbe19af00266407116775b3e4d.1636559119.git.asml.silence@gmail.com Signed-off-by: Jens Axboe --- fs/io_uring.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 7d3589e3a277..f01263a31ea4 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -321,6 +321,7 @@ struct io_submit_state { bool plug_started; bool need_plug; + bool flush_cqes; unsigned short submit_nr; struct blk_plug plug; }; @@ -1525,8 +1526,11 @@ static void io_prep_async_link(struct io_kiocb *req) static inline void io_req_add_compl_list(struct io_kiocb *req) { + struct io_ring_ctx *ctx = req->ctx; struct io_submit_state *state = &req->ctx->submit_state; + if (!(req->flags & REQ_F_CQE_SKIP)) + ctx->submit_state.flush_cqes = true; wq_list_add_tail(&req->comp_list, &state->compl_reqs); } @@ -2386,18 +2390,22 @@ static void __io_submit_flush_completions(struct io_ring_ctx *ctx) struct io_wq_work_node *node, *prev; struct io_submit_state *state = &ctx->submit_state; - spin_lock(&ctx->completion_lock); - wq_list_for_each(node, prev, &state->compl_reqs) { - struct io_kiocb *req = container_of(node, struct io_kiocb, + if (state->flush_cqes) { + spin_lock(&ctx->completion_lock); + wq_list_for_each(node, prev, &state->compl_reqs) { + struct io_kiocb *req = container_of(node, struct io_kiocb, comp_list); - if (!(req->flags & REQ_F_CQE_SKIP)) - __io_fill_cqe(ctx, req->user_data, req->result, - req->cflags); + if (!(req->flags & REQ_F_CQE_SKIP)) + __io_fill_cqe(ctx, req->user_data, req->result, + req->cflags); + } + + io_commit_cqring(ctx); + spin_unlock(&ctx->completion_lock); + io_cqring_ev_posted(ctx); + state->flush_cqes = false; } - io_commit_cqring(ctx); - spin_unlock(&ctx->completion_lock); - io_cqring_ev_posted(ctx); io_free_batch_list(ctx, state->compl_reqs.first); INIT_WQ_LIST(&state->compl_reqs); -- cgit From 5562a8d71aa32ea27133d8b10406b3dcd57c01a5 Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Wed, 10 Nov 2021 15:49:34 +0000 Subject: io_uring: disable drain with cqe skip Current IOSQE_IO_DRAIN implementation doesn't work well with CQE skipping and it's not allowed, otherwise some requests might be not executed until the ring is destroyed and the userspace would hang. Let's fail all drain requests after seeing IOSQE_CQE_SKIP_SUCCESS at least once. All drained requests prior to that will get run normally, so there should be no stalls. However, even though such mixing wouldn't lead to issues at the moment, it's still not allowed as the behaviour may change. Signed-off-by: Pavel Begunkov Link: https://lore.kernel.org/r/bcf7164f8bf3eb54b7bb7b4fd119907fa4d4d43b.1636559119.git.asml.silence@gmail.com Signed-off-by: Jens Axboe --- fs/io_uring.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index f01263a31ea4..f666a0e7f5e8 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -106,10 +106,10 @@ #define IORING_MAX_REG_BUFFERS (1U << 14) #define SQE_COMMON_FLAGS (IOSQE_FIXED_FILE | IOSQE_IO_LINK | \ - IOSQE_IO_HARDLINK | IOSQE_ASYNC | \ - IOSQE_CQE_SKIP_SUCCESS) + IOSQE_IO_HARDLINK | IOSQE_ASYNC) -#define SQE_VALID_FLAGS (SQE_COMMON_FLAGS|IOSQE_BUFFER_SELECT|IOSQE_IO_DRAIN) +#define SQE_VALID_FLAGS (SQE_COMMON_FLAGS | IOSQE_BUFFER_SELECT | \ + IOSQE_IO_DRAIN | IOSQE_CQE_SKIP_SUCCESS) #define IO_REQ_CLEAN_FLAGS (REQ_F_BUFFER_SELECTED | REQ_F_NEED_CLEANUP | \ REQ_F_POLLED | REQ_F_INFLIGHT | REQ_F_CREDS | \ @@ -339,6 +339,7 @@ struct io_ring_ctx { unsigned int restricted: 1; unsigned int off_timeout_used: 1; unsigned int drain_active: 1; + unsigned int drain_disabled: 1; } ____cacheline_aligned_in_smp; /* submission data */ @@ -7127,8 +7128,13 @@ static int io_init_req(struct io_ring_ctx *ctx, struct io_kiocb *req, if ((sqe_flags & IOSQE_BUFFER_SELECT) && !io_op_defs[opcode].buffer_select) return -EOPNOTSUPP; - if (sqe_flags & IOSQE_IO_DRAIN) + if (sqe_flags & IOSQE_CQE_SKIP_SUCCESS) + ctx->drain_disabled = true; + if (sqe_flags & IOSQE_IO_DRAIN) { + if (ctx->drain_disabled) + return -EOPNOTSUPP; io_init_req_drain(req); + } } if (unlikely(ctx->restricted || ctx->drain_active || ctx->drain_next)) { if (ctx->restricted && !io_check_restriction(ctx, req, sqe_flags)) -- cgit From e302f1046f4c209291b07ff7bc4d15ca26891f16 Mon Sep 17 00:00:00 2001 From: Hao Xu Date: Thu, 25 Nov 2021 17:21:02 +0800 Subject: io_uring: fix no lock protection for ctx->cq_extra ctx->cq_extra should be protected by completion lock so that the req_need_defer() does the right check. Cc: stable@vger.kernel.org Signed-off-by: Hao Xu Link: https://lore.kernel.org/r/20211125092103.224502-2-haoxu@linux.alibaba.com Signed-off-by: Jens Axboe --- fs/io_uring.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fs/io_uring.c b/fs/io_uring.c index f666a0e7f5e8..ae9534382b26 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -6537,12 +6537,15 @@ static __cold void io_drain_req(struct io_kiocb *req) u32 seq = io_get_sequence(req); /* Still need defer if there is pending req in defer list. */ + spin_lock(&ctx->completion_lock); if (!req_need_defer(req, seq) && list_empty_careful(&ctx->defer_list)) { + spin_unlock(&ctx->completion_lock); queue: ctx->drain_active = false; io_req_task_queue(req); return; } + spin_unlock(&ctx->completion_lock); ret = io_req_prep_async(req); if (ret) { -- cgit From b6c7db32183251204f124b10d6177d46558ca7b8 Mon Sep 17 00:00:00 2001 From: Hao Xu Date: Thu, 25 Nov 2021 17:21:03 +0800 Subject: io_uring: better to use REQ_F_IO_DRAIN for req->flags It's better to use REQ_F_IO_DRAIN for req->flags rather than IOSQE_IO_DRAIN though they have same value. Signed-off-by: Hao Xu Link: https://lore.kernel.org/r/20211125092103.224502-3-haoxu@linux.alibaba.com Signed-off-by: Jens Axboe --- fs/io_uring.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index ae9534382b26..08b1b3de9b3f 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -7095,10 +7095,10 @@ static void io_init_req_drain(struct io_kiocb *req) * If we need to drain a request in the middle of a link, drain * the head request and the next request/link after the current * link. Considering sequential execution of links, - * IOSQE_IO_DRAIN will be maintained for every request of our + * REQ_F_IO_DRAIN will be maintained for every request of our * link. */ - head->flags |= IOSQE_IO_DRAIN | REQ_F_FORCE_ASYNC; + head->flags |= REQ_F_IO_DRAIN | REQ_F_FORCE_ASYNC; ctx->drain_next = true; } } @@ -7149,7 +7149,7 @@ static int io_init_req(struct io_ring_ctx *ctx, struct io_kiocb *req, if (unlikely(ctx->drain_next) && !ctx->submit_state.link.head) { ctx->drain_next = false; ctx->drain_active = true; - req->flags |= IOSQE_IO_DRAIN | REQ_F_FORCE_ASYNC; + req->flags |= REQ_F_IO_DRAIN | REQ_F_FORCE_ASYNC; } } -- cgit From 2087009c74d41ab8579f08157bca55b7d0857ee5 Mon Sep 17 00:00:00 2001 From: Ye Bin Date: Mon, 29 Nov 2021 12:15:37 +0800 Subject: io_uring: validate timespec for timeout removals Like commit f6223ff79966, timeout removal should also validate the timespec that is being passed in. Signed-off-by: Ye Bin Link: https://lore.kernel.org/r/20211129041537.1936270-1-yebin10@huawei.com Signed-off-by: Jens Axboe --- fs/io_uring.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fs/io_uring.c b/fs/io_uring.c index 08b1b3de9b3f..8b6bfed16f65 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -6111,6 +6111,8 @@ static int io_timeout_remove_prep(struct io_kiocb *req, return -EINVAL; if (get_timespec64(&tr->ts, u64_to_user_ptr(sqe->addr2))) return -EFAULT; + if (tr->ts.tv_sec < 0 || tr->ts.tv_nsec < 0) + return -EINVAL; } else if (tr->flags) { /* timeout removal doesn't support flags */ return -EINVAL; -- cgit From 3648e5265cfa51492a65ee5a01f151807ec46dee Mon Sep 17 00:00:00 2001 From: Hao Xu Date: Sun, 5 Dec 2021 14:37:57 +0000 Subject: io_uring: move up io_put_kbuf() and io_put_rw_kbuf() Move them up to avoid explicit declaration. We will use them in later patches. Reviewed-by: Pavel Begunkov Signed-off-by: Hao Xu Signed-off-by: Pavel Begunkov Link: https://lore.kernel.org/r/3631243d6fc4a79bbba0cd62597fc8cd5be95924.1638714983.git.asml.silence@gmail.com Signed-off-by: Jens Axboe --- fs/io_uring.c | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 8b6bfed16f65..ffbe1b76f3a0 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -1273,6 +1273,24 @@ static inline void io_req_set_rsrc_node(struct io_kiocb *req, } } +static unsigned int io_put_kbuf(struct io_kiocb *req, struct io_buffer *kbuf) +{ + unsigned int cflags; + + cflags = kbuf->bid << IORING_CQE_BUFFER_SHIFT; + cflags |= IORING_CQE_F_BUFFER; + req->flags &= ~REQ_F_BUFFER_SELECTED; + kfree(kbuf); + return cflags; +} + +static inline unsigned int io_put_rw_kbuf(struct io_kiocb *req) +{ + if (likely(!(req->flags & REQ_F_BUFFER_SELECTED))) + return 0; + return io_put_kbuf(req, req->kbuf); +} + static void io_refs_resurrect(struct percpu_ref *ref, struct completion *compl) { bool got = percpu_ref_tryget(ref); @@ -2456,24 +2474,6 @@ static inline unsigned int io_sqring_entries(struct io_ring_ctx *ctx) return smp_load_acquire(&rings->sq.tail) - ctx->cached_sq_head; } -static unsigned int io_put_kbuf(struct io_kiocb *req, struct io_buffer *kbuf) -{ - unsigned int cflags; - - cflags = kbuf->bid << IORING_CQE_BUFFER_SHIFT; - cflags |= IORING_CQE_F_BUFFER; - req->flags &= ~REQ_F_BUFFER_SELECTED; - kfree(kbuf); - return cflags; -} - -static inline unsigned int io_put_rw_kbuf(struct io_kiocb *req) -{ - if (likely(!(req->flags & REQ_F_BUFFER_SELECTED))) - return 0; - return io_put_kbuf(req, req->kbuf); -} - static inline bool io_run_task_work(void) { if (test_thread_flag(TIF_NOTIFY_SIGNAL) || current->task_works) { -- cgit From d1fd1c201d750711e17377acb4914d3ea29a608c Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Sun, 5 Dec 2021 14:37:58 +0000 Subject: io_uring: simplify selected buf handling As selected buffers are now stored in a separate field in a request, get rid of rw/recv specific helpers and simplify the code. Signed-off-by: Pavel Begunkov Link: https://lore.kernel.org/r/bd4a866d8d91b044f748c40efff9e4eacd07536e.1638714983.git.asml.silence@gmail.com Signed-off-by: Jens Axboe --- fs/io_uring.c | 44 +++++++++++++++++--------------------------- 1 file changed, 17 insertions(+), 27 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index ffbe1b76f3a0..64add8260abb 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -1273,22 +1273,24 @@ static inline void io_req_set_rsrc_node(struct io_kiocb *req, } } -static unsigned int io_put_kbuf(struct io_kiocb *req, struct io_buffer *kbuf) +static unsigned int __io_put_kbuf(struct io_kiocb *req) { + struct io_buffer *kbuf = req->kbuf; unsigned int cflags; cflags = kbuf->bid << IORING_CQE_BUFFER_SHIFT; cflags |= IORING_CQE_F_BUFFER; req->flags &= ~REQ_F_BUFFER_SELECTED; kfree(kbuf); + req->kbuf = NULL; return cflags; } -static inline unsigned int io_put_rw_kbuf(struct io_kiocb *req) +static inline unsigned int io_put_kbuf(struct io_kiocb *req) { if (likely(!(req->flags & REQ_F_BUFFER_SELECTED))) return 0; - return io_put_kbuf(req, req->kbuf); + return __io_put_kbuf(req); } static void io_refs_resurrect(struct percpu_ref *ref, struct completion *compl) @@ -2532,14 +2534,14 @@ static int io_do_iopoll(struct io_ring_ctx *ctx, bool force_nonspin) prev = start; wq_list_for_each_resume(pos, prev) { struct io_kiocb *req = container_of(pos, struct io_kiocb, comp_list); - u32 cflags; /* order with io_complete_rw_iopoll(), e.g. ->result updates */ if (!smp_load_acquire(&req->iopoll_completed)) break; - cflags = io_put_rw_kbuf(req); + if (!(req->flags & REQ_F_CQE_SKIP)) - __io_fill_cqe(ctx, req->user_data, req->result, cflags); + __io_fill_cqe(ctx, req->user_data, req->result, + io_put_kbuf(req)); nr_events++; } @@ -2715,7 +2717,7 @@ static bool __io_complete_rw_common(struct io_kiocb *req, long res) static void io_req_task_complete(struct io_kiocb *req, bool *locked) { - unsigned int cflags = io_put_rw_kbuf(req); + unsigned int cflags = io_put_kbuf(req); int res = req->result; if (*locked) { @@ -2731,7 +2733,7 @@ static void __io_complete_rw(struct io_kiocb *req, long res, long res2, { if (__io_complete_rw_common(req, res)) return; - __io_req_complete(req, issue_flags, req->result, io_put_rw_kbuf(req)); + __io_req_complete(req, issue_flags, req->result, io_put_kbuf(req)); } static void io_complete_rw(struct kiocb *kiocb, long res) @@ -4979,11 +4981,6 @@ static struct io_buffer *io_recv_buffer_select(struct io_kiocb *req, return io_buffer_select(req, &sr->len, sr->bgid, issue_flags); } -static inline unsigned int io_put_recv_kbuf(struct io_kiocb *req) -{ - return io_put_kbuf(req, req->kbuf); -} - static int io_recvmsg_prep_async(struct io_kiocb *req) { int ret; @@ -5021,8 +5018,7 @@ static int io_recvmsg(struct io_kiocb *req, unsigned int issue_flags) struct socket *sock; struct io_buffer *kbuf; unsigned flags; - int min_ret = 0; - int ret, cflags = 0; + int ret, min_ret = 0; bool force_nonblock = issue_flags & IO_URING_F_NONBLOCK; sock = sock_from_file(req->file); @@ -5066,13 +5062,11 @@ static int io_recvmsg(struct io_kiocb *req, unsigned int issue_flags) req_set_fail(req); } - if (req->flags & REQ_F_BUFFER_SELECTED) - cflags = io_put_recv_kbuf(req); /* fast path, check for non-NULL to avoid function call */ if (kmsg->free_iov) kfree(kmsg->free_iov); req->flags &= ~REQ_F_NEED_CLEANUP; - __io_req_complete(req, issue_flags, ret, cflags); + __io_req_complete(req, issue_flags, ret, io_put_kbuf(req)); return 0; } @@ -5085,8 +5079,7 @@ static int io_recv(struct io_kiocb *req, unsigned int issue_flags) struct socket *sock; struct iovec iov; unsigned flags; - int min_ret = 0; - int ret, cflags = 0; + int ret, min_ret = 0; bool force_nonblock = issue_flags & IO_URING_F_NONBLOCK; sock = sock_from_file(req->file); @@ -5128,9 +5121,8 @@ out_free: } else if ((flags & MSG_WAITALL) && (msg.msg_flags & (MSG_TRUNC | MSG_CTRUNC))) { req_set_fail(req); } - if (req->flags & REQ_F_BUFFER_SELECTED) - cflags = io_put_recv_kbuf(req); - __io_req_complete(req, issue_flags, ret, cflags); + + __io_req_complete(req, issue_flags, ret, io_put_kbuf(req)); return 0; } @@ -6578,10 +6570,8 @@ fail: static void io_clean_op(struct io_kiocb *req) { - if (req->flags & REQ_F_BUFFER_SELECTED) { - kfree(req->kbuf); - req->kbuf = NULL; - } + if (req->flags & REQ_F_BUFFER_SELECTED) + io_put_kbuf(req); if (req->flags & REQ_F_NEED_CLEANUP) { switch (req->opcode) { -- cgit From 83a13a4181b0e874d1f196e11b953c3c9f009f68 Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Sun, 5 Dec 2021 14:37:59 +0000 Subject: io_uring: tweak iopoll CQE_SKIP event counting When iopolling the userspace specifies the minimum number of "events" it expects. Previously, we had one CQE per request, so the definition of an "event" was unequivocal, but that's not more the case anymore with REQ_F_CQE_SKIP. Currently it counts the number of completed requests, replace it with the number of posted CQEs. This allows users of the "one CQE per link" scheme to wait for all N links in a single syscall, which is not possible without the patch and requires extra context switches. Signed-off-by: Pavel Begunkov Link: https://lore.kernel.org/r/d5a965c4d2249827392037bbd0186f87fea49c55.1638714983.git.asml.silence@gmail.com Signed-off-by: Jens Axboe --- fs/io_uring.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 64add8260abb..ea7a0daa0b3b 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -2538,10 +2538,10 @@ static int io_do_iopoll(struct io_ring_ctx *ctx, bool force_nonspin) /* order with io_complete_rw_iopoll(), e.g. ->result updates */ if (!smp_load_acquire(&req->iopoll_completed)) break; + if (unlikely(req->flags & REQ_F_CQE_SKIP)) + continue; - if (!(req->flags & REQ_F_CQE_SKIP)) - __io_fill_cqe(ctx, req->user_data, req->result, - io_put_kbuf(req)); + __io_fill_cqe(ctx, req->user_data, req->result, io_put_kbuf(req)); nr_events++; } -- cgit From a90c8bf6590676035336ae98cc51bce1aeb96c33 Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Sun, 5 Dec 2021 14:38:00 +0000 Subject: io_uring: reuse io_req_task_complete for timeouts With kbuf unification io_req_task_complete() is now a generic function, use it for timeout's tw completions. Signed-off-by: Pavel Begunkov Link: https://lore.kernel.org/r/7142fa3cbaf3a4140d59bcba45cbe168cf40fac2.1638714983.git.asml.silence@gmail.com Signed-off-by: Jens Axboe --- fs/io_uring.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index ea7a0daa0b3b..1265dc1942eb 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -5953,15 +5953,6 @@ err: return 0; } -static void io_req_task_timeout(struct io_kiocb *req, bool *locked) -{ - struct io_timeout_data *data = req->async_data; - - if (!(data->flags & IORING_TIMEOUT_ETIME_SUCCESS)) - req_set_fail(req); - io_req_complete_post(req, -ETIME, 0); -} - static enum hrtimer_restart io_timeout_fn(struct hrtimer *timer) { struct io_timeout_data *data = container_of(timer, @@ -5976,7 +5967,11 @@ static enum hrtimer_restart io_timeout_fn(struct hrtimer *timer) atomic_read(&req->ctx->cq_timeouts) + 1); spin_unlock_irqrestore(&ctx->timeout_lock, flags); - req->io_task_work.func = io_req_task_timeout; + if (!(data->flags & IORING_TIMEOUT_ETIME_SUCCESS)) + req_set_fail(req); + + req->result = -ETIME; + req->io_task_work.func = io_req_task_complete; io_req_task_work_add(req); return HRTIMER_NORESTART; } -- cgit From 24115c4e95e137b73954bbbd94354889552a4b08 Mon Sep 17 00:00:00 2001 From: Hao Xu Date: Tue, 7 Dec 2021 17:39:47 +0800 Subject: io-wq: add helper to merge two wq_lists add a helper to merge two wq_lists, it will be useful in the next patches. Reviewed-by: Pavel Begunkov Signed-off-by: Hao Xu Link: https://lore.kernel.org/r/20211207093951.247840-2-haoxu@linux.alibaba.com Signed-off-by: Jens Axboe --- fs/io-wq.h | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/fs/io-wq.h b/fs/io-wq.h index 41bf37674a49..3709b7c5ec98 100644 --- a/fs/io-wq.h +++ b/fs/io-wq.h @@ -52,6 +52,28 @@ static inline void wq_list_add_after(struct io_wq_work_node *node, list->last = node; } +/** + * wq_list_merge - merge the second list to the first one. + * @list0: the first list + * @list1: the second list + * Return the first node after mergence. + */ +static inline struct io_wq_work_node *wq_list_merge(struct io_wq_work_list *list0, + struct io_wq_work_list *list1) +{ + struct io_wq_work_node *ret; + + if (!list0->first) { + ret = list1->first; + } else { + ret = list0->first; + list0->last->next = list1->first; + } + INIT_WQ_LIST(list0); + INIT_WQ_LIST(list1); + return ret; +} + static inline void wq_list_add_tail(struct io_wq_work_node *node, struct io_wq_work_list *list) { -- cgit From 4813c3779261fab4067edea28155a98c65a41b5f Mon Sep 17 00:00:00 2001 From: Hao Xu Date: Tue, 7 Dec 2021 17:39:48 +0800 Subject: io_uring: add a priority tw list for irq completion work Now we have a lot of task_work users, some are just to complete a req and generate a cqe. Let's put the work to a new tw list which has a higher priority, so that it can be handled quickly and thus to reduce avg req latency and users can issue next round of sqes earlier. An explanatory case: origin timeline: submit_sqe-->irq-->add completion task_work -->run heavy work0~n-->run completion task_work now timeline: submit_sqe-->irq-->add completion task_work -->run completion task_work-->run heavy work0~n Limitation: this optimization is only for those that submission and reaping process are in different threads. Otherwise anyhow we have to submit new sqes after returning to userspace, then the order of TWs doesn't matter. Tested this patch(and the following ones) by manually replace __io_queue_sqe() in io_queue_sqe() by io_req_task_queue() to construct 'heavy' task works. Then test with fio: ioengine=io_uring sqpoll=1 thread=1 bs=4k direct=1 rw=randread time_based=1 runtime=600 randrepeat=0 group_reporting=1 filename=/dev/nvme0n1 Tried various iodepth. The peak IOPS for this patch is 710K, while the old one is 665K. For avg latency, difference shows when iodepth grow: depth and avg latency(usec): depth new old 1 7.05 7.10 2 8.47 8.60 4 10.42 10.42 8 13.78 13.22 16 27.41 24.33 32 49.40 53.08 64 102.53 103.36 128 196.98 205.61 256 372.99 414.88 512 747.23 791.30 1024 1472.59 1538.72 2048 3153.49 3329.01 4096 6387.86 6682.54 8192 12150.25 12774.14 16384 23085.58 26044.71 Signed-off-by: Hao Xu Signed-off-by: Pavel Begunkov Link: https://lore.kernel.org/r/20211207093951.247840-3-haoxu@linux.alibaba.com Signed-off-by: Jens Axboe --- fs/io_uring.c | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 1265dc1942eb..ad389466a912 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -474,6 +474,7 @@ struct io_uring_task { spinlock_t task_lock; struct io_wq_work_list task_list; + struct io_wq_work_list prior_task_list; struct callback_head task_work; bool task_running; }; @@ -2226,12 +2227,12 @@ static void tctx_task_work(struct callback_head *cb) while (1) { struct io_wq_work_node *node; - if (!tctx->task_list.first && locked) + if (!tctx->prior_task_list.first && + !tctx->task_list.first && locked) io_submit_flush_completions(ctx); spin_lock_irq(&tctx->task_lock); - node = tctx->task_list.first; - INIT_WQ_LIST(&tctx->task_list); + node= wq_list_merge(&tctx->prior_task_list, &tctx->task_list); if (!node) tctx->task_running = false; spin_unlock_irq(&tctx->task_lock); @@ -2260,7 +2261,7 @@ static void tctx_task_work(struct callback_head *cb) ctx_flush_and_put(ctx, &locked); } -static void io_req_task_work_add(struct io_kiocb *req) +static void io_req_task_work_add(struct io_kiocb *req, bool priority) { struct task_struct *tsk = req->task; struct io_uring_task *tctx = tsk->io_uring; @@ -2272,7 +2273,10 @@ static void io_req_task_work_add(struct io_kiocb *req) WARN_ON_ONCE(!tctx); spin_lock_irqsave(&tctx->task_lock, flags); - wq_list_add_tail(&req->io_task_work.node, &tctx->task_list); + if (priority) + wq_list_add_tail(&req->io_task_work.node, &tctx->prior_task_list); + else + wq_list_add_tail(&req->io_task_work.node, &tctx->task_list); running = tctx->task_running; if (!running) tctx->task_running = true; @@ -2297,8 +2301,7 @@ static void io_req_task_work_add(struct io_kiocb *req) spin_lock_irqsave(&tctx->task_lock, flags); tctx->task_running = false; - node = tctx->task_list.first; - INIT_WQ_LIST(&tctx->task_list); + node = wq_list_merge(&tctx->prior_task_list, &tctx->task_list); spin_unlock_irqrestore(&tctx->task_lock, flags); while (node) { @@ -2335,19 +2338,19 @@ static void io_req_task_queue_fail(struct io_kiocb *req, int ret) { req->result = ret; req->io_task_work.func = io_req_task_cancel; - io_req_task_work_add(req); + io_req_task_work_add(req, false); } static void io_req_task_queue(struct io_kiocb *req) { req->io_task_work.func = io_req_task_submit; - io_req_task_work_add(req); + io_req_task_work_add(req, false); } static void io_req_task_queue_reissue(struct io_kiocb *req) { req->io_task_work.func = io_queue_async_work; - io_req_task_work_add(req); + io_req_task_work_add(req, false); } static inline void io_queue_next(struct io_kiocb *req) @@ -2457,7 +2460,7 @@ static inline void io_put_req_deferred(struct io_kiocb *req) { if (req_ref_put_and_test(req)) { req->io_task_work.func = io_free_req_work; - io_req_task_work_add(req); + io_req_task_work_add(req, false); } } @@ -2744,7 +2747,7 @@ static void io_complete_rw(struct kiocb *kiocb, long res) return; req->result = res; req->io_task_work.func = io_req_task_complete; - io_req_task_work_add(req); + io_req_task_work_add(req, true); } static void io_complete_rw_iopoll(struct kiocb *kiocb, long res) @@ -2986,7 +2989,7 @@ static void kiocb_done(struct io_kiocb *req, ssize_t ret, req_set_fail(req); req->result = ret; req->io_task_work.func = io_req_task_complete; - io_req_task_work_add(req); + io_req_task_work_add(req, false); } } } @@ -5309,7 +5312,7 @@ static int __io_async_wake(struct io_kiocb *req, struct io_poll_iocb *poll, * of executing it. We can't safely execute it anyway, as we may not * have the needed state needed for it anyway. */ - io_req_task_work_add(req); + io_req_task_work_add(req, false); return 1; } @@ -5972,7 +5975,7 @@ static enum hrtimer_restart io_timeout_fn(struct hrtimer *timer) req->result = -ETIME; req->io_task_work.func = io_req_task_complete; - io_req_task_work_add(req); + io_req_task_work_add(req, false); return HRTIMER_NORESTART; } @@ -6947,7 +6950,7 @@ static enum hrtimer_restart io_link_timeout_fn(struct hrtimer *timer) spin_unlock_irqrestore(&ctx->timeout_lock, flags); req->io_task_work.func = io_req_task_link_timeout; - io_req_task_work_add(req); + io_req_task_work_add(req, false); return HRTIMER_NORESTART; } @@ -8662,6 +8665,7 @@ static __cold int io_uring_alloc_task_context(struct task_struct *task, task->io_uring = tctx; spin_lock_init(&tctx->task_lock); INIT_WQ_LIST(&tctx->task_list); + INIT_WQ_LIST(&tctx->prior_task_list); init_task_work(&tctx->task_work, tctx_task_work); return 0; } -- cgit From 9f8d032a364b2b579c6ce5a62b967056f8711e69 Mon Sep 17 00:00:00 2001 From: Hao Xu Date: Tue, 7 Dec 2021 17:39:49 +0800 Subject: io_uring: add helper for task work execution code Add a helper for task work execution code. We will use it later. Reviewed-by: Pavel Begunkov Signed-off-by: Hao Xu Link: https://lore.kernel.org/r/20211207093951.247840-4-haoxu@linux.alibaba.com Signed-off-by: Jens Axboe --- fs/io_uring.c | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index ad389466a912..85f9459e9072 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -2217,6 +2217,25 @@ static void ctx_flush_and_put(struct io_ring_ctx *ctx, bool *locked) percpu_ref_put(&ctx->refs); } +static void handle_tw_list(struct io_wq_work_node *node, struct io_ring_ctx **ctx, bool *locked) +{ + do { + struct io_wq_work_node *next = node->next; + struct io_kiocb *req = container_of(node, struct io_kiocb, + io_task_work.node); + + if (req->ctx != *ctx) { + ctx_flush_and_put(*ctx, locked); + *ctx = req->ctx; + /* if not contended, grab and improve batching */ + *locked = mutex_trylock(&(*ctx)->uring_lock); + percpu_ref_get(&(*ctx)->refs); + } + req->io_task_work.func(req, locked); + node = next; + } while (node); +} + static void tctx_task_work(struct callback_head *cb) { bool locked = false; @@ -2239,22 +2258,7 @@ static void tctx_task_work(struct callback_head *cb) if (!node) break; - do { - struct io_wq_work_node *next = node->next; - struct io_kiocb *req = container_of(node, struct io_kiocb, - io_task_work.node); - - if (req->ctx != ctx) { - ctx_flush_and_put(ctx, &locked); - ctx = req->ctx; - /* if not contended, grab and improve batching */ - locked = mutex_trylock(&ctx->uring_lock); - percpu_ref_get(&ctx->refs); - } - req->io_task_work.func(req, &locked); - node = next; - } while (node); - + handle_tw_list(node, &ctx, &locked); cond_resched(); } -- cgit From a37fae8aaa62b05c11f059fee8fedf4313975abd Mon Sep 17 00:00:00 2001 From: Hao Xu Date: Tue, 7 Dec 2021 17:39:50 +0800 Subject: io_uring: split io_req_complete_post() and add a helper Split io_req_complete_post(), this is a prep for the next patch. Signed-off-by: Hao Xu Link: https://lore.kernel.org/r/20211207093951.247840-5-haoxu@linux.alibaba.com Signed-off-by: Jens Axboe --- fs/io_uring.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 85f9459e9072..21738ed7521e 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -1891,12 +1891,11 @@ static noinline bool io_fill_cqe_aux(struct io_ring_ctx *ctx, u64 user_data, return __io_fill_cqe(ctx, user_data, res, cflags); } -static void io_req_complete_post(struct io_kiocb *req, s32 res, - u32 cflags) +static void __io_req_complete_post(struct io_kiocb *req, s32 res, + u32 cflags) { struct io_ring_ctx *ctx = req->ctx; - spin_lock(&ctx->completion_lock); if (!(req->flags & REQ_F_CQE_SKIP)) __io_fill_cqe(ctx, req->user_data, res, cflags); /* @@ -1918,6 +1917,15 @@ static void io_req_complete_post(struct io_kiocb *req, s32 res, wq_list_add_head(&req->comp_list, &ctx->locked_free_list); ctx->locked_free_nr++; } +} + +static void io_req_complete_post(struct io_kiocb *req, s32 res, + u32 cflags) +{ + struct io_ring_ctx *ctx = req->ctx; + + spin_lock(&ctx->completion_lock); + __io_req_complete_post(req, res, cflags); io_commit_cqring(ctx); spin_unlock(&ctx->completion_lock); io_cqring_ev_posted(ctx); -- cgit From f28c240e7152462f0750a8939db28d985ecf7c67 Mon Sep 17 00:00:00 2001 From: Hao Xu Date: Wed, 8 Dec 2021 13:21:25 +0800 Subject: io_uring: batch completion in prior_task_list In previous patches, we have already gathered some tw with io_req_task_complete() as callback in prior_task_list, let's complete them in batch while we cannot grab uring lock. In this way, we batch the req_complete_post path. Signed-off-by: Hao Xu Link: https://lore.kernel.org/r/20211208052125.351587-1-haoxu@linux.alibaba.com Reviewed-by: Pavel Begunkov Signed-off-by: Jens Axboe --- fs/io_uring.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 60 insertions(+), 11 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 21738ed7521e..92dc33519466 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -2225,7 +2225,49 @@ static void ctx_flush_and_put(struct io_ring_ctx *ctx, bool *locked) percpu_ref_put(&ctx->refs); } -static void handle_tw_list(struct io_wq_work_node *node, struct io_ring_ctx **ctx, bool *locked) +static inline void ctx_commit_and_unlock(struct io_ring_ctx *ctx) +{ + io_commit_cqring(ctx); + spin_unlock(&ctx->completion_lock); + io_cqring_ev_posted(ctx); +} + +static void handle_prev_tw_list(struct io_wq_work_node *node, + struct io_ring_ctx **ctx, bool *uring_locked) +{ + if (*ctx && !*uring_locked) + spin_lock(&(*ctx)->completion_lock); + + do { + struct io_wq_work_node *next = node->next; + struct io_kiocb *req = container_of(node, struct io_kiocb, + io_task_work.node); + + if (req->ctx != *ctx) { + if (unlikely(!*uring_locked && *ctx)) + ctx_commit_and_unlock(*ctx); + + ctx_flush_and_put(*ctx, uring_locked); + *ctx = req->ctx; + /* if not contended, grab and improve batching */ + *uring_locked = mutex_trylock(&(*ctx)->uring_lock); + percpu_ref_get(&(*ctx)->refs); + if (unlikely(!*uring_locked)) + spin_lock(&(*ctx)->completion_lock); + } + if (likely(*uring_locked)) + req->io_task_work.func(req, uring_locked); + else + __io_req_complete_post(req, req->result, io_put_kbuf(req)); + node = next; + } while (node); + + if (unlikely(!*uring_locked)) + ctx_commit_and_unlock(*ctx); +} + +static void handle_tw_list(struct io_wq_work_node *node, + struct io_ring_ctx **ctx, bool *locked) { do { struct io_wq_work_node *next = node->next; @@ -2246,31 +2288,38 @@ static void handle_tw_list(struct io_wq_work_node *node, struct io_ring_ctx **ct static void tctx_task_work(struct callback_head *cb) { - bool locked = false; + bool uring_locked = false; struct io_ring_ctx *ctx = NULL; struct io_uring_task *tctx = container_of(cb, struct io_uring_task, task_work); while (1) { - struct io_wq_work_node *node; + struct io_wq_work_node *node1, *node2; - if (!tctx->prior_task_list.first && - !tctx->task_list.first && locked) + if (!tctx->task_list.first && + !tctx->prior_task_list.first && uring_locked) io_submit_flush_completions(ctx); spin_lock_irq(&tctx->task_lock); - node= wq_list_merge(&tctx->prior_task_list, &tctx->task_list); - if (!node) + node1 = tctx->prior_task_list.first; + node2 = tctx->task_list.first; + INIT_WQ_LIST(&tctx->task_list); + INIT_WQ_LIST(&tctx->prior_task_list); + if (!node2 && !node1) tctx->task_running = false; spin_unlock_irq(&tctx->task_lock); - if (!node) + if (!node2 && !node1) break; - handle_tw_list(node, &ctx, &locked); + if (node1) + handle_prev_tw_list(node1, &ctx, &uring_locked); + + if (node2) + handle_tw_list(node2, &ctx, &uring_locked); cond_resched(); } - ctx_flush_and_put(ctx, &locked); + ctx_flush_and_put(ctx, &uring_locked); } static void io_req_task_work_add(struct io_kiocb *req, bool priority) @@ -2759,7 +2808,7 @@ static void io_complete_rw(struct kiocb *kiocb, long res) return; req->result = res; req->io_task_work.func = io_req_task_complete; - io_req_task_work_add(req, true); + io_req_task_work_add(req, !!(req->ctx->flags & IORING_SETUP_SQPOLL)); } static void io_complete_rw_iopoll(struct kiocb *kiocb, long res) -- cgit From 33ce2aff7d340bf48875ccd80628c884cf8017ae Mon Sep 17 00:00:00 2001 From: Hao Xu Date: Tue, 14 Dec 2021 13:59:04 +0800 Subject: io_uring: code clean for some ctx usage There are some functions doing ctx = req->ctx while still using req->ctx, update those places. Signed-off-by: Hao Xu Link: https://lore.kernel.org/r/20211214055904.61772-1-haoxu@linux.alibaba.com Signed-off-by: Jens Axboe --- fs/io_uring.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 92dc33519466..1f2341d87588 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -1549,7 +1549,7 @@ static void io_prep_async_link(struct io_kiocb *req) static inline void io_req_add_compl_list(struct io_kiocb *req) { struct io_ring_ctx *ctx = req->ctx; - struct io_submit_state *state = &req->ctx->submit_state; + struct io_submit_state *state = &ctx->submit_state; if (!(req->flags & REQ_F_CQE_SKIP)) ctx->submit_state.flush_cqes = true; @@ -2188,7 +2188,7 @@ static void __io_req_find_next_prep(struct io_kiocb *req) spin_lock(&ctx->completion_lock); posted = io_disarm_next(req); if (posted) - io_commit_cqring(req->ctx); + io_commit_cqring(ctx); spin_unlock(&ctx->completion_lock); if (posted) io_cqring_ev_posted(ctx); -- cgit From e840b4baf3cfb37e2ead4f649a45bb78178677ff Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Wed, 15 Dec 2021 22:08:44 +0000 Subject: io_uring: remove double poll on poll update Before updating a poll request we should remove it from poll queues, including the double poll entry. Fixes: b69de288e913 ("io_uring: allow events and user_data update of running poll requests") Signed-off-by: Pavel Begunkov Link: https://lore.kernel.org/r/ac39e7f80152613603b8a6cc29a2b6063ac2434f.1639605189.git.asml.silence@gmail.com Signed-off-by: Jens Axboe --- fs/io_uring.c | 1 + 1 file changed, 1 insertion(+) diff --git a/fs/io_uring.c b/fs/io_uring.c index 1f2341d87588..39d50124bdea 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -5980,6 +5980,7 @@ static int io_poll_update(struct io_kiocb *req, unsigned int issue_flags) * update those. For multishot, if we're racing with completion, just * let completion re-add it. */ + io_poll_remove_double(preq); completing = !__io_poll_remove_one(preq, &preq->poll, false); if (completing && (preq->poll.events & EPOLLONESHOT)) { ret = -EALREADY; -- cgit From 2bbb146d96f4b45e17d6aeede300796bc1a96d68 Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Wed, 15 Dec 2021 22:08:45 +0000 Subject: io_uring: refactor poll update Clean up io_poll_update() and unify cancellation paths for remove and update. Signed-off-by: Pavel Begunkov Link: https://lore.kernel.org/r/5937138b6265a1285220e2fab1b28132c1d73ce3.1639605189.git.asml.silence@gmail.com Signed-off-by: Jens Axboe --- fs/io_uring.c | 62 +++++++++++++++++++++++++---------------------------------- 1 file changed, 26 insertions(+), 36 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 39d50124bdea..105593455775 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -5960,61 +5960,51 @@ static int io_poll_update(struct io_kiocb *req, unsigned int issue_flags) struct io_ring_ctx *ctx = req->ctx; struct io_kiocb *preq; bool completing; - int ret; + int ret2, ret = 0; spin_lock(&ctx->completion_lock); preq = io_poll_find(ctx, req->poll_update.old_user_data, true); if (!preq) { ret = -ENOENT; - goto err; - } - - if (!req->poll_update.update_events && !req->poll_update.update_user_data) { - completing = true; - ret = io_poll_remove_one(preq) ? 0 : -EALREADY; - goto err; +fail: + spin_unlock(&ctx->completion_lock); + goto out; } - + io_poll_remove_double(preq); /* * Don't allow racy completion with singleshot, as we cannot safely * update those. For multishot, if we're racing with completion, just * let completion re-add it. */ - io_poll_remove_double(preq); completing = !__io_poll_remove_one(preq, &preq->poll, false); if (completing && (preq->poll.events & EPOLLONESHOT)) { ret = -EALREADY; - goto err; - } - /* we now have a detached poll request. reissue. */ - ret = 0; -err: - if (ret < 0) { - spin_unlock(&ctx->completion_lock); - req_set_fail(req); - io_req_complete(req, ret); - return 0; - } - /* only mask one event flags, keep behavior flags */ - if (req->poll_update.update_events) { - preq->poll.events &= ~0xffff; - preq->poll.events |= req->poll_update.events & 0xffff; - preq->poll.events |= IO_POLL_UNMASK; + goto fail; } - if (req->poll_update.update_user_data) - preq->user_data = req->poll_update.new_user_data; spin_unlock(&ctx->completion_lock); - /* complete update request, we're done with it */ - io_req_complete(req, ret); - - if (!completing) { - ret = io_poll_add(preq, issue_flags); - if (ret < 0) { - req_set_fail(preq); - io_req_complete(preq, ret); + if (req->poll_update.update_events || req->poll_update.update_user_data) { + /* only mask one event flags, keep behavior flags */ + if (req->poll_update.update_events) { + preq->poll.events &= ~0xffff; + preq->poll.events |= req->poll_update.events & 0xffff; + preq->poll.events |= IO_POLL_UNMASK; } + if (req->poll_update.update_user_data) + preq->user_data = req->poll_update.new_user_data; + + ret2 = io_poll_add(preq, issue_flags); + /* successfully updated, don't complete poll request */ + if (!ret2) + goto out; } + req_set_fail(preq); + io_req_complete(preq, -ECANCELED); +out: + if (ret < 0) + req_set_fail(req); + /* complete update request, we're done with it */ + io_req_complete(req, ret); return 0; } -- cgit From 5641897a5e8fb8abeb07e89c71a788d3db3ec75e Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Wed, 15 Dec 2021 22:08:46 +0000 Subject: io_uring: move common poll bits Move some poll helpers/etc up, we'll need them there shortly Signed-off-by: Pavel Begunkov Link: https://lore.kernel.org/r/6c5c3dba24c86aad5cd389a54a8c7412e6a0621d.1639605189.git.asml.silence@gmail.com Signed-off-by: Jens Axboe --- fs/io_uring.c | 74 +++++++++++++++++++++++++++++------------------------------ 1 file changed, 37 insertions(+), 37 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 105593455775..8cabe4a0d38f 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -5353,6 +5353,43 @@ struct io_poll_table { int error; }; +static struct io_poll_iocb *io_poll_get_double(struct io_kiocb *req) +{ + /* pure poll stashes this in ->async_data, poll driven retry elsewhere */ + if (req->opcode == IORING_OP_POLL_ADD) + return req->async_data; + return req->apoll->double_poll; +} + +static struct io_poll_iocb *io_poll_get_single(struct io_kiocb *req) +{ + if (req->opcode == IORING_OP_POLL_ADD) + return &req->poll; + return &req->apoll->poll; +} + +static void io_poll_req_insert(struct io_kiocb *req) +{ + struct io_ring_ctx *ctx = req->ctx; + struct hlist_head *list; + + list = &ctx->cancel_hash[hash_long(req->user_data, ctx->cancel_hash_bits)]; + hlist_add_head(&req->hash_node, list); +} + +static void io_init_poll_iocb(struct io_poll_iocb *poll, __poll_t events, + wait_queue_func_t wake_func) +{ + poll->head = NULL; + poll->done = false; + poll->canceled = false; +#define IO_POLL_UNMASK (EPOLLERR|EPOLLHUP|EPOLLNVAL|EPOLLRDHUP) + /* mask in events that we always want/need */ + poll->events = events | IO_POLL_UNMASK; + INIT_LIST_HEAD(&poll->wait.entry); + init_waitqueue_func_entry(&poll->wait, wake_func); +} + static int __io_async_wake(struct io_kiocb *req, struct io_poll_iocb *poll, __poll_t mask, io_req_tw_func_t func) { @@ -5401,21 +5438,6 @@ static bool io_poll_rewait(struct io_kiocb *req, struct io_poll_iocb *poll) return false; } -static struct io_poll_iocb *io_poll_get_double(struct io_kiocb *req) -{ - /* pure poll stashes this in ->async_data, poll driven retry elsewhere */ - if (req->opcode == IORING_OP_POLL_ADD) - return req->async_data; - return req->apoll->double_poll; -} - -static struct io_poll_iocb *io_poll_get_single(struct io_kiocb *req) -{ - if (req->opcode == IORING_OP_POLL_ADD) - return &req->poll; - return &req->apoll->poll; -} - static void io_poll_remove_double(struct io_kiocb *req) __must_hold(&req->ctx->completion_lock) { @@ -5530,19 +5552,6 @@ static int io_poll_double_wake(struct wait_queue_entry *wait, unsigned mode, return 1; } -static void io_init_poll_iocb(struct io_poll_iocb *poll, __poll_t events, - wait_queue_func_t wake_func) -{ - poll->head = NULL; - poll->done = false; - poll->canceled = false; -#define IO_POLL_UNMASK (EPOLLERR|EPOLLHUP|EPOLLNVAL|EPOLLRDHUP) - /* mask in events that we always want/need */ - poll->events = events | IO_POLL_UNMASK; - INIT_LIST_HEAD(&poll->wait.entry); - init_waitqueue_func_entry(&poll->wait, wake_func); -} - static void __io_queue_proc(struct io_poll_iocb *poll, struct io_poll_table *pt, struct wait_queue_head *head, struct io_poll_iocb **poll_ptr) @@ -5640,15 +5649,6 @@ static int io_async_wake(struct wait_queue_entry *wait, unsigned mode, int sync, return __io_async_wake(req, poll, key_to_poll(key), io_async_task_func); } -static void io_poll_req_insert(struct io_kiocb *req) -{ - struct io_ring_ctx *ctx = req->ctx; - struct hlist_head *list; - - list = &ctx->cancel_hash[hash_long(req->user_data, ctx->cancel_hash_bits)]; - hlist_add_head(&req->hash_node, list); -} - static __poll_t __io_arm_poll_handler(struct io_kiocb *req, struct io_poll_iocb *poll, struct io_poll_table *ipt, __poll_t mask, -- cgit From ab1dab960b8352cee082db0f8a54dc92a948bfd7 Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Wed, 15 Dec 2021 22:08:47 +0000 Subject: io_uring: kill poll linking optimisation With IORING_FEAT_FAST_POLL in place, io_put_req_find_next() for poll requests doesn't make much sense, and in any case re-adding it shouldn't be a problem considering batching in tctx_task_work(). We can remove it. Signed-off-by: Pavel Begunkov Link: https://lore.kernel.org/r/15699682bf81610ec901d4e79d6da64baa9f70be.1639605189.git.asml.silence@gmail.com Signed-off-by: Jens Axboe --- fs/io_uring.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 8cabe4a0d38f..0215813e9f89 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -5485,7 +5485,6 @@ static bool __io_poll_complete(struct io_kiocb *req, __poll_t mask) static void io_poll_task_func(struct io_kiocb *req, bool *locked) { struct io_ring_ctx *ctx = req->ctx; - struct io_kiocb *nxt; if (io_poll_rewait(req, &req->poll)) { spin_unlock(&ctx->completion_lock); @@ -5509,11 +5508,8 @@ static void io_poll_task_func(struct io_kiocb *req, bool *locked) spin_unlock(&ctx->completion_lock); io_cqring_ev_posted(ctx); - if (done) { - nxt = io_put_req_find_next(req); - if (nxt) - io_req_task_submit(nxt, locked); - } + if (done) + io_put_req(req); } } -- cgit From aa43477b040251f451db0d844073ac00a8ab66ee Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Wed, 15 Dec 2021 22:08:48 +0000 Subject: io_uring: poll rework It's not possible to go forward with the current state of io_uring polling, we need a more straightforward and easier synchronisation. There are a lot of problems with how it is at the moment, including missing events on rewait. The main idea here is to introduce a notion of request ownership while polling, no one but the owner can modify any part but ->poll_refs of struct io_kiocb, that grants us protection against all sorts of races. Main users of such exclusivity are poll task_work handler, so before queueing a tw one should have/acquire ownership, which will be handed off to the tw handler. The other user is __io_arm_poll_handler() do initial poll arming. It starts taking the ownership, so tw handlers won't be run until it's released later in the function after vfs_poll. note: also prevents races in __io_queue_proc(). Poll wake/etc. may not be able to get ownership, then they need to increase the poll refcount and the task_work should notice it and retry if necessary, see io_poll_check_events(). There is also IO_POLL_CANCEL_FLAG flag to notify that we want to kill request. It makes cancellations more reliable, enables double multishot polling, fixes double poll rewait, fixes missing poll events and fixes another bunch of races. Even though it adds some overhead for new refcounting, and there are a couple of nice performance wins: - no req->refs refcounting for poll requests anymore - if the data is already there (once measured for some test to be 1-2% of all apoll requests), it removes it doesn't add atomics and removes spin_lock/unlock pair. - works well with multishots, we don't do remove from queue / add to queue for each new poll event. Signed-off-by: Pavel Begunkov Link: https://lore.kernel.org/r/6b652927c77ed9580ea4330ac5612f0e0848c946.1639605189.git.asml.silence@gmail.com Signed-off-by: Jens Axboe --- fs/io_uring.c | 526 +++++++++++++++++++++++++--------------------------------- 1 file changed, 227 insertions(+), 299 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 0215813e9f89..a36eb6060e7e 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -487,8 +487,6 @@ struct io_poll_iocb { struct file *file; struct wait_queue_head *head; __poll_t events; - bool done; - bool canceled; struct wait_queue_entry wait; }; @@ -892,6 +890,7 @@ struct io_kiocb { const struct cred *creds; /* stores selected buf, valid IFF REQ_F_BUFFER_SELECTED is set */ struct io_buffer *kbuf; + atomic_t poll_refs; }; struct io_tctx_node { @@ -5353,6 +5352,25 @@ struct io_poll_table { int error; }; +#define IO_POLL_CANCEL_FLAG BIT(31) +#define IO_POLL_REF_MASK ((1u << 20)-1) + +/* + * If refs part of ->poll_refs (see IO_POLL_REF_MASK) is 0, it's free. We can + * bump it and acquire ownership. It's disallowed to modify requests while not + * owning it, that prevents from races for enqueueing task_work's and b/w + * arming poll and wakeups. + */ +static inline bool io_poll_get_ownership(struct io_kiocb *req) +{ + return !(atomic_fetch_inc(&req->poll_refs) & IO_POLL_REF_MASK); +} + +static void io_poll_mark_cancelled(struct io_kiocb *req) +{ + atomic_or(IO_POLL_CANCEL_FLAG, &req->poll_refs); +} + static struct io_poll_iocb *io_poll_get_double(struct io_kiocb *req) { /* pure poll stashes this in ->async_data, poll driven retry elsewhere */ @@ -5381,8 +5399,6 @@ static void io_init_poll_iocb(struct io_poll_iocb *poll, __poll_t events, wait_queue_func_t wake_func) { poll->head = NULL; - poll->done = false; - poll->canceled = false; #define IO_POLL_UNMASK (EPOLLERR|EPOLLHUP|EPOLLNVAL|EPOLLRDHUP) /* mask in events that we always want/need */ poll->events = events | IO_POLL_UNMASK; @@ -5390,161 +5406,170 @@ static void io_init_poll_iocb(struct io_poll_iocb *poll, __poll_t events, init_waitqueue_func_entry(&poll->wait, wake_func); } -static int __io_async_wake(struct io_kiocb *req, struct io_poll_iocb *poll, - __poll_t mask, io_req_tw_func_t func) +static inline void io_poll_remove_entry(struct io_poll_iocb *poll) { - /* for instances that support it check for an event match first: */ - if (mask && !(mask & poll->events)) - return 0; - - trace_io_uring_task_add(req->ctx, req->opcode, req->user_data, mask); + struct wait_queue_head *head = poll->head; + spin_lock_irq(&head->lock); list_del_init(&poll->wait.entry); + poll->head = NULL; + spin_unlock_irq(&head->lock); +} - req->result = mask; - req->io_task_work.func = func; +static void io_poll_remove_entries(struct io_kiocb *req) +{ + struct io_poll_iocb *poll = io_poll_get_single(req); + struct io_poll_iocb *poll_double = io_poll_get_double(req); - /* - * If this fails, then the task is exiting. When a task exits, the - * work gets canceled, so just cancel this request as well instead - * of executing it. We can't safely execute it anyway, as we may not - * have the needed state needed for it anyway. - */ - io_req_task_work_add(req, false); - return 1; + if (poll->head) + io_poll_remove_entry(poll); + if (poll_double && poll_double->head) + io_poll_remove_entry(poll_double); } -static bool io_poll_rewait(struct io_kiocb *req, struct io_poll_iocb *poll) - __acquires(&req->ctx->completion_lock) +/* + * All poll tw should go through this. Checks for poll events, manages + * references, does rewait, etc. + * + * Returns a negative error on failure. >0 when no action require, which is + * either spurious wakeup or multishot CQE is served. 0 when it's done with + * the request, then the mask is stored in req->result. + */ +static int io_poll_check_events(struct io_kiocb *req) { struct io_ring_ctx *ctx = req->ctx; + struct io_poll_iocb *poll = io_poll_get_single(req); + int v; /* req->task == current here, checking PF_EXITING is safe */ if (unlikely(req->task->flags & PF_EXITING)) - WRITE_ONCE(poll->canceled, true); + io_poll_mark_cancelled(req); - if (!req->result && !READ_ONCE(poll->canceled)) { - struct poll_table_struct pt = { ._key = poll->events }; + do { + v = atomic_read(&req->poll_refs); - req->result = vfs_poll(req->file, &pt) & poll->events; - } + /* tw handler should be the owner, and so have some references */ + if (WARN_ON_ONCE(!(v & IO_POLL_REF_MASK))) + return 0; + if (v & IO_POLL_CANCEL_FLAG) + return -ECANCELED; - spin_lock(&ctx->completion_lock); - if (!req->result && !READ_ONCE(poll->canceled)) { - add_wait_queue(poll->head, &poll->wait); - return true; - } + if (!req->result) { + struct poll_table_struct pt = { ._key = poll->events }; - return false; -} + req->result = vfs_poll(req->file, &pt) & poll->events; + } -static void io_poll_remove_double(struct io_kiocb *req) - __must_hold(&req->ctx->completion_lock) -{ - struct io_poll_iocb *poll = io_poll_get_double(req); + /* multishot, just fill an CQE and proceed */ + if (req->result && !(poll->events & EPOLLONESHOT)) { + __poll_t mask = mangle_poll(req->result & poll->events); + bool filled; - lockdep_assert_held(&req->ctx->completion_lock); + spin_lock(&ctx->completion_lock); + filled = io_fill_cqe_aux(ctx, req->user_data, mask, + IORING_CQE_F_MORE); + io_commit_cqring(ctx); + spin_unlock(&ctx->completion_lock); + if (unlikely(!filled)) + return -ECANCELED; + io_cqring_ev_posted(ctx); + } else if (req->result) { + return 0; + } - if (poll && poll->head) { - struct wait_queue_head *head = poll->head; + /* + * Release all references, retry if someone tried to restart + * task_work while we were executing it. + */ + } while (atomic_sub_return(v & IO_POLL_REF_MASK, &req->poll_refs)); - spin_lock_irq(&head->lock); - list_del_init(&poll->wait.entry); - if (poll->wait.private) - req_ref_put(req); - poll->head = NULL; - spin_unlock_irq(&head->lock); - } + return 1; } -static bool __io_poll_complete(struct io_kiocb *req, __poll_t mask) - __must_hold(&req->ctx->completion_lock) +static void io_poll_task_func(struct io_kiocb *req, bool *locked) { struct io_ring_ctx *ctx = req->ctx; - unsigned flags = IORING_CQE_F_MORE; - int error; + int ret; - if (READ_ONCE(req->poll.canceled)) { - error = -ECANCELED; - req->poll.events |= EPOLLONESHOT; + ret = io_poll_check_events(req); + if (ret > 0) + return; + + if (!ret) { + req->result = mangle_poll(req->result & req->poll.events); } else { - error = mangle_poll(mask); + req->result = ret; + req_set_fail(req); } - if (req->poll.events & EPOLLONESHOT) - flags = 0; - if (!(flags & IORING_CQE_F_MORE)) { - io_fill_cqe_req(req, error, flags); - } else if (!io_fill_cqe_aux(ctx, req->user_data, error, flags)) { - req->poll.events |= EPOLLONESHOT; - flags = 0; - } - return !(flags & IORING_CQE_F_MORE); + io_poll_remove_entries(req); + spin_lock(&ctx->completion_lock); + hash_del(&req->hash_node); + __io_req_complete_post(req, req->result, 0); + io_commit_cqring(ctx); + spin_unlock(&ctx->completion_lock); + io_cqring_ev_posted(ctx); } -static void io_poll_task_func(struct io_kiocb *req, bool *locked) +static void io_apoll_task_func(struct io_kiocb *req, bool *locked) { struct io_ring_ctx *ctx = req->ctx; + int ret; - if (io_poll_rewait(req, &req->poll)) { - spin_unlock(&ctx->completion_lock); - } else { - bool done; + ret = io_poll_check_events(req); + if (ret > 0) + return; - if (req->poll.done) { - spin_unlock(&ctx->completion_lock); - return; - } - done = __io_poll_complete(req, req->result); - if (done) { - io_poll_remove_double(req); - hash_del(&req->hash_node); - req->poll.done = true; - } else { - req->result = 0; - add_wait_queue(req->poll.head, &req->poll.wait); - } - io_commit_cqring(ctx); - spin_unlock(&ctx->completion_lock); - io_cqring_ev_posted(ctx); + io_poll_remove_entries(req); + spin_lock(&ctx->completion_lock); + hash_del(&req->hash_node); + spin_unlock(&ctx->completion_lock); - if (done) - io_put_req(req); - } + if (!ret) + io_req_task_submit(req, locked); + else + io_req_complete_failed(req, ret); } -static int io_poll_double_wake(struct wait_queue_entry *wait, unsigned mode, - int sync, void *key) +static void __io_poll_execute(struct io_kiocb *req, int mask) +{ + req->result = mask; + if (req->opcode == IORING_OP_POLL_ADD) + req->io_task_work.func = io_poll_task_func; + else + req->io_task_work.func = io_apoll_task_func; + + trace_io_uring_task_add(req->ctx, req->opcode, req->user_data, mask); + io_req_task_work_add(req, false); +} + +static inline void io_poll_execute(struct io_kiocb *req, int res) +{ + if (io_poll_get_ownership(req)) + __io_poll_execute(req, res); +} + +static void io_poll_cancel_req(struct io_kiocb *req) +{ + io_poll_mark_cancelled(req); + /* kick tw, which should complete the request */ + io_poll_execute(req, 0); +} + +static int io_poll_wake(struct wait_queue_entry *wait, unsigned mode, int sync, + void *key) { struct io_kiocb *req = wait->private; - struct io_poll_iocb *poll = io_poll_get_single(req); + struct io_poll_iocb *poll = container_of(wait, struct io_poll_iocb, + wait); __poll_t mask = key_to_poll(key); - unsigned long flags; - /* for instances that support it check for an event match first: */ + /* for instances that support it check for an event match first */ if (mask && !(mask & poll->events)) return 0; - if (!(poll->events & EPOLLONESHOT)) - return poll->wait.func(&poll->wait, mode, sync, key); - - list_del_init(&wait->entry); - if (poll->head) { - bool done; - - spin_lock_irqsave(&poll->head->lock, flags); - done = list_empty(&poll->wait.entry); - if (!done) - list_del_init(&poll->wait.entry); - /* make sure double remove sees this as being gone */ - wait->private = NULL; - spin_unlock_irqrestore(&poll->head->lock, flags); - if (!done) { - /* use wait func handler, so it matches the rq type */ - poll->wait.func(&poll->wait, mode, sync, key); - } - } - req_ref_put(req); + if (io_poll_get_ownership(req)) + __io_poll_execute(req, mask); return 1; } @@ -5560,10 +5585,10 @@ static void __io_queue_proc(struct io_poll_iocb *poll, struct io_poll_table *pt, * if this happens. */ if (unlikely(pt->nr_entries)) { - struct io_poll_iocb *poll_one = poll; + struct io_poll_iocb *first = poll; /* double add on the same waitqueue head, ignore */ - if (poll_one->head == head) + if (first->head == head) return; /* already have a 2nd entry, fail a third attempt */ if (*poll_ptr) { @@ -5572,21 +5597,13 @@ static void __io_queue_proc(struct io_poll_iocb *poll, struct io_poll_table *pt, pt->error = -EINVAL; return; } - /* - * Can't handle multishot for double wait for now, turn it - * into one-shot mode. - */ - if (!(poll_one->events & EPOLLONESHOT)) - poll_one->events |= EPOLLONESHOT; + poll = kmalloc(sizeof(*poll), GFP_ATOMIC); if (!poll) { pt->error = -ENOMEM; return; } - io_init_poll_iocb(poll, poll_one->events, io_poll_double_wake); - req_ref_get(req); - poll->wait.private = req; - + io_init_poll_iocb(poll, first->events, first->wait.func); *poll_ptr = poll; if (req->opcode == IORING_OP_POLL_ADD) req->flags |= REQ_F_ASYNC_DATA; @@ -5594,6 +5611,7 @@ static void __io_queue_proc(struct io_poll_iocb *poll, struct io_poll_table *pt, pt->nr_entries++; poll->head = head; + poll->wait.private = req; if (poll->events & EPOLLEXCLUSIVE) add_wait_queue_exclusive(head, &poll->wait); @@ -5601,61 +5619,24 @@ static void __io_queue_proc(struct io_poll_iocb *poll, struct io_poll_table *pt, add_wait_queue(head, &poll->wait); } -static void io_async_queue_proc(struct file *file, struct wait_queue_head *head, +static void io_poll_queue_proc(struct file *file, struct wait_queue_head *head, struct poll_table_struct *p) { struct io_poll_table *pt = container_of(p, struct io_poll_table, pt); - struct async_poll *apoll = pt->req->apoll; - __io_queue_proc(&apoll->poll, pt, head, &apoll->double_poll); -} - -static void io_async_task_func(struct io_kiocb *req, bool *locked) -{ - struct async_poll *apoll = req->apoll; - struct io_ring_ctx *ctx = req->ctx; - - trace_io_uring_task_run(req->ctx, req, req->opcode, req->user_data); - - if (io_poll_rewait(req, &apoll->poll)) { - spin_unlock(&ctx->completion_lock); - return; - } - - hash_del(&req->hash_node); - io_poll_remove_double(req); - apoll->poll.done = true; - spin_unlock(&ctx->completion_lock); - - if (!READ_ONCE(apoll->poll.canceled)) - io_req_task_submit(req, locked); - else - io_req_complete_failed(req, -ECANCELED); -} - -static int io_async_wake(struct wait_queue_entry *wait, unsigned mode, int sync, - void *key) -{ - struct io_kiocb *req = wait->private; - struct io_poll_iocb *poll = &req->apoll->poll; - - trace_io_uring_poll_wake(req->ctx, req->opcode, req->user_data, - key_to_poll(key)); - - return __io_async_wake(req, poll, key_to_poll(key), io_async_task_func); + __io_queue_proc(&pt->req->poll, pt, head, + (struct io_poll_iocb **) &pt->req->async_data); } -static __poll_t __io_arm_poll_handler(struct io_kiocb *req, - struct io_poll_iocb *poll, - struct io_poll_table *ipt, __poll_t mask, - wait_queue_func_t wake_func) - __acquires(&ctx->completion_lock) +static int __io_arm_poll_handler(struct io_kiocb *req, + struct io_poll_iocb *poll, + struct io_poll_table *ipt, __poll_t mask) { struct io_ring_ctx *ctx = req->ctx; - bool cancel = false; + int v; INIT_HLIST_NODE(&req->hash_node); - io_init_poll_iocb(poll, mask, wake_func); + io_init_poll_iocb(poll, mask, io_poll_wake); poll->file = req->file; poll->wait.private = req; @@ -5664,31 +5645,54 @@ static __poll_t __io_arm_poll_handler(struct io_kiocb *req, ipt->error = 0; ipt->nr_entries = 0; + /* + * Take the ownership to delay any tw execution up until we're done + * with poll arming. see io_poll_get_ownership(). + */ + atomic_set(&req->poll_refs, 1); mask = vfs_poll(req->file, &ipt->pt) & poll->events; - if (unlikely(!ipt->nr_entries) && !ipt->error) - ipt->error = -EINVAL; + + if (mask && (poll->events & EPOLLONESHOT)) { + io_poll_remove_entries(req); + /* no one else has access to the req, forget about the ref */ + return mask; + } + if (!mask && unlikely(ipt->error || !ipt->nr_entries)) { + io_poll_remove_entries(req); + if (!ipt->error) + ipt->error = -EINVAL; + return 0; + } spin_lock(&ctx->completion_lock); - if (ipt->error || (mask && (poll->events & EPOLLONESHOT))) - io_poll_remove_double(req); - if (likely(poll->head)) { - spin_lock_irq(&poll->head->lock); - if (unlikely(list_empty(&poll->wait.entry))) { - if (ipt->error) - cancel = true; - ipt->error = 0; - mask = 0; - } - if ((mask && (poll->events & EPOLLONESHOT)) || ipt->error) - list_del_init(&poll->wait.entry); - else if (cancel) - WRITE_ONCE(poll->canceled, true); - else if (!poll->done) /* actually waiting for an event */ - io_poll_req_insert(req); - spin_unlock_irq(&poll->head->lock); + io_poll_req_insert(req); + spin_unlock(&ctx->completion_lock); + + if (mask) { + /* can't multishot if failed, just queue the event we've got */ + if (unlikely(ipt->error || !ipt->nr_entries)) + poll->events |= EPOLLONESHOT; + __io_poll_execute(req, mask); + return 0; } - return mask; + /* + * Release ownership. If someone tried to queue a tw while it was + * locked, kick it off for them. + */ + v = atomic_dec_return(&req->poll_refs); + if (unlikely(v & IO_POLL_REF_MASK)) + __io_poll_execute(req, 0); + return 0; +} + +static void io_async_queue_proc(struct file *file, struct wait_queue_head *head, + struct poll_table_struct *p) +{ + struct io_poll_table *pt = container_of(p, struct io_poll_table, pt); + struct async_poll *apoll = pt->req->apoll; + + __io_queue_proc(&apoll->poll, pt, head, &apoll->double_poll); } enum { @@ -5703,7 +5707,8 @@ static int io_arm_poll_handler(struct io_kiocb *req) struct io_ring_ctx *ctx = req->ctx; struct async_poll *apoll; struct io_poll_table ipt; - __poll_t ret, mask = EPOLLONESHOT | POLLERR | POLLPRI; + __poll_t mask = EPOLLONESHOT | POLLERR | POLLPRI; + int ret; if (!def->pollin && !def->pollout) return IO_APOLL_ABORTED; @@ -5728,11 +5733,8 @@ static int io_arm_poll_handler(struct io_kiocb *req) req->apoll = apoll; req->flags |= REQ_F_POLLED; ipt.pt._qproc = io_async_queue_proc; - io_req_set_refcount(req); - ret = __io_arm_poll_handler(req, &apoll->poll, &ipt, mask, - io_async_wake); - spin_unlock(&ctx->completion_lock); + ret = __io_arm_poll_handler(req, &apoll->poll, &ipt, mask); if (ret || ipt.error) return ret ? IO_APOLL_READY : IO_APOLL_ABORTED; @@ -5741,43 +5743,6 @@ static int io_arm_poll_handler(struct io_kiocb *req) return IO_APOLL_OK; } -static bool __io_poll_remove_one(struct io_kiocb *req, - struct io_poll_iocb *poll, bool do_cancel) - __must_hold(&req->ctx->completion_lock) -{ - bool do_complete = false; - - if (!poll->head) - return false; - spin_lock_irq(&poll->head->lock); - if (do_cancel) - WRITE_ONCE(poll->canceled, true); - if (!list_empty(&poll->wait.entry)) { - list_del_init(&poll->wait.entry); - do_complete = true; - } - spin_unlock_irq(&poll->head->lock); - hash_del(&req->hash_node); - return do_complete; -} - -static bool io_poll_remove_one(struct io_kiocb *req) - __must_hold(&req->ctx->completion_lock) -{ - bool do_complete; - - io_poll_remove_double(req); - do_complete = __io_poll_remove_one(req, io_poll_get_single(req), true); - - if (do_complete) { - req_set_fail(req); - io_fill_cqe_req(req, -ECANCELED, 0); - io_commit_cqring(req->ctx); - io_put_req_deferred(req); - } - return do_complete; -} - /* * Returns true if we found and killed one or more poll requests */ @@ -5786,7 +5751,8 @@ static __cold bool io_poll_remove_all(struct io_ring_ctx *ctx, { struct hlist_node *tmp; struct io_kiocb *req; - int posted = 0, i; + bool found = false; + int i; spin_lock(&ctx->completion_lock); for (i = 0; i < (1U << ctx->cancel_hash_bits); i++) { @@ -5794,16 +5760,14 @@ static __cold bool io_poll_remove_all(struct io_ring_ctx *ctx, list = &ctx->cancel_hash[i]; hlist_for_each_entry_safe(req, tmp, list, hash_node) { - if (io_match_task(req, tsk, cancel_all)) - posted += io_poll_remove_one(req); + if (io_match_task(req, tsk, cancel_all)) { + io_poll_cancel_req(req); + found = true; + } } } spin_unlock(&ctx->completion_lock); - - if (posted) - io_cqring_ev_posted(ctx); - - return posted != 0; + return found; } static struct io_kiocb *io_poll_find(struct io_ring_ctx *ctx, __u64 sqe_addr, @@ -5824,19 +5788,26 @@ static struct io_kiocb *io_poll_find(struct io_ring_ctx *ctx, __u64 sqe_addr, return NULL; } +static bool io_poll_disarm(struct io_kiocb *req) + __must_hold(&ctx->completion_lock) +{ + if (!io_poll_get_ownership(req)) + return false; + io_poll_remove_entries(req); + hash_del(&req->hash_node); + return true; +} + static int io_poll_cancel(struct io_ring_ctx *ctx, __u64 sqe_addr, bool poll_only) __must_hold(&ctx->completion_lock) { - struct io_kiocb *req; + struct io_kiocb *req = io_poll_find(ctx, sqe_addr, poll_only); - req = io_poll_find(ctx, sqe_addr, poll_only); if (!req) return -ENOENT; - if (io_poll_remove_one(req)) - return 0; - - return -EALREADY; + io_poll_cancel_req(req); + return 0; } static __poll_t io_poll_parse_events(const struct io_uring_sqe *sqe, @@ -5886,23 +5857,6 @@ static int io_poll_update_prep(struct io_kiocb *req, return 0; } -static int io_poll_wake(struct wait_queue_entry *wait, unsigned mode, int sync, - void *key) -{ - struct io_kiocb *req = wait->private; - struct io_poll_iocb *poll = &req->poll; - - return __io_async_wake(req, poll, key_to_poll(key), io_poll_task_func); -} - -static void io_poll_queue_proc(struct file *file, struct wait_queue_head *head, - struct poll_table_struct *p) -{ - struct io_poll_table *pt = container_of(p, struct io_poll_table, pt); - - __io_queue_proc(&pt->req->poll, pt, head, (struct io_poll_iocb **) &pt->req->async_data); -} - static int io_poll_add_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe) { struct io_poll_iocb *poll = &req->poll; @@ -5926,57 +5880,31 @@ static int io_poll_add_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe static int io_poll_add(struct io_kiocb *req, unsigned int issue_flags) { struct io_poll_iocb *poll = &req->poll; - struct io_ring_ctx *ctx = req->ctx; struct io_poll_table ipt; - __poll_t mask; - bool done; + int ret; ipt.pt._qproc = io_poll_queue_proc; - mask = __io_arm_poll_handler(req, &req->poll, &ipt, poll->events, - io_poll_wake); - - if (mask) { /* no async, we'd stolen it */ - ipt.error = 0; - done = __io_poll_complete(req, mask); - io_commit_cqring(req->ctx); - } - spin_unlock(&ctx->completion_lock); - - if (mask) { - io_cqring_ev_posted(ctx); - if (done) - io_put_req(req); - } - return ipt.error; + ret = __io_arm_poll_handler(req, &req->poll, &ipt, poll->events); + ret = ret ?: ipt.error; + if (ret) + __io_req_complete(req, issue_flags, ret, 0); + return 0; } static int io_poll_update(struct io_kiocb *req, unsigned int issue_flags) { struct io_ring_ctx *ctx = req->ctx; struct io_kiocb *preq; - bool completing; int ret2, ret = 0; spin_lock(&ctx->completion_lock); preq = io_poll_find(ctx, req->poll_update.old_user_data, true); - if (!preq) { - ret = -ENOENT; -fail: + if (!preq || !io_poll_disarm(preq)) { spin_unlock(&ctx->completion_lock); + ret = preq ? -EALREADY : -ENOENT; goto out; } - io_poll_remove_double(preq); - /* - * Don't allow racy completion with singleshot, as we cannot safely - * update those. For multishot, if we're racing with completion, just - * let completion re-add it. - */ - completing = !__io_poll_remove_one(preq, &preq->poll, false); - if (completing && (preq->poll.events & EPOLLONESHOT)) { - ret = -EALREADY; - goto fail; - } spin_unlock(&ctx->completion_lock); if (req->poll_update.update_events || req->poll_update.update_user_data) { -- cgit From eb0089d629ba413ebf820733ad11b4b2bed45514 Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Wed, 15 Dec 2021 22:08:49 +0000 Subject: io_uring: single shot poll removal optimisation We don't need to poll oneshot request if we've got a desired mask in io_poll_wake(), task_work will clean it up correctly, but as we already hold a wq spinlock, we can remove ourselves and save on additional spinlocking in io_poll_remove_entries(). Signed-off-by: Pavel Begunkov Link: https://lore.kernel.org/r/ee170a344a18c9ef36b554d806c64caadfd61c31.1639605189.git.asml.silence@gmail.com Signed-off-by: Jens Axboe --- fs/io_uring.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index a36eb6060e7e..206066b59b62 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -5568,8 +5568,14 @@ static int io_poll_wake(struct wait_queue_entry *wait, unsigned mode, int sync, if (mask && !(mask & poll->events)) return 0; - if (io_poll_get_ownership(req)) + if (io_poll_get_ownership(req)) { + /* optional, saves extra locking for removal in tw handler */ + if (mask && poll->events & EPOLLONESHOT) { + list_del_init(&poll->wait.entry); + poll->head = NULL; + } __io_poll_execute(req, mask); + } return 1; } -- cgit From cc8e9ba71a8626bd322d1945a8fc0c8a52131a63 Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Wed, 15 Dec 2021 22:08:50 +0000 Subject: io_uring: use completion batching for poll rem/upd Use __io_req_complete() in io_poll_update(), so we can utilise completion batching for both update/remove request and the poll we're killing (if any). Signed-off-by: Pavel Begunkov Link: https://lore.kernel.org/r/e2bdc6c5abd9e9b80f09b86d8823eb1c780362cd.1639605189.git.asml.silence@gmail.com Signed-off-by: Jens Axboe --- fs/io_uring.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 206066b59b62..eda8739592d0 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -2778,7 +2778,7 @@ static bool __io_complete_rw_common(struct io_kiocb *req, long res) return false; } -static void io_req_task_complete(struct io_kiocb *req, bool *locked) +static inline void io_req_task_complete(struct io_kiocb *req, bool *locked) { unsigned int cflags = io_put_kbuf(req); int res = req->result; @@ -5903,6 +5903,7 @@ static int io_poll_update(struct io_kiocb *req, unsigned int issue_flags) struct io_ring_ctx *ctx = req->ctx; struct io_kiocb *preq; int ret2, ret = 0; + bool locked; spin_lock(&ctx->completion_lock); preq = io_poll_find(ctx, req->poll_update.old_user_data, true); @@ -5928,13 +5929,16 @@ static int io_poll_update(struct io_kiocb *req, unsigned int issue_flags) if (!ret2) goto out; } + req_set_fail(preq); - io_req_complete(preq, -ECANCELED); + preq->result = -ECANCELED; + locked = !(issue_flags & IO_URING_F_UNLOCKED); + io_req_task_complete(preq, &locked); out: if (ret < 0) req_set_fail(req); /* complete update request, we're done with it */ - io_req_complete(req, ret); + __io_req_complete(req, issue_flags, ret, 0); return 0; } -- cgit From 00f6e68b8d59bf006db54e3e257684f44d26195c Mon Sep 17 00:00:00 2001 From: GuoYong Zheng Date: Wed, 5 Jan 2022 18:12:02 +0800 Subject: io_uring: remove unused function parameter Parameter res2 is not used in __io_complete_rw, remove it. Fixes: 6b19b766e8f0 ("fs: get rid of the res2 iocb->ki_complete argument") Signed-off-by: GuoYong Zheng Link: https://lore.kernel.org/r/1641377522-1851-1-git-send-email-zhenggy@chinatelecom.cn Signed-off-by: Jens Axboe --- fs/io_uring.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index eda8739592d0..73742dacc8ae 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -2791,7 +2791,7 @@ static inline void io_req_task_complete(struct io_kiocb *req, bool *locked) } } -static void __io_complete_rw(struct io_kiocb *req, long res, long res2, +static void __io_complete_rw(struct io_kiocb *req, long res, unsigned int issue_flags) { if (__io_complete_rw_common(req, res)) @@ -3037,7 +3037,7 @@ static void kiocb_done(struct io_kiocb *req, ssize_t ret, if (req->flags & REQ_F_CUR_POS) req->file->f_pos = req->rw.kiocb.ki_pos; if (ret >= 0 && (req->rw.kiocb.ki_complete == io_complete_rw)) - __io_complete_rw(req, ret, 0, issue_flags); + __io_complete_rw(req, ret, issue_flags); else io_rw_done(&req->rw.kiocb, ret); -- cgit From c0235652ee5194fc75926daa580817e63ceb37ab Mon Sep 17 00:00:00 2001 From: GuoYong Zheng Date: Wed, 5 Jan 2022 18:13:05 +0800 Subject: io_uring: remove redundant tab space When show fdinfo, SqMask follow two tab space, which is inconsistent with other parameters. Remove one, so it lines up nicely. Signed-off-by: GuoYong Zheng Link: https://lore.kernel.org/r/1641377585-1891-1-git-send-email-zhenggy@chinatelecom.cn Signed-off-by: Jens Axboe --- fs/io_uring.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 73742dacc8ae..aed1625a26e1 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -10151,7 +10151,7 @@ static __cold void __io_uring_show_fdinfo(struct io_ring_ctx *ctx, * and sq_tail and cq_head are changed by userspace. But it's ok since * we usually use these info when it is stuck. */ - seq_printf(m, "SqMask:\t\t0x%x\n", sq_mask); + seq_printf(m, "SqMask:\t0x%x\n", sq_mask); seq_printf(m, "SqHead:\t%u\n", sq_head); seq_printf(m, "SqTail:\t%u\n", sq_tail); seq_printf(m, "CachedSqHead:\t%u\n", ctx->cached_sq_head); -- cgit From 3cc7fdb9f90a25ae92250bf9e6cf3b9556b230e9 Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Sun, 9 Jan 2022 00:53:22 +0000 Subject: io_uring: fix not released cached task refs tctx_task_work() may get run after io_uring cancellation and so there will be no one to put cached in tctx task refs that may have been added back by tw handlers using inline completion infra, Call io_uring_drop_tctx_refs() at the end of the main tw handler to release them. Cc: stable@vger.kernel.org # 5.15+ Reported-by: Lukas Bulwahn Fixes: e98e49b2bbf7 ("io_uring: extend task put optimisations") Signed-off-by: Pavel Begunkov Link: https://lore.kernel.org/r/69f226b35fbdb996ab799a8bbc1c06bf634ccec1.1641688805.git.asml.silence@gmail.com Signed-off-by: Jens Axboe --- fs/io_uring.c | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index aed1625a26e1..684d77c179a0 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -1827,6 +1827,18 @@ static inline void io_get_task_refs(int nr) io_task_refs_refill(tctx); } +static __cold void io_uring_drop_tctx_refs(struct task_struct *task) +{ + struct io_uring_task *tctx = task->io_uring; + unsigned int refs = tctx->cached_refs; + + if (refs) { + tctx->cached_refs = 0; + percpu_counter_sub(&tctx->inflight, refs); + put_task_struct_many(task, refs); + } +} + static bool io_cqring_event_overflow(struct io_ring_ctx *ctx, u64 user_data, s32 res, u32 cflags) { @@ -2319,6 +2331,10 @@ static void tctx_task_work(struct callback_head *cb) } ctx_flush_and_put(ctx, &uring_locked); + + /* relaxed read is enough as only the task itself sets ->in_idle */ + if (unlikely(atomic_read(&tctx->in_idle))) + io_uring_drop_tctx_refs(current); } static void io_req_task_work_add(struct io_kiocb *req, bool priority) @@ -9803,18 +9819,6 @@ static s64 tctx_inflight(struct io_uring_task *tctx, bool tracked) return percpu_counter_sum(&tctx->inflight); } -static __cold void io_uring_drop_tctx_refs(struct task_struct *task) -{ - struct io_uring_task *tctx = task->io_uring; - unsigned int refs = tctx->cached_refs; - - if (refs) { - tctx->cached_refs = 0; - percpu_counter_sub(&tctx->inflight, refs); - put_task_struct_many(task, refs); - } -} - /* * Find any io_uring ctx that this task has registered or done IO on, and cancel * requests. @sqd should be not-null IIF it's an SQPOLL thread cancellation. @@ -9870,10 +9874,14 @@ static __cold void io_uring_cancel_generic(bool cancel_all, schedule(); finish_wait(&tctx->wait, &wait); } while (1); - atomic_dec(&tctx->in_idle); io_uring_clean_tctx(tctx); if (cancel_all) { + /* + * We shouldn't run task_works after cancel, so just leave + * ->in_idle set for normal exit. + */ + atomic_dec(&tctx->in_idle); /* for exec all current's requests should be gone, kill tctx */ __io_uring_free(current); } -- cgit