[PATCH] nvme: Treat discovery subsystems as unique subsystems

[PATCH] nvme: Treat discovery subsystems as unique subsystems

[James Smart]

Current code matches subnqn and collapses all controllers to the
same subnqn to a single subsystem structure. This is good for
recognizing multiple controllers for the same subsystem. But with
the well-known discovery subnqn, the subsystems aren't truly the
same subsystem. As such, subsystem specific rules, such as no
overlap of controller id, do not apply. With today's behavior, the
check for overlap of controller id can fail, preventing the new
discovery controller from being created.

When searching for like subsystem nqn, exclude the discovery nqn
from matching. This will result in each discovery controller being
attached to a unique subsystem structure.

Signed-off-by: James Smart <jsmart2021@gmail.com>
---
 drivers/nvme/host/core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 4660505eded9..3edafe57c7f7 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2358,6 +2358,9 @@ static struct nvme_subsystem *__nvme_find_get_subsystem(const char *subsysnqn)
 
 	lockdep_assert_held(&nvme_subsystems_lock);
 
+	if (!strcmp(subsysnqn, NVME_DISC_SUBSYS_NAME))
+		return NULL;
+
 	list_for_each_entry(subsys, &nvme_subsystems, entry) {
 		if (strcmp(subsys->subnqn, subsysnqn))
 			continue;
-- 
2.13.7


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

Re: [PATCH] nvme: Treat discovery subsystems as unique subsystems

[Sagi Grimberg]

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

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

Re: [PATCH] nvme: Treat discovery subsystems as unique subsystems

[Max Gurtovoy]

On 8/30/2019 11:11 PM, James Smart wrote:
> Current code matches subnqn and collapses all controllers to the
> same subnqn to a single subsystem structure. This is good for
> recognizing multiple controllers for the same subsystem. But with
> the well-known discovery subnqn, the subsystems aren't truly the
> same subsystem. As such, subsystem specific rules, such as no
> overlap of controller id, do not apply. With today's behavior, the
> check for overlap of controller id can fail, preventing the new
> discovery controller from being created.

so this fixes a scenario of multiple discovery in parallel ?


> When searching for like subsystem nqn, exclude the discovery nqn
> from matching. This will result in each discovery controller being
> attached to a unique subsystem structure.
>
> Signed-off-by: James Smart <jsmart2021@gmail.com>
> ---
>   drivers/nvme/host/core.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 4660505eded9..3edafe57c7f7 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -2358,6 +2358,9 @@ static struct nvme_subsystem *__nvme_find_get_subsystem(const char *subsysnqn)
>   
>   	lockdep_assert_held(&nvme_subsystems_lock);
>   
> +	if (!strcmp(subsysnqn, NVME_DISC_SUBSYS_NAME))
> +		return NULL;
> +
>   	list_for_each_entry(subsys, &nvme_subsystems, entry) {
>   		if (strcmp(subsys->subnqn, subsysnqn))
>   			continue;

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

Re: [PATCH] nvme: Treat discovery subsystems as unique subsystems

[James Smart]

> On Sep 1, 2019, at 3:04 AM, Max Gurtovoy <maxg@mellanox.com> wrote:
> 
> 
> On 8/30/2019 11:11 PM, James Smart wrote:
>> Current code matches subnqn and collapses all controllers to the
>> same subnqn to a single subsystem structure. This is good for
>> recognizing multiple controllers for the same subsystem. But with
>> the well-known discovery subnqn, the subsystems aren't truly the
>> same subsystem. As such, subsystem specific rules, such as no
>> overlap of controller id, do not apply. With today's behavior, the
>> check for overlap of controller id can fail, preventing the new
>> discovery controller from being created.
> 
> so this fixes a scenario of multiple discovery in parallel ?
> 

It’s not to say that multiple discoveries in parallel doesn’t work, but I did see a scenario where it mapped into this. In most cases, the time taken for an ioctl or two to get the log, then delete the controller happens so fast it’s avoided. But I fully expect long-lived controllers to make this problematic.

— james



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

Re: [PATCH] nvme: Treat discovery subsystems as unique subsystems

[Christoph Hellwig]

On Fri, Aug 30, 2019 at 01:11:45PM -0700, James Smart wrote:
> @@ -2358,6 +2358,9 @@ static struct nvme_subsystem *__nvme_find_get_subsystem(const char *subsysnqn)
>  
>  	lockdep_assert_held(&nvme_subsystems_lock);
>  
> +	if (!strcmp(subsysnqn, NVME_DISC_SUBSYS_NAME))
> +		return NULL;
> +

Can you throw in a comment here?

Otherwise this looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

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