blk-mq: fix race with timeouts and requeue events
If a requeue event races with a timeout, we can get into the
situation where we attempt to complete a request from the
timeout handler when it's not start anymore. This causes a crash.
So have the timeout handler check that REQ_ATOM_STARTED is still
set on the request - if not, we ignore the event. If this happens,
the request has now been marked as complete. As a consequence, we
need to ensure to clear REQ_ATOM_COMPLETE in blk_mq_start_request(),
as to maintain proper request state.
Signed-off-by: Jens Axboe <axboe@fb.com>
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 7d5650d..a84112c 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -378,7 +378,15 @@
* REQ_ATOMIC_STARTED is seen.
*/
rq->deadline = jiffies + q->rq_timeout;
+
+ /*
+ * Mark us as started and clear complete. Complete might have been
+ * set if requeue raced with timeout, which then marked it as
+ * complete. So be sure to clear complete again when we start
+ * the request, otherwise we'll ignore the completion event.
+ */
set_bit(REQ_ATOM_STARTED, &rq->atomic_flags);
+ clear_bit(REQ_ATOM_COMPLETE, &rq->atomic_flags);
if (q->dma_drain_size && blk_rq_bytes(rq)) {
/*
@@ -485,6 +493,28 @@
blk_mq_tag_busy_iter(hctx->tags, blk_mq_timeout_check, &data);
}
+static enum blk_eh_timer_return blk_mq_rq_timed_out(struct request *rq)
+{
+ struct request_queue *q = rq->q;
+
+ /*
+ * We know that complete is set at this point. If STARTED isn't set
+ * anymore, then the request isn't active and the "timeout" should
+ * just be ignored. This can happen due to the bitflag ordering.
+ * Timeout first checks if STARTED is set, and if it is, assumes
+ * the request is active. But if we race with completion, then
+ * we both flags will get cleared. So check here again, and ignore
+ * a timeout event with a request that isn't active.
+ */
+ if (!test_bit(REQ_ATOM_STARTED, &rq->atomic_flags))
+ return BLK_EH_NOT_HANDLED;
+
+ if (!q->mq_ops->timeout)
+ return BLK_EH_RESET_TIMER;
+
+ return q->mq_ops->timeout(rq);
+}
+
static void blk_mq_rq_timer(unsigned long data)
{
struct request_queue *q = (struct request_queue *) data;
@@ -538,11 +568,6 @@
return false;
}
-void blk_mq_add_timer(struct request *rq)
-{
- __blk_add_timer(rq, NULL);
-}
-
/*
* Run this hardware queue, pulling any software queues mapped to it in.
* Note that this function currently has various problems around ordering
@@ -799,7 +824,7 @@
/*
* We do this early, to ensure we are on the right CPU.
*/
- blk_mq_add_timer(rq);
+ blk_add_timer(rq);
}
void blk_mq_insert_request(struct request *rq, bool at_head, bool run_queue,
@@ -1400,7 +1425,7 @@
q->sg_reserved_size = INT_MAX;
blk_queue_make_request(q, blk_mq_make_request);
- blk_queue_rq_timed_out(q, set->ops->timeout);
+ blk_queue_rq_timed_out(q, blk_mq_rq_timed_out);
if (set->timeout)
blk_queue_rq_timeout(q, set->timeout);