2015-11-20 12:48:07

by Matias Bjørling

[permalink] [raw]
Subject: [PATCH 0/5] LightNVM fixes

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


2015-11-20 12:48:10

by Matias Bjørling

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

From: Wenwei Tao <[email protected]>

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 <[email protected]>
Signed-off-by: Matias Bjørling <[email protected]>
---
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

2015-11-20 12:48:11

by Matias Bjørling

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

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

Signed-off-by: Matias Bjørling <[email protected]>
---
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

2015-11-20 12:49:17

by Matias Bjørling

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

From: Wenwei Tao <[email protected]>

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 <[email protected]>
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 <[email protected]>
---
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

2015-11-20 12:48:42

by Matias Bjørling

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

From: Javier Gonzalez <[email protected]>

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 <[email protected]>
Signed-off-by: Matias Bjørling <[email protected]>
---
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

2015-11-20 12:48:40

by Matias Bjørling

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

From: Javier Gonzalez <[email protected]>

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 <[email protected]>
Signed-off-by: Matias Bjørling <[email protected]>
---
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

2015-11-20 15:51:59

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH 0/5] LightNVM fixes

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