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

From: Sungup Moon
Date: Tue Mar 15 2022 - 06:58:07 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.

Ok, I will review my patch with my team and reply your comment asap.

> > 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.
>

Ok, I agree your opinion. My patch about nvme_mpath_set_disk_name will make confusion
of the naming in subsystem, and also find the relation-ship between controller <->
namespace using the sysfs directory structure. I'll remove that patch line and resummit
with first issue (ns_head leak issue) after review that.