* [PATCH 1/5] nvme: Add more command status translation
2018-01-04 22:46 [PATCH 0/5] Failover criteria unification Keith Busch
@ 2018-01-04 22:46 ` Keith Busch
2018-01-04 23:41 ` Mike Snitzer
` (2 more replies)
2018-01-04 22:46 ` [PATCH 2/5] nvme/multipath: Consult blk_status_t for failover Keith Busch
` (4 subsequent siblings)
5 siblings, 3 replies; 30+ messages in thread
From: Keith Busch @ 2018-01-04 22:46 UTC (permalink / raw)
To: Linux Block, Linux NVMe, Device Mapper, Christoph Hellwig,
Mike Snitzer, Jens Axboe
Cc: Bart VanAssche, James Smart, Martin K . Petersen, Hannes Reinecke,
Sagi Grimberg, Keith Busch
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:
--
2.13.6
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH 1/5] nvme: Add more command status translation
2018-01-04 22:46 ` [PATCH 1/5] nvme: Add more command status translation Keith Busch
@ 2018-01-04 23:41 ` Mike Snitzer
2018-01-08 8:39 ` Hannes Reinecke
2018-01-08 9:55 ` Christoph Hellwig
2 siblings, 0 replies; 30+ messages in thread
From: Mike Snitzer @ 2018-01-04 23:41 UTC (permalink / raw)
To: Keith Busch
Cc: Linux Block, Linux NVMe, Device Mapper, Christoph Hellwig,
Jens Axboe, Bart VanAssche, James Smart, Martin K . Petersen,
Hannes Reinecke, Sagi Grimberg
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>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 1/5] nvme: Add more command status translation
2018-01-04 22:46 ` [PATCH 1/5] nvme: Add more command status translation Keith Busch
2018-01-04 23:41 ` Mike Snitzer
@ 2018-01-08 8:39 ` Hannes Reinecke
2018-01-08 9:55 ` Christoph Hellwig
2 siblings, 0 replies; 30+ messages in thread
From: Hannes Reinecke @ 2018-01-08 8:39 UTC (permalink / raw)
To: Keith Busch, Linux Block, Linux NVMe, Device Mapper,
Christoph Hellwig, Mike Snitzer, Jens Axboe
Cc: Bart VanAssche, James Smart, Martin K . Petersen, Sagi Grimberg
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
--
Dr. Hannes Reinecke zSeries & Storage
hare@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)
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 1/5] nvme: Add more command status translation
2018-01-04 22:46 ` [PATCH 1/5] nvme: Add more command status translation Keith Busch
2018-01-04 23:41 ` Mike Snitzer
2018-01-08 8:39 ` Hannes Reinecke
@ 2018-01-08 9:55 ` Christoph Hellwig
2018-01-08 10:09 ` Hannes Reinecke
2 siblings, 1 reply; 30+ messages in thread
From: Christoph Hellwig @ 2018-01-08 9:55 UTC (permalink / raw)
To: Keith Busch
Cc: Linux Block, Linux NVMe, Device Mapper, Christoph Hellwig,
Mike Snitzer, Jens Axboe, Bart VanAssche, James Smart,
Martin K . Petersen, Hannes Reinecke, Sagi Grimberg
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.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 1/5] nvme: Add more command status translation
2018-01-08 9:55 ` Christoph Hellwig
@ 2018-01-08 10:09 ` Hannes Reinecke
2018-01-08 10:19 ` Christoph Hellwig
0 siblings, 1 reply; 30+ messages in thread
From: Hannes Reinecke @ 2018-01-08 10:09 UTC (permalink / raw)
To: Christoph Hellwig, Keith Busch
Cc: Linux Block, Linux NVMe, Device Mapper, Mike Snitzer, Jens Axboe,
Bart VanAssche, James Smart, Martin K . Petersen, Sagi Grimberg
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
--
Dr. Hannes Reinecke zSeries & Storage
hare@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)
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 1/5] nvme: Add more command status translation
2018-01-08 10:09 ` Hannes Reinecke
@ 2018-01-08 10:19 ` Christoph Hellwig
2018-01-08 15:29 ` Mike Snitzer
0 siblings, 1 reply; 30+ messages in thread
From: Christoph Hellwig @ 2018-01-08 10:19 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Christoph Hellwig, Keith Busch, Linux Block, Linux NVMe,
Device Mapper, Mike Snitzer, Jens Axboe, Bart VanAssche,
James Smart, Martin K . Petersen, Sagi Grimberg
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.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 1/5] nvme: Add more command status translation
2018-01-08 10:19 ` Christoph Hellwig
@ 2018-01-08 15:29 ` Mike Snitzer
2018-01-08 15:34 ` Christoph Hellwig
0 siblings, 1 reply; 30+ messages in thread
From: Mike Snitzer @ 2018-01-08 15:29 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Hannes Reinecke, Keith Busch, Linux Block, Linux NVMe,
Device Mapper, Jens Axboe, Bart VanAssche, James Smart,
Martin K . Petersen, Sagi Grimberg
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
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 1/5] nvme: Add more command status translation
2018-01-08 15:29 ` Mike Snitzer
@ 2018-01-08 15:34 ` Christoph Hellwig
2018-01-08 16:12 ` Keith Busch
0 siblings, 1 reply; 30+ messages in thread
From: Christoph Hellwig @ 2018-01-08 15:34 UTC (permalink / raw)
To: Mike Snitzer
Cc: Christoph Hellwig, Hannes Reinecke, Keith Busch, Linux Block,
Linux NVMe, Device Mapper, Jens Axboe, Bart VanAssche,
James Smart, Martin K . Petersen, Sagi Grimberg
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.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 1/5] nvme: Add more command status translation
2018-01-08 15:34 ` Christoph Hellwig
@ 2018-01-08 16:12 ` Keith Busch
0 siblings, 0 replies; 30+ messages in thread
From: Keith Busch @ 2018-01-08 16:12 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Mike Snitzer, Hannes Reinecke, Linux Block, Linux NVMe,
Device Mapper, Jens Axboe, Bart VanAssche, James Smart,
Martin K . Petersen, Sagi Grimberg
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.
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 2/5] nvme/multipath: Consult blk_status_t for failover
2018-01-04 22:46 [PATCH 0/5] Failover criteria unification Keith Busch
2018-01-04 22:46 ` [PATCH 1/5] nvme: Add more command status translation Keith Busch
@ 2018-01-04 22:46 ` Keith Busch
2018-01-04 23:42 ` Mike Snitzer
` (2 more replies)
2018-01-04 22:46 ` [PATCH 3/5] block: Provide blk_status_t decoding for retryable errors Keith Busch
` (3 subsequent siblings)
5 siblings, 3 replies; 30+ messages in thread
From: Keith Busch @ 2018-01-04 22:46 UTC (permalink / raw)
To: Linux Block, Linux NVMe, Device Mapper, Christoph Hellwig,
Mike Snitzer, Jens Axboe
Cc: Bart VanAssche, James Smart, Martin K . Petersen, Hannes Reinecke,
Sagi Grimberg, Keith Busch
This removes nvme multipath's specific status decoding to see if failover
is needed, using the generic blk_status_t that was translated earlier. This
abstraction from the raw NVMe status means nvme status decoding exists
in just one place.
Signed-off-by: Keith Busch <keith.busch@intel.com>
---
drivers/nvme/host/core.c | 9 +++++----
drivers/nvme/host/multipath.c | 44 ++++++++-----------------------------------
drivers/nvme/host/nvme.h | 4 ++--
3 files changed, 15 insertions(+), 42 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index f1cf1f6c5b28..4afc681f7089 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -196,8 +196,10 @@ static inline bool nvme_req_needs_retry(struct request *req)
void nvme_complete_rq(struct request *req)
{
- if (unlikely(nvme_req(req)->status && nvme_req_needs_retry(req))) {
- if (nvme_req_needs_failover(req)) {
+ blk_status_t status = nvme_error_status(req);
+
+ if (unlikely(status != BLK_STS_OK && nvme_req_needs_retry(req))) {
+ if (nvme_req_needs_failover(req, status)) {
nvme_failover_req(req);
return;
}
@@ -208,8 +210,7 @@ void nvme_complete_rq(struct request *req)
return;
}
}
-
- blk_mq_end_request(req, nvme_error_status(req));
+ blk_mq_end_request(req, status);
}
EXPORT_SYMBOL_GPL(nvme_complete_rq);
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 1218a9fca846..ae9abb600c0f 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -33,46 +33,18 @@ void nvme_failover_req(struct request *req)
kblockd_schedule_work(&ns->head->requeue_work);
}
-bool nvme_req_needs_failover(struct request *req)
+bool nvme_req_needs_failover(struct request *req, blk_status_t error)
{
if (!(req->cmd_flags & REQ_NVME_MPATH))
return false;
- switch (nvme_req(req)->status & 0x7ff) {
- /*
- * Generic command status:
- */
- 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_RESERVATION_CONFLICT:
- return false;
-
- /*
- * I/O command set specific error. Unfortunately these values are
- * reused for fabrics commands, but those should never get here.
- */
- case NVME_SC_BAD_ATTRIBUTES:
- case NVME_SC_INVALID_PI:
- case NVME_SC_READ_ONLY:
- case NVME_SC_ONCS_NOT_SUPPORTED:
- WARN_ON_ONCE(nvme_req(req)->cmd->common.opcode ==
- nvme_fabrics_command);
- return false;
-
- /*
- * Media and Data Integrity Errors:
- */
- case NVME_SC_WRITE_FAULT:
- case NVME_SC_READ_ERROR:
- case NVME_SC_GUARD_CHECK:
- case NVME_SC_APPTAG_CHECK:
- case NVME_SC_REFTAG_CHECK:
- case NVME_SC_COMPARE_FAILED:
- case NVME_SC_ACCESS_DENIED:
- case NVME_SC_UNWRITTEN_BLOCK:
+ switch (error) {
+ case BLK_STS_NOTSUPP:
+ case BLK_STS_NOSPC:
+ case BLK_STS_TARGET:
+ case BLK_STS_NEXUS:
+ case BLK_STS_MEDIUM:
+ case BLK_STS_PROTECTION:
return false;
}
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index ea1aa5283e8e..cf8548c1d7ec 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -401,7 +401,7 @@ extern const struct block_device_operations nvme_ns_head_ops;
#ifdef CONFIG_NVME_MULTIPATH
void nvme_failover_req(struct request *req);
-bool nvme_req_needs_failover(struct request *req);
+bool nvme_req_needs_failover(struct request *req, blk_status_t error);
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_head *head);
@@ -421,7 +421,7 @@ struct nvme_ns *nvme_find_path(struct nvme_ns_head *head);
static inline void nvme_failover_req(struct request *req)
{
}
-static inline bool nvme_req_needs_failover(struct request *req)
+static inline bool nvme_req_needs_failover(struct request *req, blk_status_t error)
{
return false;
}
--
2.13.6
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH 2/5] nvme/multipath: Consult blk_status_t for failover
2018-01-04 22:46 ` [PATCH 2/5] nvme/multipath: Consult blk_status_t for failover Keith Busch
@ 2018-01-04 23:42 ` Mike Snitzer
2018-01-08 8:57 ` Hannes Reinecke
2018-01-08 9:57 ` Christoph Hellwig
2 siblings, 0 replies; 30+ messages in thread
From: Mike Snitzer @ 2018-01-04 23:42 UTC (permalink / raw)
To: Keith Busch
Cc: Linux Block, Linux NVMe, Device Mapper, Christoph Hellwig,
Jens Axboe, Bart VanAssche, James Smart, Martin K . Petersen,
Hannes Reinecke, Sagi Grimberg
On Thu, Jan 04 2018 at 5:46pm -0500,
Keith Busch <keith.busch@intel.com> wrote:
> This removes nvme multipath's specific status decoding to see if failover
> is needed, using the generic blk_status_t that was translated earlier. This
> abstraction from the raw NVMe status means nvme status decoding exists
> in just one place.
>
> Signed-off-by: Keith Busch <keith.busch@intel.com>
Acked-by: Mike Snitzer <snitzer@redhat.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 2/5] nvme/multipath: Consult blk_status_t for failover
2018-01-04 22:46 ` [PATCH 2/5] nvme/multipath: Consult blk_status_t for failover Keith Busch
2018-01-04 23:42 ` Mike Snitzer
@ 2018-01-08 8:57 ` Hannes Reinecke
2018-01-08 9:57 ` Christoph Hellwig
2 siblings, 0 replies; 30+ messages in thread
From: Hannes Reinecke @ 2018-01-08 8:57 UTC (permalink / raw)
To: Keith Busch, Linux Block, Linux NVMe, Device Mapper,
Christoph Hellwig, Mike Snitzer, Jens Axboe
Cc: Bart VanAssche, James Smart, Martin K . Petersen, Sagi Grimberg
On 01/04/2018 11:46 PM, Keith Busch wrote:
> This removes nvme multipath's specific status decoding to see if failover
> is needed, using the generic blk_status_t that was translated earlier. This
> abstraction from the raw NVMe status means nvme status decoding exists
> in just one place.
>
> Signed-off-by: Keith Busch <keith.busch@intel.com>
> ---
> drivers/nvme/host/core.c | 9 +++++----
> drivers/nvme/host/multipath.c | 44 ++++++++-----------------------------------
> drivers/nvme/host/nvme.h | 4 ++--
> 3 files changed, 15 insertions(+), 42 deletions(-)
> Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
hare@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)
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 2/5] nvme/multipath: Consult blk_status_t for failover
2018-01-04 22:46 ` [PATCH 2/5] nvme/multipath: Consult blk_status_t for failover Keith Busch
2018-01-04 23:42 ` Mike Snitzer
2018-01-08 8:57 ` Hannes Reinecke
@ 2018-01-08 9:57 ` Christoph Hellwig
2018-01-09 17:38 ` Keith Busch
2 siblings, 1 reply; 30+ messages in thread
From: Christoph Hellwig @ 2018-01-08 9:57 UTC (permalink / raw)
To: Keith Busch
Cc: Linux Block, Linux NVMe, Device Mapper, Christoph Hellwig,
Mike Snitzer, Jens Axboe, Bart VanAssche, James Smart,
Martin K . Petersen, Hannes Reinecke, Sagi Grimberg
> - if (unlikely(nvme_req(req)->status && nvme_req_needs_retry(req))) {
> - if (nvme_req_needs_failover(req)) {
> + blk_status_t status = nvme_error_status(req);
> +
> + if (unlikely(status != BLK_STS_OK && nvme_req_needs_retry(req))) {
> + if (nvme_req_needs_failover(req, status)) {
We don't really need to call nvme_error_status if nvme_req(req)->status
is zero.
> -static inline bool nvme_req_needs_failover(struct request *req)
> +static inline bool nvme_req_needs_failover(struct request *req, blk_status_t error)
line break after 80 characters, please.
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH 2/5] nvme/multipath: Consult blk_status_t for failover
2018-01-08 9:57 ` Christoph Hellwig
@ 2018-01-09 17:38 ` Keith Busch
2018-01-09 17:40 ` Christoph Hellwig
0 siblings, 1 reply; 30+ messages in thread
From: Keith Busch @ 2018-01-09 17:38 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Linux Block, Linux NVMe, Device Mapper, Mike Snitzer, Jens Axboe,
Bart VanAssche, James Smart, Martin K . Petersen, Hannes Reinecke,
Sagi Grimberg
On Mon, Jan 08, 2018 at 01:57:07AM -0800, Christoph Hellwig wrote:
> > - if (unlikely(nvme_req(req)->status && nvme_req_needs_retry(req))) {
> > - if (nvme_req_needs_failover(req)) {
> > + blk_status_t status = nvme_error_status(req);
> > +
> > + if (unlikely(status != BLK_STS_OK && nvme_req_needs_retry(req))) {
> > + if (nvme_req_needs_failover(req, status)) {
>
> We don't really need to call nvme_error_status if nvme_req(req)->status
> is zero.
We are already calling nvme_error_status unconditionally for
blk_mq_end_request, so we currently read nvme_req(req)->status multiple
times in the completion path. I think we'd prefer to read it just once.
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH 2/5] nvme/multipath: Consult blk_status_t for failover
2018-01-09 17:38 ` Keith Busch
@ 2018-01-09 17:40 ` Christoph Hellwig
0 siblings, 0 replies; 30+ messages in thread
From: Christoph Hellwig @ 2018-01-09 17:40 UTC (permalink / raw)
To: Keith Busch
Cc: Christoph Hellwig, Linux Block, Linux NVMe, Device Mapper,
Mike Snitzer, Jens Axboe, Bart VanAssche, James Smart,
Martin K . Petersen, Hannes Reinecke, Sagi Grimberg
On Tue, Jan 09, 2018 at 10:38:58AM -0700, Keith Busch wrote:
> On Mon, Jan 08, 2018 at 01:57:07AM -0800, Christoph Hellwig wrote:
> > > - if (unlikely(nvme_req(req)->status && nvme_req_needs_retry(req))) {
> > > - if (nvme_req_needs_failover(req)) {
> > > + blk_status_t status = nvme_error_status(req);
> > > +
> > > + if (unlikely(status != BLK_STS_OK && nvme_req_needs_retry(req))) {
> > > + if (nvme_req_needs_failover(req, status)) {
> >
> > We don't really need to call nvme_error_status if nvme_req(req)->status
> > is zero.
>
> We are already calling nvme_error_status unconditionally for
> blk_mq_end_request, so we currently read nvme_req(req)->status multiple
> times in the completion path. I think we'd prefer to read it just once.
Indeed. Objection retracted.
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 3/5] block: Provide blk_status_t decoding for retryable errors
2018-01-04 22:46 [PATCH 0/5] Failover criteria unification Keith Busch
2018-01-04 22:46 ` [PATCH 1/5] nvme: Add more command status translation Keith Busch
2018-01-04 22:46 ` [PATCH 2/5] nvme/multipath: Consult blk_status_t for failover Keith Busch
@ 2018-01-04 22:46 ` Keith Busch
2018-01-04 23:43 ` Mike Snitzer
` (2 more replies)
2018-01-04 22:46 ` [PATCH 4/5] nvme/multipath: Use blk_retryable Keith Busch
` (2 subsequent siblings)
5 siblings, 3 replies; 30+ messages in thread
From: Keith Busch @ 2018-01-04 22:46 UTC (permalink / raw)
To: Linux Block, Linux NVMe, Device Mapper, Christoph Hellwig,
Mike Snitzer, Jens Axboe
Cc: Bart VanAssche, James Smart, Martin K . Petersen, Hannes Reinecke,
Sagi Grimberg, Keith Busch
This patch provides a common decoder for block status that may be retried
so various entities wishing to consult this do not have to duplicate
this decision.
Signed-off-by: Keith Busch <keith.busch@intel.com>
---
include/linux/blk_types.h | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index a1e628e032da..b6a8723b493c 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -39,6 +39,22 @@ typedef u8 __bitwise blk_status_t;
#define BLK_STS_AGAIN ((__force blk_status_t)12)
+static inline bool blk_retryable(blk_status_t error)
+{
+ switch (error) {
+ case BLK_STS_NOTSUPP:
+ case BLK_STS_NOSPC:
+ case BLK_STS_TARGET:
+ case BLK_STS_NEXUS:
+ case BLK_STS_MEDIUM:
+ case BLK_STS_PROTECTION:
+ return false;
+ }
+
+ /* Anything else could be a path failure, so should be retried */
+ return true;
+}
+
struct blk_issue_stat {
u64 stat;
};
--
2.13.6
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH 3/5] block: Provide blk_status_t decoding for retryable errors
2018-01-04 22:46 ` [PATCH 3/5] block: Provide blk_status_t decoding for retryable errors Keith Busch
@ 2018-01-04 23:43 ` Mike Snitzer
2018-01-08 8:58 ` Hannes Reinecke
2018-01-08 9:58 ` Christoph Hellwig
2 siblings, 0 replies; 30+ messages in thread
From: Mike Snitzer @ 2018-01-04 23:43 UTC (permalink / raw)
To: Keith Busch
Cc: Linux Block, Linux NVMe, Device Mapper, Christoph Hellwig,
Jens Axboe, Bart VanAssche, James Smart, Martin K . Petersen,
Hannes Reinecke, Sagi Grimberg
On Thu, Jan 04 2018 at 5:46pm -0500,
Keith Busch <keith.busch@intel.com> wrote:
> This patch provides a common decoder for block status that may be retried
> so various entities wishing to consult this do not have to duplicate
> this decision.
>
> Signed-off-by: Keith Busch <keith.busch@intel.com>
Acked-by: Mike Snitzer <snitzer@redhat.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 3/5] block: Provide blk_status_t decoding for retryable errors
2018-01-04 22:46 ` [PATCH 3/5] block: Provide blk_status_t decoding for retryable errors Keith Busch
2018-01-04 23:43 ` Mike Snitzer
@ 2018-01-08 8:58 ` Hannes Reinecke
2018-01-08 9:58 ` Christoph Hellwig
2 siblings, 0 replies; 30+ messages in thread
From: Hannes Reinecke @ 2018-01-08 8:58 UTC (permalink / raw)
To: Keith Busch, Linux Block, Linux NVMe, Device Mapper,
Christoph Hellwig, Mike Snitzer, Jens Axboe
Cc: Bart VanAssche, James Smart, Martin K . Petersen, Sagi Grimberg
On 01/04/2018 11:46 PM, Keith Busch wrote:
> This patch provides a common decoder for block status that may be retried
> so various entities wishing to consult this do not have to duplicate
> this decision.
>
> Signed-off-by: Keith Busch <keith.busch@intel.com>
> ---
> include/linux/blk_types.h | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
> index a1e628e032da..b6a8723b493c 100644
> --- a/include/linux/blk_types.h
> +++ b/include/linux/blk_types.h
> @@ -39,6 +39,22 @@ typedef u8 __bitwise blk_status_t;
>
> #define BLK_STS_AGAIN ((__force blk_status_t)12)
>
> +static inline bool blk_retryable(blk_status_t error)
> +{
> + switch (error) {
> + case BLK_STS_NOTSUPP:
> + case BLK_STS_NOSPC:
> + case BLK_STS_TARGET:
> + case BLK_STS_NEXUS:
> + case BLK_STS_MEDIUM:
> + case BLK_STS_PROTECTION:
> + return false;
> + }
> +
> + /* Anything else could be a path failure, so should be retried */
> + return true;
> +}
> +
> struct blk_issue_stat {
> u64 stat;
> };
>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
hare@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)
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH 3/5] block: Provide blk_status_t decoding for retryable errors
2018-01-04 22:46 ` [PATCH 3/5] block: Provide blk_status_t decoding for retryable errors Keith Busch
2018-01-04 23:43 ` Mike Snitzer
2018-01-08 8:58 ` Hannes Reinecke
@ 2018-01-08 9:58 ` Christoph Hellwig
2 siblings, 0 replies; 30+ messages in thread
From: Christoph Hellwig @ 2018-01-08 9:58 UTC (permalink / raw)
To: Keith Busch
Cc: Linux Block, Linux NVMe, Device Mapper, Christoph Hellwig,
Mike Snitzer, Jens Axboe, Bart VanAssche, James Smart,
Martin K . Petersen, Hannes Reinecke, Sagi Grimberg
> +static inline bool blk_retryable(blk_status_t error)
The naming isn't really useful - it is about the fact that it's
worth retrying on another path. So please chose a better name,
and add a kerneldoc comment describing it.
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 4/5] nvme/multipath: Use blk_retryable
2018-01-04 22:46 [PATCH 0/5] Failover criteria unification Keith Busch
` (2 preceding siblings ...)
2018-01-04 22:46 ` [PATCH 3/5] block: Provide blk_status_t decoding for retryable errors Keith Busch
@ 2018-01-04 22:46 ` Keith Busch
2018-01-04 23:44 ` Mike Snitzer
2018-01-08 8:58 ` Hannes Reinecke
2018-01-04 22:46 ` [PATCH 5/5] dm mpath: " Keith Busch
2018-01-04 23:36 ` [PATCH 0/5] Failover criteria unification Mike Snitzer
5 siblings, 2 replies; 30+ messages in thread
From: Keith Busch @ 2018-01-04 22:46 UTC (permalink / raw)
To: Linux Block, Linux NVMe, Device Mapper, Christoph Hellwig,
Mike Snitzer, Jens Axboe
Cc: Bart VanAssche, James Smart, Martin K . Petersen, Hannes Reinecke,
Sagi Grimberg, Keith Busch
Uses common code for determining if an error should be retried on
alternate path.
Signed-off-by: Keith Busch <keith.busch@intel.com>
---
drivers/nvme/host/multipath.c | 14 +-------------
1 file changed, 1 insertion(+), 13 deletions(-)
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index ae9abb600c0f..93bb72b6efb6 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -37,19 +37,7 @@ bool nvme_req_needs_failover(struct request *req, blk_status_t error)
{
if (!(req->cmd_flags & REQ_NVME_MPATH))
return false;
-
- switch (error) {
- case BLK_STS_NOTSUPP:
- case BLK_STS_NOSPC:
- case BLK_STS_TARGET:
- case BLK_STS_NEXUS:
- case BLK_STS_MEDIUM:
- case BLK_STS_PROTECTION:
- return false;
- }
-
- /* Everything else could be a path failure, so should be retried */
- return true;
+ return blk_retryable(error);
}
void nvme_kick_requeue_lists(struct nvme_ctrl *ctrl)
--
2.13.6
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH 4/5] nvme/multipath: Use blk_retryable
2018-01-04 22:46 ` [PATCH 4/5] nvme/multipath: Use blk_retryable Keith Busch
@ 2018-01-04 23:44 ` Mike Snitzer
2018-01-08 8:58 ` Hannes Reinecke
1 sibling, 0 replies; 30+ messages in thread
From: Mike Snitzer @ 2018-01-04 23:44 UTC (permalink / raw)
To: Keith Busch
Cc: Linux Block, Linux NVMe, Device Mapper, Christoph Hellwig,
Jens Axboe, Bart VanAssche, James Smart, Martin K . Petersen,
Hannes Reinecke, Sagi Grimberg
On Thu, Jan 04 2018 at 5:46pm -0500,
Keith Busch <keith.busch@intel.com> wrote:
> Uses common code for determining if an error should be retried on
> alternate path.
>
> Signed-off-by: Keith Busch <keith.busch@intel.com>
Acked-by: Mike Snitzer <snitzer@redhat.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 4/5] nvme/multipath: Use blk_retryable
2018-01-04 22:46 ` [PATCH 4/5] nvme/multipath: Use blk_retryable Keith Busch
2018-01-04 23:44 ` Mike Snitzer
@ 2018-01-08 8:58 ` Hannes Reinecke
1 sibling, 0 replies; 30+ messages in thread
From: Hannes Reinecke @ 2018-01-08 8:58 UTC (permalink / raw)
To: Keith Busch, Linux Block, Linux NVMe, Device Mapper,
Christoph Hellwig, Mike Snitzer, Jens Axboe
Cc: Bart VanAssche, James Smart, Martin K . Petersen, Sagi Grimberg
On 01/04/2018 11:46 PM, Keith Busch wrote:
> Uses common code for determining if an error should be retried on
> alternate path.
>
> Signed-off-by: Keith Busch <keith.busch@intel.com>
> ---
> drivers/nvme/host/multipath.c | 14 +-------------
> 1 file changed, 1 insertion(+), 13 deletions(-)
>
> diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
> index ae9abb600c0f..93bb72b6efb6 100644
> --- a/drivers/nvme/host/multipath.c
> +++ b/drivers/nvme/host/multipath.c
> @@ -37,19 +37,7 @@ bool nvme_req_needs_failover(struct request *req, blk_status_t error)
> {
> if (!(req->cmd_flags & REQ_NVME_MPATH))
> return false;
> -
> - switch (error) {
> - case BLK_STS_NOTSUPP:
> - case BLK_STS_NOSPC:
> - case BLK_STS_TARGET:
> - case BLK_STS_NEXUS:
> - case BLK_STS_MEDIUM:
> - case BLK_STS_PROTECTION:
> - return false;
> - }
> -
> - /* Everything else could be a path failure, so should be retried */
> - return true;
> + return blk_retryable(error);
> }
>
> void nvme_kick_requeue_lists(struct nvme_ctrl *ctrl)
>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
hare@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)
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 5/5] dm mpath: Use blk_retryable
2018-01-04 22:46 [PATCH 0/5] Failover criteria unification Keith Busch
` (3 preceding siblings ...)
2018-01-04 22:46 ` [PATCH 4/5] nvme/multipath: Use blk_retryable Keith Busch
@ 2018-01-04 22:46 ` Keith Busch
2018-01-04 23:45 ` Mike Snitzer
2018-01-08 8:59 ` Hannes Reinecke
2018-01-04 23:36 ` [PATCH 0/5] Failover criteria unification Mike Snitzer
5 siblings, 2 replies; 30+ messages in thread
From: Keith Busch @ 2018-01-04 22:46 UTC (permalink / raw)
To: Linux Block, Linux NVMe, Device Mapper, Christoph Hellwig,
Mike Snitzer, Jens Axboe
Cc: Bart VanAssche, James Smart, Martin K . Petersen, Hannes Reinecke,
Sagi Grimberg, Keith Busch
Uses common code for determining if an error should be retried on
alternate path.
Signed-off-by: Keith Busch <keith.busch@intel.com>
---
drivers/md/dm-mpath.c | 19 ++-----------------
1 file changed, 2 insertions(+), 17 deletions(-)
diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index f7810cc869ac..3eb9500db1b3 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -1475,21 +1475,6 @@ static void activate_path_work(struct work_struct *work)
activate_or_offline_path(pgpath);
}
-static int noretry_error(blk_status_t error)
-{
- switch (error) {
- case BLK_STS_NOTSUPP:
- case BLK_STS_NOSPC:
- case BLK_STS_TARGET:
- case BLK_STS_NEXUS:
- case BLK_STS_MEDIUM:
- return 1;
- }
-
- /* Anything else could be a path failure, so should be retried */
- return 0;
-}
-
static int multipath_end_io(struct dm_target *ti, struct request *clone,
blk_status_t error, union map_info *map_context)
{
@@ -1508,7 +1493,7 @@ static int multipath_end_io(struct dm_target *ti, struct request *clone,
* request into dm core, which will remake a clone request and
* clone bios for it and resubmit it later.
*/
- if (error && !noretry_error(error)) {
+ if (error && blk_retryable(error)) {
struct multipath *m = ti->private;
r = DM_ENDIO_REQUEUE;
@@ -1544,7 +1529,7 @@ static int multipath_end_io_bio(struct dm_target *ti, struct bio *clone,
unsigned long flags;
int r = DM_ENDIO_DONE;
- if (!*error || noretry_error(*error))
+ if (!*error || !blk_retryable(*error))
goto done;
if (pgpath)
--
2.13.6
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH 5/5] dm mpath: Use blk_retryable
2018-01-04 22:46 ` [PATCH 5/5] dm mpath: " Keith Busch
@ 2018-01-04 23:45 ` Mike Snitzer
2018-01-08 8:59 ` Hannes Reinecke
1 sibling, 0 replies; 30+ messages in thread
From: Mike Snitzer @ 2018-01-04 23:45 UTC (permalink / raw)
To: Keith Busch
Cc: Linux Block, Linux NVMe, Device Mapper, Christoph Hellwig,
Jens Axboe, Bart VanAssche, James Smart, Martin K . Petersen,
Hannes Reinecke, Sagi Grimberg
On Thu, Jan 04 2018 at 5:46pm -0500,
Keith Busch <keith.busch@intel.com> wrote:
> Uses common code for determining if an error should be retried on
> alternate path.
>
> Signed-off-by: Keith Busch <keith.busch@intel.com>
Acked-by: Mike Snitzer <snitzer@redhat.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 5/5] dm mpath: Use blk_retryable
2018-01-04 22:46 ` [PATCH 5/5] dm mpath: " Keith Busch
2018-01-04 23:45 ` Mike Snitzer
@ 2018-01-08 8:59 ` Hannes Reinecke
1 sibling, 0 replies; 30+ messages in thread
From: Hannes Reinecke @ 2018-01-08 8:59 UTC (permalink / raw)
To: Keith Busch, Linux Block, Linux NVMe, Device Mapper,
Christoph Hellwig, Mike Snitzer, Jens Axboe
Cc: Bart VanAssche, James Smart, Martin K . Petersen, Sagi Grimberg
On 01/04/2018 11:46 PM, Keith Busch wrote:
> Uses common code for determining if an error should be retried on
> alternate path.
>
> Signed-off-by: Keith Busch <keith.busch@intel.com>
> ---
> drivers/md/dm-mpath.c | 19 ++-----------------
> 1 file changed, 2 insertions(+), 17 deletions(-)
>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
hare@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)
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/5] Failover criteria unification
2018-01-04 22:46 [PATCH 0/5] Failover criteria unification Keith Busch
` (4 preceding siblings ...)
2018-01-04 22:46 ` [PATCH 5/5] dm mpath: " Keith Busch
@ 2018-01-04 23:36 ` Mike Snitzer
2018-01-04 23:47 ` Keith Busch
5 siblings, 1 reply; 30+ messages in thread
From: Mike Snitzer @ 2018-01-04 23:36 UTC (permalink / raw)
To: Keith Busch
Cc: Linux Block, Linux NVMe, Device Mapper, Christoph Hellwig,
Jens Axboe, Bart VanAssche, James Smart, Martin K . Petersen,
Hannes Reinecke, Sagi Grimberg
On Thu, Jan 04 2018 at 5:46pm -0500,
Keith Busch <keith.busch@intel.com> wrote:
> The nvme native multipath provided a separate NVMe status decoder,
> complicating maintenance as new statuses need to be accounted for. This
> was already diverging from the generic nvme status decoder, which has
> implications for other components that rely on accurate generic block
> errors.
>
> This series unifies common code among nvme and device-mapper multipath
> so that user experience regarding the failover fate of a command is the
> same.
>
> Mike:
>
> I split this up because I thought there'd be trouble merging the dm
> mpath update with the inverted retry logic, but I think you may have
> rebased the dm-4.16 without that patch, as I'm not seeing it in the most
> current branch.
Right, I dropped that patch since it'd have only resulted in conflicts
come merge time. As such, this series can easily go through the nvme
tree to Jens.
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH 0/5] Failover criteria unification
2018-01-04 23:36 ` [PATCH 0/5] Failover criteria unification Mike Snitzer
@ 2018-01-04 23:47 ` Keith Busch
2018-01-05 0:07 ` Mike Snitzer
2018-01-08 9:58 ` Christoph Hellwig
0 siblings, 2 replies; 30+ messages in thread
From: Keith Busch @ 2018-01-04 23:47 UTC (permalink / raw)
To: Mike Snitzer
Cc: Linux Block, Linux NVMe, Device Mapper, Christoph Hellwig,
Jens Axboe, Bart VanAssche, James Smart, Martin K . Petersen,
Hannes Reinecke, Sagi Grimberg
On Thu, Jan 04, 2018 at 06:36:27PM -0500, Mike Snitzer wrote:
> Right, I dropped that patch since it'd have only resulted in conflicts
> come merge time. As such, this series can easily go through the nvme
> tree to Jens.
It looks like you can also touch up dm to allow it to multipath nvme
even if CONFIG_NVME_MULTIPATH is set. It may be useful since native NVMe
doesn't multipath namespaces across subsystems, and some crack smoking
people want to create something that requires that.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/5] Failover criteria unification
2018-01-04 23:47 ` Keith Busch
@ 2018-01-05 0:07 ` Mike Snitzer
2018-01-08 9:58 ` Christoph Hellwig
1 sibling, 0 replies; 30+ messages in thread
From: Mike Snitzer @ 2018-01-05 0:07 UTC (permalink / raw)
To: Keith Busch
Cc: Linux Block, Linux NVMe, Device Mapper, Christoph Hellwig,
Jens Axboe, Bart VanAssche, James Smart, Martin K . Petersen,
Hannes Reinecke, Sagi Grimberg
On Thu, Jan 04 2018 at 6:47pm -0500,
Keith Busch <keith.busch@intel.com> wrote:
> On Thu, Jan 04, 2018 at 06:36:27PM -0500, Mike Snitzer wrote:
> > Right, I dropped that patch since it'd have only resulted in conflicts
> > come merge time. As such, this series can easily go through the nvme
> > tree to Jens.
>
> It looks like you can also touch up dm to allow it to multipath nvme
> even if CONFIG_NVME_MULTIPATH is set. It may be useful since native NVMe
> doesn't multipath namespaces across subsystems, and some crack smoking
> people want to create something that requires that.
I had a "CONFIG_NVME_MULITIPATH=Y" typo in the dm-mpath.c commit that
added the CONFIG_NVME_MULTIPATH mutual exclussion in dm-4.16; so I
just dropped it via rebase.
So now the underlying nvme devices (already multipath'd by NVMe) should
be discoverable by multipathd. Who am I to judge crack smokers?
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/5] Failover criteria unification
2018-01-04 23:47 ` Keith Busch
2018-01-05 0:07 ` Mike Snitzer
@ 2018-01-08 9:58 ` Christoph Hellwig
1 sibling, 0 replies; 30+ messages in thread
From: Christoph Hellwig @ 2018-01-08 9:58 UTC (permalink / raw)
To: Keith Busch
Cc: Mike Snitzer, Linux Block, Linux NVMe, Device Mapper,
Christoph Hellwig, Jens Axboe, Bart VanAssche, James Smart,
Martin K . Petersen, Hannes Reinecke, Sagi Grimberg
On Thu, Jan 04, 2018 at 04:47:27PM -0700, Keith Busch wrote:
> It looks like you can also touch up dm to allow it to multipath nvme
> even if CONFIG_NVME_MULTIPATH is set. It may be useful since native NVMe
> doesn't multipath namespaces across subsystems, and some crack smoking
> people want to create something that requires that.
Right now that is simply not allowed by the spec. If Dave comes up
with a palatable way to retrofit that into the spec we'll support it.
^ permalink raw reply [flat|nested] 30+ messages in thread