lightnvm: refactor device ops->get_bb_tbl()

The device ops->get_bb_tbl() takes a callback, that allows the caller
to use its own callback function to update its data structures in the
returning function.

This makes it difficult to send parameters to the callback, and usually
is circumvented by small private structures, that both carry the callers
state and any flags needed to fulfill the update.

Refactor ops->get_bb_tbl() to fill a data buffer with the status of the
blocks returned, and let the user call the callback function manually.
That will provide the necessary flags and data structures and simplify
the logic around ops->get_bb_tbl().
Signed-off-by: Matias Bjørling <m@bjorling.me>
Signed-off-by: Jens Axboe <axboe@fb.com>

drivers/lightnvm/core.c

@@ -467,6 +467,14 @@ int nvm_bb_tbl_fold(struct nvm_dev *dev, u8 *blks, int nr_blks)
 }
 EXPORT_SYMBOL(nvm_bb_tbl_fold);
 
+int nvm_get_bb_tbl(struct nvm_dev *dev, struct ppa_addr ppa, u8 *blks)
+{
+	ppa = generic_to_dev_addr(dev, ppa);
+
+	return dev->ops->get_bb_tbl(dev, ppa, blks);
+}
+EXPORT_SYMBOL(nvm_get_bb_tbl);
+
 static int nvm_init_slc_tbl(struct nvm_dev *dev, struct nvm_id_group *grp)
 {
 	int i;

drivers/lightnvm/gennvm.c

@@ -129,10 +129,10 @@ static int gennvm_luns_init(struct nvm_dev *dev, struct gen_nvm *gn)
 	return 0;
 }
 
-static int gennvm_block_bb(struct nvm_dev *dev, struct ppa_addr ppa,
-					u8 *blks, int nr_blks, void *private)
+static int gennvm_block_bb(struct gen_nvm *gn, struct ppa_addr ppa,
+							u8 *blks, int nr_blks)
 {
-	struct gen_nvm *gn = private;
+	struct nvm_dev *dev = gn->dev;
 	struct gen_lun *lun;
 	struct nvm_block *blk;
 	int i;
@@ -219,13 +219,21 @@ static int gennvm_blocks_init(struct nvm_dev *dev, struct gen_nvm *gn)
 	struct gen_lun *lun;
 	struct nvm_block *block;
 	sector_t lun_iter, blk_iter, cur_block_id = 0;
-	int ret;
+	int ret, nr_blks;
+	u8 *blks;
+
+	nr_blks = dev->blks_per_lun * dev->plane_mode;
+	blks = kmalloc(nr_blks, GFP_KERNEL);
+	if (!blks)
+		return -ENOMEM;
 
 	gennvm_for_each_lun(gn, lun, lun_iter) {
 		lun->vlun.blocks = vzalloc(sizeof(struct nvm_block) *
 							dev->blks_per_lun);
-		if (!lun->vlun.blocks)
+		if (!lun->vlun.blocks) {
+			kfree(blks);
 			return -ENOMEM;
+		}
 
 		for (blk_iter = 0; blk_iter < dev->blks_per_lun; blk_iter++) {
 			block = &lun->vlun.blocks[blk_iter];
@@ -250,12 +258,14 @@ static int gennvm_blocks_init(struct nvm_dev *dev, struct gen_nvm *gn)
 			ppa.ppa = 0;
 			ppa.g.ch = lun->vlun.chnl_id;
 			ppa.g.lun = lun->vlun.id;
-			ppa = generic_to_dev_addr(dev, ppa);
 
-			ret = dev->ops->get_bb_tbl(dev, ppa,
-							gennvm_block_bb, gn);
+			ret = nvm_get_bb_tbl(dev, ppa, blks);
+			if (ret)
+				pr_err("gennvm: could not get BB table\n");
+
+			ret = gennvm_block_bb(gn, ppa, blks, nr_blks);
 			if (ret)
-				pr_err("gennvm: could not read BB table\n");
+				pr_err("gennvm: BB table map failed\n");
 		}
 	}
 
@@ -268,6 +278,7 @@ static int gennvm_blocks_init(struct nvm_dev *dev, struct gen_nvm *gn)
 		}
 	}
 
+	kfree(blks);
 	return 0;
 }
 

drivers/lightnvm/sysblk.c

@@ -93,10 +93,45 @@ void nvm_setup_sysblk_scan(struct nvm_dev *dev, struct sysblk_scan *s,
 	s->nr_rows = nvm_setup_sysblks(dev, sysblk_ppas);
 }
 
+static int sysblk_get_free_blks(struct nvm_dev *dev, struct ppa_addr ppa,
+					u8 *blks, int nr_blks,
+					struct sysblk_scan *s)
+{
+	struct ppa_addr *sppa;
+	int i, blkid = 0;
+
+	nr_blks = nvm_bb_tbl_fold(dev, blks, nr_blks);
+	if (nr_blks < 0)
+		return nr_blks;
+
+	for (i = 0; i < nr_blks; i++) {
+		if (blks[i] == NVM_BLK_T_HOST)
+			return -EEXIST;
+
+		if (blks[i] != NVM_BLK_T_FREE)
+			continue;
+
+		sppa = &s->ppas[scan_ppa_idx(s->row, blkid)];
+		sppa->g.ch = ppa.g.ch;
+		sppa->g.lun = ppa.g.lun;
+		sppa->g.blk = i;
+		s->nr_ppas++;
+		blkid++;
+
+		pr_debug("nvm: use (%u %u %u) as sysblk\n",
+					sppa->g.ch, sppa->g.lun, sppa->g.blk);
+		if (blkid > MAX_BLKS_PR_SYSBLK - 1)
+			return 0;
+	}
+
+	pr_err("nvm: sysblk failed get sysblk\n");
+	return -EINVAL;
+}
+
 static int sysblk_get_host_blks(struct nvm_dev *dev, struct ppa_addr ppa,
-					u8 *blks, int nr_blks, void *private)
+					u8 *blks, int nr_blks,
+					struct sysblk_scan *s)
 {
-	struct sysblk_scan *s = private;
 	int i, nr_sysblk = 0;
 
 	nr_blks = nvm_bb_tbl_fold(dev, blks, nr_blks);
@@ -123,26 +158,42 @@ static int sysblk_get_host_blks(struct nvm_dev *dev, struct ppa_addr ppa,
 }
 
 static int nvm_get_all_sysblks(struct nvm_dev *dev, struct sysblk_scan *s,
-				struct ppa_addr *ppas, nvm_bb_update_fn *fn)
+				struct ppa_addr *ppas, int get_free)
 {
-	struct ppa_addr dppa;
-	int i, ret = 0;
+	int i, nr_blks, ret = 0;
+	u8 *blks;
 
 	s->nr_ppas = 0;
+	nr_blks = dev->blks_per_lun * dev->plane_mode;
+
+	blks = kmalloc(nr_blks, GFP_KERNEL);
+	if (!blks)
+		return -ENOMEM;
 
 	for (i = 0; i < s->nr_rows; i++) {
-		dppa = generic_to_dev_addr(dev, ppas[i]);
 		s->row = i;
 
-		ret = dev->ops->get_bb_tbl(dev, dppa, fn, s);
+		ret = nvm_get_bb_tbl(dev, ppas[i], blks);
 		if (ret) {
 			pr_err("nvm: failed bb tbl for ppa (%u %u)\n",
 							ppas[i].g.ch,
 							ppas[i].g.blk);
-			return ret;
+			goto err_get;
 		}
+
+		if (get_free)
+			ret = sysblk_get_free_blks(dev, ppas[i], blks, nr_blks,
+									s);
+		else
+			ret = sysblk_get_host_blks(dev, ppas[i], blks, nr_blks,
+									s);
+
+		if (ret)
+			goto err_get;
 	}
 
+err_get:
+	kfree(blks);
 	return ret;
 }
 
@@ -239,41 +290,6 @@ static int nvm_set_bb_tbl(struct nvm_dev *dev, struct sysblk_scan *s, int type)
 	return 0;
 }
 
-static int sysblk_get_free_blks(struct nvm_dev *dev, struct ppa_addr ppa,
-					u8 *blks, int nr_blks, void *private)
-{
-	struct sysblk_scan *s = private;
-	struct ppa_addr *sppa;
-	int i, blkid = 0;
-
-	nr_blks = nvm_bb_tbl_fold(dev, blks, nr_blks);
-	if (nr_blks < 0)
-		return nr_blks;
-
-	for (i = 0; i < nr_blks; i++) {
-		if (blks[i] == NVM_BLK_T_HOST)
-			return -EEXIST;
-
-		if (blks[i] != NVM_BLK_T_FREE)
-			continue;
-
-		sppa = &s->ppas[scan_ppa_idx(s->row, blkid)];
-		sppa->g.ch = ppa.g.ch;
-		sppa->g.lun = ppa.g.lun;
-		sppa->g.blk = i;
-		s->nr_ppas++;
-		blkid++;
-
-		pr_debug("nvm: use (%u %u %u) as sysblk\n",
-					sppa->g.ch, sppa->g.lun, sppa->g.blk);
-		if (blkid > MAX_BLKS_PR_SYSBLK - 1)
-			return 0;
-	}
-
-	pr_err("nvm: sysblk failed get sysblk\n");
-	return -EINVAL;
-}
-
 static int nvm_write_and_verify(struct nvm_dev *dev, struct nvm_sb_info *info,
 							struct sysblk_scan *s)
 {
@@ -393,7 +409,7 @@ int nvm_get_sysblock(struct nvm_dev *dev, struct nvm_sb_info *info)
 	nvm_setup_sysblk_scan(dev, &s, sysblk_ppas);
 
 	mutex_lock(&dev->mlock);
-	ret = nvm_get_all_sysblks(dev, &s, sysblk_ppas, sysblk_get_host_blks);
+	ret = nvm_get_all_sysblks(dev, &s, sysblk_ppas, 0);
 	if (ret)
 		goto err_sysblk;
 
@@ -453,7 +469,7 @@ int nvm_update_sysblock(struct nvm_dev *dev, struct nvm_sb_info *new)
 	nvm_setup_sysblk_scan(dev, &s, sysblk_ppas);
 
 	mutex_lock(&dev->mlock);
-	ret = nvm_get_all_sysblks(dev, &s, sysblk_ppas, sysblk_get_host_blks);
+	ret = nvm_get_all_sysblks(dev, &s, sysblk_ppas, 0);
 	if (ret)
 		goto err_sysblk;
 
@@ -551,7 +567,7 @@ int nvm_init_sysblock(struct nvm_dev *dev, struct nvm_sb_info *info)
 	nvm_setup_sysblk_scan(dev, &s, sysblk_ppas);
 
 	mutex_lock(&dev->mlock);
-	ret = nvm_get_all_sysblks(dev, &s, sysblk_ppas, sysblk_get_free_blks);
+	ret = nvm_get_all_sysblks(dev, &s, sysblk_ppas, 1);
 	if (ret)
 		goto err_mark;
 
@@ -587,9 +603,9 @@ static unsigned int factory_blk_offset(struct nvm_dev *dev, int ch, int lun)
 }
 
 static int nvm_factory_blks(struct nvm_dev *dev, struct ppa_addr ppa,
-					u8 *blks, int nr_blks, void *private)
+					u8 *blks, int nr_blks,
+					struct factory_blks *f)
 {
-	struct factory_blks *f = private;
 	int i, lunoff;
 
 	nr_blks = nvm_bb_tbl_fold(dev, blks, nr_blks);
@@ -659,32 +675,29 @@ static int nvm_fact_get_blks(struct nvm_dev *dev, struct ppa_addr *erase_list,
 	return ppa_cnt;
 }
 
-static int nvm_fact_get_bb_tbl(struct nvm_dev *dev, struct ppa_addr ppa,
-					nvm_bb_update_fn *fn, void *priv)
+static int nvm_fact_select_blks(struct nvm_dev *dev, struct factory_blks *f)
 {
-	struct ppa_addr dev_ppa;
-	int ret;
+	struct ppa_addr ppa;
+	int ch, lun, nr_blks, ret;
+	u8 *blks;
 
-	dev_ppa = generic_to_dev_addr(dev, ppa);
+	nr_blks = dev->blks_per_lun * dev->plane_mode;
+	blks = kmalloc(nr_blks, GFP_KERNEL);
+	if (!blks)
+		return -ENOMEM;
 
-	ret = dev->ops->get_bb_tbl(dev, dev_ppa, fn, priv);
+	nvm_for_each_lun_ppa(dev, ppa, ch, lun) {
+		ret = nvm_get_bb_tbl(dev, ppa, blks);
 		if (ret)
 			pr_err("nvm: failed bb tbl for ch%u lun%u\n",
 							ppa.g.ch, ppa.g.blk);
-	return ret;
-}
 
-static int nvm_fact_select_blks(struct nvm_dev *dev, struct factory_blks *f)
-{
-	int ch, lun, ret;
-	struct ppa_addr ppa;
-
-	nvm_for_each_lun_ppa(dev, ppa, ch, lun) {
-		ret = nvm_fact_get_bb_tbl(dev, ppa, nvm_factory_blks, f);
+		ret = nvm_factory_blks(dev, ppa, blks, nr_blks, f);
 		if (ret)
 			return ret;
 	}
 
+	kfree(blks);
 	return 0;
 }
 
@@ -722,8 +735,7 @@ int nvm_dev_factory(struct nvm_dev *dev, int flags)
 	if (flags & NVM_FACTORY_RESET_HOST_BLKS) {
 		nvm_setup_sysblk_scan(dev, &s, sysblk_ppas);
 		mutex_lock(&dev->mlock);
-		ret = nvm_get_all_sysblks(dev, &s, sysblk_ppas,
-							sysblk_get_host_blks);
+		ret = nvm_get_all_sysblks(dev, &s, sysblk_ppas, 0);
 		if (!ret)
 			ret = nvm_set_bb_tbl(dev, &s, NVM_BLK_T_FREE);
 		mutex_unlock(&dev->mlock);

drivers/nvme/host/lightnvm.c

@@ -388,7 +388,7 @@ static int nvme_nvm_get_l2p_tbl(struct nvm_dev *nvmdev, u64 slba, u32 nlb,
 }
 
 static int nvme_nvm_get_bb_tbl(struct nvm_dev *nvmdev, struct ppa_addr ppa,
-				nvm_bb_update_fn *update_bbtbl, void *priv)
+								u8 *blks)
 {
 	struct request_queue *q = nvmdev->q;
 	struct nvme_ns *ns = q->queuedata;
@@ -435,9 +435,7 @@ static int nvme_nvm_get_bb_tbl(struct nvm_dev *nvmdev, struct ppa_addr ppa,
 		goto out;
 	}
 
-	ppa = dev_to_generic_addr(nvmdev, ppa);
-	ret = update_bbtbl(nvmdev, ppa, bb_tbl->blk, nr_blks, priv);
-
+	memcpy(blks, bb_tbl->blk, nvmdev->blks_per_lun * nvmdev->plane_mode);
 out:
 	kfree(bb_tbl);
 	return ret;

include/linux/lightnvm.h

@@ -41,13 +41,10 @@ struct nvm_id;
 struct nvm_dev;
 
 typedef int (nvm_l2p_update_fn)(u64, u32, __le64 *, void *);
-typedef int (nvm_bb_update_fn)(struct nvm_dev *, struct ppa_addr, u8 *, int,
-								void *);
 typedef int (nvm_id_fn)(struct nvm_dev *, struct nvm_id *);
 typedef int (nvm_get_l2p_tbl_fn)(struct nvm_dev *, u64, u32,
 				nvm_l2p_update_fn *, void *);
-typedef int (nvm_op_bb_tbl_fn)(struct nvm_dev *, struct ppa_addr,
-				nvm_bb_update_fn *, void *);
+typedef int (nvm_op_bb_tbl_fn)(struct nvm_dev *, struct ppa_addr, u8 *);
 typedef int (nvm_op_set_bb_fn)(struct nvm_dev *, struct nvm_rq *, int);
 typedef int (nvm_submit_io_fn)(struct nvm_dev *, struct nvm_rq *);
 typedef int (nvm_erase_blk_fn)(struct nvm_dev *, struct nvm_rq *);
@@ -539,6 +536,7 @@ extern int nvm_submit_ppa(struct nvm_dev *, struct ppa_addr *, int, int, int,
 extern int nvm_submit_ppa_list(struct nvm_dev *, struct ppa_addr *, int, int,
 							int, void *, int);
 extern int nvm_bb_tbl_fold(struct nvm_dev *, u8 *, int);
+extern int nvm_get_bb_tbl(struct nvm_dev *, struct ppa_addr, u8 *);
 
 /* sysblk.c */
 #define NVM_SYSBLK_MAGIC 0x4E564D53 /* "NVMS" */