Re: (2) [PATCH v2] driver/nvme/host: Support duplicated nsid for the private ns

From: hch@xxxxxx
Date: Tue Mar 15 2022 - 06:07:09 EST


On Tue, Mar 15, 2022 at 06:56:12PM +0900, Sungup Moon wrote:
> I'll answer your opinion.
>
> 1. ns_head leak issue
>
> I don't think that is leaked ns_head. Because although all ids and nsid are same
> through all namespaces, each namespaces are indenpendent namespace and each of that
> should have independent data structure.
> Duplicated nsid private namespace is different from the shared namespace even though
> same information.

In your patch if nvme_find_ns_head returns a head for a private namespace
with a non-uniqueue ID, that returned head already has an additional
reference, which we'll never drop as the head variable is overriden just
below.

> 2. nvme_mpath_set_disk_name issue
>
> Yes, I also agree that subsystem-wide IDA is very important data. However, I
> implemented without nvme_mpath_set_disk_name modification at the first time, it is
> hard to decide which namespace are connected to private controller.

It is not just very important, it can't work without that. The two
separate IDAs can will have conflicts, so you can up with two namespaces
with the same name without that change.

> As you know, each nvme controller start initiating at a time. So, each controller
> structures are sequentially initiated, but each namespaces structures are initiated
> independently because of multi-processing on cpu. So, all namespace can have different
> instance number every boot-up time, and it makes hard to track and control the private
> namespace on the controller or device failure.

Yes, but that is true of all Linux device enumeration. That is why
everyone should use table identifiers like the UUID or GUID to identify
the namespaces.

> Anyway, the private namespace is same condition with no-multipath situation (because
> private namespace cannot shared between controllers) so I think that the private
> namespace should follow the naming rule with no-multipath situation.

We can't use the non-multipath cabale naming as it will cause conflicts
in the naming. If anything in the system supports multipathing we have
to use subsystem based instances for the naming.