Re: [PATCH v3 3/3] lightnvm: add non-continuous lun target creation support

From: Matias BjÃrling
Date: Fri Feb 05 2016 - 07:55:50 EST

Next message: Thomas Petazzoni: "Re: [PATCH v10 5/5] Watchdog: ARM SBSA Generic Watchdog half timeout panic support"
Previous message: Alexander Shishkin: "Re: [PATCH V2 1/6] stm class: Add ioctl get_options interface"
In reply to: Wenwei Tao: "[PATCH v3 3/3] lightnvm: add non-continuous lun target creation support"
Next in thread: Wenwei Tao: "Re: [PATCH v3 3/3] lightnvm: add non-continuous lun target creation support"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On 02/05/2016 03:42 AM, Wenwei Tao wrote:
> When create a target, we specify the begin lunid and
> the end lunid, and get the corresponding continuous
> luns from media manager, if one of the luns is not free,
> we failed to create the target, even if the device's
> total free luns are enough.
>
> So add non-continuous lun target creation support,
> thus we can improve the backend device's space utilization.
>
> Signed-off-by: Wenwei Tao <ww.tao0320@xxxxxxxxx>
> ---
> Changes since v2
> -rebase on for-next branch
> -move luns bitmap to PATCH 2
> -remove the logic to dynamically select another lun than
> the one requested
> -implement lunid list in the lnvm ioctl interface
>
> Changes since v1
> -use NVM_FIXED instead NVM_C_FIXED in gennvm_get_lun
> -add target creation flags check
> -rebase to v4.5-rc1
>
> drivers/lightnvm/core.c | 101 ++++++++++++++------
> drivers/lightnvm/rrpc.c | 208 ++++++++++++++++++++++++++----------------
> drivers/lightnvm/rrpc.h | 7 +-
> include/linux/lightnvm.h | 4 +-
> include/uapi/linux/lightnvm.h | 18 ++++
> 5 files changed, 228 insertions(+), 110 deletions(-)
>
> diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
> index 11b8e2d..cbfd575 100644
> --- a/drivers/lightnvm/core.c
> +++ b/drivers/lightnvm/core.c
> @@ -622,7 +622,7 @@ static const struct block_device_operations nvm_fops = {
> static int nvm_create_target(struct nvm_dev *dev,
> struct nvm_ioctl_create *create)
> {
> - struct nvm_ioctl_create_simple *s = &create->conf.s;
> + struct nvm_ioctl_create_conf *conf = &create->conf;
> struct request_queue *tqueue;
> struct gendisk *tdisk;
> struct nvm_tgt_type *tt;
> @@ -671,7 +671,7 @@ static int nvm_create_target(struct nvm_dev *dev,
> tdisk->fops = &nvm_fops;
> tdisk->queue = tqueue;
>
> - targetdata = tt->init(dev, tdisk, s->lun_begin, s->lun_end);
> + targetdata = tt->init(dev, tdisk, conf);
> if (IS_ERR(targetdata))
> goto err_init;
>
> @@ -723,7 +723,6 @@ static void nvm_remove_target(struct nvm_target *t)
> static int __nvm_configure_create(struct nvm_ioctl_create *create)
> {
> struct nvm_dev *dev;
> - struct nvm_ioctl_create_simple *s;
>
> down_write(&nvm_lock);
> dev = nvm_find_nvm_dev(create->dev);
> @@ -733,17 +732,11 @@ static int __nvm_configure_create(struct nvm_ioctl_create *create)
> return -EINVAL;
> }
>
> - if (create->conf.type != NVM_CONFIG_TYPE_SIMPLE) {
> + if (create->conf.type != NVM_CONFIG_TYPE_SIMPLE &&
> + create->conf.type != NVM_CONFIG_TYPE_LIST) {
> pr_err("nvm: config type not valid\n");
> return -EINVAL;
> }
> - s = &create->conf.s;
> -
> - if (s->lun_begin > s->lun_end || s->lun_end > dev->nr_luns) {
> - pr_err("nvm: lun out of bound (%u:%u > %u)\n",
> - s->lun_begin, s->lun_end, dev->nr_luns);
> - return -EINVAL;
> - }
>
> return nvm_create_target(dev, create);
> }
> @@ -821,24 +814,29 @@ static int nvm_configure_remove(const char *val)
>
> static int nvm_configure_create(const char *val)
> {
> - struct nvm_ioctl_create create;
> + struct nvm_ioctl_create *create;
> char opcode;
> int lun_begin, lun_end, ret;
>
> - ret = sscanf(val, "%c %256s %256s %48s %u:%u", &opcode, create.dev,
> - create.tgtname, create.tgttype,
> + create = kzalloc(sizeof(struct nvm_ioctl_create), GFP_KERNEL);
> + if (!create)
> + return -ENOMEM;
> +
> + ret = sscanf(val, "%c %256s %256s %48s %u:%u", &opcode, create->dev,
> + create->tgtname, create->tgttype,
> &lun_begin, &lun_end);
> if (ret != 6) {
> pr_err("nvm: invalid command. Use \"opcode device name tgttype lun_begin:lun_end\".\n");
> + kfree(create);
> return -EINVAL;
> }
>
> - create.flags = 0;
> - create.conf.type = NVM_CONFIG_TYPE_SIMPLE;
> - create.conf.s.lun_begin = lun_begin;
> - create.conf.s.lun_end = lun_end;
> + create->flags = 0;
> + create->conf.type = NVM_CONFIG_TYPE_SIMPLE;
> + create->conf.s.lun_begin = lun_begin;
> + create->conf.s.lun_end = lun_end;
>
> - return __nvm_configure_create(&create);
> + return __nvm_configure_create(create);
> }
>
>
> @@ -991,24 +989,30 @@ static long nvm_ioctl_get_devices(struct file *file, void __user *arg)
>
> static long nvm_ioctl_dev_create(struct file *file, void __user *arg)
> {
> - struct nvm_ioctl_create create;
> + struct nvm_ioctl_create *create;
>
> if (!capable(CAP_SYS_ADMIN))
> return -EPERM;
> + create = kzalloc(sizeof(struct nvm_ioctl_create), GFP_KERNEL);
> + if (!create)
> + return -ENOMEM;
>
> - if (copy_from_user(&create, arg, sizeof(struct nvm_ioctl_create)))
> + if (copy_from_user(&create, arg, sizeof(struct nvm_ioctl_create))) {
> + kfree(create);
> return -EFAULT;
> + }
>
> - create.dev[DISK_NAME_LEN - 1] = '\0';
> - create.tgttype[NVM_TTYPE_NAME_MAX - 1] = '\0';
> - create.tgtname[DISK_NAME_LEN - 1] = '\0';
> + create->dev[DISK_NAME_LEN - 1] = '\0';
> + create->tgttype[NVM_TTYPE_NAME_MAX - 1] = '\0';
> + create->tgtname[DISK_NAME_LEN - 1] = '\0';
>
> - if (create.flags != 0) {
> + if (create->flags != 0) {
> pr_err("nvm: no flags supported\n");
> + kfree(create);
> return -EINVAL;
> }
>
> - return __nvm_configure_create(&create);
> + return __nvm_configure_create(create);
> }
>
> static long nvm_ioctl_dev_remove(struct file *file, void __user *arg)
> @@ -1031,6 +1035,49 @@ static long nvm_ioctl_dev_remove(struct file *file, void __user *arg)
> return __nvm_configure_remove(&remove);
> }
>
> +static long nvm_ioctl_dev_free_luns(struct file *file, void __user *arg)
> +{
> + struct nvm_ioctl_free_luns *free_luns;
> + struct nvm_dev *dev;
> + int lunid = 0;
> +
> + if (!capable(CAP_SYS_ADMIN))
> + return -EPERM;
> +
> + free_luns = kzalloc(sizeof(struct nvm_ioctl_free_luns), GFP_KERNEL);
> + if (!free_luns)
> + return -ENOMEM;
> +
> + if (copy_from_user(&free_luns, arg,
> + sizeof(struct nvm_ioctl_free_luns))) {
> + kfree(free_luns);
> + return -EFAULT;
> + }
> + free_luns->dev[DISK_NAME_LEN - 1] = '\0';
> + down_write(&nvm_lock);
> + dev = nvm_find_nvm_dev(free_luns->dev);
> + up_write(&nvm_lock);
> + if (!dev) {
> + pr_err("nvm: device not found\n");
> + kfree(free_luns);
> + return -EINVAL;
> + }
> +
> + free_luns->nr_free_luns = 0;
> + while ((lunid = find_next_zero_bit(dev->lun_map, dev->nr_luns,
> + lunid)) < dev->nr_luns) {
> +
> + if (free_luns->nr_free_luns >= NVM_LUNS_MAX) {
> + pr_err("nvm: max %u free luns can be reported.\n",
> + NVM_LUNS_MAX);
> + break;
> + }
> + free_luns->free_lunid[free_luns->nr_free_luns++] = lunid;
> + lunid++;
> + }
> + return 0;
> +}
> +
> static void nvm_setup_nvm_sb_info(struct nvm_sb_info *info)
> {
> info->seqnr = 1;
> @@ -1135,6 +1182,8 @@ static long nvm_ctl_ioctl(struct file *file, uint cmd, unsigned long arg)
> return nvm_ioctl_dev_create(file, argp);
> case NVM_DEV_REMOVE:
> return nvm_ioctl_dev_remove(file, argp);
> + case NVM_DEV_FREE_LUNS:
> + return nvm_ioctl_dev_free_luns(file, argp);
> case NVM_DEV_INIT:
> return nvm_ioctl_dev_init(file, argp);
> case NVM_DEV_FACTORY:
> diff --git a/drivers/lightnvm/rrpc.c b/drivers/lightnvm/rrpc.c
> index 2bd5789..88b395d 100644
> --- a/drivers/lightnvm/rrpc.c
> +++ b/drivers/lightnvm/rrpc.c
> @@ -23,28 +23,35 @@ static int rrpc_submit_io(struct rrpc *rrpc, struct bio *bio,
> struct nvm_rq *rqd, unsigned long flags);
>
> #define rrpc_for_each_lun(rrpc, rlun, i) \
> - for ((i) = 0, rlun = &(rrpc)->luns[0]; \
> - (i) < (rrpc)->nr_luns; (i)++, rlun = &(rrpc)->luns[(i)])
> + for ((i) = 0, rlun = &(rrpc)->luns[0]; \
> + (i) < (rrpc)->nr_luns; (i)++, rlun = &(rrpc)->luns[(i)])
> +
> +static inline u64 lun_poffset(struct nvm_lun *lun, struct nvm_dev *dev)
> +{
> + return lun->id * dev->sec_per_lun;
> +}
>
> static void rrpc_page_invalidate(struct rrpc *rrpc, struct rrpc_addr *a)
> {
> struct rrpc_block *rblk = a->rblk;
> - unsigned int pg_offset;
> + struct rrpc_lun *rlun = rblk->rlun;
> + u64 pg_offset;
>
> - lockdep_assert_held(&rrpc->rev_lock);
> + lockdep_assert_held(&rlun->rev_lock);
>
> if (a->addr == ADDR_EMPTY || !rblk)
> return;
>
> spin_lock(&rblk->lock);
>
> - div_u64_rem(a->addr, rrpc->dev->pgs_per_blk, &pg_offset);
> + div_u64_rem(a->addr, rrpc->dev->pgs_per_blk, (u32 *)&pg_offset);
> WARN_ON(test_and_set_bit(pg_offset, rblk->invalid_pages));
> rblk->nr_invalid_pages++;
>
> spin_unlock(&rblk->lock);
>
> - rrpc->rev_trans_map[a->addr - rrpc->poffset].addr = ADDR_EMPTY;
> + pg_offset = lun_poffset(rlun->parent, rrpc->dev);
> + rlun->rev_trans_map[a->addr - pg_offset].addr = ADDR_EMPTY;
> }
>
> static void rrpc_invalidate_range(struct rrpc *rrpc, sector_t slba,
> @@ -52,14 +59,15 @@ static void rrpc_invalidate_range(struct rrpc *rrpc, sector_t slba,
> {
> sector_t i;
>
> - spin_lock(&rrpc->rev_lock);
> for (i = slba; i < slba + len; i++) {
> struct rrpc_addr *gp = &rrpc->trans_map[i];
> + struct rrpc_lun *rlun = gp->rblk->rlun;
>
> + spin_lock(&rlun->rev_lock);
> rrpc_page_invalidate(rrpc, gp);
> + spin_unlock(&rlun->rev_lock);
> gp->rblk = NULL;
> }
> - spin_unlock(&rrpc->rev_lock);
> }
>
> static struct nvm_rq *rrpc_inflight_laddr_acquire(struct rrpc *rrpc,
> @@ -281,13 +289,14 @@ static void rrpc_end_sync_bio(struct bio *bio)
> static int rrpc_move_valid_pages(struct rrpc *rrpc, struct rrpc_block *rblk)
> {
> struct request_queue *q = rrpc->dev->q;
> + struct rrpc_lun *rlun = rblk->rlun;
> struct rrpc_rev_addr *rev;
> struct nvm_rq *rqd;
> struct bio *bio;
> struct page *page;
> int slot;
> int nr_pgs_per_blk = rrpc->dev->pgs_per_blk;
> - u64 phys_addr;
> + u64 phys_addr, poffset;
> DECLARE_COMPLETION_ONSTACK(wait);
>
> if (bitmap_full(rblk->invalid_pages, nr_pgs_per_blk))
> @@ -303,6 +312,7 @@ static int rrpc_move_valid_pages(struct rrpc *rrpc, struct rrpc_block *rblk)
> if (!page)
> return -ENOMEM;
>
> + poffset = lun_poffset(rlun->parent, rrpc->dev);
> while ((slot = find_first_zero_bit(rblk->invalid_pages,
> nr_pgs_per_blk)) < nr_pgs_per_blk) {
>
> @@ -310,23 +320,23 @@ static int rrpc_move_valid_pages(struct rrpc *rrpc, struct rrpc_block *rblk)
> phys_addr = (rblk->parent->id * nr_pgs_per_blk) + slot;
>
> try:
> - spin_lock(&rrpc->rev_lock);
> + spin_lock(&rlun->rev_lock);
> /* Get logical address from physical to logical table */
> - rev = &rrpc->rev_trans_map[phys_addr - rrpc->poffset];
> + rev = &rlun->rev_trans_map[phys_addr - poffset];
> /* already updated by previous regular write */
> if (rev->addr == ADDR_EMPTY) {
> - spin_unlock(&rrpc->rev_lock);
> + spin_unlock(&rlun->rev_lock);
> continue;
> }
>
> rqd = rrpc_inflight_laddr_acquire(rrpc, rev->addr, 1);
> if (IS_ERR_OR_NULL(rqd)) {
> - spin_unlock(&rrpc->rev_lock);
> + spin_unlock(&rlun->rev_lock);
> schedule();
> goto try;
> }
>
> - spin_unlock(&rrpc->rev_lock);
> + spin_unlock(&rlun->rev_lock);
>
> /* Perform read to do GC */
> bio->bi_iter.bi_sector = rrpc_get_sector(rev->addr);
> @@ -395,7 +405,7 @@ static void rrpc_block_gc(struct work_struct *work)
> struct rrpc_block *rblk = gcb->rblk;
> struct nvm_dev *dev = rrpc->dev;
> struct nvm_lun *lun = rblk->parent->lun;
> - struct rrpc_lun *rlun = &rrpc->luns[lun->id - rrpc->lun_offset];
> + struct rrpc_lun *rlun = lun->private;
>
> mempool_free(gcb, rrpc->gcb_pool);
> pr_debug("nvm: block '%lu' being reclaimed\n", rblk->parent->id);
> @@ -498,7 +508,7 @@ static void rrpc_gc_queue(struct work_struct *work)
> struct rrpc_block *rblk = gcb->rblk;
> struct nvm_lun *lun = rblk->parent->lun;
> struct nvm_block *blk = rblk->parent;
> - struct rrpc_lun *rlun = &rrpc->luns[lun->id - rrpc->lun_offset];
> + struct rrpc_lun *rlun = lun->private;
>
> spin_lock(&rlun->lock);
> list_add_tail(&rblk->prio, &rlun->prio_list);
> @@ -549,22 +559,24 @@ static struct rrpc_lun *rrpc_get_lun_rr(struct rrpc *rrpc, int is_gc)
> static struct rrpc_addr *rrpc_update_map(struct rrpc *rrpc, sector_t laddr,
> struct rrpc_block *rblk, u64 paddr)
> {
> + struct rrpc_lun *rlun = rblk->rlun;
> struct rrpc_addr *gp;
> struct rrpc_rev_addr *rev;
> + u64 poffset = lun_poffset(rlun->parent, rrpc->dev);
>
> BUG_ON(laddr >= rrpc->nr_sects);
>
> gp = &rrpc->trans_map[laddr];
> - spin_lock(&rrpc->rev_lock);
> + spin_lock(&rlun->rev_lock);
> if (gp->rblk)
> rrpc_page_invalidate(rrpc, gp);Having a new line would increase readability
>
> gp->addr = paddr;
> gp->rblk = rblk;
>
> - rev = &rrpc->rev_trans_map[gp->addr - rrpc->poffset];
> + rev = &rlun->rev_trans_map[gp->addr - poffset];
> rev->addr = laddr;
> - spin_unlock(&rrpc->rev_lock);
> + spin_unlock(&rlun->rev_lock);
>
> return gp;
> }
> @@ -953,8 +965,6 @@ static void rrpc_requeue(struct work_struct *work)
>
> static void rrpc_gc_free(struct rrpc *rrpc)
> {
> - struct rrpc_lun *rlun;
> - int i;
>
> if (rrpc->krqd_wq)
> destroy_workqueue(rrpc->krqd_wq);
> @@ -962,16 +972,6 @@ static void rrpc_gc_free(struct rrpc *rrpc)
> if (rrpc->kgc_wq)
> destroy_workqueue(rrpc->kgc_wq);
>
> - if (!rrpc->luns)
> - return;
> -
> - for (i = 0; i < rrpc->nr_luns; i++) {
> - rlun = &rrpc->luns[i];
> -
> - if (!rlun->blocks)
> - break;
> - vfree(rlun->blocks);
> - }
> }
>
> static int rrpc_gc_init(struct rrpc *rrpc)
> @@ -992,7 +992,6 @@ static int rrpc_gc_init(struct rrpc *rrpc)
>
> static void rrpc_map_free(struct rrpc *rrpc)
> {
> - vfree(rrpc->rev_trans_map);
> vfree(rrpc->trans_map);
> }
>
> @@ -1000,8 +999,8 @@ static int rrpc_l2p_update(u64 slba, u32 nlb, __le64 *entries, void *private)
> {
> struct rrpc *rrpc = (struct rrpc *)private;
> struct nvm_dev *dev = rrpc->dev;
> - struct rrpc_addr *addr = rrpc->trans_map + slba;
> - struct rrpc_rev_addr *raddr = rrpc->rev_trans_map;
> + struct rrpc_addr *addr;
> + struct rrpc_rev_addr *raddr;
> u64 elba = slba + nlb;
> u64 i;
>
> @@ -1010,8 +1009,15 @@ static int rrpc_l2p_update(u64 slba, u32 nlb, __le64 *entries, void *private)
> return -EINVAL;
> }
>
> + slba -= rrpc->soffset >> (ilog2(dev->sec_size) - 9);
> + addr = rrpc->trans_map + slba;
> for (i = 0; i < nlb; i++) {
> + struct rrpc_lun *rlun;
> + struct nvm_lun *lun;
> u64 pba = le64_to_cpu(entries[i]);
> + u64 poffset;
> + int lunid;
> +
> /* LNVM treats address-spaces as silos, LBA and PBA are
> * equally large and zero-indexed.
> */
> @@ -1026,9 +1032,16 @@ static int rrpc_l2p_update(u64 slba, u32 nlb, __le64 *entries, void *private)
> */
> if (!pba)
> continue;

A new line will increase readability-

> + lunid = div_u64(pba, dev->sec_per_lun);
> + lun = dev->mt->get_lun(dev, lunid);
> + if (unlikely(!lun))
> + return -EINVAL;

Same

> + rlun = lun->private;
> + raddr = rlun->rev_trans_map;
> + poffset = lun_poffset(lun, dev);
>
> addr[i].addr = pba;
> - raddr[pba].addr = slba + i;
> + raddr[pba - poffset].addr = slba + i;
> }
>
> return 0;
> @@ -1047,17 +1060,10 @@ static int rrpc_map_init(struct rrpc *rrpc)
> if (!rrpc->trans_map)
> return -ENOMEM;
>
> - rrpc->rev_trans_map = vmalloc(sizeof(struct rrpc_rev_addr)
> - * rrpc->nr_sects);
> - if (!rrpc->rev_trans_map)
> - return -ENOMEM;
> -
> for (i = 0; i < rrpc->nr_sects; i++) {
> struct rrpc_addr *p = &rrpc->trans_map[i];
> - struct rrpc_rev_addr *r = &rrpc->rev_trans_map[i];
>
> p->addr = ADDR_EMPTY;
> - r->addr = ADDR_EMPTY;
> }
>
> if (!dev->ops->get_l2p_tbl)
> @@ -1129,8 +1135,8 @@ static void rrpc_core_free(struct rrpc *rrpc)
> static void rrpc_luns_free(struct rrpc *rrpc)
> {
> struct nvm_dev *dev = rrpc->dev;
> - struct nvm_lun *lun;
> struct rrpc_lun *rlun;
> + struct nvm_lun *lun;
> int i;
>
> if (!rrpc->luns)
> @@ -1142,24 +1148,68 @@ static void rrpc_luns_free(struct rrpc *rrpc)
> if (!lun)
> break;
> dev->mt->release_lun(dev, lun->id);
> + vfree(rlun->rev_trans_map);
> vfree(rlun->blocks);
> }
> kfree(rrpc->luns);
> + rrpc->luns = NULL;
> +

No new line needed-

> +}
> +
> +static int rrpc_lun_init(struct rrpc *rrpc, struct rrpc_lun *rlun,
> + struct nvm_lun *lun)
> +{
> + struct nvm_dev *dev = rrpc->dev;
> + int i;
> +
> + rlun->rev_trans_map = vmalloc(sizeof(struct rrpc_rev_addr) *
> + dev->sec_per_lun);
> + if (!rlun->rev_trans_map)
> + return -ENOMEM;
> +
> + for (i = 0; i < dev->sec_per_lun; i++) {
> + struct rrpc_rev_addr *r = &rlun->rev_trans_map[i];
> +
> + r->addr = ADDR_EMPTY;
> + }
> + rlun->blocks = vzalloc(sizeof(struct rrpc_block) * dev->blks_per_lun);
> + if (!rlun->blocks) {
> + vfree(rlun->rev_trans_map);
> + return -ENOMEM;
> + }
> +
> + for (i = 0; i < dev->blks_per_lun; i++) {
> + struct rrpc_block *rblk = &rlun->blocks[i];
> + struct nvm_block *blk = &lun->blocks[i];
> +
> + rblk->parent = blk;
> + rblk->rlun = rlun;
> + INIT_LIST_HEAD(&rblk->prio);
> + spin_lock_init(&rblk->lock);
> + }
> +
> + rlun->rrpc = rrpc;
> + lun->private = rlun;
> + INIT_LIST_HEAD(&rlun->prio_list);
> + INIT_LIST_HEAD(&rlun->open_list);
> + INIT_LIST_HEAD(&rlun->closed_list);
> + INIT_WORK(&rlun->ws_gc, rrpc_lun_gc);
> + spin_lock_init(&rlun->lock);
> + spin_lock_init(&rlun->rev_lock);

A new line would be great here.

> + return 0;
> }
>
> -static int rrpc_luns_init(struct rrpc *rrpc, int lun_begin, int lun_end)
> +static int rrpc_luns_init(struct rrpc *rrpc, struct nvm_ioctl_create_conf *conf)
> {
> struct nvm_dev *dev = rrpc->dev;
> struct rrpc_lun *rlun;
> - int i, j, ret = -EINVAL;
> + int i, ret = -EINVAL;
>
> if (dev->pgs_per_blk > MAX_INVALID_PAGES_STORAGE * BITS_PER_LONG) {
> pr_err("rrpc: number of pages per block too high.");
> return -EINVAL;
> }
>
> - spin_lock_init(&rrpc->rev_lock);
> -
> rrpc->luns = kcalloc(rrpc->nr_luns, sizeof(struct rrpc_lun),
> GFP_KERNEL);
> if (!rrpc->luns)
> @@ -1167,9 +1217,20 @@ static int rrpc_luns_init(struct rrpc *rrpc, int lun_begin, int lun_end)
>
> /* 1:1 mapping */
> for (i = 0; i < rrpc->nr_luns; i++) {
> - int lunid = lun_begin + i;
> struct nvm_lun *lun;
> + int lunid;
>
> + if (conf->type == NVM_CONFIG_TYPE_SIMPLE)
> + lunid = conf->s.lun_begin + i;
> + else if (conf->type == NVM_CONFIG_TYPE_LIST)
> + lunid = conf->l.lunid[i];
> + else
> + goto err;

It makes it more readable to insert a blank line here.

> + if (lunid >= dev->nr_luns) {
> + pr_err("rrpc: lun out of bound (%u >= %u)\n",
> + lunid, dev->nr_luns);
> + goto err;
> + }

Same

> if (dev->mt->reserve_lun(dev, lunid)) {
> pr_err("rrpc: lun %u is already allocated\n", lunid);
> goto err;
> @@ -1181,31 +1242,9 @@ static int rrpc_luns_init(struct rrpc *rrpc, int lun_begin, int lun_end)
>
> rlun = &rrpc->luns[i];
> rlun->parent = lun;
> - rlun->blocks = vzalloc(sizeof(struct rrpc_block) *
> - rrpc->dev->blks_per_lun);
> - if (!rlun->blocks) {
> - ret = -ENOMEM;
> + ret = rrpc_lun_init(rrpc, rlun, lun);
> + if (!ret)
> goto err;
> - }
> -
> - for (j = 0; j < rrpc->dev->blks_per_lun; j++) {
> - struct rrpc_block *rblk = &rlun->blocks[j];
> - struct nvm_block *blk = &lun->blocks[j];
> -
> - rblk->parent = blk;
> - rblk->rlun = rlun;
> - INIT_LIST_HEAD(&rblk->prio);
> - spin_lock_init(&rblk->lock);
> - }
> -
> - rlun->rrpc = rrpc;
> - INIT_LIST_HEAD(&rlun->prio_list);
> - INIT_LIST_HEAD(&rlun->open_list);
> - INIT_LIST_HEAD(&rlun->closed_list);
> -
> - INIT_WORK(&rlun->ws_gc, rrpc_lun_gc);
> - spin_lock_init(&rlun->lock);
> -
> rrpc->total_blocks += dev->blks_per_lun;
> rrpc->nr_sects += dev->sec_per_lun;
>
> @@ -1213,6 +1252,7 @@ static int rrpc_luns_init(struct rrpc *rrpc, int lun_begin, int lun_end)
>
> return 0;
> err:
> + rrpc_luns_free(rrpc);
> return ret;
> }
>
> @@ -1285,14 +1325,16 @@ static sector_t rrpc_capacity(void *private)
> static void rrpc_block_map_update(struct rrpc *rrpc, struct rrpc_block *rblk)
> {
> struct nvm_dev *dev = rrpc->dev;
> + struct rrpc_lun *rlun = rblk->rlun;
> int offset;
> struct rrpc_addr *laddr;
> - u64 paddr, pladdr;
> + u64 paddr, pladdr, poffset;
>
> + poffset = lun_poffset(rlun->parent, dev);
> for (offset = 0; offset < dev->pgs_per_blk; offset++) {
> paddr = block_to_addr(rrpc, rblk) + offset;
>
> - pladdr = rrpc->rev_trans_map[paddr].addr;
> + pladdr = rlun->rev_trans_map[paddr - poffset].addr;
> if (pladdr == ADDR_EMPTY)
> continue;
>
> @@ -1357,7 +1399,7 @@ err:
> static struct nvm_tgt_type tt_rrpc;
>
> static void *rrpc_init(struct nvm_dev *dev, struct gendisk *tdisk,
> - int lun_begin, int lun_end)
> + struct nvm_ioctl_create_conf *conf)
> {
> struct request_queue *bqueue = dev->q;
> struct request_queue *tqueue = tdisk->queue;
> @@ -1383,7 +1425,16 @@ static void *rrpc_init(struct nvm_dev *dev, struct gendisk *tdisk,
> spin_lock_init(&rrpc->bio_lock);
> INIT_WORK(&rrpc->ws_requeue, rrpc_requeue);
>
> - rrpc->nr_luns = lun_end - lun_begin + 1;
> + if (conf->type == NVM_CONFIG_TYPE_SIMPLE)
> + rrpc->nr_luns =
> + conf->s.lun_end - conf->s.lun_begin + 1;
> +
> + else if (conf->type == NVM_CONFIG_TYPE_LIST)
> + rrpc->nr_luns = conf->l.nr_luns;
> + else {
> + kfree(rrpc);
> + return ERR_PTR(-EINVAL);
> + }
>
> /* simple round-robin strategy */
> atomic_set(&rrpc->next_lun, -1);
> @@ -1395,15 +1446,12 @@ static void *rrpc_init(struct nvm_dev *dev, struct gendisk *tdisk,
> }
> rrpc->soffset = soffset;
>
> - ret = rrpc_luns_init(rrpc, lun_begin, lun_end);
> + ret = rrpc_luns_init(rrpc, conf);
> if (ret) {
> pr_err("nvm: rrpc: could not initialize luns\n");
> goto err;
> }
>
> - rrpc->poffset = dev->sec_per_lun * lun_begin;
> - rrpc->lun_offset = lun_begin;
> -
> ret = rrpc_core_init(rrpc);
> if (ret) {
> pr_err("nvm: rrpc: could not initialize core\n");
> diff --git a/drivers/lightnvm/rrpc.h b/drivers/lightnvm/rrpc.h
> index 6148b14..abe9135 100644
> --- a/drivers/lightnvm/rrpc.h
> +++ b/drivers/lightnvm/rrpc.h
> @@ -87,6 +87,10 @@ struct rrpc_lun {
>
> struct work_struct ws_gc;
>
> + /* store a reverse map for garbage collection */
> + struct rrpc_rev_addr *rev_trans_map;
> + spinlock_t rev_lock;
> +
> spinlock_t lock;
> };
>
> @@ -124,9 +128,6 @@ struct rrpc {
> * addresses are used when writing to the disk block device.
> */
> struct rrpc_addr *trans_map;
> - /* also store a reverse map for garbage collection */
> - struct rrpc_rev_addr *rev_trans_map;
> - spinlock_t rev_lock;
>
> struct rrpc_inflight inflights;
>
> diff --git a/include/linux/lightnvm.h b/include/linux/lightnvm.h
> index 2a17dc1..d2f2632 100644
> --- a/include/linux/lightnvm.h
> +++ b/include/linux/lightnvm.h
> @@ -271,6 +271,7 @@ struct nvm_lun {
> spinlock_t lock;
>
> struct nvm_block *blocks;
> + void *private;
> };
>
> enum {
> @@ -425,7 +426,8 @@ static inline int ppa_to_slc(struct nvm_dev *dev, int slc_pg)
>
> typedef blk_qc_t (nvm_tgt_make_rq_fn)(struct request_queue *, struct bio *);
> typedef sector_t (nvm_tgt_capacity_fn)(void *);
> -typedef void *(nvm_tgt_init_fn)(struct nvm_dev *, struct gendisk *, int, int);
> +typedef void *(nvm_tgt_init_fn)(struct nvm_dev *, struct gendisk *,
> + struct nvm_ioctl_create_conf *);
> typedef void (nvm_tgt_exit_fn)(void *);
>
> struct nvm_tgt_type {
> diff --git a/include/uapi/linux/lightnvm.h b/include/uapi/linux/lightnvm.h
> index 774a431..23ebc0c9 100644
> --- a/include/uapi/linux/lightnvm.h
> +++ b/include/uapi/linux/lightnvm.h
> @@ -35,6 +35,8 @@
> #define NVM_TTYPE_MAX 63
> #define NVM_MMTYPE_LEN 8
>
> +#define NVM_LUNS_MAX 1024

Let's limit it to 768. That way we don't go above a single memory page,
and we have room for other variables when needed.

> +
> #define NVM_CTRL_FILE "/dev/lightnvm/control"
>
> struct nvm_ioctl_info_tgt {
> @@ -74,14 +76,21 @@ struct nvm_ioctl_create_simple {
> __u32 lun_end;
> };
>
> +struct nvm_ioctl_create_list {
> + __u32 nr_luns;
> + __u32 lunid[NVM_LUNS_MAX];
> +};
> +
> enum {
> NVM_CONFIG_TYPE_SIMPLE = 0,
> + NVM_CONFIG_TYPE_LIST,
> };
>
> struct nvm_ioctl_create_conf {
> __u32 type;
> union {
> struct nvm_ioctl_create_simple s;
> + struct nvm_ioctl_create_list l;
> };
> };
>
> @@ -101,6 +110,12 @@ struct nvm_ioctl_remove {
> __u32 flags;
> };
>
> +struct nvm_ioctl_free_luns {
> + char dev[DISK_NAME_LEN];
> + __u32 nr_free_luns;
> + __u32 free_lunid[NVM_LUNS_MAX];
> +};
> +
> struct nvm_ioctl_dev_init {
> char dev[DISK_NAME_LEN]; /* open-channel SSD device */
> char mmtype[NVM_MMTYPE_LEN]; /* register to media manager */
> @@ -131,6 +146,7 @@ enum {
> /* device level cmds */
> NVM_DEV_CREATE_CMD,
> NVM_DEV_REMOVE_CMD,
> + NVM_DEV_FREE_LUNS_CMD,
>
> /* Init a device to support LightNVM media managers */
> NVM_DEV_INIT_CMD,
> @@ -149,6 +165,8 @@ enum {
> struct nvm_ioctl_create)
> #define NVM_DEV_REMOVE _IOW(NVM_IOCTL, NVM_DEV_REMOVE_CMD, \
> struct nvm_ioctl_remove)
> +#define NVM_DEV_FREE_LUNS _IOW(NVM_IOCTL, NVM_DEV_FREE_LUNS_CMD, \
> + struct nvm_ioctl_free_luns)
> #define NVM_DEV_INIT _IOW(NVM_IOCTL, NVM_DEV_INIT_CMD, \
> struct nvm_ioctl_dev_init)
> #define NVM_DEV_FACTORY _IOW(NVM_IOCTL, NVM_DEV_FACTORY_CMD, \
>

The patch is starting to look good.

I think it would be great to move NVM_DEV_FREE_LUNS into its own patch
and rename it to:

NVM_DEV_LUNS_STATUS, and then return per lun information such as

- Id
- Lun is reserved/available
- Local lunid on channel
- Channel id
- Nr_open_blocks, nr_closed_blocks, nr_free_blocks, nr_bad_blocks

or something similar?

Thanks

Next message: Thomas Petazzoni: "Re: [PATCH v10 5/5] Watchdog: ARM SBSA Generic Watchdog half timeout panic support"
Previous message: Alexander Shishkin: "Re: [PATCH V2 1/6] stm class: Add ioctl get_options interface"
In reply to: Wenwei Tao: "[PATCH v3 3/3] lightnvm: add non-continuous lun target creation support"
Next in thread: Wenwei Tao: "Re: [PATCH v3 3/3] lightnvm: add non-continuous lun target creation support"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]