[PATCH] nvme: bump genctr when cancelling a request

[PATCH] nvme: bump genctr when cancelling a request

[Mateusz Nowicki]

nvme_find_rq() rejects a completion whose genctr does not match the
request, so a command completed once cannot be completed again by a
stale CQE. nvme_try_complete_req() bumps genctr for that reason.
nvme_cancel_request() completes the command too, but leaves genctr
alone. A CQE the controller posts for the cancelled command later still
matches and gets applied.

The cancel path is taken when the host stopped waiting for the
controller: CSTS.CFS set (dead path in nvme_dev_disable(), no CC.EN=0,
no wait), or CSTS.RDY not cleared within CAP.TO. Nothing stops the
controller from posting completions after that, and the second
nvme_dev_disable() from nvme_reset_work() reaps them in
nvme_reap_pending_cqes().

The cancelled request sits on the requeue list after the RETRY
disposition. The late CQE ends and frees it from there, and the next
dispatch hits req->mq_hctx == NULL:

  BUG: kernel NULL pointer dereference, address: 0000000000000158
  RIP: nvme_prep_rq+0x1a6
  nvme_queue_rq
  blk_mq_dispatch_rq_list
  __blk_mq_sched_dispatch_requests
  blk_mq_run_work_fn

Bump genctr in nvme_cancel_request() like a real completion does.

Reproduced with vnvme (https://github.com/Mateusz-Nowicki-Embedded/vnvme),
a virtual NVMe endpoint that holds completions back under I/O, sets
CSTS.CFS, and releases them after the tagset was cancelled.

Signed-off-by: Mateusz Nowicki <mateusz.nowicki@posteo.net>
---
 drivers/nvme/host/core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -532,6 +532,8 @@ bool nvme_cancel_request(struct request *req, void *data)
 	if (blk_mq_rq_state(req) != MQ_RQ_IN_FLIGHT)
 		return true;
 
+	if (!(nvme_req(req)->ctrl->quirks & NVME_QUIRK_SKIP_CID_GEN))
+		nvme_req(req)->genctr++;
 	nvme_req(req)->status = NVME_SC_HOST_ABORTED_CMD;
 	nvme_req(req)->flags |= NVME_REQ_CANCELLED;
 	blk_mq_complete_request(req);

Re: [PATCH] nvme: bump genctr when cancelling a request

[Keith Busch]

On Sun, Sep 13, 2026 at 12:41:01PM +0000, Mateusz Nowicki wrote:
> The cancel path is taken when the host stopped waiting for the
> controller: CSTS.CFS set (dead path in nvme_dev_disable(), no CC.EN=0,
> no wait), or CSTS.RDY not cleared within CAP.TO. Nothing stops the
> controller from posting completions after that, and the second
> nvme_dev_disable() from nvme_reset_work() reaps them in
> nvme_reap_pending_cqes().

I think it was supposed to be that we don't cancel a request while it's
still possible to see a natural completion. So with that in mind,
perhaps the call to "nvme_disable_ctrl" should be called unconditionally
instead of only on a live controller.

The sequence you're describing is weird, though. Your controller
has fatal status, the driver hasn't done anything yet to recover the
device, so why were there no completions available the first go around,
then completions became available on the second? The controller
shouldn't have done anything in that time, nor should it have been able
to since Bus Master Enable was off.

Re: [PATCH] nvme: bump genctr when cancelling a request

[Mateusz Nowicki]

> The sequence you're describing is weird, though. Your controller
> has fatal status, the driver hasn't done anything yet to recover the
> device, so why were there no completions available the first go around,
> then completions became available on the second? The controller
> shouldn't have done anything in that time, nor should it have been able
> to since Bus Master Enable was off.

Thanks Keith for the feedback. You're right, vnvme (a virtual NVMe
endpoint I use to create corner cases that are hard to reproduce with
publicly available drives, like delayed completions) ignored BME, and
that is what produced the late completions.

I have seen firmware that ignores BME too. Unlikely in the field, but
the question is whether the driver wants to be resilient to that class
of firmware bug. If not, I'll drop this and can test the unconditional
nvme_disable_ctrl() variant you suggested instead.