Re: [PATCH v4 4/5] nvme-rdma: avoid IO error for nvme native multipath

From: Chao Leng <lengchao@huawei.com>
To: Sagi Grimberg <sagi@grimberg.me>, <linux-nvme@lists.infradead.org>
Cc: <kbusch@kernel.org>, <axboe@fb.com>, <hch@lst.de>,
	<linux-block@vger.kernel.org>, <axboe@kernel.dk>
Subject: Re: [PATCH v4 4/5] nvme-rdma: avoid IO error for nvme native multipath
Date: Fri, 29 Jan 2021 11:37:21 +0800	[thread overview]
Message-ID: <6b01cd8e-a153-1913-4e2f-0c3ce1029ccf@huawei.com> (raw)
In-Reply-To: <1651456a-3251-77db-42c9-fc1a2d2c5c13@grimberg.me>


On 2021/1/29 11:24, Sagi Grimberg wrote:
> 
>>>>> Why not just call set the status and call nvme_rdma_complete_rq and
>>>>> return here?
>>>> If the err is ENOMEM or EAGAIN, I am not sure the err must be a
>>>> path-related error for all HBA drivers. So reused the error check code.
>>>> I think it would be more reasonable to assume any errors returned by HBA
>>>> driver as path-related errors.
>>>> If you think so, I will modify it in next patch version.
>>>
>>> Meant to do that only for -EIO. We should absolutely not do any of this
>>> for stuff like EINVAL, EOPNOTSUPP, EPERM or any strange error that may
>>> return due to a bug or anything like that.
>> ok, please review again, thank you.
>> ---
>>   drivers/nvme/host/rdma.c | 9 +++++++--
>>   1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
>> index b7ce4f221d99..66b697461bd9 100644
>> --- a/drivers/nvme/host/rdma.c
>> +++ b/drivers/nvme/host/rdma.c
>> @@ -2084,8 +2084,13 @@ static blk_status_t nvme_rdma_queue_rq(struct blk_mq_hw_ctx *hctx,
>>
>>          err = nvme_rdma_post_send(queue, sqe, req->sge, req->num_sge,
>>                          req->mr ? &req->reg_wr.wr : NULL);
>> -       if (unlikely(err))
>> +       if (unlikely(err)) {
>> +               if (err == -EIO) {
>> +                       nvme_complete_failed_rq(rq, NVME_SC_HOST_PATH_ERROR);
> 
> I was thinking about:
> -- 
>          err = nvme_rdma_post_send(queue, sqe, req->sge, req->num_sge,
>                          req->mr ? &req->reg_wr.wr : NULL);
>          if (unlikely(err)) {
>                  if (err == -EIO) {
>                          /*
>                           * Fail the reuqest so upper layer can failover I/O
>                           * if another path is available
>                           */
>                          req->status = NVME_SC_HOST_PATH_ERROR;
>                          nvme_rdma_complete_rq(rq);
>                          return BLK_STS_OK;
Need to do clean. so can not directly return.
> 
>                  }
>                  goto err_unmap;
>          }
> -- 
> .