* [PATCH 0/3] fixes fo LightNVM
@ 2015-11-25 12:42 Wenwei Tao
2015-11-25 12:42 ` [PATCH 1/3] lightnvm: missing nvm_lock acquire Wenwei Tao
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Wenwei Tao @ 2015-11-25 12:42 UTC (permalink / raw)
To: mb, keith.busch, axboe; +Cc: linux-kernel, linux-block, linux-nvme
Hi
A couple of fixes.
Patch 1 add the missing lock aquire
Patch 2-3 handle the targets when corresponding underlying devices were removed
Wenwei Tao (3):
lightnvm: missing nvm_lock acquire
lightnvm: handle targets when corresponding nvm device exit
nvme: change the interface between nvme and lightnvm
drivers/lightnvm/core.c | 140 ++++++++++++++++++++++++++++---------------
drivers/nvme/host/lightnvm.c | 17 +++++-
drivers/nvme/host/nvme.h | 1 +
drivers/nvme/host/pci.c | 10 ++--
include/linux/lightnvm.h | 3 +
5 files changed, 118 insertions(+), 53 deletions(-)
--
1.8.3.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/3] lightnvm: missing nvm_lock acquire
2015-11-25 12:42 [PATCH 0/3] fixes fo LightNVM Wenwei Tao
@ 2015-11-25 12:42 ` Wenwei Tao
2015-11-25 15:35 ` Matias Bjørling
2015-11-25 12:42 ` [PATCH 2/3] lightnvm: handle targets when corresponding nvm device exit Wenwei Tao
2015-11-25 12:42 ` [PATCH 3/3] nvme: change the interface between nvme and lightnvm Wenwei Tao
2 siblings, 1 reply; 9+ messages in thread
From: Wenwei Tao @ 2015-11-25 12:42 UTC (permalink / raw)
To: mb, keith.busch, axboe; +Cc: linux-kernel, linux-block, linux-nvme
To avoid race conditions, traverse dev, media manager,
and targeet lists and also register, unregister entries
to/from them, should be always under the nvm_lock control.
Signed-off-by: Wenwei Tao <ww.tao0320@gmail.com>
---
drivers/lightnvm/core.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
index f659e60..39aec3a 100644
--- a/drivers/lightnvm/core.c
+++ b/drivers/lightnvm/core.c
@@ -333,19 +333,19 @@ EXPORT_SYMBOL(nvm_register);
void nvm_unregister(char *disk_name)
{
+ down_write(&nvm_lock);
struct nvm_dev *dev = nvm_find_nvm_dev(disk_name);
if (!dev) {
pr_err("nvm: could not find device %s to unregister\n",
disk_name);
+ up_write(&nvm_lock);
return;
}
- nvm_exit(dev);
-
- down_write(&nvm_lock);
list_del(&dev->devices);
up_write(&nvm_lock);
+ nvm_exit(dev);
}
EXPORT_SYMBOL(nvm_unregister);
@@ -365,12 +365,15 @@ static int nvm_create_target(struct nvm_dev *dev,
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)
+ if (ret < 0) {
+ up_write(&nvm_lock);
return ret; /* initialization failed */
+ }
if (ret > 0) {
dev->mt = mt;
break; /* successfully initialized */
@@ -379,6 +382,7 @@ static int nvm_create_target(struct nvm_dev *dev,
if (!ret) {
pr_info("nvm: no compatible nvm manager found.\n");
+ up_write(&nvm_lock);
return -ENODEV;
}
}
@@ -386,10 +390,10 @@ static int nvm_create_target(struct nvm_dev *dev,
tt = nvm_find_target_type(create->tgttype);
if (!tt) {
pr_err("nvm: target type %s not found\n", create->tgttype);
+ up_write(&nvm_lock);
return -EINVAL;
}
- down_write(&nvm_lock);
list_for_each_entry(t, &dev->online_targets, list) {
if (!strcmp(create->tgtname, t->disk->disk_name)) {
pr_err("nvm: target name already exists.\n");
@@ -472,8 +476,9 @@ static int __nvm_configure_create(struct nvm_ioctl_create *create)
{
struct nvm_dev *dev;
struct nvm_ioctl_create_simple *s;
-
+ down_write(&nvm_lock);
dev = nvm_find_nvm_dev(create->dev);
+ up_write(&nvm_lock);
if (!dev) {
pr_err("nvm: device not found\n");
return -EINVAL;
@@ -532,7 +537,9 @@ static int nvm_configure_show(const char *val)
return -EINVAL;
}
+ down_write(&nvm_lock);
dev = nvm_find_nvm_dev(devname);
+ up_write(&nvm_lock);
if (!dev) {
pr_err("nvm: device not found\n");
return -EINVAL;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/3] lightnvm: handle targets when corresponding nvm device exit
2015-11-25 12:42 [PATCH 0/3] fixes fo LightNVM Wenwei Tao
2015-11-25 12:42 ` [PATCH 1/3] lightnvm: missing nvm_lock acquire Wenwei Tao
@ 2015-11-25 12:42 ` Wenwei Tao
2015-11-25 15:39 ` Matias Bjørling
2015-11-25 12:42 ` [PATCH 3/3] nvme: change the interface between nvme and lightnvm Wenwei Tao
2 siblings, 1 reply; 9+ messages in thread
From: Wenwei Tao @ 2015-11-25 12:42 UTC (permalink / raw)
To: mb, keith.busch, axboe; +Cc: linux-kernel, linux-block, linux-nvme
block creations of new targets, remove exiting targets when
underlying device was gone.
Signed-off-by: Wenwei Tao <ww.tao0320@gmail.com>
---
drivers/lightnvm/core.c | 127 ++++++++++++++++++++++++++++++-----------------
include/linux/lightnvm.h | 3 ++
2 files changed, 85 insertions(+), 45 deletions(-)
diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
index 39aec3a..0b71dd2 100644
--- a/drivers/lightnvm/core.c
+++ b/drivers/lightnvm/core.c
@@ -33,6 +33,17 @@ static LIST_HEAD(nvm_targets);
static LIST_HEAD(nvm_mgrs);
static LIST_HEAD(nvm_devices);
static DECLARE_RWSEM(nvm_lock);
+#define NVM_EXITING 1
+
+static inline int NvmExiting(struct nvm_dev *dev)
+{
+ return ((unsigned long)dev->ppalist_pool & NVM_EXITING) != 0;
+}
+
+static inline void *ppalist_pool(struct nvm_dev *dev)
+{
+ return (void *)((unsigned long)dev->ppalist_pool & ~NVM_EXITING);
+}
static struct nvm_tgt_type *nvm_find_target_type(const char *name)
{
@@ -74,7 +85,7 @@ EXPORT_SYMBOL(nvm_unregister_target);
void *nvm_dev_dma_alloc(struct nvm_dev *dev, gfp_t mem_flags,
dma_addr_t *dma_handler)
{
- return dev->ops->dev_dma_alloc(dev->q, dev->ppalist_pool, mem_flags,
+ return dev->ops->dev_dma_alloc(dev->q, ppalist_pool(dev), mem_flags,
dma_handler);
}
EXPORT_SYMBOL(nvm_dev_dma_alloc);
@@ -82,7 +93,7 @@ EXPORT_SYMBOL(nvm_dev_dma_alloc);
void nvm_dev_dma_free(struct nvm_dev *dev, void *ppa_list,
dma_addr_t dma_handler)
{
- dev->ops->dev_dma_free(dev->ppalist_pool, ppa_list, dma_handler);
+ dev->ops->dev_dma_free(ppalist_pool(dev), ppa_list, dma_handler);
}
EXPORT_SYMBOL(nvm_dev_dma_free);
@@ -206,17 +217,6 @@ static int nvm_core_init(struct nvm_dev *dev)
return 0;
}
-static void nvm_free(struct nvm_dev *dev)
-{
- if (!dev)
- return;
-
- if (dev->mt)
- dev->mt->unregister_mgr(dev);
-
- nvm_core_free(dev);
-}
-
static int nvm_init(struct nvm_dev *dev)
{
struct nvmm_type *mt;
@@ -262,6 +262,7 @@ static int nvm_init(struct nvm_dev *dev)
}
}
+ kref_init(&dev->kref);
if (!ret) {
pr_info("nvm: no compatible manager found.\n");
return 0;
@@ -278,11 +279,47 @@ err:
return ret;
}
-static void nvm_exit(struct nvm_dev *dev)
+static void nvm_remove_target(struct nvm_target *t)
{
- if (dev->ppalist_pool)
- dev->ops->destroy_dma_pool(dev->ppalist_pool);
- nvm_free(dev);
+ struct nvm_tgt_type *tt = t->type;
+ struct gendisk *tdisk = t->disk;
+ struct request_queue *q = tdisk->queue;
+
+ lockdep_assert_held(&nvm_lock);
+
+ del_gendisk(tdisk);
+ if (tt->exit)
+ tt->exit(tdisk->private_data);
+
+ blk_cleanup_queue(q);
+
+ put_disk(tdisk);
+
+ list_del(&t->list);
+ kfree(t);
+}
+
+static inline void nvm_remove_targets(struct nvm_dev *dev)
+{
+ struct nvm_target *t, *n;
+
+ list_for_each_entry_safe(t, n, &dev->online_targets, list)
+ nvm_remove_target(t);
+}
+
+static void nvm_exit(struct kref *kref)
+{
+ struct nvm_dev *dev;
+
+ dev = container_of(kref, struct nvm_dev, kref);
+ if (ppalist_pool(dev))
+ dev->ops->destroy_dma_pool(ppalist_pool(dev));
+
+ if (dev->mt)
+ dev->mt->unregister_mgr(dev);
+
+ if (dev->ops->dev_remove)
+ dev->ops->dev_remove(dev->q);
pr_info("nvm: successfully unloaded\n");
}
@@ -344,8 +381,10 @@ void nvm_unregister(char *disk_name)
}
list_del(&dev->devices);
+ nvm_remove_targets(dev);
+ dev->ppalist_pool += NVM_EXITING;
up_write(&nvm_lock);
- nvm_exit(dev);
+ kref_put(&dev->kref, nvm_exit);
}
EXPORT_SYMBOL(nvm_unregister);
@@ -366,6 +405,10 @@ static int nvm_create_target(struct nvm_dev *dev,
int ret = 0;
down_write(&nvm_lock);
+ if (NvmExiting(dev)) {
+ up_write(&nvm_lock);
+ return -ENODEV;
+ }
if (!dev->mt) {
/* register with device with a supported NVM manager */
list_for_each_entry(mt, &nvm_mgrs, list) {
@@ -439,10 +482,16 @@ static int nvm_create_target(struct nvm_dev *dev,
t->disk = tdisk;
down_write(&nvm_lock);
+ if (NvmExiting(dev)) {
+ up_write(&nvm_lock);
+ goto err_nvm_exiting;
+ }
list_add_tail(&t->list, &dev->online_targets);
up_write(&nvm_lock);
return 0;
+err_nvm_exiting:
+ del_gendisk(tdisk);
err_init:
put_disk(tdisk);
err_queue:
@@ -452,62 +501,50 @@ err_t:
return -ENOMEM;
}
-static void nvm_remove_target(struct nvm_target *t)
-{
- struct nvm_tgt_type *tt = t->type;
- struct gendisk *tdisk = t->disk;
- struct request_queue *q = tdisk->queue;
-
- lockdep_assert_held(&nvm_lock);
-
- del_gendisk(tdisk);
- if (tt->exit)
- tt->exit(tdisk->private_data);
-
- blk_cleanup_queue(q);
-
- put_disk(tdisk);
-
- list_del(&t->list);
- kfree(t);
-}
-
static int __nvm_configure_create(struct nvm_ioctl_create *create)
{
struct nvm_dev *dev;
struct nvm_ioctl_create_simple *s;
+ int ret = -EINVAL;
+
down_write(&nvm_lock);
dev = nvm_find_nvm_dev(create->dev);
- up_write(&nvm_lock);
if (!dev) {
pr_err("nvm: device not found\n");
- return -EINVAL;
+ up_write(&nvm_lock);
+ goto out;
}
+ kref_get(&dev->kref);
+ up_write(&nvm_lock);
if (create->conf.type != NVM_CONFIG_TYPE_SIMPLE) {
pr_err("nvm: config type not valid\n");
- return -EINVAL;
+ goto out;
}
s = &create->conf.s;
if (s->lun_begin > s->lun_end || s->lun_end > dev->nr_luns) {
pr_err("nvm: lun out of bound (%u:%u > %u)\n",
s->lun_begin, s->lun_end, dev->nr_luns);
- return -EINVAL;
+ goto out;
}
- return nvm_create_target(dev, create);
+ ret = nvm_create_target(dev, create);
+out:
+ if (dev)
+ kref_put(&dev->kref, nvm_exit);
+ return ret;
}
static int __nvm_configure_remove(struct nvm_ioctl_remove *remove)
{
- struct nvm_target *t = NULL;
+ struct nvm_target *n, *t = NULL;
struct nvm_dev *dev;
int ret = -1;
down_write(&nvm_lock);
list_for_each_entry(dev, &nvm_devices, devices)
- list_for_each_entry(t, &dev->online_targets, list) {
+ list_for_each_entry_safe(t, n, &dev->online_targets, list) {
if (!strcmp(remove->tgtname, t->disk->disk_name)) {
nvm_remove_target(t);
ret = 0;
diff --git a/include/linux/lightnvm.h b/include/linux/lightnvm.h
index 69c9057..1b42305 100644
--- a/include/linux/lightnvm.h
+++ b/include/linux/lightnvm.h
@@ -205,6 +205,7 @@ typedef void (nvm_destroy_dma_pool_fn)(void *);
typedef void *(nvm_dev_dma_alloc_fn)(struct request_queue *, void *, gfp_t,
dma_addr_t *);
typedef void (nvm_dev_dma_free_fn)(void *, void*, dma_addr_t);
+typedef void (nvm_dev_remove_fn) (struct request_queue *);
struct nvm_dev_ops {
nvm_id_fn *identity;
@@ -219,6 +220,7 @@ struct nvm_dev_ops {
nvm_destroy_dma_pool_fn *destroy_dma_pool;
nvm_dev_dma_alloc_fn *dev_dma_alloc;
nvm_dev_dma_free_fn *dev_dma_free;
+ nvm_dev_remove_fn *dev_remove;
uint8_t max_phys_sect;
};
@@ -252,6 +254,7 @@ struct nvm_dev {
/* Media manager */
struct nvmm_type *mt;
+ struct kref kref;
void *mp;
/* Device information */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/3] nvme: change the interface between nvme and lightnvm
2015-11-25 12:42 [PATCH 0/3] fixes fo LightNVM Wenwei Tao
2015-11-25 12:42 ` [PATCH 1/3] lightnvm: missing nvm_lock acquire Wenwei Tao
2015-11-25 12:42 ` [PATCH 2/3] lightnvm: handle targets when corresponding nvm device exit Wenwei Tao
@ 2015-11-25 12:42 ` Wenwei Tao
2015-11-25 15:13 ` Matias Bjørling
2 siblings, 1 reply; 9+ messages in thread
From: Wenwei Tao @ 2015-11-25 12:42 UTC (permalink / raw)
To: mb, keith.busch, axboe; +Cc: linux-kernel, linux-block, linux-nvme
When nvme devices were removed, we need to handle the targets
build upon them properly: remove the existing targets, block
creations of new ones. To do this clean up job well, we
need to change the interface between nvme and lightnvm.
Signed-off-by: Wenwei Tao <ww.tao0320@gmail.com>
---
drivers/nvme/host/lightnvm.c | 17 ++++++++++++++++-
drivers/nvme/host/nvme.h | 1 +
drivers/nvme/host/pci.c | 10 ++++++----
3 files changed, 23 insertions(+), 5 deletions(-)
diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c
index e0b7b95..3f4ffb7 100644
--- a/drivers/nvme/host/lightnvm.c
+++ b/drivers/nvme/host/lightnvm.c
@@ -468,6 +468,14 @@ static void nvme_nvm_dev_dma_free(void *pool, void *ppa_list,
dma_pool_free(pool, ppa_list, dma_handler);
}
+static void nvme_nvm_dev_remove(struct request_queue *q)
+{
+ struct nvme_ns *ns = q->queuedata;
+
+ kref_put(&ns->kref, nvme_free_ns);
+
+}
+
static struct nvm_dev_ops nvme_nvm_dev_ops = {
.identity = nvme_nvm_identity,
@@ -482,13 +490,20 @@ static struct nvm_dev_ops nvme_nvm_dev_ops = {
.destroy_dma_pool = nvme_nvm_destroy_dma_pool,
.dev_dma_alloc = nvme_nvm_dev_dma_alloc,
.dev_dma_free = nvme_nvm_dev_dma_free,
+ .dev_remove = nvme_nvm_dev_remove,
.max_phys_sect = 64,
};
int nvme_nvm_register(struct request_queue *q, char *disk_name)
{
- return nvm_register(q, disk_name, &nvme_nvm_dev_ops);
+ int ret;
+ struct nvme_ns *ns = q->queuedata;
+
+ ret = nvm_register(q, disk_name, &nvme_nvm_dev_ops);
+ if (!ret)
+ kref_get(&ns->kref);
+ return ret;
}
void nvme_nvm_unregister(struct request_queue *q, char *disk_name)
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index fdb4e5b..251ec9d 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -116,6 +116,7 @@ static inline u64 nvme_block_nr(struct nvme_ns *ns, sector_t sector)
return (sector >> (ns->lba_shift - 9));
}
+void nvme_free_ns(struct kref *kref);
int nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd,
void *buf, unsigned bufflen);
int __nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd,
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 8187df2..f63223d 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -1960,13 +1960,10 @@ static int nvme_compat_ioctl(struct block_device *bdev, fmode_t mode,
#endif
static void nvme_free_dev(struct kref *kref);
-static void nvme_free_ns(struct kref *kref)
+void nvme_free_ns(struct kref *kref)
{
struct nvme_ns *ns = container_of(kref, struct nvme_ns, kref);
- if (ns->type == NVME_NS_LIGHTNVM)
- nvme_nvm_unregister(ns->queue, ns->disk->disk_name);
-
spin_lock(&dev_list_lock);
ns->disk->private_data = NULL;
spin_unlock(&dev_list_lock);
@@ -2533,6 +2530,11 @@ static void nvme_ns_remove(struct nvme_ns *ns)
{
bool kill = nvme_io_incapable(ns->dev) && !blk_queue_dying(ns->queue);
+ if (ns->type == NVME_NS_LIGHTNVM) {
+ nvme_nvm_unregister(ns->queue, ns->disk->disk_name);
+ ns->type = 0;
+ }
+
if (kill)
blk_set_queue_dying(ns->queue);
if (ns->disk->flags & GENHD_FL_UP)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] nvme: change the interface between nvme and lightnvm
2015-11-25 12:42 ` [PATCH 3/3] nvme: change the interface between nvme and lightnvm Wenwei Tao
@ 2015-11-25 15:13 ` Matias Bjørling
2015-11-26 3:33 ` Wenwei Tao
0 siblings, 1 reply; 9+ messages in thread
From: Matias Bjørling @ 2015-11-25 15:13 UTC (permalink / raw)
To: Wenwei Tao, keith.busch, axboe; +Cc: linux-kernel, linux-block, linux-nvme
On 11/25/2015 01:42 PM, Wenwei Tao wrote:
> When nvme devices were removed, we need to handle the targets
> build upon them properly: remove the existing targets, block
> creations of new ones. To do this clean up job well, we
> need to change the interface between nvme and lightnvm.
>
> Signed-off-by: Wenwei Tao <ww.tao0320@gmail.com>
> ---
> drivers/nvme/host/lightnvm.c | 17 ++++++++++++++++-
> drivers/nvme/host/nvme.h | 1 +
> drivers/nvme/host/pci.c | 10 ++++++----
> 3 files changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c
> index e0b7b95..3f4ffb7 100644
> --- a/drivers/nvme/host/lightnvm.c
> +++ b/drivers/nvme/host/lightnvm.c
> @@ -468,6 +468,14 @@ static void nvme_nvm_dev_dma_free(void *pool, void *ppa_list,
> dma_pool_free(pool, ppa_list, dma_handler);
> }
>
> +static void nvme_nvm_dev_remove(struct request_queue *q)
> +{
> + struct nvme_ns *ns = q->queuedata;
> +
> + kref_put(&ns->kref, nvme_free_ns);
> +
> +}
> +
> static struct nvm_dev_ops nvme_nvm_dev_ops = {
> .identity = nvme_nvm_identity,
>
> @@ -482,13 +490,20 @@ static struct nvm_dev_ops nvme_nvm_dev_ops = {
> .destroy_dma_pool = nvme_nvm_destroy_dma_pool,
> .dev_dma_alloc = nvme_nvm_dev_dma_alloc,
> .dev_dma_free = nvme_nvm_dev_dma_free,
> + .dev_remove = nvme_nvm_dev_remove,
>
> .max_phys_sect = 64,
> };
>
> int nvme_nvm_register(struct request_queue *q, char *disk_name)
> {
> - return nvm_register(q, disk_name, &nvme_nvm_dev_ops);
> + int ret;
> + struct nvme_ns *ns = q->queuedata;
> +
> + ret = nvm_register(q, disk_name, &nvme_nvm_dev_ops);
> + if (!ret)
> + kref_get(&ns->kref);
> + return ret;
> }
>
> void nvme_nvm_unregister(struct request_queue *q, char *disk_name)
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index fdb4e5b..251ec9d 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -116,6 +116,7 @@ static inline u64 nvme_block_nr(struct nvme_ns *ns, sector_t sector)
> return (sector >> (ns->lba_shift - 9));
> }
>
> +void nvme_free_ns(struct kref *kref);
> int nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd,
> void *buf, unsigned bufflen);
> int __nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd,
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index 8187df2..f63223d 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -1960,13 +1960,10 @@ static int nvme_compat_ioctl(struct block_device *bdev, fmode_t mode,
> #endif
>
> static void nvme_free_dev(struct kref *kref);
> -static void nvme_free_ns(struct kref *kref)
> +void nvme_free_ns(struct kref *kref)
> {
> struct nvme_ns *ns = container_of(kref, struct nvme_ns, kref);
>
> - if (ns->type == NVME_NS_LIGHTNVM)
> - nvme_nvm_unregister(ns->queue, ns->disk->disk_name);
> -
> spin_lock(&dev_list_lock);
> ns->disk->private_data = NULL;
> spin_unlock(&dev_list_lock);
> @@ -2533,6 +2530,11 @@ static void nvme_ns_remove(struct nvme_ns *ns)
> {
> bool kill = nvme_io_incapable(ns->dev) && !blk_queue_dying(ns->queue);
>
> + if (ns->type == NVME_NS_LIGHTNVM) {
> + nvme_nvm_unregister(ns->queue, ns->disk->disk_name);
> + ns->type = 0;
Do we need to set it to zero, since we won't use it after its been removed?
> + }
> +
> if (kill)
> blk_set_queue_dying(ns->queue);
> if (ns->disk->flags & GENHD_FL_UP)
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] lightnvm: missing nvm_lock acquire
2015-11-25 12:42 ` [PATCH 1/3] lightnvm: missing nvm_lock acquire Wenwei Tao
@ 2015-11-25 15:35 ` Matias Bjørling
0 siblings, 0 replies; 9+ messages in thread
From: Matias Bjørling @ 2015-11-25 15:35 UTC (permalink / raw)
To: Wenwei Tao, mb, keith.busch, axboe; +Cc: linux-kernel, linux-block, linux-nvme
On 11/25/2015 01:42 PM, Wenwei Tao wrote:
> To avoid race conditions, traverse dev, media manager,
> and targeet lists and also register, unregister entries
> to/from them, should be always under the nvm_lock control.
>
> Signed-off-by: Wenwei Tao <ww.tao0320@gmail.com>
> ---
> drivers/lightnvm/core.c | 19 +++++++++++++------
> 1 file changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
> index f659e60..39aec3a 100644
> --- a/drivers/lightnvm/core.c
> +++ b/drivers/lightnvm/core.c
> @@ -333,19 +333,19 @@ EXPORT_SYMBOL(nvm_register);
>
> void nvm_unregister(char *disk_name)
> {
> + down_write(&nvm_lock);
> struct nvm_dev *dev = nvm_find_nvm_dev(disk_name);
>
> if (!dev) {
> pr_err("nvm: could not find device %s to unregister\n",
> disk_name);
> + up_write(&nvm_lock);
> return;
> }
>
> - nvm_exit(dev);
> -
> - down_write(&nvm_lock);
> list_del(&dev->devices);
> up_write(&nvm_lock);
> + nvm_exit(dev);
> }
> EXPORT_SYMBOL(nvm_unregister);
>
> @@ -365,12 +365,15 @@ static int nvm_create_target(struct nvm_dev *dev,
> 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)
> + if (ret < 0) {
> + up_write(&nvm_lock);
> return ret; /* initialization failed */
> + }
> if (ret > 0) {
> dev->mt = mt;
> break; /* successfully initialized */
> @@ -379,6 +382,7 @@ static int nvm_create_target(struct nvm_dev *dev,
>
> if (!ret) {
> pr_info("nvm: no compatible nvm manager found.\n");
> + up_write(&nvm_lock);
> return -ENODEV;
> }
> }
> @@ -386,10 +390,10 @@ static int nvm_create_target(struct nvm_dev *dev,
> tt = nvm_find_target_type(create->tgttype);
> if (!tt) {
> pr_err("nvm: target type %s not found\n", create->tgttype);
> + up_write(&nvm_lock);
> return -EINVAL;
> }
>
> - down_write(&nvm_lock);
> list_for_each_entry(t, &dev->online_targets, list) {
> if (!strcmp(create->tgtname, t->disk->disk_name)) {
> pr_err("nvm: target name already exists.\n");
> @@ -472,8 +476,9 @@ static int __nvm_configure_create(struct nvm_ioctl_create *create)
> {
> struct nvm_dev *dev;
> struct nvm_ioctl_create_simple *s;
> -
> + down_write(&nvm_lock);
> dev = nvm_find_nvm_dev(create->dev);
> + up_write(&nvm_lock);
> if (!dev) {
> pr_err("nvm: device not found\n");
> return -EINVAL;
> @@ -532,7 +537,9 @@ static int nvm_configure_show(const char *val)
> return -EINVAL;
> }
>
> + down_write(&nvm_lock);
> dev = nvm_find_nvm_dev(devname);
> + up_write(&nvm_lock);
> if (!dev) {
> pr_err("nvm: device not found\n");
> return -EINVAL;
>
Thanks Tao, applied.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] lightnvm: handle targets when corresponding nvm device exit
2015-11-25 12:42 ` [PATCH 2/3] lightnvm: handle targets when corresponding nvm device exit Wenwei Tao
@ 2015-11-25 15:39 ` Matias Bjørling
2015-11-26 9:56 ` Wenwei Tao
0 siblings, 1 reply; 9+ messages in thread
From: Matias Bjørling @ 2015-11-25 15:39 UTC (permalink / raw)
To: Wenwei Tao, mb, keith.busch, axboe; +Cc: linux-kernel, linux-block, linux-nvme
On 11/25/2015 01:42 PM, Wenwei Tao wrote:
> block creations of new targets, remove exiting targets when
> underlying device was gone.
>
> Signed-off-by: Wenwei Tao <ww.tao0320@gmail.com>
> ---
> drivers/lightnvm/core.c | 127 ++++++++++++++++++++++++++++++-----------------
> include/linux/lightnvm.h | 3 ++
> 2 files changed, 85 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
> index 39aec3a..0b71dd2 100644
> --- a/drivers/lightnvm/core.c
> +++ b/drivers/lightnvm/core.c
> @@ -33,6 +33,17 @@ static LIST_HEAD(nvm_targets);
> static LIST_HEAD(nvm_mgrs);
> static LIST_HEAD(nvm_devices);
> static DECLARE_RWSEM(nvm_lock);
> +#define NVM_EXITING 1
> +
> +static inline int NvmExiting(struct nvm_dev *dev)
> +{
> + return ((unsigned long)dev->ppalist_pool & NVM_EXITING) != 0;
> +}
> +
> +static inline void *ppalist_pool(struct nvm_dev *dev)
> +{
> + return (void *)((unsigned long)dev->ppalist_pool & ~NVM_EXITING);
> +}
>
I think I rather want to have a state variable (so we have a state
machine for three state. Initializing, running, and stopping.
What was the reason you'll like to put it in the ppalist_pool?
ps. could you rebase it on top of the latest master. Then I'll review
the rest of it.
> static struct nvm_tgt_type *nvm_find_target_type(const char *name)
> {
> @@ -74,7 +85,7 @@ EXPORT_SYMBOL(nvm_unregister_target);
> void *nvm_dev_dma_alloc(struct nvm_dev *dev, gfp_t mem_flags,
> dma_addr_t *dma_handler)
> {
> - return dev->ops->dev_dma_alloc(dev->q, dev->ppalist_pool, mem_flags,
> + return dev->ops->dev_dma_alloc(dev->q, ppalist_pool(dev), mem_flags,
> dma_handler);
> }
> EXPORT_SYMBOL(nvm_dev_dma_alloc);
> @@ -82,7 +93,7 @@ EXPORT_SYMBOL(nvm_dev_dma_alloc);
> void nvm_dev_dma_free(struct nvm_dev *dev, void *ppa_list,
> dma_addr_t dma_handler)
> {
> - dev->ops->dev_dma_free(dev->ppalist_pool, ppa_list, dma_handler);
> + dev->ops->dev_dma_free(ppalist_pool(dev), ppa_list, dma_handler);
> }
> EXPORT_SYMBOL(nvm_dev_dma_free);
>
> @@ -206,17 +217,6 @@ static int nvm_core_init(struct nvm_dev *dev)
> return 0;
> }
>
> -static void nvm_free(struct nvm_dev *dev)
> -{
> - if (!dev)
> - return;
> -
> - if (dev->mt)
> - dev->mt->unregister_mgr(dev);
> -
> - nvm_core_free(dev);
> -}
> -
> static int nvm_init(struct nvm_dev *dev)
> {
> struct nvmm_type *mt;
> @@ -262,6 +262,7 @@ static int nvm_init(struct nvm_dev *dev)
> }
> }
>
> + kref_init(&dev->kref);
> if (!ret) {
> pr_info("nvm: no compatible manager found.\n");
> return 0;
> @@ -278,11 +279,47 @@ err:
> return ret;
> }
>
> -static void nvm_exit(struct nvm_dev *dev)
> +static void nvm_remove_target(struct nvm_target *t)
> {
> - if (dev->ppalist_pool)
> - dev->ops->destroy_dma_pool(dev->ppalist_pool);
> - nvm_free(dev);
> + struct nvm_tgt_type *tt = t->type;
> + struct gendisk *tdisk = t->disk;
> + struct request_queue *q = tdisk->queue;
> +
> + lockdep_assert_held(&nvm_lock);
> +
> + del_gendisk(tdisk);
> + if (tt->exit)
> + tt->exit(tdisk->private_data);
> +
> + blk_cleanup_queue(q);
> +
> + put_disk(tdisk);
> +
> + list_del(&t->list);
> + kfree(t);
> +}
> +
> +static inline void nvm_remove_targets(struct nvm_dev *dev)
> +{
> + struct nvm_target *t, *n;
> +
> + list_for_each_entry_safe(t, n, &dev->online_targets, list)
> + nvm_remove_target(t);
> +}
> +
> +static void nvm_exit(struct kref *kref)
> +{
> + struct nvm_dev *dev;
> +
> + dev = container_of(kref, struct nvm_dev, kref);
> + if (ppalist_pool(dev))
> + dev->ops->destroy_dma_pool(ppalist_pool(dev));
> +
> + if (dev->mt)
> + dev->mt->unregister_mgr(dev);
> +
> + if (dev->ops->dev_remove)
> + dev->ops->dev_remove(dev->q);
>
> pr_info("nvm: successfully unloaded\n");
> }
> @@ -344,8 +381,10 @@ void nvm_unregister(char *disk_name)
> }
>
> list_del(&dev->devices);
> + nvm_remove_targets(dev);
> + dev->ppalist_pool += NVM_EXITING;
> up_write(&nvm_lock);
> - nvm_exit(dev);
> + kref_put(&dev->kref, nvm_exit);
> }
> EXPORT_SYMBOL(nvm_unregister);
>
> @@ -366,6 +405,10 @@ static int nvm_create_target(struct nvm_dev *dev,
> int ret = 0;
>
> down_write(&nvm_lock);
> + if (NvmExiting(dev)) {
> + up_write(&nvm_lock);
> + return -ENODEV;
> + }
> if (!dev->mt) {
> /* register with device with a supported NVM manager */
> list_for_each_entry(mt, &nvm_mgrs, list) {
> @@ -439,10 +482,16 @@ static int nvm_create_target(struct nvm_dev *dev,
> t->disk = tdisk;
>
> down_write(&nvm_lock);
> + if (NvmExiting(dev)) {
> + up_write(&nvm_lock);
> + goto err_nvm_exiting;
> + }
> list_add_tail(&t->list, &dev->online_targets);
> up_write(&nvm_lock);
>
> return 0;
> +err_nvm_exiting:
> + del_gendisk(tdisk);
> err_init:
> put_disk(tdisk);
> err_queue:
> @@ -452,62 +501,50 @@ err_t:
> return -ENOMEM;
> }
>
> -static void nvm_remove_target(struct nvm_target *t)
> -{
> - struct nvm_tgt_type *tt = t->type;
> - struct gendisk *tdisk = t->disk;
> - struct request_queue *q = tdisk->queue;
> -
> - lockdep_assert_held(&nvm_lock);
> -
> - del_gendisk(tdisk);
> - if (tt->exit)
> - tt->exit(tdisk->private_data);
> -
> - blk_cleanup_queue(q);
> -
> - put_disk(tdisk);
> -
> - list_del(&t->list);
> - kfree(t);
> -}
> -
> static int __nvm_configure_create(struct nvm_ioctl_create *create)
> {
> struct nvm_dev *dev;
> struct nvm_ioctl_create_simple *s;
> + int ret = -EINVAL;
> +
> down_write(&nvm_lock);
> dev = nvm_find_nvm_dev(create->dev);
> - up_write(&nvm_lock);
> if (!dev) {
> pr_err("nvm: device not found\n");
> - return -EINVAL;
> + up_write(&nvm_lock);
> + goto out;
> }
> + kref_get(&dev->kref);
> + up_write(&nvm_lock);
>
> if (create->conf.type != NVM_CONFIG_TYPE_SIMPLE) {
> pr_err("nvm: config type not valid\n");
> - return -EINVAL;
> + goto out;
> }
> s = &create->conf.s;
>
> if (s->lun_begin > s->lun_end || s->lun_end > dev->nr_luns) {
> pr_err("nvm: lun out of bound (%u:%u > %u)\n",
> s->lun_begin, s->lun_end, dev->nr_luns);
> - return -EINVAL;
> + goto out;
> }
>
> - return nvm_create_target(dev, create);
> + ret = nvm_create_target(dev, create);
> +out:
> + if (dev)
> + kref_put(&dev->kref, nvm_exit);
> + return ret;
> }
>
> static int __nvm_configure_remove(struct nvm_ioctl_remove *remove)
> {
> - struct nvm_target *t = NULL;
> + struct nvm_target *n, *t = NULL;
> struct nvm_dev *dev;
> int ret = -1;
>
> down_write(&nvm_lock);
> list_for_each_entry(dev, &nvm_devices, devices)
> - list_for_each_entry(t, &dev->online_targets, list) {
> + list_for_each_entry_safe(t, n, &dev->online_targets, list) {
> if (!strcmp(remove->tgtname, t->disk->disk_name)) {
> nvm_remove_target(t);
> ret = 0;
> diff --git a/include/linux/lightnvm.h b/include/linux/lightnvm.h
> index 69c9057..1b42305 100644
> --- a/include/linux/lightnvm.h
> +++ b/include/linux/lightnvm.h
> @@ -205,6 +205,7 @@ typedef void (nvm_destroy_dma_pool_fn)(void *);
> typedef void *(nvm_dev_dma_alloc_fn)(struct request_queue *, void *, gfp_t,
> dma_addr_t *);
> typedef void (nvm_dev_dma_free_fn)(void *, void*, dma_addr_t);
> +typedef void (nvm_dev_remove_fn) (struct request_queue *);
>
> struct nvm_dev_ops {
> nvm_id_fn *identity;
> @@ -219,6 +220,7 @@ struct nvm_dev_ops {
> nvm_destroy_dma_pool_fn *destroy_dma_pool;
> nvm_dev_dma_alloc_fn *dev_dma_alloc;
> nvm_dev_dma_free_fn *dev_dma_free;
> + nvm_dev_remove_fn *dev_remove;
>
> uint8_t max_phys_sect;
> };
> @@ -252,6 +254,7 @@ struct nvm_dev {
>
> /* Media manager */
> struct nvmm_type *mt;
> + struct kref kref;
> void *mp;
>
> /* Device information */
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] nvme: change the interface between nvme and lightnvm
2015-11-25 15:13 ` Matias Bjørling
@ 2015-11-26 3:33 ` Wenwei Tao
0 siblings, 0 replies; 9+ messages in thread
From: Wenwei Tao @ 2015-11-26 3:33 UTC (permalink / raw)
To: Matias Bjørling
Cc: keith.busch, axboe, linux-kernel, linux-block, linux-nvme
You are right. Reset it to zero is not necessary.
2015-11-25 23:13 GMT+08:00 Matias Bjørling <mb@lightnvm.io>:
> On 11/25/2015 01:42 PM, Wenwei Tao wrote:
>>
>> When nvme devices were removed, we need to handle the targets
>> build upon them properly: remove the existing targets, block
>> creations of new ones. To do this clean up job well, we
>> need to change the interface between nvme and lightnvm.
>>
>> Signed-off-by: Wenwei Tao <ww.tao0320@gmail.com>
>> ---
>> drivers/nvme/host/lightnvm.c | 17 ++++++++++++++++-
>> drivers/nvme/host/nvme.h | 1 +
>> drivers/nvme/host/pci.c | 10 ++++++----
>> 3 files changed, 23 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c
>> index e0b7b95..3f4ffb7 100644
>> --- a/drivers/nvme/host/lightnvm.c
>> +++ b/drivers/nvme/host/lightnvm.c
>> @@ -468,6 +468,14 @@ static void nvme_nvm_dev_dma_free(void *pool, void
>> *ppa_list,
>> dma_pool_free(pool, ppa_list, dma_handler);
>> }
>>
>> +static void nvme_nvm_dev_remove(struct request_queue *q)
>> +{
>> + struct nvme_ns *ns = q->queuedata;
>> +
>> + kref_put(&ns->kref, nvme_free_ns);
>> +
>> +}
>> +
>> static struct nvm_dev_ops nvme_nvm_dev_ops = {
>> .identity = nvme_nvm_identity,
>>
>> @@ -482,13 +490,20 @@ static struct nvm_dev_ops nvme_nvm_dev_ops = {
>> .destroy_dma_pool = nvme_nvm_destroy_dma_pool,
>> .dev_dma_alloc = nvme_nvm_dev_dma_alloc,
>> .dev_dma_free = nvme_nvm_dev_dma_free,
>> + .dev_remove = nvme_nvm_dev_remove,
>>
>> .max_phys_sect = 64,
>> };
>>
>> int nvme_nvm_register(struct request_queue *q, char *disk_name)
>> {
>> - return nvm_register(q, disk_name, &nvme_nvm_dev_ops);
>> + int ret;
>> + struct nvme_ns *ns = q->queuedata;
>> +
>> + ret = nvm_register(q, disk_name, &nvme_nvm_dev_ops);
>> + if (!ret)
>> + kref_get(&ns->kref);
>> + return ret;
>> }
>>
>> void nvme_nvm_unregister(struct request_queue *q, char *disk_name)
>> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
>> index fdb4e5b..251ec9d 100644
>> --- a/drivers/nvme/host/nvme.h
>> +++ b/drivers/nvme/host/nvme.h
>> @@ -116,6 +116,7 @@ static inline u64 nvme_block_nr(struct nvme_ns *ns,
>> sector_t sector)
>> return (sector >> (ns->lba_shift - 9));
>> }
>>
>> +void nvme_free_ns(struct kref *kref);
>> int nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command
>> *cmd,
>> void *buf, unsigned bufflen);
>> int __nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command
>> *cmd,
>> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
>> index 8187df2..f63223d 100644
>> --- a/drivers/nvme/host/pci.c
>> +++ b/drivers/nvme/host/pci.c
>> @@ -1960,13 +1960,10 @@ static int nvme_compat_ioctl(struct block_device
>> *bdev, fmode_t mode,
>> #endif
>>
>> static void nvme_free_dev(struct kref *kref);
>> -static void nvme_free_ns(struct kref *kref)
>> +void nvme_free_ns(struct kref *kref)
>> {
>> struct nvme_ns *ns = container_of(kref, struct nvme_ns, kref);
>>
>> - if (ns->type == NVME_NS_LIGHTNVM)
>> - nvme_nvm_unregister(ns->queue, ns->disk->disk_name);
>> -
>> spin_lock(&dev_list_lock);
>> ns->disk->private_data = NULL;
>> spin_unlock(&dev_list_lock);
>> @@ -2533,6 +2530,11 @@ static void nvme_ns_remove(struct nvme_ns *ns)
>> {
>> bool kill = nvme_io_incapable(ns->dev) &&
>> !blk_queue_dying(ns->queue);
>>
>> + if (ns->type == NVME_NS_LIGHTNVM) {
>> + nvme_nvm_unregister(ns->queue, ns->disk->disk_name);
>> + ns->type = 0;
>
>
> Do we need to set it to zero, since we won't use it after its been removed?
>
>
>> + }
>> +
>> if (kill)
>> blk_set_queue_dying(ns->queue);
>> if (ns->disk->flags & GENHD_FL_UP)
>>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] lightnvm: handle targets when corresponding nvm device exit
2015-11-25 15:39 ` Matias Bjørling
@ 2015-11-26 9:56 ` Wenwei Tao
0 siblings, 0 replies; 9+ messages in thread
From: Wenwei Tao @ 2015-11-26 9:56 UTC (permalink / raw)
To: Matias Bjørling
Cc: Matias, keith.busch, axboe, linux-kernel, linux-block, linux-nvme
The reason why I put it in the ppalist_pool is that when I wrote the
code I thought state we needed was only exiting/stopping, I didn't
want to add a variable for just a bit.
I will send you a patch base on the lasted master 4.4rc2.
2015-11-25 23:39 GMT+08:00 Matias Bjørling <m@bjorling.me>:
> On 11/25/2015 01:42 PM, Wenwei Tao wrote:
>>
>> block creations of new targets, remove exiting targets when
>> underlying device was gone.
>>
>> Signed-off-by: Wenwei Tao <ww.tao0320@gmail.com>
>> ---
>> drivers/lightnvm/core.c | 127
>> ++++++++++++++++++++++++++++++-----------------
>> include/linux/lightnvm.h | 3 ++
>> 2 files changed, 85 insertions(+), 45 deletions(-)
>>
>> diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
>> index 39aec3a..0b71dd2 100644
>> --- a/drivers/lightnvm/core.c
>> +++ b/drivers/lightnvm/core.c
>> @@ -33,6 +33,17 @@ static LIST_HEAD(nvm_targets);
>> static LIST_HEAD(nvm_mgrs);
>> static LIST_HEAD(nvm_devices);
>> static DECLARE_RWSEM(nvm_lock);
>> +#define NVM_EXITING 1
>> +
>> +static inline int NvmExiting(struct nvm_dev *dev)
>> +{
>> + return ((unsigned long)dev->ppalist_pool & NVM_EXITING) != 0;
>> +}
>> +
>> +static inline void *ppalist_pool(struct nvm_dev *dev)
>> +{
>> + return (void *)((unsigned long)dev->ppalist_pool & ~NVM_EXITING);
>> +}
>>
>
> I think I rather want to have a state variable (so we have a state machine
> for three state. Initializing, running, and stopping.
>
> What was the reason you'll like to put it in the ppalist_pool?
>
> ps. could you rebase it on top of the latest master. Then I'll review the
> rest of it.
>
>
>> static struct nvm_tgt_type *nvm_find_target_type(const char *name)
>> {
>> @@ -74,7 +85,7 @@ EXPORT_SYMBOL(nvm_unregister_target);
>> void *nvm_dev_dma_alloc(struct nvm_dev *dev, gfp_t mem_flags,
>> dma_addr_t
>> *dma_handler)
>> {
>> - return dev->ops->dev_dma_alloc(dev->q, dev->ppalist_pool,
>> mem_flags,
>> + return dev->ops->dev_dma_alloc(dev->q, ppalist_pool(dev),
>> mem_flags,
>>
>> dma_handler);
>> }
>> EXPORT_SYMBOL(nvm_dev_dma_alloc);
>> @@ -82,7 +93,7 @@ EXPORT_SYMBOL(nvm_dev_dma_alloc);
>> void nvm_dev_dma_free(struct nvm_dev *dev, void *ppa_list,
>> dma_addr_t
>> dma_handler)
>> {
>> - dev->ops->dev_dma_free(dev->ppalist_pool, ppa_list, dma_handler);
>> + dev->ops->dev_dma_free(ppalist_pool(dev), ppa_list, dma_handler);
>> }
>> EXPORT_SYMBOL(nvm_dev_dma_free);
>>
>> @@ -206,17 +217,6 @@ static int nvm_core_init(struct nvm_dev *dev)
>> return 0;
>> }
>>
>> -static void nvm_free(struct nvm_dev *dev)
>> -{
>> - if (!dev)
>> - return;
>> -
>> - if (dev->mt)
>> - dev->mt->unregister_mgr(dev);
>> -
>> - nvm_core_free(dev);
>> -}
>> -
>> static int nvm_init(struct nvm_dev *dev)
>> {
>> struct nvmm_type *mt;
>> @@ -262,6 +262,7 @@ static int nvm_init(struct nvm_dev *dev)
>> }
>> }
>>
>> + kref_init(&dev->kref);
>> if (!ret) {
>> pr_info("nvm: no compatible manager found.\n");
>> return 0;
>> @@ -278,11 +279,47 @@ err:
>> return ret;
>> }
>>
>> -static void nvm_exit(struct nvm_dev *dev)
>> +static void nvm_remove_target(struct nvm_target *t)
>> {
>> - if (dev->ppalist_pool)
>> - dev->ops->destroy_dma_pool(dev->ppalist_pool);
>> - nvm_free(dev);
>> + struct nvm_tgt_type *tt = t->type;
>> + struct gendisk *tdisk = t->disk;
>> + struct request_queue *q = tdisk->queue;
>> +
>> + lockdep_assert_held(&nvm_lock);
>> +
>> + del_gendisk(tdisk);
>> + if (tt->exit)
>> + tt->exit(tdisk->private_data);
>> +
>> + blk_cleanup_queue(q);
>> +
>> + put_disk(tdisk);
>> +
>> + list_del(&t->list);
>> + kfree(t);
>> +}
>> +
>> +static inline void nvm_remove_targets(struct nvm_dev *dev)
>> +{
>> + struct nvm_target *t, *n;
>> +
>> + list_for_each_entry_safe(t, n, &dev->online_targets, list)
>> + nvm_remove_target(t);
>> +}
>> +
>> +static void nvm_exit(struct kref *kref)
>> +{
>> + struct nvm_dev *dev;
>> +
>> + dev = container_of(kref, struct nvm_dev, kref);
>> + if (ppalist_pool(dev))
>> + dev->ops->destroy_dma_pool(ppalist_pool(dev));
>> +
>> + if (dev->mt)
>> + dev->mt->unregister_mgr(dev);
>> +
>> + if (dev->ops->dev_remove)
>> + dev->ops->dev_remove(dev->q);
>>
>> pr_info("nvm: successfully unloaded\n");
>> }
>> @@ -344,8 +381,10 @@ void nvm_unregister(char *disk_name)
>> }
>>
>> list_del(&dev->devices);
>> + nvm_remove_targets(dev);
>> + dev->ppalist_pool += NVM_EXITING;
>> up_write(&nvm_lock);
>> - nvm_exit(dev);
>> + kref_put(&dev->kref, nvm_exit);
>> }
>> EXPORT_SYMBOL(nvm_unregister);
>>
>> @@ -366,6 +405,10 @@ static int nvm_create_target(struct nvm_dev *dev,
>> int ret = 0;
>>
>> down_write(&nvm_lock);
>> + if (NvmExiting(dev)) {
>> + up_write(&nvm_lock);
>> + return -ENODEV;
>> + }
>> if (!dev->mt) {
>> /* register with device with a supported NVM manager */
>> list_for_each_entry(mt, &nvm_mgrs, list) {
>> @@ -439,10 +482,16 @@ static int nvm_create_target(struct nvm_dev *dev,
>> t->disk = tdisk;
>>
>> down_write(&nvm_lock);
>> + if (NvmExiting(dev)) {
>> + up_write(&nvm_lock);
>> + goto err_nvm_exiting;
>> + }
>> list_add_tail(&t->list, &dev->online_targets);
>> up_write(&nvm_lock);
>>
>> return 0;
>> +err_nvm_exiting:
>> + del_gendisk(tdisk);
>> err_init:
>> put_disk(tdisk);
>> err_queue:
>> @@ -452,62 +501,50 @@ err_t:
>> return -ENOMEM;
>> }
>>
>> -static void nvm_remove_target(struct nvm_target *t)
>> -{
>> - struct nvm_tgt_type *tt = t->type;
>> - struct gendisk *tdisk = t->disk;
>> - struct request_queue *q = tdisk->queue;
>> -
>> - lockdep_assert_held(&nvm_lock);
>> -
>> - del_gendisk(tdisk);
>> - if (tt->exit)
>> - tt->exit(tdisk->private_data);
>> -
>> - blk_cleanup_queue(q);
>> -
>> - put_disk(tdisk);
>> -
>> - list_del(&t->list);
>> - kfree(t);
>> -}
>> -
>> static int __nvm_configure_create(struct nvm_ioctl_create *create)
>> {
>> struct nvm_dev *dev;
>> struct nvm_ioctl_create_simple *s;
>> + int ret = -EINVAL;
>> +
>> down_write(&nvm_lock);
>> dev = nvm_find_nvm_dev(create->dev);
>> - up_write(&nvm_lock);
>> if (!dev) {
>> pr_err("nvm: device not found\n");
>> - return -EINVAL;
>> + up_write(&nvm_lock);
>> + goto out;
>> }
>> + kref_get(&dev->kref);
>> + up_write(&nvm_lock);
>>
>> if (create->conf.type != NVM_CONFIG_TYPE_SIMPLE) {
>> pr_err("nvm: config type not valid\n");
>> - return -EINVAL;
>> + goto out;
>> }
>> s = &create->conf.s;
>>
>> if (s->lun_begin > s->lun_end || s->lun_end > dev->nr_luns) {
>> pr_err("nvm: lun out of bound (%u:%u > %u)\n",
>> s->lun_begin, s->lun_end, dev->nr_luns);
>> - return -EINVAL;
>> + goto out;
>> }
>>
>> - return nvm_create_target(dev, create);
>> + ret = nvm_create_target(dev, create);
>> +out:
>> + if (dev)
>> + kref_put(&dev->kref, nvm_exit);
>> + return ret;
>> }
>>
>> static int __nvm_configure_remove(struct nvm_ioctl_remove *remove)
>> {
>> - struct nvm_target *t = NULL;
>> + struct nvm_target *n, *t = NULL;
>> struct nvm_dev *dev;
>> int ret = -1;
>>
>> down_write(&nvm_lock);
>> list_for_each_entry(dev, &nvm_devices, devices)
>> - list_for_each_entry(t, &dev->online_targets, list) {
>> + list_for_each_entry_safe(t, n, &dev->online_targets, list)
>> {
>> if (!strcmp(remove->tgtname, t->disk->disk_name))
>> {
>> nvm_remove_target(t);
>> ret = 0;
>> diff --git a/include/linux/lightnvm.h b/include/linux/lightnvm.h
>> index 69c9057..1b42305 100644
>> --- a/include/linux/lightnvm.h
>> +++ b/include/linux/lightnvm.h
>> @@ -205,6 +205,7 @@ typedef void (nvm_destroy_dma_pool_fn)(void *);
>> typedef void *(nvm_dev_dma_alloc_fn)(struct request_queue *, void *,
>> gfp_t,
>> dma_addr_t
>> *);
>> typedef void (nvm_dev_dma_free_fn)(void *, void*, dma_addr_t);
>> +typedef void (nvm_dev_remove_fn) (struct request_queue *);
>>
>> struct nvm_dev_ops {
>> nvm_id_fn *identity;
>> @@ -219,6 +220,7 @@ struct nvm_dev_ops {
>> nvm_destroy_dma_pool_fn *destroy_dma_pool;
>> nvm_dev_dma_alloc_fn *dev_dma_alloc;
>> nvm_dev_dma_free_fn *dev_dma_free;
>> + nvm_dev_remove_fn *dev_remove;
>>
>> uint8_t max_phys_sect;
>> };
>> @@ -252,6 +254,7 @@ struct nvm_dev {
>>
>> /* Media manager */
>> struct nvmm_type *mt;
>> + struct kref kref;
>> void *mp;
>>
>> /* Device information */
>>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-11-26 9:56 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-25 12:42 [PATCH 0/3] fixes fo LightNVM Wenwei Tao
2015-11-25 12:42 ` [PATCH 1/3] lightnvm: missing nvm_lock acquire Wenwei Tao
2015-11-25 15:35 ` Matias Bjørling
2015-11-25 12:42 ` [PATCH 2/3] lightnvm: handle targets when corresponding nvm device exit Wenwei Tao
2015-11-25 15:39 ` Matias Bjørling
2015-11-26 9:56 ` Wenwei Tao
2015-11-25 12:42 ` [PATCH 3/3] nvme: change the interface between nvme and lightnvm Wenwei Tao
2015-11-25 15:13 ` Matias Bjørling
2015-11-26 3:33 ` Wenwei Tao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox