[PATCH 3/3] nvme: redirect commands on dying queue

[Chao Leng]

On 2020/8/17 16:13, Christoph Hellwig wrote:
> On Mon, Aug 17, 2020 at 01:49:47PM +0800, Chao Leng wrote:
>> If without any multipath, will not retry.
> 
> Which is the right thing to do.
Suggestion:
Do not use blk_noretry_request. The local retry mechanism, which is
defined by nvme protocol, is conflicted with REQ_FAILFAST_TRANSPORT.
blk_noretry_request is not a good choice for nvme, even for SCSI,
this is not a good choice too, so SCSI does not use this MACRO.
We can seperate REQ_FAILFAST_TRANSPORT with other FAILFAST flag.
For REQ_FAILFAST_TRANSPORT, do local retry for non path error.
For other FAILFAST flag, complete request with error code immediately.
> 
>> This may not be what we expected.
>> If we can support both NVMe multipathing and third-party multipathing,
>> it would be great. In addition to DM-MULTIPATH, third-party multipathing
>> software is provided by storage vendors, such as PowerPath(DELL&EMC),
>> SecurePath(hp), Ultrapath(huawei), etc.
> 
> For dm-multipath we only guarantee we don't break old behavior.  ANA
No, now we do not guarantee this. For example, if target return
NVME_SC_NS_NOT_READY or NVME_SC_CMD_INTERRUPTED, we do not local retry,
and translate to BLK_STS_TARGET, blk_path_error treat it as retrying
failover path will not help, will cause io interrupt. This is serious
problem.
  /**
  * blk_path_error - returns true if error may be path related
  * @error: status the request was completed with
  *
  * Description:
  *     This classifies block error status into non-retryable errors and ones
  *     that may be successful if retried on a failover path.
  *
  * Return:
  *     %false - retrying failover path will not help
  *     %true  - may succeed if retried
  */
static inline bool blk_path_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:
	case BLK_STS_PROTECTION:
		return false;
	}

	/* Anything else could be a path failure, so should be retried */
	return true;
}
> was never supported without nvme-multipath so this isn't part of it.
> 
> Out of tree junk is completely unsupported per the kernel wide policy,
> unrelated to the nvme driver itself.
If we can support third multipathing software without extra cost,
and have no bad effect on NVMe, we will get more palyers, supporters,
and contributors, will faster nvme over fabric development.
I will try to do this base on your patch.