Skip to content

Commit 30a3366

Browse files
isilenceaxboe
authored andcommitted
io_uring/poll: fix double poll req->flags races
io_poll_double_prepare() | io_poll_wake() | poll->head = NULL smp_load(&poll->head); /* NULL */ | flags = req->flags; | | req->flags &= ~SINGLE_POLL; req->flags = flags | DOUBLE_POLL | The idea behind io_poll_double_prepare() is to serialise with the first poll entry by taking the wq lock. However, it's not safe to assume that io_poll_wake() is not running when we can't grab the lock and so we may race modifying req->flags. Skip double poll setup if that happens. It's ok because the first poll entry will only be removed when it's definitely completing, e.g. pollfree or oneshot with a valid mask. Fixes: 49f1c68 ("io_uring: optimise submission side poll_refs") Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> Link: https://lore.kernel.org/r/b7fab2d502f6121a7d7b199fe4d914a43ca9cdfd.1668184658.git.asml.silence@gmail.com Signed-off-by: Jens Axboe <axboe@kernel.dk>
1 parent 3851d25 commit 30a3366

File tree

1 file changed

+17
-12
lines changed

1 file changed

+17
-12
lines changed

io_uring/poll.c

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -394,7 +394,8 @@ static int io_poll_wake(struct wait_queue_entry *wait, unsigned mode, int sync,
394394
return 1;
395395
}
396396

397-
static void io_poll_double_prepare(struct io_kiocb *req)
397+
/* fails only when polling is already completing by the first entry */
398+
static bool io_poll_double_prepare(struct io_kiocb *req)
398399
{
399400
struct wait_queue_head *head;
400401
struct io_poll *poll = io_poll_get_single(req);
@@ -403,20 +404,20 @@ static void io_poll_double_prepare(struct io_kiocb *req)
403404
rcu_read_lock();
404405
head = smp_load_acquire(&poll->head);
405406
/*
406-
* poll arm may not hold ownership and so race with
407-
* io_poll_wake() by modifying req->flags. There is only one
408-
* poll entry queued, serialise with it by taking its head lock.
407+
* poll arm might not hold ownership and so race for req->flags with
408+
* io_poll_wake(). There is only one poll entry queued, serialise with
409+
* it by taking its head lock. As we're still arming the tw hanlder
410+
* is not going to be run, so there are no races with it.
409411
*/
410-
if (head)
412+
if (head) {
411413
spin_lock_irq(&head->lock);
412-
413-
req->flags |= REQ_F_DOUBLE_POLL;
414-
if (req->opcode == IORING_OP_POLL_ADD)
415-
req->flags |= REQ_F_ASYNC_DATA;
416-
417-
if (head)
414+
req->flags |= REQ_F_DOUBLE_POLL;
415+
if (req->opcode == IORING_OP_POLL_ADD)
416+
req->flags |= REQ_F_ASYNC_DATA;
418417
spin_unlock_irq(&head->lock);
418+
}
419419
rcu_read_unlock();
420+
return !!head;
420421
}
421422

422423
static void __io_queue_proc(struct io_poll *poll, struct io_poll_table *pt,
@@ -454,7 +455,11 @@ static void __io_queue_proc(struct io_poll *poll, struct io_poll_table *pt,
454455
/* mark as double wq entry */
455456
wqe_private |= IO_WQE_F_DOUBLE;
456457
io_init_poll_iocb(poll, first->events, first->wait.func);
457-
io_poll_double_prepare(req);
458+
if (!io_poll_double_prepare(req)) {
459+
/* the request is completing, just back off */
460+
kfree(poll);
461+
return;
462+
}
458463
*poll_ptr = poll;
459464
} else {
460465
/* fine to modify, there is no poll queued to race with us */

0 commit comments

Comments
 (0)