[OS-BUILD PATCH 0/4] nvme: update error handling and ana_state to work well with device-mapper-multipath
12:22 p.m.
From: Mike Snitzer on gitlab.com
Merge Request: https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1024
BZ: 1948690
Upstream Status: RHEL-only
Forward-port RHEL8 NVMe changes to RHEL9. These changes were rejected by
Christoph Hellwig (hch is one of upstream's 4 NVMe maintainers). The
other (3) upstream NVMe maintainers don't have a problem with these
changes, but Christoph is hostile toward any changes that make NVMe work
better with dm-multipath (because he wants everyone to use NVMe's native
multipathing, doesn't care that others have reasons why they want to use
dm-multipath).
So unfortunately we need to just keep carrying these changes in RHEL.
Can revisit getting these changes to land upstream in time for RHEL10,
but I don't care to engage upstream (specifically hch) on this topic at
this point in time.
Thankfully these NVMe changes are _not_ intrusive, have proven
stable/useful in RHEL8 and were easily forward-ported from RHEL8 to
RHEL9.
Signed-off-by: Mike Snitzer <snitzer(a)redhat.com>
12:22 p.m.
New subject: [OS-BUILD PATCH 1/4] nvme: Return BLK_STS_TARGET if the DNR bit is
set
From: Mike Snitzer <snitzer(a)redhat.com>
nvme: Return BLK_STS_TARGET if the DNR bit is set
BZ: 1948690
Upstream Status: RHEL-only
Signed-off-by: Mike Snitzer <snitzer(a)redhat.com>
rhel-8.git commit ef4ab90c12db5e0e50800ec323736b95be7a6ff5
Author: Mike Snitzer <snitzer(a)redhat.com>
Date: Tue Aug 25 21:52:45 2020 -0400
[nvme] nvme: Return BLK_STS_TARGET if the DNR bit is set
Message-id: <20200825215248.2291-8-snitzer(a)redhat.com>
Patchwork-id: 325178
Patchwork-instance: patchwork
O-Subject: [RHEL8.3 PATCH 07/10] nvme: Return BLK_STS_TARGET if the DNR bit is set
Bugzilla: 1843515
RH-Acked-by: David Milburn <dmilburn(a)redhat.com>
RH-Acked-by: Gopal Tiwari <gtiwari(a)redhat.com>
RH-Acked-by: Ewan Milne <emilne(a)redhat.com>
BZ: 1843515
Upstream Status: RHEL-only
If the DNR bit is set we should not retry the command, even if
the standard status evaluation indicates so.
SUSE is carrying this patch in their kernel:
https://lwn.net/Articles/800370/
Based on patch posted for upstream inclusion but rejected:
v1: https://lore.kernel.org/linux-nvme/20190806111036.113233-1-hare@suse.de/
v2: https://lore.kernel.org/linux-nvme/20190807071208.101882-1-hare@suse.de/
v2-keith:
https://lore.kernel.org/linux-nvme/20190807144725.GB25621@localhost.local...
v3: https://lore.kernel.org/linux-nvme/20190812075147.79598-1-hare@suse.de/
v3-keith:
https://lore.kernel.org/linux-nvme/20190813141510.GB32686@localhost.local...
This commit's change is basically "v3-keith".
Signed-off-by: Mike Snitzer <snitzer(a)redhat.com>
Signed-off-by: Frantisek Hrbata <fhrbata(a)redhat.com>
diff a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -237,6 +237,9 @@ static void nvme_delete_ctrl_sync(struct nvme_ctrl *ctrl)
static blk_status_t nvme_error_status(u16 status)
{
+ if (unlikely(status & NVME_SC_DNR))
+ return BLK_STS_TARGET;
+
switch (status & 0x7ff) {
case NVME_SC_SUCCESS:
return BLK_STS_OK;
--
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1024
12:22 p.m.
New subject: [OS-BUILD PATCH 2/4] nvme: update failover handling to work with
REQ_FAILFAST_TRANSPORT
From: Mike Snitzer <snitzer(a)redhat.com>
nvme: update failover handling to work with REQ_FAILFAST_TRANSPORT
BZ: 1948690
Upstream Status: RHEL-only
Signed-off-by: Mike Snitzer <snitzer(a)redhat.com>
rhel-8.git commit f8fb6ea1226e2abc525c88da13b346118d548eea
Author: Mike Snitzer <snitzer(a)redhat.com>
Date: Tue Aug 25 21:52:46 2020 -0400
[nvme] nvme: update failover handling to work with REQ_FAILFAST_TRANSPORT
Message-id: <20200825215248.2291-9-snitzer(a)redhat.com>
Patchwork-id: 325177
Patchwork-instance: patchwork
O-Subject: [RHEL8.3 PATCH 08/10] nvme: update failover handling to work with
REQ_FAILFAST_TRANSPORT
Bugzilla: 1843515
RH-Acked-by: David Milburn <dmilburn(a)redhat.com>
RH-Acked-by: Gopal Tiwari <gtiwari(a)redhat.com>
RH-Acked-by: Ewan Milne <emilne(a)redhat.com>
BZ: 1843515
Upstream Status: RHEL-only
If REQ_FAILFAST_TRANSPORT is set it means the driver should not retry
IO that completed with transport errors. REQ_FAILFAST_TRANSPORT is
set by multipathing software (e.g. dm-multipath) before it issues IO.
Update NVMe to prepare for failover of requests marked with either
REQ_NVME_MPATH or REQ_FAILFAST_TRANSPORT. This allows such requests
to be given a disposition of FAILOVER.
Introduce nvme_end_req_with_failover() for use in nvme_complete_rq()
if REQ_NVME_MPATH isn't set. nvme_end_req_with_failover() ensures
request is completed with a retryable IO error when appropriate.
__nvme_end_req() was factored out for use by both nvme_end_req() and
nvme_end_req_with_failover().
Signed-off-by: Mike Snitzer <snitzer(a)redhat.com>
Signed-off-by: Frantisek Hrbata <fhrbata(a)redhat.com>
diff a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -311,7 +311,7 @@ static inline enum nvme_disposition nvme_decide_disposition(struct
request *req)
nvme_req(req)->retries >= nvme_max_retries)
return COMPLETE;
- if (req->cmd_flags & REQ_NVME_MPATH) {
+ if (req->cmd_flags & (REQ_NVME_MPATH | REQ_FAILFAST_TRANSPORT)) {
if (nvme_is_path_error(nvme_req(req)->status) ||
blk_queue_dying(req->q))
return FAILOVER;
@@ -323,10 +323,8 @@ static inline enum nvme_disposition nvme_decide_disposition(struct
request *req)
return RETRY;
}
-static inline void nvme_end_req(struct request *req)
+static inline void __nvme_end_req(struct request *req, blk_status_t status)
{
- blk_status_t status = nvme_error_status(nvme_req(req)->status);
-
if (IS_ENABLED(CONFIG_BLK_DEV_ZONED) &&
req_op(req) == REQ_OP_ZONE_APPEND)
req->__sector = nvme_lba_to_sect(req->q->queuedata,
@@ -336,6 +334,28 @@ static inline void nvme_end_req(struct request *req)
blk_mq_end_request(req, status);
}
+static inline void nvme_end_req(struct request *req)
+{
+ __nvme_end_req(req, nvme_error_status(nvme_req(req)->status));
+}
+
+static inline void nvme_end_req_with_failover(struct request *req)
+{
+ u16 nvme_status = nvme_req(req)->status;
+ blk_status_t status = nvme_error_status(nvme_status);
+
+ if (unlikely(nvme_status & NVME_SC_DNR))
+ goto out;
+
+ if (!blk_path_error(status)) {
+ pr_debug("Request meant for failover but blk_status_t (errno=%d) was not
retryable.\n",
+ blk_status_to_errno(status));
+ status = BLK_STS_IOERR;
+ }
+out:
+ __nvme_end_req(req, status);
+}
+
void nvme_complete_rq(struct request *req)
{
trace_nvme_complete_rq(req);
@@ -352,7 +372,10 @@ void nvme_complete_rq(struct request *req)
nvme_retry_req(req);
return;
case FAILOVER:
- nvme_failover_req(req);
+ if (req->cmd_flags & REQ_NVME_MPATH)
+ nvme_failover_req(req);
+ else
+ nvme_end_req_with_failover(req);
return;
}
}
--
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1024
12:22 p.m.
New subject: [OS-BUILD PATCH 3/4] nvme: decouple basic ANA log page re-read
support from
native multipathing
From: Mike Snitzer <snitzer(a)redhat.com>
nvme: decouple basic ANA log page re-read support from native multipathing
BZ: 1948690
Upstream Status: RHEL-only
Signed-off-by: Mike Snitzer <snitzer(a)redhat.com>
rhel-8.git commit b904f4b8e0f90613bf1b2b9d9ccad3c015741daf
Author: Mike Snitzer <snitzer(a)redhat.com>
Date: Tue Aug 25 21:52:47 2020 -0400
[nvme] nvme: decouple basic ANA log page re-read support from native multipathing
Message-id: <20200825215248.2291-10-snitzer(a)redhat.com>
Patchwork-id: 325179
Patchwork-instance: patchwork
O-Subject: [RHEL8.3 PATCH 09/10] nvme: decouple basic ANA log page re-read support
from native multipathing
Bugzilla: 1843515
RH-Acked-by: David Milburn <dmilburn(a)redhat.com>
RH-Acked-by: Gopal Tiwari <gtiwari(a)redhat.com>
RH-Acked-by: Ewan Milne <emilne(a)redhat.com>
BZ: 1843515
Upstream Status: RHEL-only
Whether or not ANA is present is a choice of the target implementation;
the host (and whether it supports multipathing) has _zero_ influence on
this. If the target declares a path as 'inaccessible' the path _is_
inaccessible to the host. As such, ANA support should be functional
even if native multipathing is not.
Introduce ability to always re-read ANA log page as required due to ANA
error and make current ANA state available via sysfs -- even if native
multipathing is disabled on the host (e.g. nvme_core.multipath=N).
This affords userspace access to the current ANA state independent of
which layer might be doing multipathing. It also allows multipath-tools
to rely on the NVMe driver for ANA support while dm-multipath takes care
of multipathing.
And as always, if embedded NVMe users do not want any performance
overhead associated with ANA or native NVMe multipathing they can
disable CONFIG_NVME_MULTIPATH.
Signed-off-by: Mike Snitzer <snitzer(a)redhat.com>
Signed-off-by: Frantisek Hrbata <fhrbata(a)redhat.com>
diff a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -347,6 +347,8 @@ static inline void nvme_end_req_with_failover(struct request *req)
if (unlikely(nvme_status & NVME_SC_DNR))
goto out;
+ nvme_update_ana(req);
+
if (!blk_path_error(status)) {
pr_debug("Request meant for failover but blk_status_t (errno=%d) was not
retryable.\n",
blk_status_to_errno(status));
diff a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -65,10 +65,25 @@ void nvme_set_disk_name(char *disk_name, struct nvme_ns *ns,
}
}
+static inline void __nvme_update_ana(struct nvme_ns *ns)
+{
+ if (!ns->ctrl->ana_log_buf)
+ return;
+
+ set_bit(NVME_NS_ANA_PENDING, &ns->flags);
+ queue_work(nvme_wq, &ns->ctrl->ana_work);
+}
+
+
+void nvme_update_ana(struct request *req)
+{
+ if (nvme_is_ana_error(nvme_req(req)->status))
+ __nvme_update_ana(req->q->queuedata);
+}
+
void nvme_failover_req(struct request *req)
{
struct nvme_ns *ns = req->q->queuedata;
- u16 status = nvme_req(req)->status & 0x7ff;
unsigned long flags;
nvme_mpath_clear_current_path(ns);
@@ -78,10 +93,8 @@ void nvme_failover_req(struct request *req)
* ready to serve this namespace. Kick of a re-read of the ANA
* information page, and just try any other available path for now.
*/
- if (nvme_is_ana_error(status) && ns->ctrl->ana_log_buf) {
- set_bit(NVME_NS_ANA_PENDING, &ns->flags);
- queue_work(nvme_wq, &ns->ctrl->ana_work);
- }
+ if (nvme_is_ana_error(nvme_req(req)->status))
+ __nvme_update_ana(ns);
spin_lock_irqsave(&ns->head->requeue_lock, flags);
blk_steal_bios(&ns->head->requeue_list, req);
diff a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -664,6 +664,7 @@ void nvme_mpath_start_freeze(struct nvme_subsystem *subsys);
void nvme_set_disk_name(char *disk_name, struct nvme_ns *ns,
struct nvme_ctrl *ctrl, int *flags);
void nvme_failover_req(struct request *req);
+void nvme_update_ana(struct request *req);
void nvme_kick_requeue_lists(struct nvme_ctrl *ctrl);
int nvme_mpath_alloc_disk(struct nvme_ctrl *ctrl,struct nvme_ns_head *head);
void nvme_mpath_add_disk(struct nvme_ns *ns, struct nvme_id_ns *id);
@@ -714,6 +715,9 @@ static inline void nvme_set_disk_name(char *disk_name, struct nvme_ns
*ns,
static inline void nvme_failover_req(struct request *req)
{
}
+static inline void nvme_update_ana(struct request *req)
+{
+}
static inline void nvme_kick_requeue_lists(struct nvme_ctrl *ctrl)
{
}
--
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1024
12:22 p.m.
New subject: [OS-BUILD PATCH 4/4] nvme: allow retry for requests with
REQ_FAILFAST_TRANSPORT set
From: Mike Snitzer <snitzer(a)redhat.com>
nvme: allow retry for requests with REQ_FAILFAST_TRANSPORT set
BZ: 1948690
Upstream Status: RHEL-only
Signed-off-by: Mike Snitzer <snitzer(a)redhat.com>
rhel-8.git commit 7dadadb072515f243868e6fe2f7e9c97fd3516c9
Author: Mike Snitzer <snitzer(a)redhat.com>
Date: Tue Aug 25 21:52:48 2020 -0400
[nvme] nvme: allow retry for requests with REQ_FAILFAST_TRANSPORT set
Message-id: <20200825215248.2291-11-snitzer(a)redhat.com>
Patchwork-id: 325180
Patchwork-instance: patchwork
O-Subject: [RHEL8.3 PATCH 10/10] nvme: allow retry for requests with
REQ_FAILFAST_TRANSPORT set
Bugzilla: 1843515
RH-Acked-by: David Milburn <dmilburn(a)redhat.com>
RH-Acked-by: Gopal Tiwari <gtiwari(a)redhat.com>
RH-Acked-by: Ewan Milne <emilne(a)redhat.com>
BZ: 1843515
Upstream Status: RHEL-only
Based on patch that was proposed upstream but ultimately rejected, see:
https://www.spinics.net/lists/linux-block/msg57490.html
I'd have made this change even if this wasn't already posted obviously,
but I figured I'd give proper attribution due to their public post with
the same code change.
Author: Chao Leng <lengchao(a)huawei.com>
Date: Wed Aug 12 16:18:55 2020 +0800
nvme: allow retry for requests with REQ_FAILFAST_TRANSPORT set
REQ_FAILFAST_TRANSPORT may be designed for SCSI, because SCSI protocol
does not define the local retry mechanism. SCSI implements a fuzzy
local retry mechanism, so REQ_FAILFAST_TRANSPORT is needed to allow
higher-level multipathing software to perform failover/retry.
NVMe is different with SCSI about this. It defines a local retry
mechanism and path error codes, so NVMe should retry local for non
path error. If path related error, whether to retry and how to retry
is still determined by higher-level multipathing's failover.
Unlike SCSI, NVMe shouldn't prevent retry if REQ_FAILFAST_TRANSPORT
because NVMe's local retry is needed -- as is NVMe specific logic to
categorize whether an error is path related.
In this way, the mechanism of NVMe multipath or other multipath are
now equivalent. The mechanism is: non path related error will be
retry local, path related error is handled by multipath.
Signed-off-by: Chao Leng <lengchao(a)huawei.com>
[snitzer: edited header for grammar and to make clearer]
Signed-off-by: Mike Snitzer <snitzer(a)redhat.com>
Signed-off-by: Mike Snitzer <snitzer(a)redhat.com>
Signed-off-by: Frantisek Hrbata <fhrbata(a)redhat.com>
diff a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -306,7 +306,14 @@ static inline enum nvme_disposition nvme_decide_disposition(struct
request *req)
if (likely(nvme_req(req)->status == 0))
return COMPLETE;
- if (blk_noretry_request(req) ||
+ /*
+ * REQ_FAILFAST_TRANSPORT is set by upper layer software that
+ * handles multipathing. Unlike SCSI, NVMe's error handling was
+ * specifically designed to handle local retry for non-path errors.
+ * As such, allow NVMe's local retry mechanism to be used for
+ * requests marked with REQ_FAILFAST_TRANSPORT.
+ */
+ if ((req->cmd_flags & (REQ_FAILFAST_DEV | REQ_FAILFAST_DRIVER)) ||
(nvme_req(req)->status & NVME_SC_DNR) ||
nvme_req(req)->retries >= nvme_max_retries)
return COMPLETE;
--
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1024
1102
days inactive
1102
days old
kernel@lists.fedoraproject.org
4 comments
1 participants
participants (1)
-
Mike Snitzer (via Email Bridge)