hi,
On 6/14/20 8:10 AM, Xiaoguang Wang wrote:
hi,
I have taken some further thoughts about previous IPOLL race fix patch,
if io_complete_rw_iopoll() is called in interrupt context, "req->result = res"
and "WRITE_ONCE(req->iopoll_completed, 1);" are independent store operations.
So in io_do_iopoll(), if iopoll_completed is ture, can we make sure that
req->result has already been perceived by the cpu executing io_do_iopoll()?
Good point, I think if we do something like the below, we should be
totally safe against an IRQ completion. Since we batch the completions,
we can get by with just a single smp_rmb() on the completion side.
Yes, agree.
And thanks for confirming this issue, I'll make a formal patch with proper commit log.
Regards,
Xiaoguang Wang
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 155f3d830ddb..74c2a4709b63 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -1736,6 +1736,9 @@ static void io_iopoll_complete(struct io_ring_ctx *ctx, unsigned int *nr_events,
struct req_batch rb;
struct io_kiocb *req;
+ /* 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;
@@ -1976,6 +1979,8 @@ static void io_complete_rw_iopoll(struct kiocb *kiocb, long res, long res2)
if (res != req->result)
req_set_fail_links(req);
req->result = res;
+ /* order with io_poll_complete() checking ->result */
+ smp_wmb();
if (res != -EAGAIN)
WRITE_ONCE(req->iopoll_completed, 1);
}
