[PATCH 1/4] lightnvm: double-clear of dev->lun_map on target init error

[PATCH 1/4] lightnvm: double-clear of dev->lun_map on target init error

[Javier González]

The dev->lun_map bits are cleared twice if an target init error occurs.
First in the target clean routine, and then next in the nvm_tgt_create
error function. Make sure that it is only cleared once by extending
nvm_remove_tgt_devi() with a clear bit, such that clearing of bits can
ignored when cleaning up a successful initialized target.

Signed-off-by: Javier González <javier@cnexlabs.com>
Signed-off-by: Matias Bjørling <matias@cnexlabs.com>
---
 drivers/lightnvm/core.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
index 2122922..da4c082 100644
--- a/drivers/lightnvm/core.c
+++ b/drivers/lightnvm/core.c
@@ -89,7 +89,7 @@ static void nvm_release_luns_err(struct nvm_dev *dev, int lun_begin,
 		WARN_ON(!test_and_clear_bit(i, dev->lun_map));
 }
 
-static void nvm_remove_tgt_dev(struct nvm_tgt_dev *tgt_dev)
+static void nvm_remove_tgt_dev(struct nvm_tgt_dev *tgt_dev, int clear)
 {
 	struct nvm_dev *dev = tgt_dev->parent;
 	struct nvm_dev_map *dev_map = tgt_dev->map;
@@ -100,11 +100,13 @@ static void nvm_remove_tgt_dev(struct nvm_tgt_dev *tgt_dev)
 		int *lun_offs = ch_map->lun_offs;
 		int ch = i + ch_map->ch_off;
 
-		for (j = 0; j < ch_map->nr_luns; j++) {
-			int lun = j + lun_offs[j];
-			int lunid = (ch * dev->geo.luns_per_chnl) + lun;
+		if (clear) {
+			for (j = 0; j < ch_map->nr_luns; j++) {
+				int lun = j + lun_offs[j];
+				int lunid = (ch * dev->geo.luns_per_chnl) + lun;
 
-			WARN_ON(!test_and_clear_bit(lunid, dev->lun_map));
+				WARN_ON(!test_and_clear_bit(lunid, dev->lun_map));
+			}
 		}
 
 		kfree(ch_map->lun_offs);
@@ -309,7 +311,7 @@ static int nvm_create_tgt(struct nvm_dev *dev, struct nvm_ioctl_create *create)
 err_queue:
 	blk_cleanup_queue(tqueue);
 err_dev:
-	nvm_remove_tgt_dev(tgt_dev);
+	nvm_remove_tgt_dev(tgt_dev, 0);
 err_t:
 	kfree(t);
 err_reserve:
@@ -332,7 +334,7 @@ static void __nvm_remove_target(struct nvm_target *t)
 	if (tt->exit)
 		tt->exit(tdisk->private_data);
 
-	nvm_remove_tgt_dev(t->dev);
+	nvm_remove_tgt_dev(t->dev, 1);
 	put_disk(tdisk);
 
 	list_del(&t->list);
-- 
2.7.4

[PATCH 2/4] lightnvm: fix cleanup order of disk on init error

[Javier González]

Reorder disk allocation such that the disk structure can be put
safely.

Signed-off-by: Javier González <javier@cnexlabs.com>
---
 drivers/lightnvm/core.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
index da4c082..28e69a7 100644
--- a/drivers/lightnvm/core.c
+++ b/drivers/lightnvm/core.c
@@ -263,15 +263,15 @@ static int nvm_create_tgt(struct nvm_dev *dev, struct nvm_ioctl_create *create)
 		goto err_t;
 	}
 
+	tdisk = alloc_disk(0);
+	if (!tdisk)
+		goto err_dev;
+
 	tqueue = blk_alloc_queue_node(GFP_KERNEL, dev->q->node);
 	if (!tqueue)
-		goto err_dev;
+		goto err_disk;
 	blk_queue_make_request(tqueue, tt->make_rq);
 
-	tdisk = alloc_disk(0);
-	if (!tdisk)
-		goto err_queue;
-
 	sprintf(tdisk->disk_name, "%s", create->tgtname);
 	tdisk->flags = GENHD_FL_EXT_DEVT;
 	tdisk->major = 0;
@@ -307,9 +307,9 @@ static int nvm_create_tgt(struct nvm_dev *dev, struct nvm_ioctl_create *create)
 	if (tt->exit)
 		tt->exit(targetdata);
 err_init:
-	put_disk(tdisk);
-err_queue:
 	blk_cleanup_queue(tqueue);
+err_disk:
+	put_disk(tdisk);
 err_dev:
 	nvm_remove_tgt_dev(tgt_dev, 0);
 err_t:
-- 
2.7.4

[PATCH 3/4] lightnvm: bad type conversion for nvme control bits

[Javier González]

The NVMe I/O command control bits are 16 bytes, but is interpreted as
32 bytes in the lightnvm user I/O data path.

Signed-off-by: Javier González <javier@cnexlabs.com>
---
 drivers/nvme/host/lightnvm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c
index 2c8f933..83e7ea2 100644
--- a/drivers/nvme/host/lightnvm.c
+++ b/drivers/nvme/host/lightnvm.c
@@ -754,7 +754,7 @@ static int nvme_nvm_user_vcmd(struct nvme_ns *ns, int admin,
 	c.common.cdw2[1] = cpu_to_le32(vcmd.cdw3);
 	/* cdw11-12 */
 	c.ph_rw.length = cpu_to_le16(vcmd.nppas);
-	c.ph_rw.control  = cpu_to_le32(vcmd.control);
+	c.ph_rw.control  = cpu_to_le16(vcmd.control);
 	c.common.cdw10[3] = cpu_to_le32(vcmd.cdw13);
 	c.common.cdw10[4] = cpu_to_le32(vcmd.cdw14);
 	c.common.cdw10[5] = cpu_to_le32(vcmd.cdw15);
-- 
2.7.4

[PATCH 4/4] lightnvm: allow to init targets on factory mode

[Javier González]

Target initialization has two responsibilities: creating the target
partition and instantiating the target. This patch enables to create a
factory partition (e.g., do not trigger recovery on the given target).
This is useful for target development and for being able to restore the
device state at any moment in time without requiring a full-device
erase.

Signed-off-by: Javier González <javier@cnexlabs.com>
---
 drivers/lightnvm/core.c       | 14 +++++++++++---
 drivers/lightnvm/rrpc.c       |  3 ++-
 include/linux/lightnvm.h      |  3 ++-
 include/uapi/linux/lightnvm.h |  4 ++++
 4 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
index 28e69a7..e4530c2 100644
--- a/drivers/lightnvm/core.c
+++ b/drivers/lightnvm/core.c
@@ -279,7 +279,7 @@ static int nvm_create_tgt(struct nvm_dev *dev, struct nvm_ioctl_create *create)
 	tdisk->fops = &nvm_fops;
 	tdisk->queue = tqueue;
 
-	targetdata = tt->init(tgt_dev, tdisk);
+	targetdata = tt->init(tgt_dev, tdisk, create->flags);
 	if (IS_ERR(targetdata))
 		goto err_init;
 
@@ -1243,8 +1243,16 @@ static long nvm_ioctl_dev_create(struct file *file, void __user *arg)
 	create.tgtname[DISK_NAME_LEN - 1] = '\0';
 
 	if (create.flags != 0) {
-		pr_err("nvm: no flags supported\n");
-		return -EINVAL;
+		__u32 flags = create.flags;
+
+		/* Check for valid flags */
+		if (flags & NVM_TARGET_FACTORY)
+			flags &= ~NVM_TARGET_FACTORY;
+
+		if (flags) {
+			pr_err("nvm: flag not supported\n");
+			return -EINVAL;
+		}
 	}
 
 	return __nvm_configure_create(&create);
diff --git a/drivers/lightnvm/rrpc.c b/drivers/lightnvm/rrpc.c
index 4e4c299..2c04ff3 100644
--- a/drivers/lightnvm/rrpc.c
+++ b/drivers/lightnvm/rrpc.c
@@ -1515,7 +1515,8 @@ static int rrpc_luns_configure(struct rrpc *rrpc)
 
 static struct nvm_tgt_type tt_rrpc;
 
-static void *rrpc_init(struct nvm_tgt_dev *dev, struct gendisk *tdisk)
+static void *rrpc_init(struct nvm_tgt_dev *dev, struct gendisk *tdisk,
+		       int flags)
 {
 	struct request_queue *bqueue = dev->q;
 	struct request_queue *tqueue = tdisk->queue;
diff --git a/include/linux/lightnvm.h b/include/linux/lightnvm.h
index bebea80..e88f7ef 100644
--- a/include/linux/lightnvm.h
+++ b/include/linux/lightnvm.h
@@ -436,7 +436,8 @@ static inline int ppa_cmp_blk(struct ppa_addr ppa1, struct ppa_addr ppa2)
 
 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_tgt_dev *, struct gendisk *);
+typedef void *(nvm_tgt_init_fn)(struct nvm_tgt_dev *, struct gendisk *,
+				int flags);
 typedef void (nvm_tgt_exit_fn)(void *);
 typedef int (nvm_tgt_sysfs_init_fn)(struct gendisk *);
 typedef void (nvm_tgt_sysfs_exit_fn)(struct gendisk *);
diff --git a/include/uapi/linux/lightnvm.h b/include/uapi/linux/lightnvm.h
index fd19f36..c8aec4b 100644
--- a/include/uapi/linux/lightnvm.h
+++ b/include/uapi/linux/lightnvm.h
@@ -85,6 +85,10 @@ struct nvm_ioctl_create_conf {
 	};
 };
 
+enum {
+	NVM_TARGET_FACTORY = 1 << 0,	/* Init target in factory mode */
+};
+
 struct nvm_ioctl_create {
 	char dev[DISK_NAME_LEN];		/* open-channel SSD device */
 	char tgttype[NVM_TTYPE_NAME_MAX];	/* target type name */
-- 
2.7.4

Re: [PATCH 1/4] lightnvm: double-clear of dev->lun_map on target init error

[Matias Bjørling]

On 04/07/2017 08:31 PM, Javier González wrote:
> The dev->lun_map bits are cleared twice if an target init error occurs.
> First in the target clean routine, and then next in the nvm_tgt_create
> error function. Make sure that it is only cleared once by extending
> nvm_remove_tgt_devi() with a clear bit, such that clearing of bits can
> ignored when cleaning up a successful initialized target.
>
> Signed-off-by: Javier González <javier@cnexlabs.com>
> Signed-off-by: Matias Bjørling <matias@cnexlabs.com>
> ---
>  drivers/lightnvm/core.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
> index 2122922..da4c082 100644
> --- a/drivers/lightnvm/core.c
> +++ b/drivers/lightnvm/core.c
> @@ -89,7 +89,7 @@ static void nvm_release_luns_err(struct nvm_dev *dev, int lun_begin,
>  		WARN_ON(!test_and_clear_bit(i, dev->lun_map));
>  }
>
> -static void nvm_remove_tgt_dev(struct nvm_tgt_dev *tgt_dev)
> +static void nvm_remove_tgt_dev(struct nvm_tgt_dev *tgt_dev, int clear)
>  {
>  	struct nvm_dev *dev = tgt_dev->parent;
>  	struct nvm_dev_map *dev_map = tgt_dev->map;
> @@ -100,11 +100,13 @@ static void nvm_remove_tgt_dev(struct nvm_tgt_dev *tgt_dev)
>  		int *lun_offs = ch_map->lun_offs;
>  		int ch = i + ch_map->ch_off;
>
> -		for (j = 0; j < ch_map->nr_luns; j++) {
> -			int lun = j + lun_offs[j];
> -			int lunid = (ch * dev->geo.luns_per_chnl) + lun;
> +		if (clear) {
> +			for (j = 0; j < ch_map->nr_luns; j++) {
> +				int lun = j + lun_offs[j];
> +				int lunid = (ch * dev->geo.luns_per_chnl) + lun;
>
> -			WARN_ON(!test_and_clear_bit(lunid, dev->lun_map));
> +				WARN_ON(!test_and_clear_bit(lunid, dev->lun_map));
> +			}
>  		}
>
>  		kfree(ch_map->lun_offs);
> @@ -309,7 +311,7 @@ static int nvm_create_tgt(struct nvm_dev *dev, struct nvm_ioctl_create *create)
>  err_queue:
>  	blk_cleanup_queue(tqueue);
>  err_dev:
> -	nvm_remove_tgt_dev(tgt_dev);
> +	nvm_remove_tgt_dev(tgt_dev, 0);
>  err_t:
>  	kfree(t);
>  err_reserve:
> @@ -332,7 +334,7 @@ static void __nvm_remove_target(struct nvm_target *t)
>  	if (tt->exit)
>  		tt->exit(tdisk->private_data);
>
> -	nvm_remove_tgt_dev(t->dev);
> +	nvm_remove_tgt_dev(t->dev, 1);
>  	put_disk(tdisk);
>
>  	list_del(&t->list);
>

Thanks. Applied for 4.12.

Re: [PATCH 2/4] lightnvm: fix cleanup order of disk on init error

[Matias Bjørling]

On 04/07/2017 08:31 PM, Javier González wrote:
> Reorder disk allocation such that the disk structure can be put
> safely.
>
> Signed-off-by: Javier González <javier@cnexlabs.com>
> ---
>  drivers/lightnvm/core.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
> index da4c082..28e69a7 100644
> --- a/drivers/lightnvm/core.c
> +++ b/drivers/lightnvm/core.c
> @@ -263,15 +263,15 @@ static int nvm_create_tgt(struct nvm_dev *dev, struct nvm_ioctl_create *create)
>  		goto err_t;
>  	}
>
> +	tdisk = alloc_disk(0);
> +	if (!tdisk)
> +		goto err_dev;
> +
>  	tqueue = blk_alloc_queue_node(GFP_KERNEL, dev->q->node);
>  	if (!tqueue)
> -		goto err_dev;
> +		goto err_disk;
>  	blk_queue_make_request(tqueue, tt->make_rq);
>
> -	tdisk = alloc_disk(0);
> -	if (!tdisk)
> -		goto err_queue;
> -
>  	sprintf(tdisk->disk_name, "%s", create->tgtname);
>  	tdisk->flags = GENHD_FL_EXT_DEVT;
>  	tdisk->major = 0;
> @@ -307,9 +307,9 @@ static int nvm_create_tgt(struct nvm_dev *dev, struct nvm_ioctl_create *create)
>  	if (tt->exit)
>  		tt->exit(targetdata);
>  err_init:
> -	put_disk(tdisk);
> -err_queue:
>  	blk_cleanup_queue(tqueue);
> +err_disk:
> +	put_disk(tdisk);
>  err_dev:
>  	nvm_remove_tgt_dev(tgt_dev, 0);
>  err_t:
>
Thanks. Applied for 4.12.

Re: [PATCH 3/4] lightnvm: bad type conversion for nvme control bits

[Matias Bjørling]

On 04/07/2017 08:31 PM, Javier González wrote:
> The NVMe I/O command control bits are 16 bytes, but is interpreted as
> 32 bytes in the lightnvm user I/O data path.
>
> Signed-off-by: Javier González <javier@cnexlabs.com>
> ---
>  drivers/nvme/host/lightnvm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c
> index 2c8f933..83e7ea2 100644
> --- a/drivers/nvme/host/lightnvm.c
> +++ b/drivers/nvme/host/lightnvm.c
> @@ -754,7 +754,7 @@ static int nvme_nvm_user_vcmd(struct nvme_ns *ns, int admin,
>  	c.common.cdw2[1] = cpu_to_le32(vcmd.cdw3);
>  	/* cdw11-12 */
>  	c.ph_rw.length = cpu_to_le16(vcmd.nppas);
> -	c.ph_rw.control  = cpu_to_le32(vcmd.control);
> +	c.ph_rw.control  = cpu_to_le16(vcmd.control);
>  	c.common.cdw10[3] = cpu_to_le32(vcmd.cdw13);
>  	c.common.cdw10[4] = cpu_to_le32(vcmd.cdw14);
>  	c.common.cdw10[5] = cpu_to_le32(vcmd.cdw15);
>
Thanks. Applied for 4.12.

Re: [PATCH 4/4] lightnvm: allow to init targets on factory mode

[Matias Bjørling]

On 04/07/2017 08:31 PM, Javier González wrote:
> Target initialization has two responsibilities: creating the target
> partition and instantiating the target. This patch enables to create a
> factory partition (e.g., do not trigger recovery on the given target).
> This is useful for target development and for being able to restore the
> device state at any moment in time without requiring a full-device
> erase.
>
> Signed-off-by: Javier González <javier@cnexlabs.com>
> ---
>  drivers/lightnvm/core.c       | 14 +++++++++++---
>  drivers/lightnvm/rrpc.c       |  3 ++-
>  include/linux/lightnvm.h      |  3 ++-
>  include/uapi/linux/lightnvm.h |  4 ++++
>  4 files changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
> index 28e69a7..e4530c2 100644
> --- a/drivers/lightnvm/core.c
> +++ b/drivers/lightnvm/core.c
> @@ -279,7 +279,7 @@ static int nvm_create_tgt(struct nvm_dev *dev, struct nvm_ioctl_create *create)
>  	tdisk->fops = &nvm_fops;
>  	tdisk->queue = tqueue;
>
> -	targetdata = tt->init(tgt_dev, tdisk);
> +	targetdata = tt->init(tgt_dev, tdisk, create->flags);
>  	if (IS_ERR(targetdata))
>  		goto err_init;
>
> @@ -1243,8 +1243,16 @@ static long nvm_ioctl_dev_create(struct file *file, void __user *arg)
>  	create.tgtname[DISK_NAME_LEN - 1] = '\0';
>
>  	if (create.flags != 0) {
> -		pr_err("nvm: no flags supported\n");
> -		return -EINVAL;
> +		__u32 flags = create.flags;
> +
> +		/* Check for valid flags */
> +		if (flags & NVM_TARGET_FACTORY)
> +			flags &= ~NVM_TARGET_FACTORY;
> +
> +		if (flags) {
> +			pr_err("nvm: flag not supported\n");
> +			return -EINVAL;
> +		}
>  	}
>
>  	return __nvm_configure_create(&create);
> diff --git a/drivers/lightnvm/rrpc.c b/drivers/lightnvm/rrpc.c
> index 4e4c299..2c04ff3 100644
> --- a/drivers/lightnvm/rrpc.c
> +++ b/drivers/lightnvm/rrpc.c
> @@ -1515,7 +1515,8 @@ static int rrpc_luns_configure(struct rrpc *rrpc)
>
>  static struct nvm_tgt_type tt_rrpc;
>
> -static void *rrpc_init(struct nvm_tgt_dev *dev, struct gendisk *tdisk)
> +static void *rrpc_init(struct nvm_tgt_dev *dev, struct gendisk *tdisk,
> +		       int flags)
>  {
>  	struct request_queue *bqueue = dev->q;
>  	struct request_queue *tqueue = tdisk->queue;
> diff --git a/include/linux/lightnvm.h b/include/linux/lightnvm.h
> index bebea80..e88f7ef 100644
> --- a/include/linux/lightnvm.h
> +++ b/include/linux/lightnvm.h
> @@ -436,7 +436,8 @@ static inline int ppa_cmp_blk(struct ppa_addr ppa1, struct ppa_addr ppa2)
>
>  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_tgt_dev *, struct gendisk *);
> +typedef void *(nvm_tgt_init_fn)(struct nvm_tgt_dev *, struct gendisk *,
> +				int flags);
>  typedef void (nvm_tgt_exit_fn)(void *);
>  typedef int (nvm_tgt_sysfs_init_fn)(struct gendisk *);
>  typedef void (nvm_tgt_sysfs_exit_fn)(struct gendisk *);
> diff --git a/include/uapi/linux/lightnvm.h b/include/uapi/linux/lightnvm.h
> index fd19f36..c8aec4b 100644
> --- a/include/uapi/linux/lightnvm.h
> +++ b/include/uapi/linux/lightnvm.h
> @@ -85,6 +85,10 @@ struct nvm_ioctl_create_conf {
>  	};
>  };
>
> +enum {
> +	NVM_TARGET_FACTORY = 1 << 0,	/* Init target in factory mode */
> +};
> +
>  struct nvm_ioctl_create {
>  	char dev[DISK_NAME_LEN];		/* open-channel SSD device */
>  	char tgttype[NVM_TTYPE_NAME_MAX];	/* target type name */
>
Thanks. Applied for 4.12. Feel free to kick the part to Keith in the 
nvme-cli tool.