Skip to content

Commit

Permalink
io_uring: fix potential hang with polled IO
Browse files Browse the repository at this point in the history
If a request issue ends up being punted to async context to avoid
blocking, we can get into a situation where the original application
enters the poll loop for that very request before it has been issued.
This should not be an issue, except that the polling will hold the
io_uring uring_ctx mutex for the duration of the poll. When the async
worker has actually issued the request, it needs to acquire this mutex
to add the request to the poll issued list. Since the application
polling is already holding this mutex, the workqueue sleeps on the
mutex forever, and the application thus never gets a chance to poll for
the very request it was interested in.

Fix this by ensuring that the polling drops the uring_ctx occasionally
if it's not making any progress.

Reported-by: Jeffrey M. Birnbaum <jmbnyc@gmail.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
  • Loading branch information
axboe committed Aug 20, 2019
1 parent d1abaeb commit 500f9fb
Showing 1 changed file with 25 additions and 11 deletions.
36 changes: 25 additions & 11 deletions fs/io_uring.c
Original file line number Diff line number Diff line change
Expand Up @@ -805,11 +805,34 @@ static void io_iopoll_reap_events(struct io_ring_ctx *ctx)
static int io_iopoll_check(struct io_ring_ctx *ctx, unsigned *nr_events,
long min)
{
int ret = 0;
int iters, ret = 0;

/*
* We disallow the app entering submit/complete with polling, but we
* still need to lock the ring to prevent racing with polled issue
* that got punted to a workqueue.
*/
mutex_lock(&ctx->uring_lock);

iters = 0;
do {
int tmin = 0;

/*
* If a submit got punted to a workqueue, we can have the
* application entering polling for a command before it gets
* issued. That app will hold the uring_lock for the duration
* of the poll right here, so we need to take a breather every
* now and then to ensure that the issue has a chance to add
* the poll to the issued list. Otherwise we can spin here
* forever, while the workqueue is stuck trying to acquire the
* very same mutex.
*/
if (!(++iters & 7)) {
mutex_unlock(&ctx->uring_lock);
mutex_lock(&ctx->uring_lock);
}

if (*nr_events < min)
tmin = min - *nr_events;

Expand All @@ -819,6 +842,7 @@ static int io_iopoll_check(struct io_ring_ctx *ctx, unsigned *nr_events,
ret = 0;
} while (min && !*nr_events && !need_resched());

mutex_unlock(&ctx->uring_lock);
return ret;
}

Expand Down Expand Up @@ -2280,15 +2304,7 @@ static int io_sq_thread(void *data)
unsigned nr_events = 0;

if (ctx->flags & IORING_SETUP_IOPOLL) {
/*
* We disallow the app entering submit/complete
* with polling, but we still need to lock the
* ring to prevent racing with polled issue
* that got punted to a workqueue.
*/
mutex_lock(&ctx->uring_lock);
io_iopoll_check(ctx, &nr_events, 0);
mutex_unlock(&ctx->uring_lock);
} else {
/*
* Normal IO, just pretend everything completed.
Expand Down Expand Up @@ -3190,9 +3206,7 @@ SYSCALL_DEFINE6(io_uring_enter, unsigned int, fd, u32, to_submit,
min_complete = min(min_complete, ctx->cq_entries);

if (ctx->flags & IORING_SETUP_IOPOLL) {
mutex_lock(&ctx->uring_lock);
ret = io_iopoll_check(ctx, &nr_events, min_complete);
mutex_unlock(&ctx->uring_lock);
} else {
ret = io_cqring_wait(ctx, min_complete, sig, sigsz);
}
Expand Down

0 comments on commit 500f9fb

Please sign in to comment.