| Message ID | 1449058583-27940-3-git-send-email-m@bjorling.me (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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-block" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> > +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-block" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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);
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(-)