From: Mike Snitzer <snitzer@redhat.com>
To: Christoph Hellwig <hch@lst.de>, Keith Busch <keith.busch@intel.com>
Cc: axboe@kernel.dk, emilne@redhat.com, james.smart@broadcom.com,
hare@suse.de, Bart.VanAssche@wdc.com,
linux-block@vger.kernel.org, linux-nvme@lists.infradead.org,
dm-devel@redhat.com
Subject: Re: [for-4.16 PATCH v2 0/5] block, nvme, dm: allow DM multipath to use NVMe's error handler
Date: Thu, 4 Jan 2018 11:26:03 -0500 [thread overview]
Message-ID: <20180104162603.GA20081@redhat.com> (raw)
In-Reply-To: <20180104102631.GA4864@lst.de>
On Thu, Jan 04 2018 at 5:26am -0500,
Christoph Hellwig <hch@lst.de> wrote:
> On Tue, Jan 02, 2018 at 04:29:43PM -0700, Keith Busch wrote:
> > Instead of hiding NVMe path related errors, the NVMe driver needs to
> > code an appropriate generic block status from an NVMe status.
> >
> > We already do this translation whether or not CONFIG_NVME_MULTIPATHING is
> > set, so I think it's silly NVMe native multipathing has a second status
> > decoder. This just doubles the work if we need to handle any new NVMe
> > status codes in the future.
> >
> > I have a counter-proposal below that unifies NVMe-to-block status
> > translations, and combines common code for determining if an error is a
> > path failure. This should work for both NVMe and DM, and DM won't need
> > NVMe specifics.
> >
> > I can split this into a series if there's indication this is ok and
> > satisfies the need.
>
> You'll need to update nvme_error_status to account for all errors
> handled in nvme_req_needs_failover
These weren't accounted for (I may have mis-categorized them in this
patch, so please feel free to correct):
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 25ef349fd4e4..835a9c60200e 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -165,10 +165,18 @@ static blk_status_t nvme_error_status(struct request *req)
case NVME_SC_ACCESS_DENIED:
case NVME_SC_READ_ONLY:
return BLK_STS_MEDIUM;
+ case NVME_SC_INVALID_OPCODE:
+ case NVME_SC_INVALID_FIELD:
+ case NVME_SC_INVALID_NS:
+ case NVME_SC_LBA_RANGE:
+ case NVME_SC_CAP_EXCEEDED:
+ case NVME_SC_BAD_ATTRIBUTES:
+ return BLK_STS_TARGET;
case NVME_SC_GUARD_CHECK:
case NVME_SC_APPTAG_CHECK:
case NVME_SC_REFTAG_CHECK:
case NVME_SC_INVALID_PI:
+ case NVME_SC_COMPARE_FAILED:
return BLK_STS_PROTECTION;
case NVME_SC_RESERVATION_CONFLICT:
return BLK_STS_NEXUS;
And of those, these aren't currently used:
NVME_SC_LBA_RANGE
NVME_SC_CAP_EXCEEDED
NVME_SC_BAD_ATTRIBUTES
NVME_SC_COMPARE_FAILED
At least not that I can see.
WARNING: multiple messages have this Message-ID (diff)
From: snitzer@redhat.com (Mike Snitzer)
Subject: [for-4.16 PATCH v2 0/5] block, nvme, dm: allow DM multipath to use NVMe's error handler
Date: Thu, 4 Jan 2018 11:26:03 -0500 [thread overview]
Message-ID: <20180104162603.GA20081@redhat.com> (raw)
In-Reply-To: <20180104102631.GA4864@lst.de>
On Thu, Jan 04 2018 at 5:26am -0500,
Christoph Hellwig <hch@lst.de> wrote:
> On Tue, Jan 02, 2018@04:29:43PM -0700, Keith Busch wrote:
> > Instead of hiding NVMe path related errors, the NVMe driver needs to
> > code an appropriate generic block status from an NVMe status.
> >
> > We already do this translation whether or not CONFIG_NVME_MULTIPATHING is
> > set, so I think it's silly NVMe native multipathing has a second status
> > decoder. This just doubles the work if we need to handle any new NVMe
> > status codes in the future.
> >
> > I have a counter-proposal below that unifies NVMe-to-block status
> > translations, and combines common code for determining if an error is a
> > path failure. This should work for both NVMe and DM, and DM won't need
> > NVMe specifics.
> >
> > I can split this into a series if there's indication this is ok and
> > satisfies the need.
>
> You'll need to update nvme_error_status to account for all errors
> handled in nvme_req_needs_failover
These weren't accounted for (I may have mis-categorized them in this
patch, so please feel free to correct):
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 25ef349fd4e4..835a9c60200e 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -165,10 +165,18 @@ static blk_status_t nvme_error_status(struct request *req)
case NVME_SC_ACCESS_DENIED:
case NVME_SC_READ_ONLY:
return BLK_STS_MEDIUM;
+ case NVME_SC_INVALID_OPCODE:
+ case NVME_SC_INVALID_FIELD:
+ case NVME_SC_INVALID_NS:
+ case NVME_SC_LBA_RANGE:
+ case NVME_SC_CAP_EXCEEDED:
+ case NVME_SC_BAD_ATTRIBUTES:
+ return BLK_STS_TARGET;
case NVME_SC_GUARD_CHECK:
case NVME_SC_APPTAG_CHECK:
case NVME_SC_REFTAG_CHECK:
case NVME_SC_INVALID_PI:
+ case NVME_SC_COMPARE_FAILED:
return BLK_STS_PROTECTION;
case NVME_SC_RESERVATION_CONFLICT:
return BLK_STS_NEXUS;
And of those, these aren't currently used:
NVME_SC_LBA_RANGE
NVME_SC_CAP_EXCEEDED
NVME_SC_BAD_ATTRIBUTES
NVME_SC_COMPARE_FAILED
At least not that I can see.
next prev parent reply other threads:[~2018-01-04 16:26 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-27 3:22 [for-4.16 PATCH v2 0/5] block, nvme, dm: allow DM multipath to use NVMe's error handler Mike Snitzer
2017-12-27 3:22 ` Mike Snitzer
2017-12-27 3:22 ` [for-4.16 PATCH v2 1/5] block: establish request failover callback Mike Snitzer
2017-12-27 3:22 ` Mike Snitzer
2017-12-29 10:10 ` Christoph Hellwig
2017-12-29 10:10 ` Christoph Hellwig
2017-12-29 20:19 ` Mike Snitzer
2017-12-29 20:19 ` Mike Snitzer
2018-01-04 10:28 ` Christoph Hellwig
2018-01-04 10:28 ` Christoph Hellwig
2018-01-04 14:42 ` Mike Snitzer
2018-01-04 14:42 ` Mike Snitzer
2017-12-27 3:22 ` [for-4.16 PATCH v2 2/5] nvme: use request_queue's failover_rq_fn callback for multipath failover Mike Snitzer
2017-12-27 3:22 ` Mike Snitzer
2017-12-29 10:11 ` Christoph Hellwig
2017-12-29 10:11 ` Christoph Hellwig
2017-12-29 20:22 ` Mike Snitzer
2017-12-29 20:22 ` Mike Snitzer
2017-12-27 3:22 ` [for-4.16 PATCH v2 3/5] nvme: move nvme_req_needs_failover() from multipath to core Mike Snitzer
2017-12-27 3:22 ` Mike Snitzer
2017-12-27 3:22 ` [for-4.16 PATCH v2 4/5] dm mpath: use NVMe error handling to know when an error is retryable Mike Snitzer
2017-12-27 3:22 ` Mike Snitzer
2017-12-27 3:22 ` [for-4.16 PATCH v2 5/5] dm mpath: skip calls to end_io_bio if using NVMe bio-based and round-robin Mike Snitzer
2017-12-27 3:22 ` Mike Snitzer
2018-01-02 23:29 ` [for-4.16 PATCH v2 0/5] block, nvme, dm: allow DM multipath to use NVMe's error handler Keith Busch
2018-01-02 23:29 ` Keith Busch
2018-01-03 0:24 ` Mike Snitzer
2018-01-03 0:24 ` Mike Snitzer
2018-01-04 10:26 ` Christoph Hellwig
2018-01-04 10:26 ` Christoph Hellwig
2018-01-04 14:08 ` Mike Snitzer
2018-01-04 14:08 ` Mike Snitzer
2018-01-04 16:26 ` Mike Snitzer [this message]
2018-01-04 16:26 ` Mike Snitzer
2018-01-08 6:52 ` Hannes Reinecke
2018-01-08 6:52 ` Hannes Reinecke
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=20180104162603.GA20081@redhat.com \
--to=snitzer@redhat.com \
--cc=Bart.VanAssche@wdc.com \
--cc=axboe@kernel.dk \
--cc=dm-devel@redhat.com \
--cc=emilne@redhat.com \
--cc=hare@suse.de \
--cc=hch@lst.de \
--cc=james.smart@broadcom.com \
--cc=keith.busch@intel.com \
--cc=linux-block@vger.kernel.org \
--cc=linux-nvme@lists.infradead.org \
/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.