| Message ID | 20180104224623.8944-2-keith.busch@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jan 04 2018 at 5:46pm -0500, Keith Busch <keith.busch@intel.com> wrote: > This adds more NVMe status code translations to blk_status_t values, > and captures all the current status codes NVMe multipath uses. > > Signed-off-by: Keith Busch <keith.busch@intel.com> Acked-by: Mike Snitzer <snitzer@redhat.com>
On 01/04/2018 11:46 PM, Keith Busch wrote: > This adds more NVMe status code translations to blk_status_t values, > and captures all the current status codes NVMe multipath uses. > > Signed-off-by: Keith Busch <keith.busch@intel.com> > --- > drivers/nvme/host/core.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c > index 2a69d735efbc..f1cf1f6c5b28 100644 > --- a/drivers/nvme/host/core.c > +++ b/drivers/nvme/host/core.c > @@ -156,14 +156,20 @@ static blk_status_t nvme_error_status(struct request *req) > case NVME_SC_SUCCESS: > return BLK_STS_OK; > case NVME_SC_CAP_EXCEEDED: > + case NVME_SC_LBA_RANGE: > return BLK_STS_NOSPC; > + case NVME_SC_BAD_ATTRIBUTES: > case NVME_SC_ONCS_NOT_SUPPORTED: > + case NVME_SC_INVALID_OPCODE: > + case NVME_SC_INVALID_FIELD: > + case NVME_SC_INVALID_NS: > return BLK_STS_NOTSUPP; > case NVME_SC_WRITE_FAULT: > case NVME_SC_READ_ERROR: > case NVME_SC_UNWRITTEN_BLOCK: > case NVME_SC_ACCESS_DENIED: > case NVME_SC_READ_ONLY: > + case NVME_SC_COMPARE_FAILED: > return BLK_STS_MEDIUM; > case NVME_SC_GUARD_CHECK: > case NVME_SC_APPTAG_CHECK: > Reviewed-by: Hannes Reinecke <hare@suse.com> Cheers, Hannes
On Thu, Jan 04, 2018 at 03:46:19PM -0700, Keith Busch wrote: > This adds more NVMe status code translations to blk_status_t values, > and captures all the current status codes NVMe multipath uses. > > Signed-off-by: Keith Busch <keith.busch@intel.com> > --- > drivers/nvme/host/core.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c > index 2a69d735efbc..f1cf1f6c5b28 100644 > --- a/drivers/nvme/host/core.c > +++ b/drivers/nvme/host/core.c > @@ -156,14 +156,20 @@ static blk_status_t nvme_error_status(struct request *req) > case NVME_SC_SUCCESS: > return BLK_STS_OK; > case NVME_SC_CAP_EXCEEDED: > + case NVME_SC_LBA_RANGE: > return BLK_STS_NOSPC; lba range isn't really enospc. It is returned when the lba in the command is outside the logical size of the namespace.
On 01/08/2018 10:55 AM, Christoph Hellwig wrote: > On Thu, Jan 04, 2018 at 03:46:19PM -0700, Keith Busch wrote: >> This adds more NVMe status code translations to blk_status_t values, >> and captures all the current status codes NVMe multipath uses. >> >> Signed-off-by: Keith Busch <keith.busch@intel.com> >> --- >> drivers/nvme/host/core.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c >> index 2a69d735efbc..f1cf1f6c5b28 100644 >> --- a/drivers/nvme/host/core.c >> +++ b/drivers/nvme/host/core.c >> @@ -156,14 +156,20 @@ static blk_status_t nvme_error_status(struct request *req) >> case NVME_SC_SUCCESS: >> return BLK_STS_OK; >> case NVME_SC_CAP_EXCEEDED: >> + case NVME_SC_LBA_RANGE: >> return BLK_STS_NOSPC; > > lba range isn't really enospc. It is returned when the lba in > the command is outside the logical size of the namespace. > Isn't that distinction pretty academic? The entire block-to-POSIX error mapping is pretty much ad-hoc anyway... Cheers, Hannes
On Mon, Jan 08, 2018 at 11:09:03AM +0100, Hannes Reinecke wrote: > >> case NVME_SC_SUCCESS: > >> return BLK_STS_OK; > >> case NVME_SC_CAP_EXCEEDED: > >> + case NVME_SC_LBA_RANGE: > >> return BLK_STS_NOSPC; > > > > lba range isn't really enospc. It is returned when the lba in > > the command is outside the logical size of the namespace. > > > Isn't that distinction pretty academic? > The entire block-to-POSIX error mapping is pretty much ad-hoc anyway... Yes, BLK_STS_NOSPC matters. And the fix is pretty trivial, so there is no point in arguing.
On Mon, Jan 08 2018 at 5:19am -0500, Christoph Hellwig <hch@lst.de> wrote: > On Mon, Jan 08, 2018 at 11:09:03AM +0100, Hannes Reinecke wrote: > > >> case NVME_SC_SUCCESS: > > >> return BLK_STS_OK; > > >> case NVME_SC_CAP_EXCEEDED: > > >> + case NVME_SC_LBA_RANGE: > > >> return BLK_STS_NOSPC; > > > > > > lba range isn't really enospc. It is returned when the lba in > > > the command is outside the logical size of the namespace. > > > > > Isn't that distinction pretty academic? > > The entire block-to-POSIX error mapping is pretty much ad-hoc anyway... > > Yes, BLK_STS_NOSPC matters. And the fix is pretty trivial, so there is > no point in arguing. No argument needed. Definitely needs fixing. Too many upper layers consider BLK_STS_NOSPC retryable (XFS, ext4, dm-thinp, etc). Which NVME_SC_LBA_RANGE absolutely isn't. When I backfilled NVME_SC_LBA_RANGE handling I categorized it as BLK_STS_TARGET. Do you have a better suggestion for how NVME_SC_LBA_RANGE should be categorized? Mike
On Mon, Jan 08, 2018 at 10:29:33AM -0500, Mike Snitzer wrote: > No argument needed. Definitely needs fixing. Too many upper layers > consider BLK_STS_NOSPC retryable (XFS, ext4, dm-thinp, etc). Which > NVME_SC_LBA_RANGE absolutely isn't. > > When I backfilled NVME_SC_LBA_RANGE handling I categorized it as > BLK_STS_TARGET. Do you have a better suggestion for how > NVME_SC_LBA_RANGE should be categorized? It's basically a kernel bug as it tries to access lbas that do not exist. BLK_STS_TARGET should be fine.
On Mon, Jan 08, 2018 at 04:34:36PM +0100, Christoph Hellwig wrote: > It's basically a kernel bug as it tries to access lbas that do not > exist. BLK_STS_TARGET should be fine. Okay, I'll fix this and address your other comments, and resend. Thanks for the feedback.
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index 2a69d735efbc..f1cf1f6c5b28 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -156,14 +156,20 @@ static blk_status_t nvme_error_status(struct request *req) case NVME_SC_SUCCESS: return BLK_STS_OK; case NVME_SC_CAP_EXCEEDED: + case NVME_SC_LBA_RANGE: return BLK_STS_NOSPC; + case NVME_SC_BAD_ATTRIBUTES: case NVME_SC_ONCS_NOT_SUPPORTED: + case NVME_SC_INVALID_OPCODE: + case NVME_SC_INVALID_FIELD: + case NVME_SC_INVALID_NS: return BLK_STS_NOTSUPP; case NVME_SC_WRITE_FAULT: case NVME_SC_READ_ERROR: case NVME_SC_UNWRITTEN_BLOCK: case NVME_SC_ACCESS_DENIED: case NVME_SC_READ_ONLY: + case NVME_SC_COMPARE_FAILED: return BLK_STS_MEDIUM; case NVME_SC_GUARD_CHECK: case NVME_SC_APPTAG_CHECK:
This adds more NVMe status code translations to blk_status_t values, and captures all the current status codes NVMe multipath uses. Signed-off-by: Keith Busch <keith.busch@intel.com> --- drivers/nvme/host/core.c | 6 ++++++ 1 file changed, 6 insertions(+)