io_uring: add memory barrier to synchronize io_kiocb's result and iopoll_completed

In io_complete_rw_iopoll(), stores to io_kiocb's result and iopoll
completed are two independent store operations, to ensure that once
iopoll_completed is ture and then req->result must been perceived by
the cpu executing io_do_iopoll(), proper memory barrier should be used.

And in io_do_iopoll(), we check whether req->result is EAGAIN, if it is,
we'll need to issue this io request using io-wq again. In order to just
issue a single smp_rmb() on the completion side, move the re-submit work
to io_iopoll_complete().

Cc: stable@vger.kernel.org
Signed-off-by: Xiaoguang Wang <xiaoguang.wang@linux.alibaba.com>
[axboe: don't set ->iopoll_completed for -EAGAIN retry]
Signed-off-by: Jens Axboe <axboe@kernel.dk>

diff --git a/fs/io_uring.c b/fs/io_uring.c
index eb37977..9d2ae9a 100644 (file)
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -1742,6 +1742,18 @@ static int io_put_kbuf(struct io_kiocb *req)
        return cflags;
 }
 
+static void io_iopoll_queue(struct list_head *again)
+{
+       struct io_kiocb *req;
+
+       do {
+               req = list_first_entry(again, struct io_kiocb, list);
+               list_del(&req->list);
+               refcount_inc(&req->refs);
+               io_queue_async_work(req);
+       } while (!list_empty(again));
+}
+
 /*
  * Find and free completed poll iocbs
  */
@@ -1750,12 +1762,21 @@ static void io_iopoll_complete(struct io_ring_ctx *ctx, unsigned int *nr_events,
 {
        struct req_batch rb;
        struct io_kiocb *req;
+       LIST_HEAD(again);
+
+       /* order with ->result store in io_complete_rw_iopoll() */
+       smp_rmb();
 
        rb.to_free = rb.need_iter = 0;
        while (!list_empty(done)) {
                int cflags = 0;
 
                req = list_first_entry(done, struct io_kiocb, list);
+               if (READ_ONCE(req->result) == -EAGAIN) {
+                       req->iopoll_completed = 0;
+                       list_move_tail(&req->list, &again);
+                       continue;
+               }
                list_del(&req->list);
 
                if (req->flags & REQ_F_BUFFER_SELECTED)
@@ -1773,18 +1794,9 @@ static void io_iopoll_complete(struct io_ring_ctx *ctx, unsigned int *nr_events,
        if (ctx->flags & IORING_SETUP_SQPOLL)
                io_cqring_ev_posted(ctx);
        io_free_req_many(ctx, &rb);
-}
-
-static void io_iopoll_queue(struct list_head *again)
-{
-       struct io_kiocb *req;
 
-       do {
-               req = list_first_entry(again, struct io_kiocb, list);
-               list_del(&req->list);
-               refcount_inc(&req->refs);
-               io_queue_async_work(req);
-       } while (!list_empty(again));
+       if (!list_empty(&again))
+               io_iopoll_queue(&again);
 }
 
 static int io_do_iopoll(struct io_ring_ctx *ctx, unsigned int *nr_events,
@@ -1792,7 +1804,6 @@ static int io_do_iopoll(struct io_ring_ctx *ctx, unsigned int *nr_events,
 {
        struct io_kiocb *req, *tmp;
        LIST_HEAD(done);
-       LIST_HEAD(again);
        bool spin;
        int ret;
 
@@ -1818,13 +1829,6 @@ static int io_do_iopoll(struct io_ring_ctx *ctx, unsigned int *nr_events,
                if (!list_empty(&done))
                        break;
 
-               if (req->result == -EAGAIN) {
-                       list_move_tail(&req->list, &again);
-                       continue;
-               }
-               if (!list_empty(&again))
-                       break;
-
                ret = kiocb->ki_filp->f_op->iopoll(kiocb, spin);
                if (ret < 0)
                        break;
@@ -1837,9 +1841,6 @@ static int io_do_iopoll(struct io_ring_ctx *ctx, unsigned int *nr_events,
        if (!list_empty(&done))
                io_iopoll_complete(ctx, nr_events, &done);
 
-       if (!list_empty(&again))
-               io_iopoll_queue(&again);
-
        return ret;
 }
 
@@ -1990,9 +1991,13 @@ static void io_complete_rw_iopoll(struct kiocb *kiocb, long res, long res2)
 
        if (res != -EAGAIN && res != req->result)
                req_set_fail_links(req);
-       req->result = res;
-       if (res != -EAGAIN)
+
+       WRITE_ONCE(req->result, res);
+       /* order with io_poll_complete() checking ->result */
+       if (res != -EAGAIN) {
+               smp_wmb();
                WRITE_ONCE(req->iopoll_completed, 1);
+       }
 }
 
 /*