[PATCH 0/5] LightNVM fixes

[PATCH 0/5] LightNVM fixes

[Matias Bjørling]

Hi Jens,

A couple of small fixes.

Patch 1-2: fixes a couple of bugs when init fails.
Patch 3: changes the lightnvm admin commands to the passed through
         the admin NVMe queue.
Patch 4-5: adds extra debugging information for luns.

Javier Gonzalez (2):
  lightnvm: keep track of block counts
  lightnvm: add free and bad lun info to show luns

Matias Bjørling (1):
  lightnvm: missing free on init error

Wenwei Tao (2):
  lightnvm: wrong return value and redundant free
  nvme: lightnvm: use admin queues for admin cmds

 drivers/lightnvm/core.c      | 14 +++++++-------
 drivers/lightnvm/gennvm.c    | 33 +++++++++++++++++++++++++++------
 drivers/nvme/host/lightnvm.c | 17 ++++++++++-------
 include/linux/lightnvm.h     |  6 ++++--
 4 files changed, 48 insertions(+), 22 deletions(-)

-- 
2.1.4

[PATCH 1/5] lightnvm: wrong return value and redundant free

[Matias Bjørling]

From: Wenwei Tao <ww.tao0320@gmail.com>

The return value should be non-zero under error conditions.
Remove nvme_free(dev) to avoid free dev more than once.

Signed-off-by: Wenwei Tao <ww.tao0320@gmail.com>
Signed-off-by: Matias Bjørling <m@bjorling.me>
---
 drivers/lightnvm/core.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
index 8a556f3..bed47e7 100644
--- a/drivers/lightnvm/core.c
+++ b/drivers/lightnvm/core.c
@@ -222,14 +222,13 @@ static void nvm_free(struct nvm_dev *dev)
 static int nvm_init(struct nvm_dev *dev)
 {
 	struct nvmm_type *mt;
-	int ret = 0;
+	int ret = -EINVAL;
 
 	if (!dev->q || !dev->ops)
-		return -EINVAL;
+		return ret;
 
 	if (dev->ops->identity(dev->q, &dev->identity)) {
 		pr_err("nvm: device could not be identified\n");
-		ret = -EINVAL;
 		goto err;
 	}
 
@@ -275,7 +274,6 @@ static int nvm_init(struct nvm_dev *dev)
 			dev->nr_chnls);
 	return 0;
 err:
-	nvm_free(dev);
 	pr_err("nvm: failed to initialize nvm\n");
 	return ret;
 }
-- 
2.1.4

[PATCH 2/5] lightnvm: missing free on init error

[Matias Bjørling]

If either max_phys_sect is out of bound, the nvm_dev structure is not
freed.

Signed-off-by: Matias Bjørling <m@bjorling.me>
---
 drivers/lightnvm/core.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
index bed47e7..f61d325 100644
--- a/drivers/lightnvm/core.c
+++ b/drivers/lightnvm/core.c
@@ -313,11 +313,13 @@ int nvm_register(struct request_queue *q, char *disk_name,
 								"ppalist");
 		if (!dev->ppalist_pool) {
 			pr_err("nvm: could not create ppa pool\n");
-			return -ENOMEM;
+			ret = -ENOMEM;
+			goto err_init;
 		}
 	} else if (dev->ops->max_phys_sect > 256) {
 		pr_info("nvm: max sectors supported is 256.\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err_init;
 	}
 
 	down_write(&nvm_lock);
-- 
2.1.4

[PATCH 3/5] nvme: lightnvm: use admin queues for admin cmds

[Matias Bjørling]

From: Wenwei Tao <ww.tao0320@gmail.com>

According to the Open-Channel SSD Specification, the NVMe-NVM admin
commands use vendor specific opcodes of NVMe, so use the NVMe admin
queue to dispatch these commands.

Signed-off-by: Wenwei Tao <ww.tao0320@gmail.com>
Updated by me to include set bad block table as well and also use
the admin queue for l2p len calculation.

Signed-off-by: Matias Bjørling <m@bjorling.me>
---
 drivers/nvme/host/lightnvm.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c
index 7e82fe3..9202d1a 100644
--- a/drivers/nvme/host/lightnvm.c
+++ b/drivers/nvme/host/lightnvm.c
@@ -276,6 +276,7 @@ static int init_grps(struct nvm_id *nvm_id, struct nvme_nvm_id *nvme_nvm_id)
 static int nvme_nvm_identity(struct request_queue *q, struct nvm_id *nvm_id)
 {
 	struct nvme_ns *ns = q->queuedata;
+	struct nvme_dev *dev = ns->dev;
 	struct nvme_nvm_id *nvme_nvm_id;
 	struct nvme_nvm_command c = {};
 	int ret;
@@ -288,8 +289,8 @@ static int nvme_nvm_identity(struct request_queue *q, struct nvm_id *nvm_id)
 	if (!nvme_nvm_id)
 		return -ENOMEM;
 
-	ret = nvme_submit_sync_cmd(q, (struct nvme_command *)&c, nvme_nvm_id,
-						sizeof(struct nvme_nvm_id));
+	ret = nvme_submit_sync_cmd(dev->admin_q, (struct nvme_command *)&c,
+				nvme_nvm_id, sizeof(struct nvme_nvm_id));
 	if (ret) {
 		ret = -EIO;
 		goto out;
@@ -315,7 +316,7 @@ static int nvme_nvm_get_l2p_tbl(struct request_queue *q, u64 slba, u32 nlb,
 	struct nvme_ns *ns = q->queuedata;
 	struct nvme_dev *dev = ns->dev;
 	struct nvme_nvm_command c = {};
-	u32 len = queue_max_hw_sectors(q) << 9;
+	u32 len = queue_max_hw_sectors(dev->admin_q) << 9;
 	u32 nlb_pr_rq = len / sizeof(u64);
 	u64 cmd_slba = slba;
 	void *entries;
@@ -333,8 +334,8 @@ static int nvme_nvm_get_l2p_tbl(struct request_queue *q, u64 slba, u32 nlb,
 		c.l2p.slba = cpu_to_le64(cmd_slba);
 		c.l2p.nlb = cpu_to_le32(cmd_nlb);
 
-		ret = nvme_submit_sync_cmd(q, (struct nvme_command *)&c,
-								entries, len);
+		ret = nvme_submit_sync_cmd(dev->admin_q,
+				(struct nvme_command *)&c, entries, len);
 		if (ret) {
 			dev_err(dev->dev, "L2P table transfer failed (%d)\n",
 									ret);
@@ -375,7 +376,8 @@ static int nvme_nvm_get_bb_tbl(struct request_queue *q, struct ppa_addr ppa,
 	if (!bb_tbl)
 		return -ENOMEM;
 
-	ret = nvme_submit_sync_cmd(q, (struct nvme_command *)&c, bb_tbl, tblsz);
+	ret = nvme_submit_sync_cmd(dev->admin_q, (struct nvme_command *)&c,
+								bb_tbl, tblsz);
 	if (ret) {
 		dev_err(dev->dev, "get bad block table failed (%d)\n", ret);
 		ret = -EIO;
@@ -427,7 +429,8 @@ static int nvme_nvm_set_bb_tbl(struct request_queue *q, struct nvm_rq *rqd,
 	c.set_bb.nlb = cpu_to_le16(rqd->nr_pages - 1);
 	c.set_bb.value = type;
 
-	ret = nvme_submit_sync_cmd(q, (struct nvme_command *)&c, NULL, 0);
+	ret = nvme_submit_sync_cmd(dev->admin_q, (struct nvme_command *)&c,
+								NULL, 0);
 	if (ret)
 		dev_err(dev->dev, "set bad block table failed (%d)\n", ret);
 	return ret;
-- 
2.1.4

[PATCH 4/5] lightnvm: keep track of block counts

[Matias Bjørling]

From: Javier Gonzalez <javier@javigon.com>

Maintain number of in use blocks, free blocks, and bad blocks in a per
lun basis. This allows the upper layers to get information about the
state of each lun.

Also, account for blocks reserved to the device on the free block count.
nr_free_blocks matches now the actual number of blocks on the free list
when the device is booted.

Signed-off-by: Javier Gonzalez <javier@cnexlabs.com>
Signed-off-by: Matias Bjørling <m@bjorling.me>
---
 drivers/lightnvm/gennvm.c | 14 +++++++++++++-
 include/linux/lightnvm.h  |  2 ++
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/lightnvm/gennvm.c b/drivers/lightnvm/gennvm.c
index c0d0eb2..43c01e0 100644
--- a/drivers/lightnvm/gennvm.c
+++ b/drivers/lightnvm/gennvm.c
@@ -60,6 +60,8 @@ static int gennvm_luns_init(struct nvm_dev *dev, struct gen_nvm *gn)
 		lun->vlun.lun_id = i % dev->luns_per_chnl;
 		lun->vlun.chnl_id = i / dev->luns_per_chnl;
 		lun->vlun.nr_free_blocks = dev->blks_per_lun;
+		lun->vlun.nr_inuse_blocks = 0;
+		lun->vlun.nr_bad_blocks = 0;
 	}
 	return 0;
 }
@@ -87,6 +89,7 @@ static int gennvm_block_bb(struct ppa_addr ppa, int nr_blocks, u8 *blks,
 		}
 
 		list_move_tail(&blk->list, &lun->bb_list);
+		lun->vlun.nr_bad_blocks++;
 	}
 
 	return 0;
@@ -139,6 +142,7 @@ static int gennvm_block_map(u64 slba, u32 nlb, __le64 *entries, void *private)
 			list_move_tail(&blk->list, &lun->used_list);
 			blk->type = 1;
 			lun->vlun.nr_free_blocks--;
+			lun->vlun.nr_inuse_blocks++;
 		}
 	}
 
@@ -167,8 +171,10 @@ static int gennvm_blocks_init(struct nvm_dev *dev, struct gen_nvm *gn)
 			block->id = cur_block_id++;
 
 			/* First block is reserved for device */
-			if (unlikely(lun_iter == 0 && blk_iter == 0))
+			if (unlikely(lun_iter == 0 && blk_iter == 0)) {
+				lun->vlun.nr_free_blocks--;
 				continue;
+			}
 
 			list_add_tail(&block->list, &lun->free_list);
 		}
@@ -266,6 +272,7 @@ static struct nvm_block *gennvm_get_blk(struct nvm_dev *dev,
 	blk->type = 1;
 
 	lun->vlun.nr_free_blocks--;
+	lun->vlun.nr_inuse_blocks++;
 
 	spin_unlock(&vlun->lock);
 out:
@@ -283,16 +290,21 @@ static void gennvm_put_blk(struct nvm_dev *dev, struct nvm_block *blk)
 	case 1:
 		list_move_tail(&blk->list, &lun->free_list);
 		lun->vlun.nr_free_blocks++;
+		lun->vlun.nr_inuse_blocks--;
 		blk->type = 0;
 		break;
 	case 2:
 		list_move_tail(&blk->list, &lun->bb_list);
+		lun->vlun.nr_bad_blocks++;
+		lun->vlun.nr_inuse_blocks--;
 		break;
 	default:
 		WARN_ON_ONCE(1);
 		pr_err("gennvm: erroneous block type (%lu -> %u)\n",
 							blk->id, blk->type);
 		list_move_tail(&blk->list, &lun->bb_list);
+		lun->vlun.nr_bad_blocks++;
+		lun->vlun.nr_inuse_blocks--;
 	}
 
 	spin_unlock(&vlun->lock);
diff --git a/include/linux/lightnvm.h b/include/linux/lightnvm.h
index cbe288a..831a20c 100644
--- a/include/linux/lightnvm.h
+++ b/include/linux/lightnvm.h
@@ -213,7 +213,9 @@ struct nvm_lun {
 	int lun_id;
 	int chnl_id;
 
+	unsigned int nr_inuse_blocks;	/* Number of used blocks */
 	unsigned int nr_free_blocks;	/* Number of unused blocks */
+	unsigned int nr_bad_blocks;	/* Number of bad blocks */
 	struct nvm_block *blocks;
 
 	spinlock_t lock;
-- 
2.1.4

[PATCH 5/5] lightnvm: add free and bad lun info to show luns

[Matias Bjørling]

From: Javier Gonzalez <javier@javigon.com>

Add free block, used block, and bad block information to the show debug
interface. This information is used to debug how targets track blocks.

Also, change debug function name to make it more generic.

Signed-off-by: Javier Gonzalez <javier@cnexlabs.com>
Signed-off-by: Matias Bjørling <m@bjorling.me>
---
 drivers/lightnvm/core.c   |  2 +-
 drivers/lightnvm/gennvm.c | 19 ++++++++++++++-----
 include/linux/lightnvm.h  |  4 ++--
 3 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
index f61d325..5178645 100644
--- a/drivers/lightnvm/core.c
+++ b/drivers/lightnvm/core.c
@@ -544,7 +544,7 @@ static int nvm_configure_show(const char *val)
 	if (!dev->mt)
 		return 0;
 
-	dev->mt->free_blocks_print(dev);
+	dev->mt->lun_info_print(dev);
 
 	return 0;
 }
diff --git a/drivers/lightnvm/gennvm.c b/drivers/lightnvm/gennvm.c
index 43c01e0..e20e74e 100644
--- a/drivers/lightnvm/gennvm.c
+++ b/drivers/lightnvm/gennvm.c
@@ -464,15 +464,24 @@ static struct nvm_lun *gennvm_get_lun(struct nvm_dev *dev, int lunid)
 	return &gn->luns[lunid].vlun;
 }
 
-static void gennvm_free_blocks_print(struct nvm_dev *dev)
+static void gennvm_lun_info_print(struct nvm_dev *dev)
 {
 	struct gen_nvm *gn = dev->mp;
 	struct gen_lun *lun;
 	unsigned int i;
 
-	gennvm_for_each_lun(gn, lun, i)
-		pr_info("%s: lun%8u\t%u\n",
-					dev->name, i, lun->vlun.nr_free_blocks);
+
+	gennvm_for_each_lun(gn, lun, i) {
+		spin_lock(&lun->vlun.lock);
+
+		pr_info("%s: lun%8u\t%u\t%u\t%u\n",
+				dev->name, i,
+				lun->vlun.nr_free_blocks,
+				lun->vlun.nr_inuse_blocks,
+				lun->vlun.nr_bad_blocks);
+
+		spin_unlock(&lun->vlun.lock);
+	}
 }
 
 static struct nvmm_type gennvm = {
@@ -490,7 +499,7 @@ static struct nvmm_type gennvm = {
 	.erase_blk	= gennvm_erase_blk,
 
 	.get_lun	= gennvm_get_lun,
-	.free_blocks_print = gennvm_free_blocks_print,
+	.lun_info_print = gennvm_lun_info_print,
 };
 
 static int __init gennvm_module_init(void)
diff --git a/include/linux/lightnvm.h b/include/linux/lightnvm.h
index 831a20c..3db5552 100644
--- a/include/linux/lightnvm.h
+++ b/include/linux/lightnvm.h
@@ -380,7 +380,7 @@ typedef int (nvmm_end_io_fn)(struct nvm_rq *, int);
 typedef int (nvmm_erase_blk_fn)(struct nvm_dev *, struct nvm_block *,
 								unsigned long);
 typedef struct nvm_lun *(nvmm_get_lun_fn)(struct nvm_dev *, int);
-typedef void (nvmm_free_blocks_print_fn)(struct nvm_dev *);
+typedef void (nvmm_lun_info_print_fn)(struct nvm_dev *);
 
 struct nvmm_type {
 	const char *name;
@@ -404,7 +404,7 @@ struct nvmm_type {
 	nvmm_get_lun_fn *get_lun;
 
 	/* Statistics */
-	nvmm_free_blocks_print_fn *free_blocks_print;
+	nvmm_lun_info_print_fn *lun_info_print;
 	struct list_head list;
 };
 
-- 
2.1.4

Re: [PATCH 0/5] LightNVM fixes

[Jens Axboe]

On 11/20/2015 05:47 AM, Matias Bjørling wrote:
> Hi Jens,
>
> A couple of small fixes.
>
> Patch 1-2: fixes a couple of bugs when init fails.
> Patch 3: changes the lightnvm admin commands to the passed through
>           the admin NVMe queue.
> Patch 4-5: adds extra debugging information for luns.

Applied, thanks.

-- 
Jens Axboe

Re: [PATCH 0/5] LightNVM fixes

[Matias Bjørling]

On 04/26/2016 04:35 PM, Jens Axboe wrote:
> On 04/26/2016 04:31 AM, Matias Bjørling wrote:
>> Hi,
>>
>> A couple of patches for the 4.7 cycle. The first three are cleanups for
>> iterating ppas, better get_bb_tbl interface and removal of struct
>> factory_blks. The forth patch enables the caller to control if mapping
>> should by a single plane or multiple planes. The last patch fixes a bug
>> when unfolding a virtual block onto multiple die planes.
>
> What are these against? Patch #2 fails horribly on both for-4.7/drivers
> and current Linus' master.
>

Ah yes, forgot to add what they were based upon. They are based on the 
https://github.com/OpenChannelSSD/linux.git for-next branch.

I wanted to post these before applying. Will send you a combined patch 
set around -rc6.

Re: [PATCH 0/5] LightNVM fixes

[Jens Axboe]

On 04/26/2016 04:31 AM, Matias Bjørling wrote:
> Hi,
>
> A couple of patches for the 4.7 cycle. The first three are cleanups for
> iterating ppas, better get_bb_tbl interface and removal of struct
> factory_blks. The forth patch enables the caller to control if mapping
> should by a single plane or multiple planes. The last patch fixes a bug
> when unfolding a virtual block onto multiple die planes.

What are these against? Patch #2 fails horribly on both for-4.7/drivers 
and current Linus' master.

-- 
Jens Axboe

[PATCH 0/5] LightNVM fixes

[Matias Bjørling]

Hi,

A couple of patches for the 4.7 cycle. The first three are cleanups for
iterating ppas, better get_bb_tbl interface and removal of struct
factory_blks. The forth patch enables the caller to control if mapping
should by a single plane or multiple planes. The last patch fixes a bug
when unfolding a virtual block onto multiple die planes.

Matias Bjørling (5):
  lightnvm: introduce nvm_for_each_lun_ppa() macro
  lightnvm: refactor device ops->get_bb_tbl()
  lightnvm: remove struct factory_blks
  lightnvm: make nvm_set_rqd_ppalist() aware of vblks
  lightnvm: set block priority in nvm_set_rqd_ppalist()

 drivers/lightnvm/core.c      |  40 ++++---
 drivers/lightnvm/gennvm.c    |  29 +++--
 drivers/lightnvm/sysblk.c    | 248 +++++++++++++++++++++----------------------
 drivers/nvme/host/lightnvm.c |   6 +-
 include/linux/lightnvm.h     |  15 ++-
 5 files changed, 179 insertions(+), 159 deletions(-)

-- 
2.1.4

[PATCH 0/5] LightNVM fixes

[Matias Bjørling]

Hi Jens,

A couple of fixes for -rc3.

Patch 1: Fixes the device id detection for qemu.
Patch 2: Fix for conditional compilation of lightnvm with nvme by Keith.
Patch 3-5: Fixes for memory leaks and boundary check by Tao and Sudip.

Please pick up. Thanks.

Keith Busch (1):
  lightnvm: Simplify config when disabled

Matias Bjørling (1):
  lightnvm: change vendor and dev id for qemu

Sudip Mukherjee (1):
  lightnvm: fix ioctl memory leaks

Wenwei Tao (2):
  lightnvm: free memory when gennvm register fails
  lightnvm: do device max sectors boundary check first

 drivers/lightnvm/core.c      | 19 +++++++++++++------
 drivers/lightnvm/gennvm.c    | 15 ++++++++++-----
 drivers/nvme/host/Makefile   |  3 ++-
 drivers/nvme/host/lightnvm.c | 15 +--------------
 drivers/nvme/host/nvme.h     | 14 ++++++++++++++
 5 files changed, 40 insertions(+), 26 deletions(-)

-- 
2.1.4