All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mike Snitzer <snitzer@redhat.com>
To: Sagi Grimberg <sagi@grimberg.me>
Cc: linux-nvme@lists.infradead.org,
	Christoph Hellwig <hch@infradead.org>,
	dm-devel@redhat.com, Chao Leng <lengchao@huawei.com>,
	Keith Busch <kbusch@kernel.org>,
	"Meneghini, John" <John.Meneghini@netapp.com>,
	Ewan Milne <emilne@redhat.com>
Subject: [RESEND PATCH] nvme: explicitly use normal NVMe error handling when appropriate
Date: Thu, 13 Aug 2020 10:48:11 -0400	[thread overview]
Message-ID: <20200813144811.GA5452@redhat.com> (raw)
In-Reply-To: <20200810172209.GA19535@redhat.com>

Commit 764e9332098c0 ("nvme-multipath: do not reset on unknown
status"), among other things, fixed NVME_SC_CMD_INTERRUPTED error
handling by changing multipathing's nvme_failover_req() to short-circuit
path failover and then fallback to NVMe's normal error handling (which
takes care of NVME_SC_CMD_INTERRUPTED).

This detour through native NVMe multipathing code is unwelcome because
it prevents NVMe core from handling NVME_SC_CMD_INTERRUPTED independent
of any multipathing concerns.

Introduce nvme_status_needs_local_error_handling() to prioritize
non-failover retry, when appropriate, in terms of normal NVMe error
handling.  nvme_status_needs_local_error_handling() will naturely evolve
to include handling of any other errors that normal error handling must
be used for.

nvme_failover_req()'s ability to fallback to normal NVMe error handling
has been preserved because it may be useful for future NVME_SC that
nvme_status_needs_local_error_handling() hasn't been trained for yet.

Signed-off-by: Mike Snitzer <snitzer@redhat.com>
---
 drivers/nvme/host/core.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 88cff309d8e4..be749b690af7 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -252,6 +252,16 @@ static inline bool nvme_req_needs_retry(struct request *req)
 	return true;
 }
 
+static inline bool nvme_status_needs_local_error_handling(u16 status)
+{
+	switch (status & 0x7ff) {
+	case NVME_SC_CMD_INTERRUPTED:
+		return true;
+	default:
+		return false;
+	}
+}
+
 static void nvme_retry_req(struct request *req)
 {
 	struct nvme_ns *ns = req->q->queuedata;
@@ -270,7 +280,8 @@ static void nvme_retry_req(struct request *req)
 
 void nvme_complete_rq(struct request *req)
 {
-	blk_status_t status = nvme_error_status(nvme_req(req)->status);
+	u16 nvme_status = nvme_req(req)->status;
+	blk_status_t status = nvme_error_status(nvme_status);
 
 	trace_nvme_complete_rq(req);
 
@@ -280,7 +291,8 @@ void nvme_complete_rq(struct request *req)
 		nvme_req(req)->ctrl->comp_seen = true;
 
 	if (unlikely(status != BLK_STS_OK && nvme_req_needs_retry(req))) {
-		if ((req->cmd_flags & REQ_NVME_MPATH) && nvme_failover_req(req))
+		if (!nvme_status_needs_local_error_handling(nvme_status) &&
+		    (req->cmd_flags & REQ_NVME_MPATH) && nvme_failover_req(req))
 			return;
 
 		if (!blk_queue_dying(req->q)) {
-- 
2.18.0

WARNING: multiple messages have this Message-ID (diff)
From: Mike Snitzer <snitzer@redhat.com>
To: Sagi Grimberg <sagi@grimberg.me>
Cc: linux-nvme@lists.infradead.org,
	Christoph Hellwig <hch@infradead.org>,
	dm-devel@redhat.com, Hannes Reinecke <hare@suse.de>,
	Chao Leng <lengchao@huawei.com>, Keith Busch <kbusch@kernel.org>,
	"Meneghini, John" <John.Meneghini@netapp.com>,
	Ewan Milne <emilne@redhat.com>
Subject: [RESEND PATCH] nvme: explicitly use normal NVMe error handling when appropriate
Date: Thu, 13 Aug 2020 10:48:11 -0400	[thread overview]
Message-ID: <20200813144811.GA5452@redhat.com> (raw)
In-Reply-To: <20200810172209.GA19535@redhat.com>

Commit 764e9332098c0 ("nvme-multipath: do not reset on unknown
status"), among other things, fixed NVME_SC_CMD_INTERRUPTED error
handling by changing multipathing's nvme_failover_req() to short-circuit
path failover and then fallback to NVMe's normal error handling (which
takes care of NVME_SC_CMD_INTERRUPTED).

This detour through native NVMe multipathing code is unwelcome because
it prevents NVMe core from handling NVME_SC_CMD_INTERRUPTED independent
of any multipathing concerns.

Introduce nvme_status_needs_local_error_handling() to prioritize
non-failover retry, when appropriate, in terms of normal NVMe error
handling.  nvme_status_needs_local_error_handling() will naturely evolve
to include handling of any other errors that normal error handling must
be used for.

nvme_failover_req()'s ability to fallback to normal NVMe error handling
has been preserved because it may be useful for future NVME_SC that
nvme_status_needs_local_error_handling() hasn't been trained for yet.

Signed-off-by: Mike Snitzer <snitzer@redhat.com>
---
 drivers/nvme/host/core.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 88cff309d8e4..be749b690af7 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -252,6 +252,16 @@ static inline bool nvme_req_needs_retry(struct request *req)
 	return true;
 }
 
+static inline bool nvme_status_needs_local_error_handling(u16 status)
+{
+	switch (status & 0x7ff) {
+	case NVME_SC_CMD_INTERRUPTED:
+		return true;
+	default:
+		return false;
+	}
+}
+
 static void nvme_retry_req(struct request *req)
 {
 	struct nvme_ns *ns = req->q->queuedata;
@@ -270,7 +280,8 @@ static void nvme_retry_req(struct request *req)
 
 void nvme_complete_rq(struct request *req)
 {
-	blk_status_t status = nvme_error_status(nvme_req(req)->status);
+	u16 nvme_status = nvme_req(req)->status;
+	blk_status_t status = nvme_error_status(nvme_status);
 
 	trace_nvme_complete_rq(req);
 
@@ -280,7 +291,8 @@ void nvme_complete_rq(struct request *req)
 		nvme_req(req)->ctrl->comp_seen = true;
 
 	if (unlikely(status != BLK_STS_OK && nvme_req_needs_retry(req))) {
-		if ((req->cmd_flags & REQ_NVME_MPATH) && nvme_failover_req(req))
+		if (!nvme_status_needs_local_error_handling(nvme_status) &&
+		    (req->cmd_flags & REQ_NVME_MPATH) && nvme_failover_req(req))
 			return;
 
 		if (!blk_queue_dying(req->q)) {
-- 
2.18.0


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

  parent reply	other threads:[~2020-08-13 14:48 UTC|newest]

Thread overview: 80+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-27  5:58 [PATCH] nvme-core: fix io interrupt when work with dm-multipah Chao Leng
2020-07-28 11:19 ` Christoph Hellwig
2020-07-29  2:54   ` Chao Leng
2020-07-29  5:59     ` Christoph Hellwig
2020-07-30  1:49       ` Chao Leng
2020-08-05  6:40         ` Chao Leng
2020-08-05 15:29           ` Keith Busch
2020-08-06  5:52             ` Chao Leng
2020-08-06 14:26               ` Keith Busch
2020-08-06 15:59                 ` Meneghini, John
2020-08-06 16:17                   ` Meneghini, John
2020-08-06 18:40                     ` Mike Snitzer
2020-08-06 19:19                       ` [PATCH] nvme: restore use of blk_path_error() in nvme_complete_rq() Mike Snitzer
2020-08-06 22:42                         ` Meneghini, John
2020-08-07  0:07                           ` Mike Snitzer
2020-08-07  0:07                             ` Mike Snitzer
2020-08-07  1:21                             ` Sagi Grimberg
2020-08-07  1:21                               ` Sagi Grimberg
2020-08-07  4:50                               ` Mike Snitzer
2020-08-07  4:50                                 ` Mike Snitzer
2020-08-07 23:35                                 ` Sagi Grimberg
2020-08-07 23:35                                   ` Sagi Grimberg
2020-08-08 21:08                                   ` Meneghini, John
2020-08-08 21:08                                     ` Meneghini, John
2020-08-08 21:11                                     ` Meneghini, John
2020-08-08 21:11                                       ` Meneghini, John
2020-08-10 14:48                                       ` Mike Snitzer
2020-08-10 14:48                                         ` Mike Snitzer
2020-08-11 12:54                                         ` Meneghini, John
2020-08-11 12:54                                           ` Meneghini, John
2020-08-10  8:10                                     ` Chao Leng
2020-08-10  8:10                                       ` Chao Leng
2020-08-11 12:36                                       ` Meneghini, John
2020-08-11 12:36                                         ` Meneghini, John
2020-08-12  7:51                                         ` Chao Leng
2020-08-12  7:51                                           ` Chao Leng
2020-08-10 14:36                                   ` Mike Snitzer
2020-08-10 14:36                                     ` Mike Snitzer
2020-08-10 17:22                                     ` [PATCH] nvme: explicitly use normal NVMe error handling when appropriate Mike Snitzer
2020-08-10 17:22                                       ` Mike Snitzer
2020-08-11  3:32                                       ` Chao Leng
2020-08-11  3:32                                         ` Chao Leng
2020-08-11  4:20                                         ` Mike Snitzer
2020-08-11  4:20                                           ` Mike Snitzer
2020-08-11  6:17                                           ` Chao Leng
2020-08-11  6:17                                             ` Chao Leng
2020-08-11 14:12                                             ` Mike Snitzer
2020-08-11 14:12                                               ` Mike Snitzer
2020-08-13 14:48                                       ` Mike Snitzer [this message]
2020-08-13 14:48                                         ` [RESEND PATCH] " Mike Snitzer
2020-08-13 15:29                                         ` Meneghini, John
2020-08-13 15:29                                           ` Meneghini, John
2020-08-13 15:43                                           ` Mike Snitzer
2020-08-13 15:43                                             ` Mike Snitzer
2020-08-13 15:59                                             ` Meneghini, John
2020-08-13 15:59                                               ` Meneghini, John
2020-08-13 15:36                                         ` Christoph Hellwig
2020-08-13 15:36                                           ` Christoph Hellwig
2020-08-13 17:47                                           ` Mike Snitzer
2020-08-13 17:47                                             ` Mike Snitzer
2020-08-13 18:43                                             ` Christoph Hellwig
2020-08-13 18:43                                               ` Christoph Hellwig
2020-08-13 19:03                                               ` Mike Snitzer
2020-08-13 19:03                                                 ` Mike Snitzer
2020-08-14  4:26                                               ` Meneghini, John
2020-08-14  4:26                                                 ` Meneghini, John
2020-08-14  6:53                                               ` Sagi Grimberg
2020-08-14  6:53                                                 ` Sagi Grimberg
2020-08-14  6:55                                                 ` Christoph Hellwig
2020-08-14  6:55                                                   ` Christoph Hellwig
2020-08-14  7:02                                                   ` Sagi Grimberg
2020-08-14  7:02                                                     ` Sagi Grimberg
2020-08-14  3:23                                         ` Meneghini, John
2020-08-14  3:23                                           ` Meneghini, John
2020-08-07  0:44                         ` [PATCH] nvme: restore use of blk_path_error() in nvme_complete_rq() Sagi Grimberg
2020-08-10 12:43                         ` Christoph Hellwig
2020-08-10 15:06                           ` Mike Snitzer
2020-08-11  3:45                           ` [PATCH] " Chao Leng
2020-08-07  0:03                   ` [PATCH] nvme-core: fix io interrupt when work with dm-multipah Sagi Grimberg
2020-08-07  2:28                     ` Chao Leng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200813144811.GA5452@redhat.com \
    --to=snitzer@redhat.com \
    --cc=John.Meneghini@netapp.com \
    --cc=dm-devel@redhat.com \
    --cc=emilne@redhat.com \
    --cc=hch@infradead.org \
    --cc=kbusch@kernel.org \
    --cc=lengchao@huawei.com \
    --cc=linux-nvme@lists.infradead.org \
    --cc=sagi@grimberg.me \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.