[for-4.16,v2,2/5] nvme: use request_queue's failover_rq_fn callback for multipath failover

Commit Message

Mike Snitzer Dec. 27, 2017, 3:22 a.m. UTC

Also, remove NVMe-local REQ_NVME_MPATH since setting request_queue's
failover_rq_fn serves the same purpose.  NVMe's error handling gives a
request further consideration about it being retryable if its
request_queue has failover_rq_fn set.

Signed-off-by: Mike Snitzer <[e-mail]>
---
 drivers/nvme/host/core.c      | 5 +++--
 drivers/nvme/host/multipath.c | 4 +---
 drivers/nvme/host/nvme.h      | 5 -----
 3 files changed, 4 insertions(+), 10 deletions(-)

Comments

Christoph Hellwig Dec. 29, 2017, 10:11 a.m. UTC | #1

Independent on any policy question as raised in reply to the previous
patch:  until the ANA (ALUA equivalent for NVMe) spec has been finalized
and our code for it has landed there is no way we're going to expose
anything from this code based as it will be highly in flux.

Mike Snitzer Dec. 29, 2017, 8:22 p.m. UTC | #2

On Fri, Dec 29 2017 at  5:11am -0500,
Christoph Hellwig <[e-mail]> wrote:

> Independent on any policy question as raised in reply to the previous
> patch:  until the ANA (ALUA equivalent for NVMe) spec has been finalized
> and our code for it has landed there is no way we're going to expose
> anything from this code based as it will be highly in flux.

Please stop hiding behind specs like they are governing this aspect of
Linux's NVMe implementation.

Please just own the need for NVMe to provide DM multipathing
compatibility.

Thanks.

Patch

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index f837d666cbd4..99f1c7d8514e 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -191,8 +191,8 @@  static inline bool nvme_req_needs_retry(struct request *req)
 void nvme_complete_rq(struct request *req)
 {
 	if (unlikely(nvme_req(req)->status && nvme_req_needs_retry(req))) {
-		if (nvme_req_needs_failover(req)) {
-			nvme_failover_req(req);
+		if (req->q->failover_rq_fn && nvme_req_needs_failover(req)) {
+			req->q->failover_rq_fn(req);
 			return;
 		}
 
@@ -2893,6 +2893,7 @@  static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid)
 		sprintf(disk_name, "nvme%dc%dn%d", ctrl->subsys->instance,
 				ctrl->cntlid, ns->head->instance);
 		flags = GENHD_FL_HIDDEN;
+		ns->queue->failover_rq_fn = nvme_failover_req;
 	} else {
 		sprintf(disk_name, "nvme%dn%d", ctrl->subsys->instance,
 				ns->head->instance);
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 1218a9fca846..b576942b96b2 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -35,8 +35,7 @@  void nvme_failover_req(struct request *req)
 
 bool nvme_req_needs_failover(struct request *req)
 {
-	if (!(req->cmd_flags & REQ_NVME_MPATH))
-		return false;
+	/* Caller verifies req->q->failover_rq_fn is set */
 
 	switch (nvme_req(req)->status & 0x7ff) {
 	/*
@@ -128,7 +127,6 @@  static blk_qc_t nvme_ns_head_make_request(struct request_queue *q,
 	ns = nvme_find_path(head);
 	if (likely(ns)) {
 		bio->bi_disk = ns->disk;
-		bio->bi_opf |= REQ_NVME_MPATH;
 		ret = direct_make_request(bio);
 	} else if (!list_empty_careful(&head->list)) {
 		dev_warn_ratelimited(dev, "no path available - requeuing I/O\n");
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index ea1aa5283e8e..5ec44935690f 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -95,11 +95,6 @@  struct nvme_request {
 	u16			status;
 };
 
-/*
- * Mark a bio as coming in through the mpath node.
- */
-#define REQ_NVME_MPATH		REQ_DRV
-
 enum {
 	NVME_REQ_CANCELLED		= (1 << 0),
 };