Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1281708 > unrolled thread

[PATCH RFC 3/3] lightnvm: fix media mgr registration

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH RFC 3/3] lightnvm: fix media mgr registration Matias Bjørling <m@bjorling.me> - 2015-12-02 13:20 +0100
    Re: [PATCH RFC 3/3] lightnvm: fix media mgr registration Wenwei Tao <ww.tao0320@gmail.com> - 2015-12-04 09:50 +0100
      Re: [PATCH RFC 3/3] lightnvm: fix media mgr registration Matias Bjørling <m@bjorling.me> - 2015-12-04 11:50 +0100

#1281708 — [PATCH RFC 3/3] lightnvm: fix media mgr registration

The ppa pool can be used at media manager registration.
Therefore the ppa pool should be allocated before registering.

If a media manager can't be found, this should not lead to the
device being unallocated. A media manager can be registered later, that
can manage a device. Only warn if a media manager fails initialization.

Additionally, protect against the media managed being registered or
deregistered while looping through available media managers. This was
reported by Wenwei Tao.

Signed-off-by: Matias Bjørling <m@bjorling.me>
---
 drivers/lightnvm/core.c | 79 ++++++++++++++++++++++++-------------------------
 1 file changed, 39 insertions(+), 40 deletions(-)

diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
index 4601cf1..f4d5291 100644
--- a/drivers/lightnvm/core.c
+++ b/drivers/lightnvm/core.c
@@ -97,15 +97,47 @@ static struct nvmm_type *nvm_find_mgr_type(const char *name)
 	return NULL;
 }
 
+struct nvmm_type *nvm_init_mgr(struct nvm_dev *dev)
+{
+	struct nvmm_type *mt;
+	int ret;
+
+	lockdep_assert_held(&nvm_lock);
+
+	list_for_each_entry(mt, &nvm_mgrs, list) {
+		ret = mt->register_mgr(dev);
+		if (ret < 0) {
+			pr_err("nvm: media mgr failed to init (%d) on dev %s\n",
+								ret, dev->name);
+			return NULL; /* initialization failed */
+		} else if (ret > 0)
+			return mt;
+	}
+
+	return NULL;
+}
+
 int nvm_register_mgr(struct nvmm_type *mt)
 {
+	struct nvm_dev *dev;
 	int ret = 0;
 
 	down_write(&nvm_lock);
-	if (nvm_find_mgr_type(mt->name))
+	if (nvm_find_mgr_type(mt->name)) {
 		ret = -EEXIST;
-	else
+		goto finish;
+	} else {
 		list_add(&mt->list, &nvm_mgrs);
+	}
+
+	/* try to register media mgr if any device have none configured */
+	list_for_each_entry(dev, &nvm_devices, devices) {
+		if (dev->mt)
+			continue;
+
+		dev->mt = nvm_init_mgr(dev);
+	}
+finish:
 	up_write(&nvm_lock);
 
 	return ret;
@@ -221,7 +253,6 @@ static void nvm_free(struct nvm_dev *dev)
 
 static int nvm_init(struct nvm_dev *dev)
 {
-	struct nvmm_type *mt;
 	int ret = -EINVAL;
 
 	if (!dev->q || !dev->ops)
@@ -252,22 +283,6 @@ static int nvm_init(struct nvm_dev *dev)
 		goto err;
 	}
 
-	/* register with device with a supported manager */
-	list_for_each_entry(mt, &nvm_mgrs, list) {
-		ret = mt->register_mgr(dev);
-		if (ret < 0)
-			goto err; /* initialization failed */
-		if (ret > 0) {
-			dev->mt = mt;
-			break; /* successfully initialized */
-		}
-	}
-
-	if (!ret) {
-		pr_info("nvm: no compatible manager found.\n");
-		return 0;
-	}
-
 	pr_info("nvm: registered %s [%u/%u/%u/%u/%u/%u]\n",
 			dev->name, dev->sec_per_pg, dev->nr_planes,
 			dev->pgs_per_blk, dev->blks_per_lun, dev->nr_luns,
@@ -324,7 +339,9 @@ int nvm_register(struct request_queue *q, char *disk_name,
 		}
 	}
 
+	/* register device with a supported media manager */
 	down_write(&nvm_lock);
+	dev->mt = nvm_init_mgr(dev);
 	list_add(&dev->devices, &nvm_devices);
 	up_write(&nvm_lock);
 
@@ -365,35 +382,17 @@ static int nvm_create_target(struct nvm_dev *dev,
 {
 	struct nvm_ioctl_create_simple *s = &create->conf.s;
 	struct request_queue *tqueue;
-	struct nvmm_type *mt;
 	struct gendisk *tdisk;
 	struct nvm_tgt_type *tt;
 	struct nvm_target *t;
 	void *targetdata;
-	int ret = 0;
 
-	down_write(&nvm_lock);
 	if (!dev->mt) {
-		/* register with device with a supported NVM manager */
-		list_for_each_entry(mt, &nvm_mgrs, list) {
-			ret = mt->register_mgr(dev);
-			if (ret < 0) {
-				up_write(&nvm_lock);
-				return ret; /* initialization failed */
-			}
-			if (ret > 0) {
-				dev->mt = mt;
-				break; /* successfully initialized */
-			}
-		}
-
-		if (!ret) {
-			up_write(&nvm_lock);
-			pr_info("nvm: no compatible nvm manager found.\n");
-			return -ENODEV;
-		}
+		pr_info("nvm: device has no media manager registered.\n");
+		return -ENODEV;
 	}
 
+	down_write(&nvm_lock);
 	tt = nvm_find_target_type(create->tgttype);
 	if (!tt) {
 		pr_err("nvm: target type %s not found\n", create->tgttype);
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1283651

2015-12-02 20:16 GMT+08:00 Matias Bjørling <m@bjorling.me>:
> The ppa pool can be used at media manager registration.
> Therefore the ppa pool should be allocated before registering.
>
> If a media manager can't be found, this should not lead to the
> device being unallocated. A media manager can be registered later, that
> can manage a device. Only warn if a media manager fails initialization.
>
> Additionally, protect against the media managed being registered or
> deregistered while looping through available media managers. This was
> reported by Wenwei Tao.
>
> Signed-off-by: Matias Bjørling <m@bjorling.me>
> ---
>  drivers/lightnvm/core.c | 79 ++++++++++++++++++++++++-------------------------
>  1 file changed, 39 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
> index 4601cf1..f4d5291 100644
> --- a/drivers/lightnvm/core.c
> +++ b/drivers/lightnvm/core.c
> @@ -97,15 +97,47 @@ static struct nvmm_type *nvm_find_mgr_type(const char *name)
>         return NULL;
>  }
>
> +struct nvmm_type *nvm_init_mgr(struct nvm_dev *dev)
> +{
> +       struct nvmm_type *mt;
> +       int ret;
> +
> +       lockdep_assert_held(&nvm_lock);
> +
> +       list_for_each_entry(mt, &nvm_mgrs, list) {
> +               ret = mt->register_mgr(dev);
> +               if (ret < 0) {
> +                       pr_err("nvm: media mgr failed to init (%d) on dev %s\n",
> +                                                               ret, dev->name);
> +                       return NULL; /* initialization failed */

Do we just return or continue to try next media manager ?  In my commit

commit d0a712ceb83ebaea32d520825ee7b997f59b168f

Author: Wenwei Tao <ww.tao0320@gmail.com>

Date:   Sat Nov 28 16:49:28 2015 +0100

    lightnvm: missing nvm_lock acquire

I use the second solution.


> +               } else if (ret > 0)
> +                       return mt;
> +       }
> +
> +       return NULL;
> +}
> +
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1283696

> > +struct nvmm_type *nvm_init_mgr(struct nvm_dev *dev)
>> +{
>> +       struct nvmm_type *mt;
>> +       int ret;
>> +
>> +       lockdep_assert_held(&nvm_lock);
>> +
>> +       list_for_each_entry(mt, &nvm_mgrs, list) {
>> +               ret = mt->register_mgr(dev);
>> +               if (ret < 0) {
>> +                       pr_err("nvm: media mgr failed to init (%d) on dev %s\n",
>> +                                                               ret, dev->name);
>> +                       return NULL; /* initialization failed */
>
> Do we just return or continue to try next media manager ?  In my commit
>
> commit d0a712ceb83ebaea32d520825ee7b997f59b168f
>
> Author: Wenwei Tao <ww.tao0320@gmail.com>
>
> Date:   Sat Nov 28 16:49:28 2015 +0100
>
>      lightnvm: missing nvm_lock acquire
>
> I use the second solution.
>

We just return. With the system blocks, there might now have been 
registered a media manager, and that is why it should just give up if 
nothing is found. On media manager module init, it will then attach 
appropriately.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web