linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* lightnvm: remove dma alloc/free helpers
@ 2018-11-27 13:39 Javier González
  2018-11-27 13:39 ` [PATCH] " Javier González
  0 siblings, 1 reply; 5+ messages in thread
From: Javier González @ 2018-11-27 13:39 UTC (permalink / raw)
  To: mb; +Cc: igor.j.konopko, linux-block, linux-kernel, Javier González

This patch applies on top of Igor's lightnvm: Flexible metadata (V3). I
took the liberty to rename the new nvm_alloc_dma_pool() to
nvm_create_dma_pool() as it is what is does. Igor, feel free to squash
this part in your patchset. You can also carry this patch on your series
if you want, when you send the V4 to make it easier to Matias to track.

Thanks,
Javier

Javier González (1):
  lightnvm: remove dma alloc/free helpers

 drivers/lightnvm/core.c          | 25 +++++++------------------
 drivers/lightnvm/pblk-core.c     | 10 +++++-----
 drivers/lightnvm/pblk-read.c     |  3 ++-
 drivers/lightnvm/pblk-recovery.c |  5 +++--
 drivers/nvme/host/lightnvm.c     | 16 +---------------
 include/linux/lightnvm.h         |  8 +-------
 6 files changed, 19 insertions(+), 48 deletions(-)

-- 
2.7.4


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] lightnvm: remove dma alloc/free helpers
  2018-11-27 13:39 lightnvm: remove dma alloc/free helpers Javier González
@ 2018-11-27 13:39 ` Javier González
  2018-11-27 14:18   ` Matias Bjørling
  0 siblings, 1 reply; 5+ messages in thread
From: Javier González @ 2018-11-27 13:39 UTC (permalink / raw)
  To: mb; +Cc: igor.j.konopko, linux-block, linux-kernel, Javier González

Time has shown that the LightNVM alloc/free dma helpers are merely
replacements of the native dma_pool operations. Thus, remove them and
let targets manage the LightNVM dma pool directly.

Signed-off-by: Javier González <javier@cnexlabs.com>
---
 drivers/lightnvm/core.c          | 25 +++++++------------------
 drivers/lightnvm/pblk-core.c     | 10 +++++-----
 drivers/lightnvm/pblk-read.c     |  3 ++-
 drivers/lightnvm/pblk-recovery.c |  5 +++--
 drivers/nvme/host/lightnvm.c     | 16 +---------------
 include/linux/lightnvm.h         |  8 +-------
 6 files changed, 19 insertions(+), 48 deletions(-)

diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
index c3650b141a30..8b6ee35e4356 100644
--- a/drivers/lightnvm/core.c
+++ b/drivers/lightnvm/core.c
@@ -641,20 +641,6 @@ void nvm_unregister_tgt_type(struct nvm_tgt_type *tt)
 }
 EXPORT_SYMBOL(nvm_unregister_tgt_type);
 
-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, dev->dma_pool, mem_flags,
-								dma_handler);
-}
-EXPORT_SYMBOL(nvm_dev_dma_alloc);
-
-void nvm_dev_dma_free(struct nvm_dev *dev, void *addr, dma_addr_t dma_handler)
-{
-	dev->ops->dev_dma_free(dev->dma_pool, addr, dma_handler);
-}
-EXPORT_SYMBOL(nvm_dev_dma_free);
-
 static struct nvm_dev *nvm_find_nvm_dev(const char *name)
 {
 	struct nvm_dev *dev;
@@ -682,7 +668,8 @@ static int nvm_set_rqd_ppalist(struct nvm_tgt_dev *tgt_dev, struct nvm_rq *rqd,
 	}
 
 	rqd->nr_ppas = nr_ppas;
-	rqd->ppa_list = nvm_dev_dma_alloc(dev, GFP_KERNEL, &rqd->dma_ppa_list);
+	rqd->ppa_list = dma_pool_alloc(dev->dma_pool, GFP_KERNEL,
+							&rqd->dma_ppa_list);
 	if (!rqd->ppa_list) {
 		pr_err("nvm: failed to allocate dma memory\n");
 		return -ENOMEM;
@@ -705,10 +692,12 @@ static int nvm_set_rqd_ppalist(struct nvm_tgt_dev *tgt_dev, struct nvm_rq *rqd,
 static void nvm_free_rqd_ppalist(struct nvm_tgt_dev *tgt_dev,
 			struct nvm_rq *rqd)
 {
+	struct nvm_dev *dev = tgt_dev->parent;
+
 	if (!rqd->ppa_list)
 		return;
 
-	nvm_dev_dma_free(tgt_dev->parent, rqd->ppa_list, rqd->dma_ppa_list);
+	dma_pool_free(dev->dma_pool, rqd->ppa_list, rqd->dma_ppa_list);
 }
 
 static int nvm_set_flags(struct nvm_geo *geo, struct nvm_rq *rqd)
@@ -1178,7 +1167,7 @@ void nvm_unregister(struct nvm_dev *dev)
 }
 EXPORT_SYMBOL(nvm_unregister);
 
-int nvm_alloc_dma_pool(struct nvm_dev *dev)
+int nvm_create_dma_pool(struct nvm_dev *dev)
 {
 	int exp_pool_size;
 
@@ -1195,7 +1184,7 @@ int nvm_alloc_dma_pool(struct nvm_dev *dev)
 
 	return 0;
 }
-EXPORT_SYMBOL(nvm_alloc_dma_pool);
+EXPORT_SYMBOL(nvm_create_dma_pool);
 
 static int __nvm_configure_create(struct nvm_ioctl_create *create)
 {
diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c
index 615817bf97e3..61a2a5330ecf 100644
--- a/drivers/lightnvm/pblk-core.c
+++ b/drivers/lightnvm/pblk-core.c
@@ -242,7 +242,7 @@ int pblk_alloc_rqd_meta(struct pblk *pblk, struct nvm_rq *rqd)
 {
 	struct nvm_tgt_dev *dev = pblk->dev;
 
-	rqd->meta_list = nvm_dev_dma_alloc(dev->parent, GFP_KERNEL,
+	rqd->meta_list = dma_pool_alloc(dev->parent->dma_pool, GFP_KERNEL,
 							&rqd->dma_meta_list);
 	if (!rqd->meta_list)
 		return -ENOMEM;
@@ -261,8 +261,8 @@ void pblk_free_rqd_meta(struct pblk *pblk, struct nvm_rq *rqd)
 	struct nvm_tgt_dev *dev = pblk->dev;
 
 	if (rqd->meta_list)
-		nvm_dev_dma_free(dev->parent, rqd->meta_list,
-				rqd->dma_meta_list);
+		dma_pool_free(dev->parent->dma_pool, rqd->meta_list,
+							rqd->dma_meta_list);
 }
 
 /* Caller must guarantee that the request is a valid type */
@@ -846,7 +846,7 @@ int pblk_line_emeta_read(struct pblk *pblk, struct pblk_line *line,
 	int i, j;
 	int ret;
 
-	meta_list = nvm_dev_dma_alloc(dev->parent, GFP_KERNEL,
+	meta_list = dma_pool_alloc(dev->parent->dma_pool, GFP_KERNEL,
 							&dma_meta_list);
 	if (!meta_list)
 		return -ENOMEM;
@@ -925,7 +925,7 @@ int pblk_line_emeta_read(struct pblk *pblk, struct pblk_line *line,
 		goto next_rq;
 
 free_rqd_dma:
-	nvm_dev_dma_free(dev->parent, rqd.meta_list, rqd.dma_meta_list);
+	dma_pool_free(dev->parent->dma_pool, rqd.meta_list, rqd.dma_meta_list);
 	return ret;
 }
 
diff --git a/drivers/lightnvm/pblk-read.c b/drivers/lightnvm/pblk-read.c
index 32b285cf5846..1576f357c9af 100644
--- a/drivers/lightnvm/pblk-read.c
+++ b/drivers/lightnvm/pblk-read.c
@@ -507,7 +507,8 @@ int pblk_submit_read(struct pblk *pblk, struct bio *bio)
 	return NVM_IO_OK;
 
 fail_meta_free:
-	nvm_dev_dma_free(dev->parent, rqd->meta_list, rqd->dma_meta_list);
+	dma_pool_free(dev->parent->dma_pool, rqd->meta_list,
+							rqd->dma_meta_list);
 fail_rqd_free:
 	pblk_free_rqd(pblk, rqd, PBLK_READ);
 	return ret;
diff --git a/drivers/lightnvm/pblk-recovery.c b/drivers/lightnvm/pblk-recovery.c
index 65abc043e268..80d5b5bd4ab7 100644
--- a/drivers/lightnvm/pblk-recovery.c
+++ b/drivers/lightnvm/pblk-recovery.c
@@ -472,7 +472,8 @@ static int pblk_recov_l2p_from_oob(struct pblk *pblk, struct pblk_line *line)
 	dma_addr_t dma_ppa_list, dma_meta_list;
 	int ret = 0;
 
-	meta_list = nvm_dev_dma_alloc(dev->parent, GFP_KERNEL, &dma_meta_list);
+	meta_list = dma_pool_alloc(dev->parent->dma_pool, GFP_KERNEL,
+								&dma_meta_list);
 	if (!meta_list)
 		return -ENOMEM;
 
@@ -508,7 +509,7 @@ static int pblk_recov_l2p_from_oob(struct pblk *pblk, struct pblk_line *line)
 	mempool_free(rqd, &pblk->r_rq_pool);
 	kfree(data);
 free_meta_list:
-	nvm_dev_dma_free(dev->parent, meta_list, dma_meta_list);
+	dma_pool_free(dev->parent->dma_pool, meta_list, dma_meta_list);
 
 	return ret;
 }
diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c
index 049425ad8592..dd300ce9983f 100644
--- a/drivers/nvme/host/lightnvm.c
+++ b/drivers/nvme/host/lightnvm.c
@@ -747,18 +747,6 @@ static void nvme_nvm_destroy_dma_pool(void *pool)
 	dma_pool_destroy(dma_pool);
 }
 
-static void *nvme_nvm_dev_dma_alloc(struct nvm_dev *dev, void *pool,
-				    gfp_t mem_flags, dma_addr_t *dma_handler)
-{
-	return dma_pool_alloc(pool, mem_flags, dma_handler);
-}
-
-static void nvme_nvm_dev_dma_free(void *pool, void *addr,
-							dma_addr_t dma_handler)
-{
-	dma_pool_free(pool, addr, dma_handler);
-}
-
 static struct nvm_dev_ops nvme_nvm_dev_ops = {
 	.identity		= nvme_nvm_identity,
 
@@ -772,8 +760,6 @@ static struct nvm_dev_ops nvme_nvm_dev_ops = {
 
 	.create_dma_pool	= nvme_nvm_create_dma_pool,
 	.destroy_dma_pool	= nvme_nvm_destroy_dma_pool,
-	.dev_dma_alloc		= nvme_nvm_dev_dma_alloc,
-	.dev_dma_free		= nvme_nvm_dev_dma_free,
 };
 
 static int nvme_nvm_submit_user_cmd(struct request_queue *q,
@@ -985,7 +971,7 @@ void nvme_nvm_update_nvm_info(struct nvme_ns *ns)
 		geo->ext = ns->ext;
 	}
 
-	if (nvm_alloc_dma_pool(ndev))
+	if (nvm_create_dma_pool(ndev))
 		nvm_unregister(ndev);
 }
 
diff --git a/include/linux/lightnvm.h b/include/linux/lightnvm.h
index fd7b519f3ad2..7ca38b8bf133 100644
--- a/include/linux/lightnvm.h
+++ b/include/linux/lightnvm.h
@@ -94,7 +94,6 @@ typedef void *(nvm_create_dma_pool_fn)(struct nvm_dev *, char *, int);
 typedef void (nvm_destroy_dma_pool_fn)(void *);
 typedef void *(nvm_dev_dma_alloc_fn)(struct nvm_dev *, void *, gfp_t,
 								dma_addr_t *);
-typedef void (nvm_dev_dma_free_fn)(void *, void*, dma_addr_t);
 
 struct nvm_dev_ops {
 	nvm_id_fn		*identity;
@@ -108,8 +107,6 @@ struct nvm_dev_ops {
 
 	nvm_create_dma_pool_fn	*create_dma_pool;
 	nvm_destroy_dma_pool_fn	*destroy_dma_pool;
-	nvm_dev_dma_alloc_fn	*dev_dma_alloc;
-	nvm_dev_dma_free_fn	*dev_dma_free;
 };
 
 #ifdef CONFIG_NVM
@@ -669,11 +666,8 @@ struct nvm_tgt_type {
 extern int nvm_register_tgt_type(struct nvm_tgt_type *);
 extern void nvm_unregister_tgt_type(struct nvm_tgt_type *);
 
-extern void *nvm_dev_dma_alloc(struct nvm_dev *, gfp_t, dma_addr_t *);
-extern void nvm_dev_dma_free(struct nvm_dev *, void *, dma_addr_t);
-
 extern struct nvm_dev *nvm_alloc_dev(int);
-extern int nvm_alloc_dma_pool(struct nvm_dev *);
+extern int nvm_create_dma_pool(struct nvm_dev *);
 extern int nvm_register(struct nvm_dev *);
 extern void nvm_unregister(struct nvm_dev *);
 
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] lightnvm: remove dma alloc/free helpers
  2018-11-27 13:39 ` [PATCH] " Javier González
@ 2018-11-27 14:18   ` Matias Bjørling
  2018-11-27 14:22     ` Javier Gonzalez
  0 siblings, 1 reply; 5+ messages in thread
From: Matias Bjørling @ 2018-11-27 14:18 UTC (permalink / raw)
  To: javier; +Cc: igor.j.konopko, linux-block, linux-kernel, javier

On 11/27/2018 02:39 PM, Javier González wrote:
> Time has shown that the LightNVM alloc/free dma helpers are merely
> replacements of the native dma_pool operations. Thus, remove them and
> let targets manage the LightNVM dma pool directly.
> 
> Signed-off-by: Javier González <javier@cnexlabs.com>
> ---
>   drivers/lightnvm/core.c          | 25 +++++++------------------
>   drivers/lightnvm/pblk-core.c     | 10 +++++-----
>   drivers/lightnvm/pblk-read.c     |  3 ++-
>   drivers/lightnvm/pblk-recovery.c |  5 +++--
>   drivers/nvme/host/lightnvm.c     | 16 +---------------
>   include/linux/lightnvm.h         |  8 +-------
>   6 files changed, 19 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
> index c3650b141a30..8b6ee35e4356 100644
> --- a/drivers/lightnvm/core.c
> +++ b/drivers/lightnvm/core.c
> @@ -641,20 +641,6 @@ void nvm_unregister_tgt_type(struct nvm_tgt_type *tt)
>   }
>   EXPORT_SYMBOL(nvm_unregister_tgt_type);
>   
> -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, dev->dma_pool, mem_flags,
> -								dma_handler);
> -}
> -EXPORT_SYMBOL(nvm_dev_dma_alloc);
> -
> -void nvm_dev_dma_free(struct nvm_dev *dev, void *addr, dma_addr_t dma_handler)
> -{
> -	dev->ops->dev_dma_free(dev->dma_pool, addr, dma_handler);
> -}
> -EXPORT_SYMBOL(nvm_dev_dma_free);
> -
>   static struct nvm_dev *nvm_find_nvm_dev(const char *name)
>   {
>   	struct nvm_dev *dev;
> @@ -682,7 +668,8 @@ static int nvm_set_rqd_ppalist(struct nvm_tgt_dev *tgt_dev, struct nvm_rq *rqd,
>   	}
>   
>   	rqd->nr_ppas = nr_ppas;
> -	rqd->ppa_list = nvm_dev_dma_alloc(dev, GFP_KERNEL, &rqd->dma_ppa_list);
> +	rqd->ppa_list = dma_pool_alloc(dev->dma_pool, GFP_KERNEL,
> +							&rqd->dma_ppa_list);
>   	if (!rqd->ppa_list) {
>   		pr_err("nvm: failed to allocate dma memory\n");
>   		return -ENOMEM;
> @@ -705,10 +692,12 @@ static int nvm_set_rqd_ppalist(struct nvm_tgt_dev *tgt_dev, struct nvm_rq *rqd,
>   static void nvm_free_rqd_ppalist(struct nvm_tgt_dev *tgt_dev,
>   			struct nvm_rq *rqd)
>   {
> +	struct nvm_dev *dev = tgt_dev->parent;
> +
>   	if (!rqd->ppa_list)
>   		return;
>   
> -	nvm_dev_dma_free(tgt_dev->parent, rqd->ppa_list, rqd->dma_ppa_list);
> +	dma_pool_free(dev->dma_pool, rqd->ppa_list, rqd->dma_ppa_list);
>   }
>   
>   static int nvm_set_flags(struct nvm_geo *geo, struct nvm_rq *rqd)
> @@ -1178,7 +1167,7 @@ void nvm_unregister(struct nvm_dev *dev)
>   }
>   EXPORT_SYMBOL(nvm_unregister);
>   
> -int nvm_alloc_dma_pool(struct nvm_dev *dev)
> +int nvm_create_dma_pool(struct nvm_dev *dev)
>   {
>   	int exp_pool_size;
>   
> @@ -1195,7 +1184,7 @@ int nvm_alloc_dma_pool(struct nvm_dev *dev)
>   
>   	return 0;
>   }
> -EXPORT_SYMBOL(nvm_alloc_dma_pool);
> +EXPORT_SYMBOL(nvm_create_dma_pool);
>   
>   static int __nvm_configure_create(struct nvm_ioctl_create *create)
>   {
> diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c
> index 615817bf97e3..61a2a5330ecf 100644
> --- a/drivers/lightnvm/pblk-core.c
> +++ b/drivers/lightnvm/pblk-core.c
> @@ -242,7 +242,7 @@ int pblk_alloc_rqd_meta(struct pblk *pblk, struct nvm_rq *rqd)
>   {
>   	struct nvm_tgt_dev *dev = pblk->dev;
>   
> -	rqd->meta_list = nvm_dev_dma_alloc(dev->parent, GFP_KERNEL,
> +	rqd->meta_list = dma_pool_alloc(dev->parent->dma_pool, GFP_KERNEL,
>   							&rqd->dma_meta_list);
>   	if (!rqd->meta_list)
>   		return -ENOMEM;
> @@ -261,8 +261,8 @@ void pblk_free_rqd_meta(struct pblk *pblk, struct nvm_rq *rqd)
>   	struct nvm_tgt_dev *dev = pblk->dev;
>   
>   	if (rqd->meta_list)
> -		nvm_dev_dma_free(dev->parent, rqd->meta_list,
> -				rqd->dma_meta_list);
> +		dma_pool_free(dev->parent->dma_pool, rqd->meta_list,
> +							rqd->dma_meta_list);
>   }
>   
>   /* Caller must guarantee that the request is a valid type */
> @@ -846,7 +846,7 @@ int pblk_line_emeta_read(struct pblk *pblk, struct pblk_line *line,
>   	int i, j;
>   	int ret;
>   
> -	meta_list = nvm_dev_dma_alloc(dev->parent, GFP_KERNEL,
> +	meta_list = dma_pool_alloc(dev->parent->dma_pool, GFP_KERNEL,
>   							&dma_meta_list);
>   	if (!meta_list)
>   		return -ENOMEM;
> @@ -925,7 +925,7 @@ int pblk_line_emeta_read(struct pblk *pblk, struct pblk_line *line,
>   		goto next_rq;
>   
>   free_rqd_dma:
> -	nvm_dev_dma_free(dev->parent, rqd.meta_list, rqd.dma_meta_list);
> +	dma_pool_free(dev->parent->dma_pool, rqd.meta_list, rqd.dma_meta_list);
>   	return ret;
>   }
>   
> diff --git a/drivers/lightnvm/pblk-read.c b/drivers/lightnvm/pblk-read.c
> index 32b285cf5846..1576f357c9af 100644
> --- a/drivers/lightnvm/pblk-read.c
> +++ b/drivers/lightnvm/pblk-read.c
> @@ -507,7 +507,8 @@ int pblk_submit_read(struct pblk *pblk, struct bio *bio)
>   	return NVM_IO_OK;
>   
>   fail_meta_free:
> -	nvm_dev_dma_free(dev->parent, rqd->meta_list, rqd->dma_meta_list);
> +	dma_pool_free(dev->parent->dma_pool, rqd->meta_list,
> +							rqd->dma_meta_list);
>   fail_rqd_free:
>   	pblk_free_rqd(pblk, rqd, PBLK_READ);
>   	return ret;
> diff --git a/drivers/lightnvm/pblk-recovery.c b/drivers/lightnvm/pblk-recovery.c
> index 65abc043e268..80d5b5bd4ab7 100644
> --- a/drivers/lightnvm/pblk-recovery.c
> +++ b/drivers/lightnvm/pblk-recovery.c
> @@ -472,7 +472,8 @@ static int pblk_recov_l2p_from_oob(struct pblk *pblk, struct pblk_line *line)
>   	dma_addr_t dma_ppa_list, dma_meta_list;
>   	int ret = 0;
>   
> -	meta_list = nvm_dev_dma_alloc(dev->parent, GFP_KERNEL, &dma_meta_list);
> +	meta_list = dma_pool_alloc(dev->parent->dma_pool, GFP_KERNEL,
> +								&dma_meta_list);
>   	if (!meta_list)
>   		return -ENOMEM;
>   
> @@ -508,7 +509,7 @@ static int pblk_recov_l2p_from_oob(struct pblk *pblk, struct pblk_line *line)
>   	mempool_free(rqd, &pblk->r_rq_pool);
>   	kfree(data);
>   free_meta_list:
> -	nvm_dev_dma_free(dev->parent, meta_list, dma_meta_list);
> +	dma_pool_free(dev->parent->dma_pool, meta_list, dma_meta_list);
>   
>   	return ret;
>   }
> diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c
> index 049425ad8592..dd300ce9983f 100644
> --- a/drivers/nvme/host/lightnvm.c
> +++ b/drivers/nvme/host/lightnvm.c
> @@ -747,18 +747,6 @@ static void nvme_nvm_destroy_dma_pool(void *pool)
>   	dma_pool_destroy(dma_pool);
>   }
>   
> -static void *nvme_nvm_dev_dma_alloc(struct nvm_dev *dev, void *pool,
> -				    gfp_t mem_flags, dma_addr_t *dma_handler)
> -{
> -	return dma_pool_alloc(pool, mem_flags, dma_handler);
> -}
> -
> -static void nvme_nvm_dev_dma_free(void *pool, void *addr,
> -							dma_addr_t dma_handler)
> -{
> -	dma_pool_free(pool, addr, dma_handler);
> -}
> -
>   static struct nvm_dev_ops nvme_nvm_dev_ops = {
>   	.identity		= nvme_nvm_identity,
>   
> @@ -772,8 +760,6 @@ static struct nvm_dev_ops nvme_nvm_dev_ops = {
>   
>   	.create_dma_pool	= nvme_nvm_create_dma_pool,
>   	.destroy_dma_pool	= nvme_nvm_destroy_dma_pool,
> -	.dev_dma_alloc		= nvme_nvm_dev_dma_alloc,
> -	.dev_dma_free		= nvme_nvm_dev_dma_free,
>   };
>   
>   static int nvme_nvm_submit_user_cmd(struct request_queue *q,
> @@ -985,7 +971,7 @@ void nvme_nvm_update_nvm_info(struct nvme_ns *ns)
>   		geo->ext = ns->ext;
>   	}
>   
> -	if (nvm_alloc_dma_pool(ndev))
> +	if (nvm_create_dma_pool(ndev))
>   		nvm_unregister(ndev);
>   }
>   
> diff --git a/include/linux/lightnvm.h b/include/linux/lightnvm.h
> index fd7b519f3ad2..7ca38b8bf133 100644
> --- a/include/linux/lightnvm.h
> +++ b/include/linux/lightnvm.h
> @@ -94,7 +94,6 @@ typedef void *(nvm_create_dma_pool_fn)(struct nvm_dev *, char *, int);
>   typedef void (nvm_destroy_dma_pool_fn)(void *);
>   typedef void *(nvm_dev_dma_alloc_fn)(struct nvm_dev *, void *, gfp_t,
>   								dma_addr_t *);
> -typedef void (nvm_dev_dma_free_fn)(void *, void*, dma_addr_t);
>   
>   struct nvm_dev_ops {
>   	nvm_id_fn		*identity;
> @@ -108,8 +107,6 @@ struct nvm_dev_ops {
>   
>   	nvm_create_dma_pool_fn	*create_dma_pool;
>   	nvm_destroy_dma_pool_fn	*destroy_dma_pool;
> -	nvm_dev_dma_alloc_fn	*dev_dma_alloc;
> -	nvm_dev_dma_free_fn	*dev_dma_free;
>   };
>   
>   #ifdef CONFIG_NVM
> @@ -669,11 +666,8 @@ struct nvm_tgt_type {
>   extern int nvm_register_tgt_type(struct nvm_tgt_type *);
>   extern void nvm_unregister_tgt_type(struct nvm_tgt_type *);
>   
> -extern void *nvm_dev_dma_alloc(struct nvm_dev *, gfp_t, dma_addr_t *);
> -extern void nvm_dev_dma_free(struct nvm_dev *, void *, dma_addr_t);
> -
>   extern struct nvm_dev *nvm_alloc_dev(int);
> -extern int nvm_alloc_dma_pool(struct nvm_dev *);
> +extern int nvm_create_dma_pool(struct nvm_dev *);
>   extern int nvm_register(struct nvm_dev *);
>   extern void nvm_unregister(struct nvm_dev *);
>   
> 

I kindly would like to reject this patch. One reason is that target's 
should not have direct access to DMA pools and another reason is that 
the code changes when OCSSD is used over Fabrics. The DMA pool is no 
longer available and data has to be allocated differently. This code 
does not belong inside a target.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] lightnvm: remove dma alloc/free helpers
  2018-11-27 14:18   ` Matias Bjørling
@ 2018-11-27 14:22     ` Javier Gonzalez
  2018-11-28  0:47       ` Igor Konopko
  0 siblings, 1 reply; 5+ messages in thread
From: Javier Gonzalez @ 2018-11-27 14:22 UTC (permalink / raw)
  To: Matias Bjørling; +Cc: javier, igor.j.konopko, linux-block, linux-kernel



> On 27 Nov 2018, at 15.18, Matias Bjørling <mb@lightnvm.io> wrote:
> 
>> On 11/27/2018 02:39 PM, Javier González wrote:
>> Time has shown that the LightNVM alloc/free dma helpers are merely
>> replacements of the native dma_pool operations. Thus, remove them and
>> let targets manage the LightNVM dma pool directly.
>> Signed-off-by: Javier González <javier@cnexlabs.com>
>> ---
>>  drivers/lightnvm/core.c          | 25 +++++++------------------
>>  drivers/lightnvm/pblk-core.c     | 10 +++++-----
>>  drivers/lightnvm/pblk-read.c     |  3 ++-
>>  drivers/lightnvm/pblk-recovery.c |  5 +++--
>>  drivers/nvme/host/lightnvm.c     | 16 +---------------
>>  include/linux/lightnvm.h         |  8 +-------
>>  6 files changed, 19 insertions(+), 48 deletions(-)
>> diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
>> index c3650b141a30..8b6ee35e4356 100644
>> --- a/drivers/lightnvm/core.c
>> +++ b/drivers/lightnvm/core.c
>> @@ -641,20 +641,6 @@ void nvm_unregister_tgt_type(struct nvm_tgt_type *tt)
>>  }
>>  EXPORT_SYMBOL(nvm_unregister_tgt_type);
>>  -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, dev->dma_pool, mem_flags,
>> -                                dma_handler);
>> -}
>> -EXPORT_SYMBOL(nvm_dev_dma_alloc);
>> -
>> -void nvm_dev_dma_free(struct nvm_dev *dev, void *addr, dma_addr_t dma_handler)
>> -{
>> -    dev->ops->dev_dma_free(dev->dma_pool, addr, dma_handler);
>> -}
>> -EXPORT_SYMBOL(nvm_dev_dma_free);
>> -
>>  static struct nvm_dev *nvm_find_nvm_dev(const char *name)
>>  {
>>      struct nvm_dev *dev;
>> @@ -682,7 +668,8 @@ static int nvm_set_rqd_ppalist(struct nvm_tgt_dev *tgt_dev, struct nvm_rq *rqd,
>>      }
>>        rqd->nr_ppas = nr_ppas;
>> -    rqd->ppa_list = nvm_dev_dma_alloc(dev, GFP_KERNEL, &rqd->dma_ppa_list);
>> +    rqd->ppa_list = dma_pool_alloc(dev->dma_pool, GFP_KERNEL,
>> +                            &rqd->dma_ppa_list);
>>      if (!rqd->ppa_list) {
>>          pr_err("nvm: failed to allocate dma memory\n");
>>          return -ENOMEM;
>> @@ -705,10 +692,12 @@ static int nvm_set_rqd_ppalist(struct nvm_tgt_dev *tgt_dev, struct nvm_rq *rqd,
>>  static void nvm_free_rqd_ppalist(struct nvm_tgt_dev *tgt_dev,
>>              struct nvm_rq *rqd)
>>  {
>> +    struct nvm_dev *dev = tgt_dev->parent;
>> +
>>      if (!rqd->ppa_list)
>>          return;
>>  -    nvm_dev_dma_free(tgt_dev->parent, rqd->ppa_list, rqd->dma_ppa_list);
>> +    dma_pool_free(dev->dma_pool, rqd->ppa_list, rqd->dma_ppa_list);
>>  }
>>    static int nvm_set_flags(struct nvm_geo *geo, struct nvm_rq *rqd)
>> @@ -1178,7 +1167,7 @@ void nvm_unregister(struct nvm_dev *dev)
>>  }
>>  EXPORT_SYMBOL(nvm_unregister);
>>  -int nvm_alloc_dma_pool(struct nvm_dev *dev)
>> +int nvm_create_dma_pool(struct nvm_dev *dev)
>>  {
>>      int exp_pool_size;
>>  @@ -1195,7 +1184,7 @@ int nvm_alloc_dma_pool(struct nvm_dev *dev)
>>        return 0;
>>  }
>> -EXPORT_SYMBOL(nvm_alloc_dma_pool);
>> +EXPORT_SYMBOL(nvm_create_dma_pool);
>>    static int __nvm_configure_create(struct nvm_ioctl_create *create)
>>  {
>> diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c
>> index 615817bf97e3..61a2a5330ecf 100644
>> --- a/drivers/lightnvm/pblk-core.c
>> +++ b/drivers/lightnvm/pblk-core.c
>> @@ -242,7 +242,7 @@ int pblk_alloc_rqd_meta(struct pblk *pblk, struct nvm_rq *rqd)
>>  {
>>      struct nvm_tgt_dev *dev = pblk->dev;
>>  -    rqd->meta_list = nvm_dev_dma_alloc(dev->parent, GFP_KERNEL,
>> +    rqd->meta_list = dma_pool_alloc(dev->parent->dma_pool, GFP_KERNEL,
>>                              &rqd->dma_meta_list);
>>      if (!rqd->meta_list)
>>          return -ENOMEM;
>> @@ -261,8 +261,8 @@ void pblk_free_rqd_meta(struct pblk *pblk, struct nvm_rq *rqd)
>>      struct nvm_tgt_dev *dev = pblk->dev;
>>        if (rqd->meta_list)
>> -        nvm_dev_dma_free(dev->parent, rqd->meta_list,
>> -                rqd->dma_meta_list);
>> +        dma_pool_free(dev->parent->dma_pool, rqd->meta_list,
>> +                            rqd->dma_meta_list);
>>  }
>>    /* Caller must guarantee that the request is a valid type */
>> @@ -846,7 +846,7 @@ int pblk_line_emeta_read(struct pblk *pblk, struct pblk_line *line,
>>      int i, j;
>>      int ret;
>>  -    meta_list = nvm_dev_dma_alloc(dev->parent, GFP_KERNEL,
>> +    meta_list = dma_pool_alloc(dev->parent->dma_pool, GFP_KERNEL,
>>                              &dma_meta_list);
>>      if (!meta_list)
>>          return -ENOMEM;
>> @@ -925,7 +925,7 @@ int pblk_line_emeta_read(struct pblk *pblk, struct pblk_line *line,
>>          goto next_rq;
>>    free_rqd_dma:
>> -    nvm_dev_dma_free(dev->parent, rqd.meta_list, rqd.dma_meta_list);
>> +    dma_pool_free(dev->parent->dma_pool, rqd.meta_list, rqd.dma_meta_list);
>>      return ret;
>>  }
>>  diff --git a/drivers/lightnvm/pblk-read.c b/drivers/lightnvm/pblk-read.c
>> index 32b285cf5846..1576f357c9af 100644
>> --- a/drivers/lightnvm/pblk-read.c
>> +++ b/drivers/lightnvm/pblk-read.c
>> @@ -507,7 +507,8 @@ int pblk_submit_read(struct pblk *pblk, struct bio *bio)
>>      return NVM_IO_OK;
>>    fail_meta_free:
>> -    nvm_dev_dma_free(dev->parent, rqd->meta_list, rqd->dma_meta_list);
>> +    dma_pool_free(dev->parent->dma_pool, rqd->meta_list,
>> +                            rqd->dma_meta_list);
>>  fail_rqd_free:
>>      pblk_free_rqd(pblk, rqd, PBLK_READ);
>>      return ret;
>> diff --git a/drivers/lightnvm/pblk-recovery.c b/drivers/lightnvm/pblk-recovery.c
>> index 65abc043e268..80d5b5bd4ab7 100644
>> --- a/drivers/lightnvm/pblk-recovery.c
>> +++ b/drivers/lightnvm/pblk-recovery.c
>> @@ -472,7 +472,8 @@ static int pblk_recov_l2p_from_oob(struct pblk *pblk, struct pblk_line *line)
>>      dma_addr_t dma_ppa_list, dma_meta_list;
>>      int ret = 0;
>>  -    meta_list = nvm_dev_dma_alloc(dev->parent, GFP_KERNEL, &dma_meta_list);
>> +    meta_list = dma_pool_alloc(dev->parent->dma_pool, GFP_KERNEL,
>> +                                &dma_meta_list);
>>      if (!meta_list)
>>          return -ENOMEM;
>>  @@ -508,7 +509,7 @@ static int pblk_recov_l2p_from_oob(struct pblk *pblk, struct pblk_line *line)
>>      mempool_free(rqd, &pblk->r_rq_pool);
>>      kfree(data);
>>  free_meta_list:
>> -    nvm_dev_dma_free(dev->parent, meta_list, dma_meta_list);
>> +    dma_pool_free(dev->parent->dma_pool, meta_list, dma_meta_list);
>>        return ret;
>>  }
>> diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c
>> index 049425ad8592..dd300ce9983f 100644
>> --- a/drivers/nvme/host/lightnvm.c
>> +++ b/drivers/nvme/host/lightnvm.c
>> @@ -747,18 +747,6 @@ static void nvme_nvm_destroy_dma_pool(void *pool)
>>      dma_pool_destroy(dma_pool);
>>  }
>>  -static void *nvme_nvm_dev_dma_alloc(struct nvm_dev *dev, void *pool,
>> -                    gfp_t mem_flags, dma_addr_t *dma_handler)
>> -{
>> -    return dma_pool_alloc(pool, mem_flags, dma_handler);
>> -}
>> -
>> -static void nvme_nvm_dev_dma_free(void *pool, void *addr,
>> -                            dma_addr_t dma_handler)
>> -{
>> -    dma_pool_free(pool, addr, dma_handler);
>> -}
>> -
>>  static struct nvm_dev_ops nvme_nvm_dev_ops = {
>>      .identity        = nvme_nvm_identity,
>>  @@ -772,8 +760,6 @@ static struct nvm_dev_ops nvme_nvm_dev_ops = {
>>        .create_dma_pool    = nvme_nvm_create_dma_pool,
>>      .destroy_dma_pool    = nvme_nvm_destroy_dma_pool,
>> -    .dev_dma_alloc        = nvme_nvm_dev_dma_alloc,
>> -    .dev_dma_free        = nvme_nvm_dev_dma_free,
>>  };
>>    static int nvme_nvm_submit_user_cmd(struct request_queue *q,
>> @@ -985,7 +971,7 @@ void nvme_nvm_update_nvm_info(struct nvme_ns *ns)
>>          geo->ext = ns->ext;
>>      }
>>  -    if (nvm_alloc_dma_pool(ndev))
>> +    if (nvm_create_dma_pool(ndev))
>>          nvm_unregister(ndev);
>>  }
>>  diff --git a/include/linux/lightnvm.h b/include/linux/lightnvm.h
>> index fd7b519f3ad2..7ca38b8bf133 100644
>> --- a/include/linux/lightnvm.h
>> +++ b/include/linux/lightnvm.h
>> @@ -94,7 +94,6 @@ typedef void *(nvm_create_dma_pool_fn)(struct nvm_dev *, char *, int);
>>  typedef void (nvm_destroy_dma_pool_fn)(void *);
>>  typedef void *(nvm_dev_dma_alloc_fn)(struct nvm_dev *, void *, gfp_t,
>>                                  dma_addr_t *);
>> -typedef void (nvm_dev_dma_free_fn)(void *, void*, dma_addr_t);
>>    struct nvm_dev_ops {
>>      nvm_id_fn        *identity;
>> @@ -108,8 +107,6 @@ struct nvm_dev_ops {
>>        nvm_create_dma_pool_fn    *create_dma_pool;
>>      nvm_destroy_dma_pool_fn    *destroy_dma_pool;
>> -    nvm_dev_dma_alloc_fn    *dev_dma_alloc;
>> -    nvm_dev_dma_free_fn    *dev_dma_free;
>>  };
>>    #ifdef CONFIG_NVM
>> @@ -669,11 +666,8 @@ struct nvm_tgt_type {
>>  extern int nvm_register_tgt_type(struct nvm_tgt_type *);
>>  extern void nvm_unregister_tgt_type(struct nvm_tgt_type *);
>>  -extern void *nvm_dev_dma_alloc(struct nvm_dev *, gfp_t, dma_addr_t *);
>> -extern void nvm_dev_dma_free(struct nvm_dev *, void *, dma_addr_t);
>> -
>>  extern struct nvm_dev *nvm_alloc_dev(int);
>> -extern int nvm_alloc_dma_pool(struct nvm_dev *);
>> +extern int nvm_create_dma_pool(struct nvm_dev *);
>>  extern int nvm_register(struct nvm_dev *);
>>  extern void nvm_unregister(struct nvm_dev *);
>>  
> 
> I kindly would like to reject this patch. One reason is that target's should not have direct access to DMA pools and another reason is that the code changes when OCSSD is used over Fabrics. The DMA pool is no longer available and data has to be allocated differently. This code does not belong inside a target.

Ok. I just sent it based on the discussion we had around the dma pools during the review of Igor’s patches. If I remember correctly, you backed this at that time... Anyway, there is no behavior change, so just ignore. 

Igor: can you still make the name change on the dma pool creation?

Javier. 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] lightnvm: remove dma alloc/free helpers
  2018-11-27 14:22     ` Javier Gonzalez
@ 2018-11-28  0:47       ` Igor Konopko
  0 siblings, 0 replies; 5+ messages in thread
From: Igor Konopko @ 2018-11-28  0:47 UTC (permalink / raw)
  To: Javier Gonzalez, Matias Bjørling; +Cc: javier, linux-block, linux-kernel



On 27.11.2018 15:22, Javier Gonzalez wrote:
> 
> 
>> On 27 Nov 2018, at 15.18, Matias Bjørling <mb@lightnvm.io> wrote:
>>
>>> On 11/27/2018 02:39 PM, Javier González wrote:
>>> Time has shown that the LightNVM alloc/free dma helpers are merely
>>> replacements of the native dma_pool operations. Thus, remove them and
>>> let targets manage the LightNVM dma pool directly.
>>> Signed-off-by: Javier González <javier@cnexlabs.com>
>>> ---
>>>   drivers/lightnvm/core.c          | 25 +++++++------------------
>>>   drivers/lightnvm/pblk-core.c     | 10 +++++-----
>>>   drivers/lightnvm/pblk-read.c     |  3 ++-
>>>   drivers/lightnvm/pblk-recovery.c |  5 +++--
>>>   drivers/nvme/host/lightnvm.c     | 16 +---------------
>>>   include/linux/lightnvm.h         |  8 +-------
>>>   6 files changed, 19 insertions(+), 48 deletions(-)
>>> diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
>>> index c3650b141a30..8b6ee35e4356 100644
>>> --- a/drivers/lightnvm/core.c
>>> +++ b/drivers/lightnvm/core.c
>>> @@ -641,20 +641,6 @@ void nvm_unregister_tgt_type(struct nvm_tgt_type *tt)
>>>   }
>>>   EXPORT_SYMBOL(nvm_unregister_tgt_type);
>>>   -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, dev->dma_pool, mem_flags,
>>> -                                dma_handler);
>>> -}
>>> -EXPORT_SYMBOL(nvm_dev_dma_alloc);
>>> -
>>> -void nvm_dev_dma_free(struct nvm_dev *dev, void *addr, dma_addr_t dma_handler)
>>> -{
>>> -    dev->ops->dev_dma_free(dev->dma_pool, addr, dma_handler);
>>> -}
>>> -EXPORT_SYMBOL(nvm_dev_dma_free);
>>> -
>>>   static struct nvm_dev *nvm_find_nvm_dev(const char *name)
>>>   {
>>>       struct nvm_dev *dev;
>>> @@ -682,7 +668,8 @@ static int nvm_set_rqd_ppalist(struct nvm_tgt_dev *tgt_dev, struct nvm_rq *rqd,
>>>       }
>>>         rqd->nr_ppas = nr_ppas;
>>> -    rqd->ppa_list = nvm_dev_dma_alloc(dev, GFP_KERNEL, &rqd->dma_ppa_list);
>>> +    rqd->ppa_list = dma_pool_alloc(dev->dma_pool, GFP_KERNEL,
>>> +                            &rqd->dma_ppa_list);
>>>       if (!rqd->ppa_list) {
>>>           pr_err("nvm: failed to allocate dma memory\n");
>>>           return -ENOMEM;
>>> @@ -705,10 +692,12 @@ static int nvm_set_rqd_ppalist(struct nvm_tgt_dev *tgt_dev, struct nvm_rq *rqd,
>>>   static void nvm_free_rqd_ppalist(struct nvm_tgt_dev *tgt_dev,
>>>               struct nvm_rq *rqd)
>>>   {
>>> +    struct nvm_dev *dev = tgt_dev->parent;
>>> +
>>>       if (!rqd->ppa_list)
>>>           return;
>>>   -    nvm_dev_dma_free(tgt_dev->parent, rqd->ppa_list, rqd->dma_ppa_list);
>>> +    dma_pool_free(dev->dma_pool, rqd->ppa_list, rqd->dma_ppa_list);
>>>   }
>>>     static int nvm_set_flags(struct nvm_geo *geo, struct nvm_rq *rqd)
>>> @@ -1178,7 +1167,7 @@ void nvm_unregister(struct nvm_dev *dev)
>>>   }
>>>   EXPORT_SYMBOL(nvm_unregister);
>>>   -int nvm_alloc_dma_pool(struct nvm_dev *dev)
>>> +int nvm_create_dma_pool(struct nvm_dev *dev)
>>>   {
>>>       int exp_pool_size;
>>>   @@ -1195,7 +1184,7 @@ int nvm_alloc_dma_pool(struct nvm_dev *dev)
>>>         return 0;
>>>   }
>>> -EXPORT_SYMBOL(nvm_alloc_dma_pool);
>>> +EXPORT_SYMBOL(nvm_create_dma_pool);
>>>     static int __nvm_configure_create(struct nvm_ioctl_create *create)
>>>   {
>>> diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c
>>> index 615817bf97e3..61a2a5330ecf 100644
>>> --- a/drivers/lightnvm/pblk-core.c
>>> +++ b/drivers/lightnvm/pblk-core.c
>>> @@ -242,7 +242,7 @@ int pblk_alloc_rqd_meta(struct pblk *pblk, struct nvm_rq *rqd)
>>>   {
>>>       struct nvm_tgt_dev *dev = pblk->dev;
>>>   -    rqd->meta_list = nvm_dev_dma_alloc(dev->parent, GFP_KERNEL,
>>> +    rqd->meta_list = dma_pool_alloc(dev->parent->dma_pool, GFP_KERNEL,
>>>                               &rqd->dma_meta_list);
>>>       if (!rqd->meta_list)
>>>           return -ENOMEM;
>>> @@ -261,8 +261,8 @@ void pblk_free_rqd_meta(struct pblk *pblk, struct nvm_rq *rqd)
>>>       struct nvm_tgt_dev *dev = pblk->dev;
>>>         if (rqd->meta_list)
>>> -        nvm_dev_dma_free(dev->parent, rqd->meta_list,
>>> -                rqd->dma_meta_list);
>>> +        dma_pool_free(dev->parent->dma_pool, rqd->meta_list,
>>> +                            rqd->dma_meta_list);
>>>   }
>>>     /* Caller must guarantee that the request is a valid type */
>>> @@ -846,7 +846,7 @@ int pblk_line_emeta_read(struct pblk *pblk, struct pblk_line *line,
>>>       int i, j;
>>>       int ret;
>>>   -    meta_list = nvm_dev_dma_alloc(dev->parent, GFP_KERNEL,
>>> +    meta_list = dma_pool_alloc(dev->parent->dma_pool, GFP_KERNEL,
>>>                               &dma_meta_list);
>>>       if (!meta_list)
>>>           return -ENOMEM;
>>> @@ -925,7 +925,7 @@ int pblk_line_emeta_read(struct pblk *pblk, struct pblk_line *line,
>>>           goto next_rq;
>>>     free_rqd_dma:
>>> -    nvm_dev_dma_free(dev->parent, rqd.meta_list, rqd.dma_meta_list);
>>> +    dma_pool_free(dev->parent->dma_pool, rqd.meta_list, rqd.dma_meta_list);
>>>       return ret;
>>>   }
>>>   diff --git a/drivers/lightnvm/pblk-read.c b/drivers/lightnvm/pblk-read.c
>>> index 32b285cf5846..1576f357c9af 100644
>>> --- a/drivers/lightnvm/pblk-read.c
>>> +++ b/drivers/lightnvm/pblk-read.c
>>> @@ -507,7 +507,8 @@ int pblk_submit_read(struct pblk *pblk, struct bio *bio)
>>>       return NVM_IO_OK;
>>>     fail_meta_free:
>>> -    nvm_dev_dma_free(dev->parent, rqd->meta_list, rqd->dma_meta_list);
>>> +    dma_pool_free(dev->parent->dma_pool, rqd->meta_list,
>>> +                            rqd->dma_meta_list);
>>>   fail_rqd_free:
>>>       pblk_free_rqd(pblk, rqd, PBLK_READ);
>>>       return ret;
>>> diff --git a/drivers/lightnvm/pblk-recovery.c b/drivers/lightnvm/pblk-recovery.c
>>> index 65abc043e268..80d5b5bd4ab7 100644
>>> --- a/drivers/lightnvm/pblk-recovery.c
>>> +++ b/drivers/lightnvm/pblk-recovery.c
>>> @@ -472,7 +472,8 @@ static int pblk_recov_l2p_from_oob(struct pblk *pblk, struct pblk_line *line)
>>>       dma_addr_t dma_ppa_list, dma_meta_list;
>>>       int ret = 0;
>>>   -    meta_list = nvm_dev_dma_alloc(dev->parent, GFP_KERNEL, &dma_meta_list);
>>> +    meta_list = dma_pool_alloc(dev->parent->dma_pool, GFP_KERNEL,
>>> +                                &dma_meta_list);
>>>       if (!meta_list)
>>>           return -ENOMEM;
>>>   @@ -508,7 +509,7 @@ static int pblk_recov_l2p_from_oob(struct pblk *pblk, struct pblk_line *line)
>>>       mempool_free(rqd, &pblk->r_rq_pool);
>>>       kfree(data);
>>>   free_meta_list:
>>> -    nvm_dev_dma_free(dev->parent, meta_list, dma_meta_list);
>>> +    dma_pool_free(dev->parent->dma_pool, meta_list, dma_meta_list);
>>>         return ret;
>>>   }
>>> diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c
>>> index 049425ad8592..dd300ce9983f 100644
>>> --- a/drivers/nvme/host/lightnvm.c
>>> +++ b/drivers/nvme/host/lightnvm.c
>>> @@ -747,18 +747,6 @@ static void nvme_nvm_destroy_dma_pool(void *pool)
>>>       dma_pool_destroy(dma_pool);
>>>   }
>>>   -static void *nvme_nvm_dev_dma_alloc(struct nvm_dev *dev, void *pool,
>>> -                    gfp_t mem_flags, dma_addr_t *dma_handler)
>>> -{
>>> -    return dma_pool_alloc(pool, mem_flags, dma_handler);
>>> -}
>>> -
>>> -static void nvme_nvm_dev_dma_free(void *pool, void *addr,
>>> -                            dma_addr_t dma_handler)
>>> -{
>>> -    dma_pool_free(pool, addr, dma_handler);
>>> -}
>>> -
>>>   static struct nvm_dev_ops nvme_nvm_dev_ops = {
>>>       .identity        = nvme_nvm_identity,
>>>   @@ -772,8 +760,6 @@ static struct nvm_dev_ops nvme_nvm_dev_ops = {
>>>         .create_dma_pool    = nvme_nvm_create_dma_pool,
>>>       .destroy_dma_pool    = nvme_nvm_destroy_dma_pool,
>>> -    .dev_dma_alloc        = nvme_nvm_dev_dma_alloc,
>>> -    .dev_dma_free        = nvme_nvm_dev_dma_free,
>>>   };
>>>     static int nvme_nvm_submit_user_cmd(struct request_queue *q,
>>> @@ -985,7 +971,7 @@ void nvme_nvm_update_nvm_info(struct nvme_ns *ns)
>>>           geo->ext = ns->ext;
>>>       }
>>>   -    if (nvm_alloc_dma_pool(ndev))
>>> +    if (nvm_create_dma_pool(ndev))
>>>           nvm_unregister(ndev);
>>>   }
>>>   diff --git a/include/linux/lightnvm.h b/include/linux/lightnvm.h
>>> index fd7b519f3ad2..7ca38b8bf133 100644
>>> --- a/include/linux/lightnvm.h
>>> +++ b/include/linux/lightnvm.h
>>> @@ -94,7 +94,6 @@ typedef void *(nvm_create_dma_pool_fn)(struct nvm_dev *, char *, int);
>>>   typedef void (nvm_destroy_dma_pool_fn)(void *);
>>>   typedef void *(nvm_dev_dma_alloc_fn)(struct nvm_dev *, void *, gfp_t,
>>>                                   dma_addr_t *);
>>> -typedef void (nvm_dev_dma_free_fn)(void *, void*, dma_addr_t);
>>>     struct nvm_dev_ops {
>>>       nvm_id_fn        *identity;
>>> @@ -108,8 +107,6 @@ struct nvm_dev_ops {
>>>         nvm_create_dma_pool_fn    *create_dma_pool;
>>>       nvm_destroy_dma_pool_fn    *destroy_dma_pool;
>>> -    nvm_dev_dma_alloc_fn    *dev_dma_alloc;
>>> -    nvm_dev_dma_free_fn    *dev_dma_free;
>>>   };
>>>     #ifdef CONFIG_NVM
>>> @@ -669,11 +666,8 @@ struct nvm_tgt_type {
>>>   extern int nvm_register_tgt_type(struct nvm_tgt_type *);
>>>   extern void nvm_unregister_tgt_type(struct nvm_tgt_type *);
>>>   -extern void *nvm_dev_dma_alloc(struct nvm_dev *, gfp_t, dma_addr_t *);
>>> -extern void nvm_dev_dma_free(struct nvm_dev *, void *, dma_addr_t);
>>> -
>>>   extern struct nvm_dev *nvm_alloc_dev(int);
>>> -extern int nvm_alloc_dma_pool(struct nvm_dev *);
>>> +extern int nvm_create_dma_pool(struct nvm_dev *);
>>>   extern int nvm_register(struct nvm_dev *);
>>>   extern void nvm_unregister(struct nvm_dev *);
>>>   
>>
>> I kindly would like to reject this patch. One reason is that target's should not have direct access to DMA pools and another reason is that the code changes when OCSSD is used over Fabrics. The DMA pool is no longer available and data has to be allocated differently. This code does not belong inside a target.
> 
> Ok. I just sent it based on the discussion we had around the dma pools during the review of Igor’s patches. If I remember correctly, you backed this at that time... Anyway, there is no behavior change, so just ignore.
> 
> Igor: can you still make the name change on the dma pool creation?
> 

Sure, I'll change the name of that function together with other 
suggested changes and will send V4 of my series.

Igor

> Javier.
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-11-28  0:47 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-27 13:39 lightnvm: remove dma alloc/free helpers Javier González
2018-11-27 13:39 ` [PATCH] " Javier González
2018-11-27 14:18   ` Matias Bjørling
2018-11-27 14:22     ` Javier Gonzalez
2018-11-28  0:47       ` Igor Konopko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).