[PATCH] nvme: increment request genctr on completion

[PATCH] nvme: increment request genctr on completion

[Keith Busch]

The nvme request generation counter is intended to catch duplicate
completions. Incrementing the counter on submission means duplicates can
only be caught if the request tag is reallocated and dispatched prior to
the driver observing the corrupted CQE. Incrementing on completion
removes this window, making it possible to detect duplicate completions
in consecutive entries.

Signed-off-by: Keith Busch <kbusch@kernel.org>
---
 drivers/nvme/host/core.c | 2 --
 drivers/nvme/host/nvme.h | 4 ++++
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index f82c098b1a61..44c375a1edbb 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1037,8 +1037,6 @@ blk_status_t nvme_setup_cmd(struct nvme_ns *ns, struct request *req)
 		return BLK_STS_IOERR;
 	}
 
-	if (!(ctrl->quirks & NVME_QUIRK_SKIP_CID_GEN))
-		nvme_req(req)->genctr++;
 	cmd->common.command_id = nvme_cid(req);
 	trace_nvme_setup_cmd(req, cmd);
 	return ret;
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index b334af8aa264..a54096ba0552 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -614,6 +614,10 @@ static inline bool nvme_try_complete_req(struct request *req, __le16 status,
 		union nvme_result result)
 {
 	struct nvme_request *rq = nvme_req(req);
+	struct nvme_ctrl *ctrl = rq->ctrl;
+
+	if (!(ctrl->quirks & NVME_QUIRK_SKIP_CID_GEN))
+		rq->genctr++;
 
 	rq->status = le16_to_cpu(status) >> 1;
 	rq->result = result;
-- 
2.25.4

Re: [PATCH] nvme: increment request genctr on completion

[Sagi Grimberg]

> The nvme request generation counter is intended to catch duplicate
> completions. Incrementing the counter on submission means duplicates can
> only be caught if the request tag is reallocated and dispatched prior to
> the driver observing the corrupted CQE. Incrementing on completion
> removes this window, making it possible to detect duplicate completions
> in consecutive entries.

OK, It just occurred to me that blk_mq_tag_to_rq will still return the
request after it was completed as tags->rqs[x] is not cleared when
ending the request.

So this looks good.
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

Re: [PATCH] nvme: increment request genctr on completion

[Christoph Hellwig]

Thanks,

applied to nvme-5.17.