* [PATCHv2] nvme: Assign subsy instance from first ctrl
@ 2019-09-05 16:33 ` Keith Busch
0 siblings, 0 replies; 10+ messages in thread
From: Keith Busch @ 2019-09-05 16:33 UTC (permalink / raw)
To: Christoph Hellwig, linux-nvme, Sagi Grimberg
Cc: Logan Gunthorpe, Hannes Reinecke, Martin K . Petersen,
linux-kernel, Jens Axboe, Keith Busch
The namespace disk names must be unique for the lifetime of the
subsystem. This was accomplished by using their parent subsystems'
instances which were allocated independently from the controllers
connected to that subsystem. This allowed name prefixes assigned to
namespaces to match a controller from an unrelated subsystem, and has
created confusion among users examining device nodes.
Ensure a namespace's subsystem instance never clashes with a controller
instance of another subsystem by transferring the instance ownership
to the parent subsystem from the first controller discovered in that
subsystem.
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
v1 -> v2:
Changelog: reduce sensationalism, fix spelling
drivers/nvme/host/core.c | 21 ++++++++++-----------
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 14c0bfb55615..8a8279ece5ee 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -81,7 +81,6 @@ EXPORT_SYMBOL_GPL(nvme_reset_wq);
struct workqueue_struct *nvme_delete_wq;
EXPORT_SYMBOL_GPL(nvme_delete_wq);
-static DEFINE_IDA(nvme_subsystems_ida);
static LIST_HEAD(nvme_subsystems);
static DEFINE_MUTEX(nvme_subsystems_lock);
@@ -2344,7 +2343,8 @@ static void nvme_release_subsystem(struct device *dev)
struct nvme_subsystem *subsys =
container_of(dev, struct nvme_subsystem, dev);
- ida_simple_remove(&nvme_subsystems_ida, subsys->instance);
+ if (subsys->instance >= 0)
+ ida_simple_remove(&nvme_instance_ida, subsys->instance);
kfree(subsys);
}
@@ -2473,12 +2473,8 @@ static int nvme_init_subsystem(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id)
subsys = kzalloc(sizeof(*subsys), GFP_KERNEL);
if (!subsys)
return -ENOMEM;
- ret = ida_simple_get(&nvme_subsystems_ida, 0, 0, GFP_KERNEL);
- if (ret < 0) {
- kfree(subsys);
- return ret;
- }
- subsys->instance = ret;
+
+ subsys->instance = -1;
mutex_init(&subsys->lock);
kref_init(&subsys->ref);
INIT_LIST_HEAD(&subsys->ctrls);
@@ -2497,7 +2493,7 @@ static int nvme_init_subsystem(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id)
subsys->dev.class = nvme_subsys_class;
subsys->dev.release = nvme_release_subsystem;
subsys->dev.groups = nvme_subsys_attrs_groups;
- dev_set_name(&subsys->dev, "nvme-subsys%d", subsys->instance);
+ dev_set_name(&subsys->dev, "nvme-subsys%d", ctrl->instance);
device_initialize(&subsys->dev);
mutex_lock(&nvme_subsystems_lock);
@@ -2528,6 +2524,8 @@ static int nvme_init_subsystem(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id)
goto out_put_subsystem;
}
+ if (!found)
+ subsys->instance = ctrl->instance;
ctrl->subsys = subsys;
list_add_tail(&ctrl->subsys_entry, &subsys->ctrls);
mutex_unlock(&nvme_subsystems_lock);
@@ -3803,7 +3801,9 @@ static void nvme_free_ctrl(struct device *dev)
container_of(dev, struct nvme_ctrl, ctrl_device);
struct nvme_subsystem *subsys = ctrl->subsys;
- ida_simple_remove(&nvme_instance_ida, ctrl->instance);
+ if (subsys && ctrl->instance != subsys->instance)
+ ida_simple_remove(&nvme_instance_ida, ctrl->instance);
+
kfree(ctrl->effects);
nvme_mpath_uninit(ctrl);
__free_page(ctrl->discard_page);
@@ -4085,7 +4085,6 @@ static int __init nvme_core_init(void)
static void __exit nvme_core_exit(void)
{
- ida_destroy(&nvme_subsystems_ida);
class_destroy(nvme_subsys_class);
class_destroy(nvme_class);
unregister_chrdev_region(nvme_chr_devt, NVME_MINORS);
--
2.14.5
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCHv2] nvme: Assign subsy instance from first ctrl
2019-09-05 16:33 ` Keith Busch
@ 2019-09-05 16:39 ` Christoph Hellwig
-1 siblings, 0 replies; 10+ messages in thread
From: Christoph Hellwig @ 2019-09-05 16:39 UTC (permalink / raw)
To: Keith Busch
Cc: Hannes Reinecke, Sagi Grimberg, Martin K . Petersen, linux-kernel,
linux-nvme, Jens Axboe, Logan Gunthorpe, Christoph Hellwig
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCHv2] nvme: Assign subsy instance from first ctrl
2019-09-05 16:33 ` Keith Busch
@ 2019-09-05 20:05 ` Sagi Grimberg
-1 siblings, 0 replies; 10+ messages in thread
From: Sagi Grimberg @ 2019-09-05 20:05 UTC (permalink / raw)
To: Keith Busch, Christoph Hellwig, linux-nvme
Cc: Jens Axboe, Hannes Reinecke, Logan Gunthorpe, linux-kernel,
Martin K . Petersen
Looks good,
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
Applied to nvme-5.4
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCHv2] nvme: Assign subsy instance from first ctrl
2019-09-05 16:33 ` Keith Busch
@ 2019-09-07 6:13 ` Minwoo Im
-1 siblings, 0 replies; 10+ messages in thread
From: Minwoo Im @ 2019-09-07 6:13 UTC (permalink / raw)
To: Keith Busch, Minwoo Im
Cc: Hannes Reinecke, Sagi Grimberg, Martin K . Petersen,
linux-kernel@vger.kernel.org, linux-nvme@lists.infradead.org,
Jens Axboe, Logan Gunthorpe, Christoph Hellwig
> Subject: [PATCHv2] nvme: Assign subsy instance from first ctrl
I'm not sure but, I have not seen 'subsy' thing before. Maybe
s/sybsy/subsys/ ?
>
> The namespace disk names must be unique for the lifetime of the
> subsystem. This was accomplished by using their parent subsystems'
> instances which were allocated independently from the controllers
> connected to that subsystem. This allowed name prefixes assigned to
> namespaces to match a controller from an unrelated subsystem, and has
> created confusion among users examining device nodes.
>
> Ensure a namespace's subsystem instance never clashes with a controller
> instance of another subsystem by transferring the instance ownership
> to the parent subsystem from the first controller discovered in that
> subsystem.
>
> Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
Keith, Thanks for this patch. I really like this concept which can
avoid from instance mistakes.
Otherwise looks good to me.
Reviewed-by: Minwoo Im <minwoo.im@samsung.com>
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCHv2] nvme: Assign subsy instance from first ctrl
@ 2019-09-07 6:13 ` Minwoo Im
0 siblings, 0 replies; 10+ messages in thread
From: Minwoo Im @ 2019-09-07 6:13 UTC (permalink / raw)
To: Keith Busch, Minwoo Im
Cc: Christoph Hellwig, linux-nvme@lists.infradead.org, Sagi Grimberg,
Hannes Reinecke, Martin K . Petersen,
linux-kernel@vger.kernel.org, Jens Axboe, Logan Gunthorpe
> Subject: [PATCHv2] nvme: Assign subsy instance from first ctrl
I'm not sure but, I have not seen 'subsy' thing before. Maybe
s/sybsy/subsys/ ?
>
> The namespace disk names must be unique for the lifetime of the
> subsystem. This was accomplished by using their parent subsystems'
> instances which were allocated independently from the controllers
> connected to that subsystem. This allowed name prefixes assigned to
> namespaces to match a controller from an unrelated subsystem, and has
> created confusion among users examining device nodes.
>
> Ensure a namespace's subsystem instance never clashes with a controller
> instance of another subsystem by transferring the instance ownership
> to the parent subsystem from the first controller discovered in that
> subsystem.
>
> Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
Keith, Thanks for this patch. I really like this concept which can
avoid from instance mistakes.
Otherwise looks good to me.
Reviewed-by: Minwoo Im <minwoo.im@samsung.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCHv2] nvme: Assign subsy instance from first ctrl
2019-09-05 16:33 ` Keith Busch
@ 2019-09-09 10:32 ` Hannes Reinecke
-1 siblings, 0 replies; 10+ messages in thread
From: Hannes Reinecke @ 2019-09-09 10:32 UTC (permalink / raw)
To: Keith Busch, Sagi Grimberg, linux-nvme, Christoph Hellwig
Cc: Jens Axboe, Hannes Reinecke, Logan Gunthorpe, linux-kernel,
Martin K . Petersen
On 9/5/19 6:33 PM, Keith Busch wrote:
> The namespace disk names must be unique for the lifetime of the
> subsystem. This was accomplished by using their parent subsystems'
> instances which were allocated independently from the controllers
> connected to that subsystem. This allowed name prefixes assigned to
> namespaces to match a controller from an unrelated subsystem, and has
> created confusion among users examining device nodes.
>
> Ensure a namespace's subsystem instance never clashes with a controller
> instance of another subsystem by transferring the instance ownership
> to the parent subsystem from the first controller discovered in that
> subsystem.
>
> Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
> v1 -> v2:
>
> Changelog: reduce sensationalism, fix spelling
>
> drivers/nvme/host/core.c | 21 ++++++++++-----------
> 1 file changed, 10 insertions(+), 11 deletions(-)
>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@suse.de +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 247165 (AG München), GF: Felix Imendörffer
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCHv2] nvme: Assign subsy instance from first ctrl
@ 2019-09-09 10:32 ` Hannes Reinecke
0 siblings, 0 replies; 10+ messages in thread
From: Hannes Reinecke @ 2019-09-09 10:32 UTC (permalink / raw)
To: Keith Busch, Sagi Grimberg, linux-nvme, Christoph Hellwig
Cc: Logan Gunthorpe, Jens Axboe, Martin K . Petersen, Hannes Reinecke,
linux-kernel
On 9/5/19 6:33 PM, Keith Busch wrote:
> The namespace disk names must be unique for the lifetime of the
> subsystem. This was accomplished by using their parent subsystems'
> instances which were allocated independently from the controllers
> connected to that subsystem. This allowed name prefixes assigned to
> namespaces to match a controller from an unrelated subsystem, and has
> created confusion among users examining device nodes.
>
> Ensure a namespace's subsystem instance never clashes with a controller
> instance of another subsystem by transferring the instance ownership
> to the parent subsystem from the first controller discovered in that
> subsystem.
>
> Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
> v1 -> v2:
>
> Changelog: reduce sensationalism, fix spelling
>
> drivers/nvme/host/core.c | 21 ++++++++++-----------
> 1 file changed, 10 insertions(+), 11 deletions(-)
>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@suse.de +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 247165 (AG München), GF: Felix Imendörffer
^ permalink raw reply [flat|nested] 10+ messages in thread