NVMe: limit max completion iterations in nvme_process_cq()
Subsystems (2)
PATCH 1/1 NVMe: limit max completion iterations in nvme_process_cq()
This is an unbounded loop. If we have per-cpu queues this is usually not a problem, but if CPUs share a queue, then we could have some of them continually queueing IO and the loop could take forever to exit.
Limit max iterations to the queue depth of the given completion queue, which seems like a nice number to use.
--- drivers/block/nvme-core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/block/nvme-core.c +2 -1
@@ -697,12 +697,13 @@ static int nvme_queue_rq(struct blk_mq_hw_ctx *hctx,static int nvme_process_cq (struct nvme_queue *nvmeq) { +unsigned int max_iter = nvmeq->q_depth;u16 head, phase; head = nvmeq->cq_head; phase = nvmeq->cq_phase; -for (;;) { +while (max_iter--) {void *ctx;nvme_completion_fn fn;struct nvme_completion cqe = nvmeq->cqes[head];
Signature · 4 lines
1.9.1 -- Jens Axboe
PATCH 1/1 NVMe: limit max completion iterations in nvme_process_cq()
Quoted 7 lines · Jens Axboe
This is an unbounded loop. If we have per-cpu queues this is usually not a problem, but if CPUs share a queue, then we could have some of them continually queueing IO and the loop could take forever to exit.
Limit max iterations to the queue depth of the given completion queue, which seems like a nice number to use.
I think the loop implicitly already has the same max iteration: the queue lock is held when it's called, so new commands can't be posted on the submission queue while the completions are reaped, so we can't loop more than the depth.
PATCH 1/1 NVMe: limit max completion iterations in nvme_process_cq()
Quoted 6 lines · Keith Busch
Quoted 5 lines · Jens Axboe
This is an unbounded loop. If we have per-cpu queues this is usually not a problem, but if CPUs share a queue, then we could have some of them continually queueing IO and the loop could take forever to exit.
Quoted 7 lines
Quoted 2 lines
Limit max iterations to the queue depth of the given completion queue, which seems like a nice number to use.
I think the loop implicitly already has the same max iteration: the queue lock is held when it's called, so new commands can't be posted on the submission queue while the completions are reaped, so we can't loop more than the depth.
That's a good point, since we need the same queue lock to submit a new command. Disregard!
Signature · 1 line
Jens Axboe