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

[Sagi Grimberg <sagi@grimberg.me>]

>>> diff --git a/drivers/nvme/host/multipath.c
>>> b/drivers/nvme/host/multipath.c index 54603bd..d8b7f45 100644
>>> --- a/drivers/nvme/host/multipath.c
>>> +++ b/drivers/nvme/host/multipath.c
>>> @@ -278,9 +278,12 @@ static bool nvme_available_path(struct
>>> nvme_ns_head *head)
>>>
>>>    	list_for_each_entry_rcu(ns, &head->list, siblings) {
>>>    		switch (ns->ctrl->state) {
>>> +		case NVME_CTRL_CONNECTING:
>>> +			if (test_bit(NVME_CTRL_FAILFAST_EXPIRED,
>>> +				     &ns->ctrl->flags))
>>> +				break;
>>>    		case NVME_CTRL_LIVE:
>>>    		case NVME_CTRL_RESETTING:
>>> -		case NVME_CTRL_CONNECTING:
>>>    			/* fallthru */
>>>    			return true;
>>>    		default:
>>
>> This is too subtle to not document.
>> The parameter is a controller property, but here it will affect the mpath device
>> node.
>>
>> This is changing the behavior of "queue as long as we have an available path"
>> to "queue until all our paths said to fail fast".
>>
>> I guess that by default we will have the same behavior, and the behavior will
>> change only if all the controller have failfast parameter tuned.
>>
>> At the very least it is an important undocumented change that needs to be
>> called in the change log.
> 
> The multipath may be stuck on reconnected controller even forever.
> Moreover, all commands will be returned with error status,
> but the path will not be switched.
> And in this case, the presence of the additional path looks pointless.

Its not pointless, currently while there is an available path that may
become active we queue it up. Your suggestion to fail multipath commands
when the last failfail timeout expires is OK, but must be documented.

> I suggest use the failfast parameter for each path separately.

It is already a controller attribute.

> It can also serve as the priority of each path.

Lets not do anything on that front, if someone wants fail certain
controllers faster than others, he can do it, but nothing should
be mandated from the driver for sure...

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