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

Groups > linux.kernel > #1742799 > unrolled thread

[PATCH 3/6] lightnvm: remove unused argument from nvm_set_tgt_bb_tbl

Back to article view | Back to linux.kernel

Contents

  [PATCH 3/6] lightnvm: remove unused argument from nvm_set_tgt_bb_tbl Rakesh Pandit <rakesh@tuxera.com> - 2017-10-01 15:30 +0200
    Re: [PATCH 3/6] lightnvm: remove unused argument from  nvm_set_tgt_bb_tbl Javier González <jg@lightnvm.io> - 2017-10-02 13:50 +0200

#1742799 — [PATCH 3/6] lightnvm: remove unused argument from nvm_set_tgt_bb_tbl

vblk isn't being used anyway and if we ever have a usecase we can
introduce this again.  This makes the logic easier and removes
unnecessary checks.

Signed-off-by: Rakesh Pandit <rakesh@tuxera.com>
---
 drivers/lightnvm/core.c  | 29 ++++++++++++-----------------
 include/linux/lightnvm.h |  2 +-
 2 files changed, 13 insertions(+), 18 deletions(-)

diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
index ddae430..9cd1c4b 100644
--- a/drivers/lightnvm/core.c
+++ b/drivers/lightnvm/core.c
@@ -616,7 +616,7 @@ int nvm_set_tgt_bb_tbl(struct nvm_tgt_dev *tgt_dev, struct ppa_addr *ppas,
 
 	memset(&rqd, 0, sizeof(struct nvm_rq));
 
-	nvm_set_rqd_ppalist(tgt_dev, &rqd, ppas, nr_ppas, 1);
+	nvm_set_rqd_ppalist(tgt_dev, &rqd, ppas, nr_ppas);
 	nvm_rq_tgt_to_dev(tgt_dev, &rqd);
 
 	ret = dev->ops->set_bb_tbl(dev, &rqd.ppa_addr, rqd.nr_ppas, type);
@@ -680,7 +680,7 @@ int nvm_erase_sync(struct nvm_tgt_dev *tgt_dev, struct ppa_addr *ppas,
 	rqd.private = &wait;
 	rqd.flags = geo->plane_mode >> 1;
 
-	ret = nvm_set_rqd_ppalist(tgt_dev, &rqd, ppas, nr_ppas, 1);
+	ret = nvm_set_rqd_ppalist(tgt_dev, &rqd, ppas, nr_ppas);
 	if (ret)
 		return ret;
 
@@ -776,14 +776,14 @@ void nvm_put_area(struct nvm_tgt_dev *tgt_dev, sector_t begin)
 EXPORT_SYMBOL(nvm_put_area);
 
 int nvm_set_rqd_ppalist(struct nvm_tgt_dev *tgt_dev, struct nvm_rq *rqd,
-			const struct ppa_addr *ppas, int nr_ppas, int vblk)
+			const struct ppa_addr *ppas, int nr_ppas)
 {
 	struct nvm_dev *dev = tgt_dev->parent;
 	struct nvm_geo *geo = &tgt_dev->geo;
 	int i, plane_cnt, pl_idx;
 	struct ppa_addr ppa;
 
-	if ((!vblk || geo->plane_mode == NVM_PLANE_SINGLE) && nr_ppas == 1) {
+	if (geo->plane_mode == NVM_PLANE_SINGLE && nr_ppas == 1) {
 		rqd->nr_ppas = nr_ppas;
 		rqd->ppa_addr = ppas[0];
 
@@ -797,19 +797,14 @@ int nvm_set_rqd_ppalist(struct nvm_tgt_dev *tgt_dev, struct nvm_rq *rqd,
 		return -ENOMEM;
 	}
 
-	if (!vblk) {
-		for (i = 0; i < nr_ppas; i++)
-			rqd->ppa_list[i] = ppas[i];
-	} else {
-		plane_cnt = geo->plane_mode;
-		rqd->nr_ppas *= plane_cnt;
-
-		for (i = 0; i < nr_ppas; i++) {
-			for (pl_idx = 0; pl_idx < plane_cnt; pl_idx++) {
-				ppa = ppas[i];
-				ppa.g.pl = pl_idx;
-				rqd->ppa_list[(pl_idx * nr_ppas) + i] = ppa;
-			}
+	plane_cnt = geo->plane_mode;
+	rqd->nr_ppas *= plane_cnt;
+
+	for (i = 0; i < nr_ppas; i++) {
+		for (pl_idx = 0; pl_idx < plane_cnt; pl_idx++) {
+			ppa = ppas[i];
+			ppa.g.pl = pl_idx;
+			rqd->ppa_list[(pl_idx * nr_ppas) + i] = ppa;
 		}
 	}
 
diff --git a/include/linux/lightnvm.h b/include/linux/lightnvm.h
index 81b71c6d..c8c014b 100644
--- a/include/linux/lightnvm.h
+++ b/include/linux/lightnvm.h
@@ -481,7 +481,7 @@ extern int nvm_max_phys_sects(struct nvm_tgt_dev *);
 extern int nvm_submit_io(struct nvm_tgt_dev *, struct nvm_rq *);
 extern int nvm_erase_sync(struct nvm_tgt_dev *, struct ppa_addr *, int);
 extern int nvm_set_rqd_ppalist(struct nvm_tgt_dev *, struct nvm_rq *,
-					const struct ppa_addr *, int, int);
+					const struct ppa_addr *, int);
 extern void nvm_free_rqd_ppalist(struct nvm_tgt_dev *, struct nvm_rq *);
 extern int nvm_get_l2p_tbl(struct nvm_tgt_dev *, u64, u32, nvm_l2p_update_fn *,
 			   void *);
-- 
2.7.4

[toc] | [next] | [standalone]

#1743139 — Re: [PATCH 3/6] lightnvm: remove unused argument from nvm_set_tgt_bb_tbl

[Multipart message — attachments visible in raw view] — view raw

> On 1 Oct 2017, at 15.24, Rakesh Pandit <rakesh@tuxera.com> wrote:
> 
> vblk isn't being used anyway and if we ever have a usecase we can
> introduce this again.  This makes the logic easier and removes
> unnecessary checks.
> 
> Signed-off-by: Rakesh Pandit <rakesh@tuxera.com>
> ---
> drivers/lightnvm/core.c  | 29 ++++++++++++-----------------
> include/linux/lightnvm.h |  2 +-
> 2 files changed, 13 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
> index ddae430..9cd1c4b 100644
> --- a/drivers/lightnvm/core.c
> +++ b/drivers/lightnvm/core.c
> @@ -616,7 +616,7 @@ int nvm_set_tgt_bb_tbl(struct nvm_tgt_dev *tgt_dev, struct ppa_addr *ppas,
> 
> 	memset(&rqd, 0, sizeof(struct nvm_rq));
> 
> -	nvm_set_rqd_ppalist(tgt_dev, &rqd, ppas, nr_ppas, 1);
> +	nvm_set_rqd_ppalist(tgt_dev, &rqd, ppas, nr_ppas);
> 	nvm_rq_tgt_to_dev(tgt_dev, &rqd);
> 
> 	ret = dev->ops->set_bb_tbl(dev, &rqd.ppa_addr, rqd.nr_ppas, type);
> @@ -680,7 +680,7 @@ int nvm_erase_sync(struct nvm_tgt_dev *tgt_dev, struct ppa_addr *ppas,
> 	rqd.private = &wait;
> 	rqd.flags = geo->plane_mode >> 1;
> 
> -	ret = nvm_set_rqd_ppalist(tgt_dev, &rqd, ppas, nr_ppas, 1);
> +	ret = nvm_set_rqd_ppalist(tgt_dev, &rqd, ppas, nr_ppas);
> 	if (ret)
> 		return ret;
> 
> @@ -776,14 +776,14 @@ void nvm_put_area(struct nvm_tgt_dev *tgt_dev, sector_t begin)
> EXPORT_SYMBOL(nvm_put_area);
> 
> int nvm_set_rqd_ppalist(struct nvm_tgt_dev *tgt_dev, struct nvm_rq *rqd,
> -			const struct ppa_addr *ppas, int nr_ppas, int vblk)
> +			const struct ppa_addr *ppas, int nr_ppas)
> {
> 	struct nvm_dev *dev = tgt_dev->parent;
> 	struct nvm_geo *geo = &tgt_dev->geo;
> 	int i, plane_cnt, pl_idx;
> 	struct ppa_addr ppa;
> 
> -	if ((!vblk || geo->plane_mode == NVM_PLANE_SINGLE) && nr_ppas == 1) {
> +	if (geo->plane_mode == NVM_PLANE_SINGLE && nr_ppas == 1) {
> 		rqd->nr_ppas = nr_ppas;
> 		rqd->ppa_addr = ppas[0];
> 
> @@ -797,19 +797,14 @@ int nvm_set_rqd_ppalist(struct nvm_tgt_dev *tgt_dev, struct nvm_rq *rqd,
> 		return -ENOMEM;
> 	}
> 
> -	if (!vblk) {
> -		for (i = 0; i < nr_ppas; i++)
> -			rqd->ppa_list[i] = ppas[i];
> -	} else {
> -		plane_cnt = geo->plane_mode;
> -		rqd->nr_ppas *= plane_cnt;
> -
> -		for (i = 0; i < nr_ppas; i++) {
> -			for (pl_idx = 0; pl_idx < plane_cnt; pl_idx++) {
> -				ppa = ppas[i];
> -				ppa.g.pl = pl_idx;
> -				rqd->ppa_list[(pl_idx * nr_ppas) + i] = ppa;
> -			}
> +	plane_cnt = geo->plane_mode;
> +	rqd->nr_ppas *= plane_cnt;
> +
> +	for (i = 0; i < nr_ppas; i++) {
> +		for (pl_idx = 0; pl_idx < plane_cnt; pl_idx++) {
> +			ppa = ppas[i];
> +			ppa.g.pl = pl_idx;
> +			rqd->ppa_list[(pl_idx * nr_ppas) + i] = ppa;
> 		}
> 	}
> 
> diff --git a/include/linux/lightnvm.h b/include/linux/lightnvm.h
> index 81b71c6d..c8c014b 100644
> --- a/include/linux/lightnvm.h
> +++ b/include/linux/lightnvm.h
> @@ -481,7 +481,7 @@ extern int nvm_max_phys_sects(struct nvm_tgt_dev *);
> extern int nvm_submit_io(struct nvm_tgt_dev *, struct nvm_rq *);
> extern int nvm_erase_sync(struct nvm_tgt_dev *, struct ppa_addr *, int);
> extern int nvm_set_rqd_ppalist(struct nvm_tgt_dev *, struct nvm_rq *,
> -					const struct ppa_addr *, int, int);
> +					const struct ppa_addr *, int);
> extern void nvm_free_rqd_ppalist(struct nvm_tgt_dev *, struct nvm_rq *);
> extern int nvm_get_l2p_tbl(struct nvm_tgt_dev *, u64, u32, nvm_l2p_update_fn *,
> 			   void *);
> --
> 2.7.4

This was used when using MLC media as SLC back in the days, when we had
the host media manager (spec 1.2). We probably want to have a path to
use NAND as SLC, but I agree that it should be separated in order to
reduce the number of checks (considering that this is for a very
specific purpose).


Reviewed-by: Javier González <javier@cnexlabs.com>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web