Re: [dm-devel] [PATCH 1/5] nvme: Add more command status translation
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. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH 1/5] nvme: Add more command status translation
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. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH 1/5] nvme: Add more command status translation
On Mon, Jan 08 2018 at 5:19am -0500, Christoph Hellwigwrote: > 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 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH 1/5] nvme: Add more command status translation
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>> --- >> 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 -- Dr. Hannes ReineckezSeries & Storage h...@suse.com +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg) -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH 1/5] nvme: Add more command status translation
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. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH 1/5] nvme: Add more command status translation
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> --- > 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. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH 1/5] nvme: Add more command status translation
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> --- > 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 Cheers, Hannes -- Dr. Hannes ReineckezSeries & Storage h...@suse.com +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg) -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH 1/5] nvme: Add more command status translation
On Thu, Jan 04 2018 at 5:46pm -0500, Keith Buschwrote: > 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 Acked-by: Mike Snitzer -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel