Re: [PATCHv3] nvme-mpath: delete disk after last connection

From: Hannes Reinecke <hare@suse.de>
To: Keith Busch <kbusch@kernel.org>, Sagi Grimberg <sagi@grimberg.me>
Cc: Christoph Hellwig <hch@lst.de>, Keith Busch <keith.busch@wdc.com>,
	linux-nvme@lists.infradead.org,
	Daniel Wagner <daniel.wagner@suse.de>
Subject: Re: [PATCHv3] nvme-mpath: delete disk after last connection
Date: Wed, 5 May 2021 18:15:46 +0200	[thread overview]
Message-ID: <6a43fe69-e73a-07f8-f050-8116add83074@suse.de> (raw)
In-Reply-To: <20210505152659.GB912679@dhcp-10-100-145-180.wdc.com>

On 5/5/21 5:26 PM, Keith Busch wrote:
> On Tue, May 04, 2021 at 12:54:14PM -0700, Sagi Grimberg wrote:
>>
>>>> As stated in the v3 review this is an incompatible change.  We'll need
>>>> the queue_if_no_path attribute first, and default it to on to keep
>>>> compatability.
>>>>
>>>
>>> That is what I tried the last time, but the direction I got was to treat
>>> both, NVMe-PCI and NVMe-oF identically:
>>> (https://lore.kernel.org/linux-nvme/34e5c178-8bc4-68d3-8374-fbc1b451b6e8@grimberg.me/)
>>
>> Yes, I'm not sure I understand your comment Christoph. This addresses an
>> issue with mdraid where hot unplug+replug does not restore the device to
>> the raid group (pci and fabrics alike), where before multipath this used
>> to work.
>>
>> queue_if_no_path is a dm-multipath feature so I'm not entirely clear
>> what is the concern? mdraid on nvme (pci/fabrics) used to work a certain
>> way, with the introduction of nvme-mpath the behavior was broken (as far
>> as I understand from Hannes).
>>
>> My thinking is that if we want queue_if_no_path functionality in nvme
>> mpath we should have it explicitly stated properly such that people
>> that actually need it will use it and have mdraid function correctly
>> again. Also, queue_if_no_path applies really if all the paths are
>> gone in the sense they are completely removed, and doesn't apply
>> to controller reset/reconnect.
>>
>> I agree we should probably have queue_if_no_path attribute on the
>> mpath device, but it doesn't sound right to default it to true given
>> that it breaks mdraid stacking on top of it..
> 
> If you want "queue_if_no_path" behavior, can't you just set really high
> reconnect_delay and ctrl_loss_tmo values? That prevents the path from
> being deleted while it is unreachable, then restart IO on the existing
> path once connection is re-established.
> 
Precisely my thinking.
We _could_ add a queue_if_no_path attribute, but we can also achieve the
same behaviour by setting the ctrl_loss_tmo value to infinity.
Provided we can change it on the fly, though; but it not that's easily
fixed.

In fact, that's what we recommend to our customers to avoid the bug
fixed by this patch.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		        Kernel Storage Architect
hare@suse.de			               +49 911 74053 688
SUSE Software Solutions Germany GmbH, 90409 Nürnberg
GF: F. Imendörffer, HRB 36809 (AG Nürnberg)

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