* [PATCH] nvme: keep ctrl->namespaces ordered
@ 2021-09-15 7:20 Christoph Hellwig
2021-09-15 7:36 ` Damien Le Moal
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Christoph Hellwig @ 2021-09-15 7:20 UTC (permalink / raw)
To: kbusch, sagi; +Cc: anton.eidelman, linux-nvme
Various places in the nvme code that rely on ctrl->namespace to be
ordered. Ensure that the namespae is inserted into the list at the
right position from the start instead of sorting it after the fact.
Fixes: 540c801c65eb ("NVMe: Implement namespace list scanning")
Reported-by: Anton Eidelman <anton.eidelman@gmail.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/nvme/host/core.c | 38 ++++++++++++++++++++------------------
1 file changed, 20 insertions(+), 18 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 97f8211cf92c1..3e3c835777458 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -13,7 +13,6 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/backing-dev.h>
-#include <linux/list_sort.h>
#include <linux/slab.h>
#include <linux/types.h>
#include <linux/pr.h>
@@ -3716,15 +3715,6 @@ static int nvme_init_ns_head(struct nvme_ns *ns, unsigned nsid,
return ret;
}
-static int ns_cmp(void *priv, const struct list_head *a,
- const struct list_head *b)
-{
- struct nvme_ns *nsa = container_of(a, struct nvme_ns, list);
- struct nvme_ns *nsb = container_of(b, struct nvme_ns, list);
-
- return nsa->head->ns_id - nsb->head->ns_id;
-}
-
struct nvme_ns *nvme_find_get_ns(struct nvme_ctrl *ctrl, unsigned nsid)
{
struct nvme_ns *ns, *ret = NULL;
@@ -3745,6 +3735,23 @@ struct nvme_ns *nvme_find_get_ns(struct nvme_ctrl *ctrl, unsigned nsid)
}
EXPORT_SYMBOL_NS_GPL(nvme_find_get_ns, NVME_TARGET_PASSTHRU);
+/*
+ * Add the namespace to the controller list while keeping the list ordered.
+ */
+static void nvme_ns_add_to_ctrl_list(struct nvme_ns *ns)
+{
+ struct nvme_ns *tmp;
+
+ list_for_each_entry_reverse(tmp, &ns->ctrl->namespaces, list) {
+ if (tmp->head->ns_id < ns->head->ns_id) {
+ list_add(&tmp->list, &ns->ctrl->namespaces);
+ return;
+ }
+ }
+
+ list_add_tail(&ns->list, &ns->ctrl->namespaces);
+}
+
static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid,
struct nvme_ns_ids *ids)
{
@@ -3794,10 +3801,9 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid,
if (nvme_update_ns_info(ns, id))
goto out_unlink_ns;
- down_write(&ctrl->namespaces_rwsem);
- list_add_tail(&ns->list, &ctrl->namespaces);
- up_write(&ctrl->namespaces_rwsem);
-
+ down_write(&ns->ctrl->namespaces_rwsem);
+ nvme_ns_add_to_ctrl_list(ns);
+ up_write(&ns->ctrl->namespaces_rwsem);
nvme_get_ctrl(ctrl);
if (device_add_disk(ctrl->device, ns->disk, nvme_ns_id_attr_groups))
@@ -4082,10 +4088,6 @@ static void nvme_scan_work(struct work_struct *work)
if (nvme_scan_ns_list(ctrl) != 0)
nvme_scan_ns_sequential(ctrl);
mutex_unlock(&ctrl->scan_lock);
-
- down_write(&ctrl->namespaces_rwsem);
- list_sort(NULL, &ctrl->namespaces, ns_cmp);
- up_write(&ctrl->namespaces_rwsem);
}
/*
--
2.30.2
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH] nvme: keep ctrl->namespaces ordered
2021-09-15 7:20 [PATCH] nvme: keep ctrl->namespaces ordered Christoph Hellwig
@ 2021-09-15 7:36 ` Damien Le Moal
2021-09-15 12:37 ` Keith Busch
2021-09-15 9:01 ` Damien Le Moal
` (2 subsequent siblings)
3 siblings, 1 reply; 12+ messages in thread
From: Damien Le Moal @ 2021-09-15 7:36 UTC (permalink / raw)
To: Christoph Hellwig, kbusch, sagi; +Cc: anton.eidelman, linux-nvme
On 2021/09/15 16:27, Christoph Hellwig wrote:
> Various places in the nvme code that rely on ctrl->namespace to be
> ordered. Ensure that the namespae is inserted into the list at the
> right position from the start instead of sorting it after the fact.
>
> Fixes: 540c801c65eb ("NVMe: Implement namespace list scanning")
> Reported-by: Anton Eidelman <anton.eidelman@gmail.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/nvme/host/core.c | 38 ++++++++++++++++++++------------------
> 1 file changed, 20 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 97f8211cf92c1..3e3c835777458 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -13,7 +13,6 @@
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/backing-dev.h>
> -#include <linux/list_sort.h>
> #include <linux/slab.h>
> #include <linux/types.h>
> #include <linux/pr.h>
> @@ -3716,15 +3715,6 @@ static int nvme_init_ns_head(struct nvme_ns *ns, unsigned nsid,
> return ret;
> }
>
> -static int ns_cmp(void *priv, const struct list_head *a,
> - const struct list_head *b)
> -{
> - struct nvme_ns *nsa = container_of(a, struct nvme_ns, list);
> - struct nvme_ns *nsb = container_of(b, struct nvme_ns, list);
> -
> - return nsa->head->ns_id - nsb->head->ns_id;
> -}
> -
> struct nvme_ns *nvme_find_get_ns(struct nvme_ctrl *ctrl, unsigned nsid)
> {
> struct nvme_ns *ns, *ret = NULL;
> @@ -3745,6 +3735,23 @@ struct nvme_ns *nvme_find_get_ns(struct nvme_ctrl *ctrl, unsigned nsid)
> }
> EXPORT_SYMBOL_NS_GPL(nvme_find_get_ns, NVME_TARGET_PASSTHRU);
>
> +/*
> + * Add the namespace to the controller list while keeping the list ordered.
> + */
> +static void nvme_ns_add_to_ctrl_list(struct nvme_ns *ns)
> +{
> + struct nvme_ns *tmp;
> +
> + list_for_each_entry_reverse(tmp, &ns->ctrl->namespaces, list) {
> + if (tmp->head->ns_id < ns->head->ns_id) {
> + list_add(&tmp->list, &ns->ctrl->namespaces);
> + return;
> + }
> + }
> +
> + list_add_tail(&ns->list, &ns->ctrl->namespaces);
I wonder if it is worth checking that ns->head->id is not equal to the ID of the
first NS in the list. That can only happen for a very buggy system or device,
but that is not unheard of...
> +}
> +
> static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid,
> struct nvme_ns_ids *ids)
> {
> @@ -3794,10 +3801,9 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid,
> if (nvme_update_ns_info(ns, id))
> goto out_unlink_ns;
>
> - down_write(&ctrl->namespaces_rwsem);
> - list_add_tail(&ns->list, &ctrl->namespaces);
> - up_write(&ctrl->namespaces_rwsem);
> -
> + down_write(&ns->ctrl->namespaces_rwsem);
> + nvme_ns_add_to_ctrl_list(ns);
> + up_write(&ns->ctrl->namespaces_rwsem);
> nvme_get_ctrl(ctrl);
>
> if (device_add_disk(ctrl->device, ns->disk, nvme_ns_id_attr_groups))
> @@ -4082,10 +4088,6 @@ static void nvme_scan_work(struct work_struct *work)
> if (nvme_scan_ns_list(ctrl) != 0)
> nvme_scan_ns_sequential(ctrl);
> mutex_unlock(&ctrl->scan_lock);
> -
> - down_write(&ctrl->namespaces_rwsem);
> - list_sort(NULL, &ctrl->namespaces, ns_cmp);
> - up_write(&ctrl->namespaces_rwsem);
> }
>
> /*
>
--
Damien Le Moal
Western Digital Research
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] nvme: keep ctrl->namespaces ordered
2021-09-15 7:36 ` Damien Le Moal
@ 2021-09-15 12:37 ` Keith Busch
2021-09-15 13:47 ` Christoph Hellwig
0 siblings, 1 reply; 12+ messages in thread
From: Keith Busch @ 2021-09-15 12:37 UTC (permalink / raw)
To: Damien Le Moal; +Cc: Christoph Hellwig, sagi, anton.eidelman, linux-nvme
On Wed, Sep 15, 2021 at 07:36:04AM +0000, Damien Le Moal wrote:
> > +static void nvme_ns_add_to_ctrl_list(struct nvme_ns *ns)
> > +{
> > + struct nvme_ns *tmp;
> > +
> > + list_for_each_entry_reverse(tmp, &ns->ctrl->namespaces, list) {
> > + if (tmp->head->ns_id < ns->head->ns_id) {
> > + list_add(&tmp->list, &ns->ctrl->namespaces);
> > + return;
> > + }
> > + }
> > +
> > + list_add_tail(&ns->list, &ns->ctrl->namespaces);
>
> I wonder if it is worth checking that ns->head->id is not equal to the ID of the
> first NS in the list. That can only happen for a very buggy system or device,
> but that is not unheard of...
We would never be able to hit the condition of two heads with matching
NSID's. If the controller was broken and provided duplicate NSIDs in the
namespace list, this driver will just recheck the first namespace rather
than allocate a new one.
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] nvme: keep ctrl->namespaces ordered
2021-09-15 12:37 ` Keith Busch
@ 2021-09-15 13:47 ` Christoph Hellwig
0 siblings, 0 replies; 12+ messages in thread
From: Christoph Hellwig @ 2021-09-15 13:47 UTC (permalink / raw)
To: Keith Busch
Cc: Damien Le Moal, Christoph Hellwig, sagi, anton.eidelman, linux-nvme
On Wed, Sep 15, 2021 at 05:37:35AM -0700, Keith Busch wrote:
> On Wed, Sep 15, 2021 at 07:36:04AM +0000, Damien Le Moal wrote:
> > > +static void nvme_ns_add_to_ctrl_list(struct nvme_ns *ns)
> > > +{
> > > + struct nvme_ns *tmp;
> > > +
> > > + list_for_each_entry_reverse(tmp, &ns->ctrl->namespaces, list) {
> > > + if (tmp->head->ns_id < ns->head->ns_id) {
> > > + list_add(&tmp->list, &ns->ctrl->namespaces);
> > > + return;
> > > + }
> > > + }
> > > +
> > > + list_add_tail(&ns->list, &ns->ctrl->namespaces);
> >
> > I wonder if it is worth checking that ns->head->id is not equal to the ID of the
> > first NS in the list. That can only happen for a very buggy system or device,
> > but that is not unheard of...
>
> We would never be able to hit the condition of two heads with matching
> NSID's. If the controller was broken and provided duplicate NSIDs in the
> namespace list, this driver will just recheck the first namespace rather
> than allocate a new one.
Indeed. So back to v1 for review then.
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] nvme: keep ctrl->namespaces ordered
2021-09-15 7:20 [PATCH] nvme: keep ctrl->namespaces ordered Christoph Hellwig
2021-09-15 7:36 ` Damien Le Moal
@ 2021-09-15 9:01 ` Damien Le Moal
2021-09-15 14:02 ` Keith Busch
2021-09-15 14:10 ` Keith Busch
3 siblings, 0 replies; 12+ messages in thread
From: Damien Le Moal @ 2021-09-15 9:01 UTC (permalink / raw)
To: Christoph Hellwig, kbusch, sagi; +Cc: anton.eidelman, linux-nvme
On 2021/09/15 16:27, Christoph Hellwig wrote:
> Various places in the nvme code that rely on ctrl->namespace to be
> ordered. Ensure that the namespae is inserted into the list at the
> right position from the start instead of sorting it after the fact.
>
> Fixes: 540c801c65eb ("NVMe: Implement namespace list scanning")
> Reported-by: Anton Eidelman <anton.eidelman@gmail.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/nvme/host/core.c | 38 ++++++++++++++++++++------------------
> 1 file changed, 20 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 97f8211cf92c1..3e3c835777458 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -13,7 +13,6 @@
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/backing-dev.h>
> -#include <linux/list_sort.h>
> #include <linux/slab.h>
> #include <linux/types.h>
> #include <linux/pr.h>
> @@ -3716,15 +3715,6 @@ static int nvme_init_ns_head(struct nvme_ns *ns, unsigned nsid,
> return ret;
> }
>
> -static int ns_cmp(void *priv, const struct list_head *a,
> - const struct list_head *b)
> -{
> - struct nvme_ns *nsa = container_of(a, struct nvme_ns, list);
> - struct nvme_ns *nsb = container_of(b, struct nvme_ns, list);
> -
> - return nsa->head->ns_id - nsb->head->ns_id;
> -}
> -
> struct nvme_ns *nvme_find_get_ns(struct nvme_ctrl *ctrl, unsigned nsid)
> {
> struct nvme_ns *ns, *ret = NULL;
> @@ -3745,6 +3735,23 @@ struct nvme_ns *nvme_find_get_ns(struct nvme_ctrl *ctrl, unsigned nsid)
> }
> EXPORT_SYMBOL_NS_GPL(nvme_find_get_ns, NVME_TARGET_PASSTHRU);
>
> +/*
> + * Add the namespace to the controller list while keeping the list ordered.
> + */
> +static void nvme_ns_add_to_ctrl_list(struct nvme_ns *ns)
> +{
> + struct nvme_ns *tmp;
> +
> + list_for_each_entry_reverse(tmp, &ns->ctrl->namespaces, list) {
> + if (tmp->head->ns_id < ns->head->ns_id) {
> + list_add(&tmp->list, &ns->ctrl->namespaces);
> + return;
> + }
> + }
> +
> + list_add_tail(&ns->list, &ns->ctrl->namespaces);
> +}
> +
> static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid,
> struct nvme_ns_ids *ids)
> {
> @@ -3794,10 +3801,9 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid,
> if (nvme_update_ns_info(ns, id))
> goto out_unlink_ns;
>
> - down_write(&ctrl->namespaces_rwsem);
> - list_add_tail(&ns->list, &ctrl->namespaces);
> - up_write(&ctrl->namespaces_rwsem);
> -
> + down_write(&ns->ctrl->namespaces_rwsem);
> + nvme_ns_add_to_ctrl_list(ns);
> + up_write(&ns->ctrl->namespaces_rwsem);
> nvme_get_ctrl(ctrl);
>
> if (device_add_disk(ctrl->device, ns->disk, nvme_ns_id_attr_groups))
> @@ -4082,10 +4088,6 @@ static void nvme_scan_work(struct work_struct *work)
> if (nvme_scan_ns_list(ctrl) != 0)
> nvme_scan_ns_sequential(ctrl);
> mutex_unlock(&ctrl->scan_lock);
> -
> - down_write(&ctrl->namespaces_rwsem);
> - list_sort(NULL, &ctrl->namespaces, ns_cmp);
> - up_write(&ctrl->namespaces_rwsem);
> }
>
> /*
>
Looks OK to me.
Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>
--
Damien Le Moal
Western Digital Research
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] nvme: keep ctrl->namespaces ordered
2021-09-15 7:20 [PATCH] nvme: keep ctrl->namespaces ordered Christoph Hellwig
2021-09-15 7:36 ` Damien Le Moal
2021-09-15 9:01 ` Damien Le Moal
@ 2021-09-15 14:02 ` Keith Busch
2021-09-15 14:10 ` Keith Busch
3 siblings, 0 replies; 12+ messages in thread
From: Keith Busch @ 2021-09-15 14:02 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: sagi, anton.eidelman, linux-nvme
On Wed, Sep 15, 2021 at 09:20:06AM +0200, Christoph Hellwig wrote:
> Various places in the nvme code that rely on ctrl->namespace to be
> ordered. Ensure that the namespae is inserted into the list at the
> right position from the start instead of sorting it after the fact.
>
> Fixes: 540c801c65eb ("NVMe: Implement namespace list scanning")
> Reported-by: Anton Eidelman <anton.eidelman@gmail.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
The v1 looks good.
Reviewed-by: Keith Busch <kbusch@kernel.org>
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] nvme: keep ctrl->namespaces ordered
2021-09-15 7:20 [PATCH] nvme: keep ctrl->namespaces ordered Christoph Hellwig
` (2 preceding siblings ...)
2021-09-15 14:02 ` Keith Busch
@ 2021-09-15 14:10 ` Keith Busch
3 siblings, 0 replies; 12+ messages in thread
From: Keith Busch @ 2021-09-15 14:10 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: sagi, anton.eidelman, linux-nvme
On Wed, Sep 15, 2021 at 09:20:06AM +0200, Christoph Hellwig wrote:
> +static void nvme_ns_add_to_ctrl_list(struct nvme_ns *ns)
> +{
> + struct nvme_ns *tmp;
> +
> + list_for_each_entry_reverse(tmp, &ns->ctrl->namespaces, list) {
> + if (tmp->head->ns_id < ns->head->ns_id) {
> + list_add(&tmp->list, &ns->ctrl->namespaces);
> + return;
> + }
> + }
> +
> + list_add_tail(&ns->list, &ns->ctrl->namespaces);
Actually, this list_add_tail() looks wrong. If the new 'ns' has the
lowest nsid, then we need it added at the head, so this should just be a
"list_add()".
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] nvme: keep ctrl->namespaces ordered
@ 2021-09-15 15:37 Christoph Hellwig
2021-09-15 15:46 ` Keith Busch
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Christoph Hellwig @ 2021-09-15 15:37 UTC (permalink / raw)
To: kbusch, sagi; +Cc: anton.eidelman, linux-nvme
Various places in the nvme code that rely on ctrl->namespace to be
ordered. Ensure that the namespae is inserted into the list at the
right position from the start instead of sorting it after the fact.
Fixes: 540c801c65eb ("NVMe: Implement namespace list scanning")
Reported-by: Anton Eidelman <anton.eidelman@gmail.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/nvme/host/core.c | 33 +++++++++++++++++----------------
1 file changed, 17 insertions(+), 16 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 7efb31b87f37a..3894a8508065d 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -13,7 +13,6 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/backing-dev.h>
-#include <linux/list_sort.h>
#include <linux/slab.h>
#include <linux/types.h>
#include <linux/pr.h>
@@ -3714,15 +3713,6 @@ static int nvme_init_ns_head(struct nvme_ns *ns, unsigned nsid,
return ret;
}
-static int ns_cmp(void *priv, const struct list_head *a,
- const struct list_head *b)
-{
- struct nvme_ns *nsa = container_of(a, struct nvme_ns, list);
- struct nvme_ns *nsb = container_of(b, struct nvme_ns, list);
-
- return nsa->head->ns_id - nsb->head->ns_id;
-}
-
struct nvme_ns *nvme_find_get_ns(struct nvme_ctrl *ctrl, unsigned nsid)
{
struct nvme_ns *ns, *ret = NULL;
@@ -3743,6 +3733,22 @@ struct nvme_ns *nvme_find_get_ns(struct nvme_ctrl *ctrl, unsigned nsid)
}
EXPORT_SYMBOL_NS_GPL(nvme_find_get_ns, NVME_TARGET_PASSTHRU);
+/*
+ * Add the namespace to the controller list while keeping the list ordered.
+ */
+static void nvme_ns_add_to_ctrl_list(struct nvme_ns *ns)
+{
+ struct nvme_ns *tmp;
+
+ list_for_each_entry_reverse(tmp, &ns->ctrl->namespaces, list) {
+ if (tmp->head->ns_id < ns->head->ns_id) {
+ list_add(&ns->list, &tmp->list);
+ return;
+ }
+ }
+ list_add(&ns->list, &ns->ctrl->namespaces);
+}
+
static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid,
struct nvme_ns_ids *ids)
{
@@ -3793,9 +3799,8 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid,
goto out_unlink_ns;
down_write(&ctrl->namespaces_rwsem);
- list_add_tail(&ns->list, &ctrl->namespaces);
+ nvme_ns_add_to_ctrl_list(ns);
up_write(&ctrl->namespaces_rwsem);
-
nvme_get_ctrl(ctrl);
if (device_add_disk(ctrl->device, ns->disk, nvme_ns_id_attr_groups))
@@ -4083,10 +4088,6 @@ static void nvme_scan_work(struct work_struct *work)
if (nvme_scan_ns_list(ctrl) != 0)
nvme_scan_ns_sequential(ctrl);
mutex_unlock(&ctrl->scan_lock);
-
- down_write(&ctrl->namespaces_rwsem);
- list_sort(NULL, &ctrl->namespaces, ns_cmp);
- up_write(&ctrl->namespaces_rwsem);
}
/*
--
2.30.2
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH] nvme: keep ctrl->namespaces ordered
2021-09-15 15:37 Christoph Hellwig
@ 2021-09-15 15:46 ` Keith Busch
2021-09-15 22:00 ` Damien Le Moal
` (2 subsequent siblings)
3 siblings, 0 replies; 12+ messages in thread
From: Keith Busch @ 2021-09-15 15:46 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: sagi, anton.eidelman, linux-nvme
On Wed, Sep 15, 2021 at 05:37:51PM +0200, Christoph Hellwig wrote:
> Various places in the nvme code that rely on ctrl->namespace to be
> ordered. Ensure that the namespae is inserted into the list at the
> right position from the start instead of sorting it after the fact.
>
> Fixes: 540c801c65eb ("NVMe: Implement namespace list scanning")
> Reported-by: Anton Eidelman <anton.eidelman@gmail.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks good:
Reviewed-by: Keith Busch <kbusch@kernel.org>
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] nvme: keep ctrl->namespaces ordered
2021-09-15 15:37 Christoph Hellwig
2021-09-15 15:46 ` Keith Busch
@ 2021-09-15 22:00 ` Damien Le Moal
2021-09-16 16:53 ` Chaitanya Kulkarni
2021-09-19 10:03 ` Sagi Grimberg
3 siblings, 0 replies; 12+ messages in thread
From: Damien Le Moal @ 2021-09-15 22:00 UTC (permalink / raw)
To: Christoph Hellwig, kbusch, sagi; +Cc: anton.eidelman, linux-nvme
On 2021/09/16 0:43, Christoph Hellwig wrote:
> Various places in the nvme code that rely on ctrl->namespace to be
> ordered. Ensure that the namespae is inserted into the list at the
> right position from the start instead of sorting it after the fact.
>
> Fixes: 540c801c65eb ("NVMe: Implement namespace list scanning")
> Reported-by: Anton Eidelman <anton.eidelman@gmail.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/nvme/host/core.c | 33 +++++++++++++++++----------------
> 1 file changed, 17 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 7efb31b87f37a..3894a8508065d 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -13,7 +13,6 @@
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/backing-dev.h>
> -#include <linux/list_sort.h>
> #include <linux/slab.h>
> #include <linux/types.h>
> #include <linux/pr.h>
> @@ -3714,15 +3713,6 @@ static int nvme_init_ns_head(struct nvme_ns *ns, unsigned nsid,
> return ret;
> }
>
> -static int ns_cmp(void *priv, const struct list_head *a,
> - const struct list_head *b)
> -{
> - struct nvme_ns *nsa = container_of(a, struct nvme_ns, list);
> - struct nvme_ns *nsb = container_of(b, struct nvme_ns, list);
> -
> - return nsa->head->ns_id - nsb->head->ns_id;
> -}
> -
> struct nvme_ns *nvme_find_get_ns(struct nvme_ctrl *ctrl, unsigned nsid)
> {
> struct nvme_ns *ns, *ret = NULL;
> @@ -3743,6 +3733,22 @@ struct nvme_ns *nvme_find_get_ns(struct nvme_ctrl *ctrl, unsigned nsid)
> }
> EXPORT_SYMBOL_NS_GPL(nvme_find_get_ns, NVME_TARGET_PASSTHRU);
>
> +/*
> + * Add the namespace to the controller list while keeping the list ordered.
> + */
May be add a note here about the impossibility to ever get a duplicate NS IDs ?
> +static void nvme_ns_add_to_ctrl_list(struct nvme_ns *ns)
> +{
> + struct nvme_ns *tmp;
> +
> + list_for_each_entry_reverse(tmp, &ns->ctrl->namespaces, list) {
> + if (tmp->head->ns_id < ns->head->ns_id) {
> + list_add(&ns->list, &tmp->list);
> + return;
> + }
> + }
> + list_add(&ns->list, &ns->ctrl->namespaces);
> +}
> +
> static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid,
> struct nvme_ns_ids *ids)
> {
> @@ -3793,9 +3799,8 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid,
> goto out_unlink_ns;
>
> down_write(&ctrl->namespaces_rwsem);
> - list_add_tail(&ns->list, &ctrl->namespaces);
> + nvme_ns_add_to_ctrl_list(ns);
> up_write(&ctrl->namespaces_rwsem);
> -
> nvme_get_ctrl(ctrl);
>
> if (device_add_disk(ctrl->device, ns->disk, nvme_ns_id_attr_groups))
> @@ -4083,10 +4088,6 @@ static void nvme_scan_work(struct work_struct *work)
> if (nvme_scan_ns_list(ctrl) != 0)
> nvme_scan_ns_sequential(ctrl);
> mutex_unlock(&ctrl->scan_lock);
> -
> - down_write(&ctrl->namespaces_rwsem);
> - list_sort(NULL, &ctrl->namespaces, ns_cmp);
> - up_write(&ctrl->namespaces_rwsem);
> }
>
> /*
>
Apart from the nit above, looks OK to me.
Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>
--
Damien Le Moal
Western Digital Research
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] nvme: keep ctrl->namespaces ordered
2021-09-15 15:37 Christoph Hellwig
2021-09-15 15:46 ` Keith Busch
2021-09-15 22:00 ` Damien Le Moal
@ 2021-09-16 16:53 ` Chaitanya Kulkarni
2021-09-19 10:03 ` Sagi Grimberg
3 siblings, 0 replies; 12+ messages in thread
From: Chaitanya Kulkarni @ 2021-09-16 16:53 UTC (permalink / raw)
To: linux-nvme
On 9/15/21 8:37 AM, Christoph Hellwig wrote:
> Various places in the nvme code that rely on ctrl->namespace to be
> ordered. Ensure that the namespae is inserted into the list at the
> right position from the start instead of sorting it after the fact.
>
> Fixes: 540c801c65eb ("NVMe: Implement namespace list scanning")
> Reported-by: Anton Eidelman <anton.eidelman@gmail.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
No sorting logic anymore :).
Looks good to me.
Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] nvme: keep ctrl->namespaces ordered
2021-09-15 15:37 Christoph Hellwig
` (2 preceding siblings ...)
2021-09-16 16:53 ` Chaitanya Kulkarni
@ 2021-09-19 10:03 ` Sagi Grimberg
3 siblings, 0 replies; 12+ messages in thread
From: Sagi Grimberg @ 2021-09-19 10:03 UTC (permalink / raw)
To: Christoph Hellwig, kbusch; +Cc: anton.eidelman, linux-nvme
Reviewed-by: Sagi Grimberg <sagi@grimberg.me
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-09-19 10:03 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-15 7:20 [PATCH] nvme: keep ctrl->namespaces ordered Christoph Hellwig
2021-09-15 7:36 ` Damien Le Moal
2021-09-15 12:37 ` Keith Busch
2021-09-15 13:47 ` Christoph Hellwig
2021-09-15 9:01 ` Damien Le Moal
2021-09-15 14:02 ` Keith Busch
2021-09-15 14:10 ` Keith Busch
2021-09-15 15:37 Christoph Hellwig
2021-09-15 15:46 ` Keith Busch
2021-09-15 22:00 ` Damien Le Moal
2021-09-16 16:53 ` Chaitanya Kulkarni
2021-09-19 10:03 ` Sagi Grimberg
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.