[PATCH 2/2] nvme-multipath: fix I/O stall when remapping namespaces

[Hannes Reinecke]

On 9/5/24 11:06, Sagi Grimberg wrote:
> 
> 
> 
> On 05/09/2024 11:39, Hannes Reinecke wrote:
>> On 9/5/24 09:06, Sagi Grimberg wrote:
>>>
>>>
>>>
>>> On 04/09/2024 11:59, Hannes Reinecke wrote:
>>>> On 9/4/24 10:20, Hannes Reinecke wrote:
[ .. ]
>>>> in addition to moving of kblockd_schedule.
>>>>
>>>> So what do you prefer, checking NVME_NS_HEAD_LIVE or NVME_NS_READY?
>>>
>>> Well, NVME_NS_READY is cleared in nvme_ns_remove (which is fine) but 
>>> also
>>> in nvme_mpath_revalidate_paths() if the capacity changed, so 
>>> theoretically,
>>> if the last remaining path changed its capacity, it will make 
>>> nvme_available_path()
>>> fail, and in turn, mpath IO will fail and not added to the 
>>> requeue_list, which would be
>>> wrong. So I think that nvme_available_path should check 
>>> NVME_NSHEAD_DISK_LIVE,
>>> but nvme_find_path is fine in its current form.
>>
>> Weelll ... not quite.
>> I never get this far, as the only place where nvme_remove_ns() can be
>> called is from the scanning process, yet this is stuck waiting for I/O.
>>
>> My testcase then simulated a namespace replacement by setting a new 
>> UUID, and then enabling the namespace again. The target will return
>> PATH ERROR during that time, causing I/O to be retried (while disabled)
>> or failed over to the same path (as it's the only one left).
>>
>> Of course we could argue that the testcase is invalid, and the target
>> should return INVALID_NS if the UUID changed.
>> But one could also argue that retrying on the same path is pointless,
>> seeing that we'll always get the same error.
>> Hmm?
> 
> Hannes, I was referring to the general case where we have more than one 
> path.
> 
> What I said is that I think that if you drop the NSHEAD_DISK_LIVE check 
> in nvme_find_path
> (it already checks for NS_READY) but have nvme_available_path check for 
> NSHEAD_DISK_LIVE
> and both your test case should work and the general case where there is 
> more than a single
> path.

Ah, _that's_ what you mean. Sure, sounds feasible. I'll give it a spin.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare at suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Ivo Totev, Andrew McDonald,
Werner Knoblich