Re: [PATCH] nvme-fabrics: reject I/O to offline device

[James Smart <james.smart@broadcom.com>]

On 12/4/2019 12:28 AM, Victor Gladkov wrote:
> On 12/03/2019 06:19 PM, James Smart wrote:
>> On 12/3/2019 2:04 AM, Victor Gladkov wrote:
>>> On 12/03/2019 00:47 AM, James Smart wrote:
>>>> O
>>>> The controller-loss-timeout should not affect IO timeout policy, these are
>> two different policies.
>> Ok - which says what does make sense to add is the portion:
>>
>>     !(ctrl->state == NVME_CTRL_CONNECTING && ((ktime_get_ns() - rq->start_time_ns) > jiffies_to_nsecs(rq->timeout)))
>>
>>
>> But I don't think we need the failfast flag.
>>
>> -- james
> OK. I think, it's good enough.
>
> This is updated patch:
>
> ---
> diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
> index 74b8818..b58abc1 100644
> --- a/drivers/nvme/host/fabrics.c
> +++ b/drivers/nvme/host/fabrics.c
> @@ -549,6 +549,8 @@ blk_status_t nvmf_fail_nonready_command(struct nvme_ctrl *ctrl,
>   {
>          if (ctrl->state != NVME_CTRL_DELETING &&
>              ctrl->state != NVME_CTRL_DEAD &&
> +           !(ctrl->state == NVME_CTRL_CONNECTING &&
> +            ((ktime_get_ns() - rq->start_time_ns) > jiffies_to_nsecs(rq->timeout))) &&
>              !blk_noretry_request(rq) && !(rq->cmd_flags & REQ_NVME_MPATH))
>                  return BLK_STS_RESOURCE;
>

Did you test this to ensure it's doing what you expect. I'm not sure 
that all the timers are set right at this point. Most I/O's timeout from 
a deadline time stamped at blk_mq_start_request(). But that routine is 
actually called by the transports post the 
nvmf_check_ready/fail_nonready calls.  E.g. the io is not yet in flight, 
thus queued, and the blk-mq internal queuing doesn't count against the 
io timeout.  I can't see anything that guarantees start_time_ns is set.

-- james


_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme