Skip to content

Commit a26a35e

Browse files
isilenceaxboe
authored andcommitted
io_uring: make poll refs more robust
poll_refs carry two functions, the first is ownership over the request. The second is notifying the io_poll_check_events() that there was an event but wake up couldn't grab the ownership, so io_poll_check_events() should retry. We want to make poll_refs more robust against overflows. Instead of always incrementing it, which covers two purposes with one atomic, check if poll_refs is elevated enough and if so set a retry flag without attempts to grab ownership. The gap between the bias check and following atomics may seem racy, but we don't need it to be strict. Moreover there might only be maximum 4 parallel updates: by the first and the second poll entries, __io_arm_poll_handler() and cancellation. From those four, only poll wake ups may be executed multiple times, but they're protected by a spin. Cc: stable@vger.kernel.org Reported-by: Lin Ma <linma@zju.edu.cn> Fixes: aa43477 ("io_uring: poll rework") Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> Link: https://lore.kernel.org/r/c762bc31f8683b3270f3587691348a7119ef9c9d.1668963050.git.asml.silence@gmail.com Signed-off-by: Jens Axboe <axboe@kernel.dk>
1 parent 2f38934 commit a26a35e

File tree

1 file changed

+35
-1
lines changed

1 file changed

+35
-1
lines changed

io_uring/poll.c

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,14 @@ struct io_poll_table {
4040
};
4141

4242
#define IO_POLL_CANCEL_FLAG BIT(31)
43-
#define IO_POLL_REF_MASK GENMASK(30, 0)
43+
#define IO_POLL_RETRY_FLAG BIT(30)
44+
#define IO_POLL_REF_MASK GENMASK(29, 0)
45+
46+
/*
47+
* We usually have 1-2 refs taken, 128 is more than enough and we want to
48+
* maximise the margin between this amount and the moment when it overflows.
49+
*/
50+
#define IO_POLL_REF_BIAS 128
4451

4552
#define IO_WQE_F_DOUBLE 1
4653

@@ -58,6 +65,21 @@ static inline bool wqe_is_double(struct wait_queue_entry *wqe)
5865
return priv & IO_WQE_F_DOUBLE;
5966
}
6067

68+
static bool io_poll_get_ownership_slowpath(struct io_kiocb *req)
69+
{
70+
int v;
71+
72+
/*
73+
* poll_refs are already elevated and we don't have much hope for
74+
* grabbing the ownership. Instead of incrementing set a retry flag
75+
* to notify the loop that there might have been some change.
76+
*/
77+
v = atomic_fetch_or(IO_POLL_RETRY_FLAG, &req->poll_refs);
78+
if (v & IO_POLL_REF_MASK)
79+
return false;
80+
return !(atomic_fetch_inc(&req->poll_refs) & IO_POLL_REF_MASK);
81+
}
82+
6183
/*
6284
* If refs part of ->poll_refs (see IO_POLL_REF_MASK) is 0, it's free. We can
6385
* bump it and acquire ownership. It's disallowed to modify requests while not
@@ -66,6 +88,8 @@ static inline bool wqe_is_double(struct wait_queue_entry *wqe)
6688
*/
6789
static inline bool io_poll_get_ownership(struct io_kiocb *req)
6890
{
91+
if (unlikely(atomic_read(&req->poll_refs) >= IO_POLL_REF_BIAS))
92+
return io_poll_get_ownership_slowpath(req);
6993
return !(atomic_fetch_inc(&req->poll_refs) & IO_POLL_REF_MASK);
7094
}
7195

@@ -235,6 +259,16 @@ static int io_poll_check_events(struct io_kiocb *req, bool *locked)
235259
*/
236260
if ((v & IO_POLL_REF_MASK) != 1)
237261
req->cqe.res = 0;
262+
if (v & IO_POLL_RETRY_FLAG) {
263+
req->cqe.res = 0;
264+
/*
265+
* We won't find new events that came in between
266+
* vfs_poll and the ref put unless we clear the flag
267+
* in advance.
268+
*/
269+
atomic_andnot(IO_POLL_RETRY_FLAG, &req->poll_refs);
270+
v &= ~IO_POLL_RETRY_FLAG;
271+
}
238272

239273
/* the mask was stashed in __io_poll_execute */
240274
if (!req->cqe.res) {

0 commit comments

Comments
 (0)