The improvements in this patch are as follows: - Change several memory barriers into load acquire / store release instructions since the latter are faster. - Ensure that the completion has been reaped from use space by using smp_load_acquire() in __io_cqring_events(). Preceding __io_cqring_events() with smp_rmb() is not sufficient because the CPU may reorder READ_ONCE() in __io_cqring_events() with later memory accesses. - Fix a race between the WRITE_ONCE(req->iopoll_completed, 1) in io_complete_rw_iopoll() and req->iopoll_completed = 0 in io_sqring_entries() by reading req->iopoll_completed before comparing req->result with -EAGAIN. This patch has been tested by running the liburing test suite. Signed-off-by: Bart Van Assche <bvanassche@xxxxxxx> --- fs/io_uring.c | 32 +++++++++++--------------------- 1 file changed, 11 insertions(+), 21 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index c07913ec0cca..2fff7250f0b1 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -1727,9 +1727,9 @@ static inline bool io_should_trigger_evfd(struct io_ring_ctx *ctx) return io_wq_current_is_worker(); } -static inline unsigned __io_cqring_events(struct io_ring_ctx *ctx) +static inline unsigned io_cqring_events(struct io_ring_ctx *ctx) { - return ctx->cached_cq_tail - READ_ONCE(ctx->rings->cq.head); + return ctx->cached_cq_tail - smp_load_acquire(&ctx->rings->cq.head); } static void io_cqring_ev_posted(struct io_ring_ctx *ctx) @@ -1778,7 +1778,7 @@ static bool __io_cqring_overflow_flush(struct io_ring_ctx *ctx, bool force, bool all_flushed; LIST_HEAD(list); - if (!force && __io_cqring_events(ctx) == rings->cq_ring_entries) + if (!force && io_cqring_events(ctx) == rings->cq_ring_entries) return false; spin_lock_irqsave(&ctx->completion_lock, flags); @@ -2385,13 +2385,6 @@ static void io_double_put_req(struct io_kiocb *req) io_free_req(req); } -static unsigned io_cqring_events(struct io_ring_ctx *ctx) -{ - /* See comment at the top of this file */ - smp_rmb(); - return __io_cqring_events(ctx); -} - static inline unsigned int io_sqring_entries(struct io_ring_ctx *ctx) { struct io_rings *rings = ctx->rings; @@ -2457,15 +2450,13 @@ static void io_iopoll_complete(struct io_ring_ctx *ctx, unsigned int *nr_events, struct io_kiocb *req; LIST_HEAD(again); - /* order with ->result store in io_complete_rw_iopoll() */ - smp_rmb(); - io_init_req_batch(&rb); while (!list_empty(done)) { int cflags = 0; req = list_first_entry(done, struct io_kiocb, inflight_entry); - if (READ_ONCE(req->result) == -EAGAIN) { + if (smp_load_acquire(&req->iopoll_completed) && + req->result == -EAGAIN) { req->result = 0; req->iopoll_completed = 0; list_move_tail(&req->inflight_entry, &again); @@ -2514,7 +2505,7 @@ static int io_do_iopoll(struct io_ring_ctx *ctx, unsigned int *nr_events, * If we find a request that requires polling, break out * and complete those lists first, if we have entries there. */ - if (READ_ONCE(req->iopoll_completed)) { + if (smp_load_acquire(&req->iopoll_completed)) { list_move_tail(&req->inflight_entry, &done); continue; } @@ -2526,7 +2517,7 @@ static int io_do_iopoll(struct io_ring_ctx *ctx, unsigned int *nr_events, break; /* iopoll may have completed current req */ - if (READ_ONCE(req->iopoll_completed)) + if (smp_load_acquire(&req->iopoll_completed)) list_move_tail(&req->inflight_entry, &done); if (ret && spin) @@ -2767,10 +2758,9 @@ static void io_complete_rw_iopoll(struct kiocb *kiocb, long res, long res2) if (res != -EAGAIN && res != req->result) req_set_fail_links(req); - WRITE_ONCE(req->result, res); - /* order with io_poll_complete() checking ->result */ - smp_wmb(); - WRITE_ONCE(req->iopoll_completed, 1); + req->result = res; + /* order with io_poll_complete() checking ->iopoll_completed */ + smp_store_release(&req->iopoll_completed, 1); } /* @@ -2803,7 +2793,7 @@ static void io_iopoll_req_issued(struct io_kiocb *req, bool in_async) * For fast devices, IO may have already completed. If it has, add * it to the front so we find it first. */ - if (READ_ONCE(req->iopoll_completed)) + if (smp_load_acquire(&req->iopoll_completed)) list_add(&req->inflight_entry, &ctx->iopoll_list); else list_add_tail(&req->inflight_entry, &ctx->iopoll_list);