All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/19] nvmet: add support for file backed namespaces
@ 2018-04-18 18:59 Chaitanya Kulkarni
  2018-04-18 18:59 ` [PATCH 01/20] nvmet: add block device guard for smart-log Chaitanya Kulkarni
                   ` (20 more replies)
  0 siblings, 21 replies; 29+ messages in thread
From: Chaitanya Kulkarni @ 2018-04-18 18:59 UTC (permalink / raw)


Hi,

This patch series implements the file backed namespaces
support for the NVMeOF target.

In current implementation NVMeOF supports block
device backed namespaces on the target side.
With this implementation regular file(s) can be used to
initialize the namespace(s). This approach provides host more
control to create and manage namespaces on target side without
using configfs interface but using tools such as nvme-cli on the
host side.

Structure of the path-series:-

This patch series is divided into two parts. The first
part (0001-0009 patches) implements the file backed
namespaces with existing target side configfs namespace
creation model. In the second part of the series
we implement the code for host side NVMe Namespace
management commands such as nvme-create-ns/nvme-delete-ns.

Example:-

Following example demonstrates how to configure
newly introduced file backed subsystem to create file backed ns:-

1. Target side configuration:-

1.1 Create a subsystem under newly introduced direcotry "fs"
# mkdir /sys/kernel/config/nvmet/fs/file
1.2 Initialize the mount_path attributes where all the backend
    files are created
# echo -n "/mnt/nvme0n1/" > /sys/kernel/config/nvmet/fs/file/attr_mount_path
1.3 Create and initialize the port
# mkdir /sys/kernel/config/nvmet/ports/1/
# echo -n "loop" > /sys/kernel/config/nvmet/ports/1/addr_trtype
# echo -n 1 > /sys/kernel/config/nvmet/fs/file/attr_allow_any_host
# ln -s /sys/kernel/config/nvmet/fs/file /sys/kernel/config/nvmet/ports/1/subsystems/
1.4 Following is the example of the configfs structure after initialization:-

	/sys/kernel/config/nvmet/
		|-- fs
		|   |-- file
		|       |-- allowed_hosts
		|       |-- attr_allow_any_host
		|       |-- attr_mount_path
		|       |-- attr_serial
		|       |-- attr_version
		|-- hosts
		|-- ports
		|   |-- 1
		|       |-- addr_adrfam
		|       |-- addr_traddr
		|       |-- addr_treq
		|       |-- addr_trsvcid
		|       |-- addr_trtype
		|       |-- referrals
		|       |-- subsystems
		|           |-- file -> ../../../../nvmet/fs/file
		|-- subsystems

Note :- We format NVMe SSD with XFS file system to initialize the "mount_path".

2. Host side configuration:-

2.1 Connect to the file backed subsystem
# echo  "transport=loop,nqn=file" > /dev/nvme-fabrics
2.2 Create and list ns on the host:-
# nvme create-ns /dev/nvme1 --nsze=204800000 --ncap=204800000 --flbas=9
create-ns: Success, created nsid:1
2.3 Delete Namespace
# nvme delete-ns /dev/nvme1 -n 1

3. Performance Numbers:-

We collect the performance numbers by configuring the target side
in different modes where target port is configured in "nvme_loop" mode:-

1. Use NVMe PCIe SSD as a block device (default Block device mode).
2. Use File backed NS with O_DIRECT.
3. USe File backed NS with O_SYNC.

I ran simple random read fio, following is the performance comparison:-

				Bandwidth (MB/s)

Iteration 	Block	|	File O_DIRECT	|	File O_SYNC
	1.	2383	|		2655	|		3115
	2.	2380	|		2775	|		3145
	3.	2500	|		2838	|		3176


				IOPS (k)

Iteration 	Block	|	File O_DIRECT	|	File O_SYNC
	1.	610	|		680	|		797
	2.	609	|		710	|		805
	3.	640	|		726	|		813


				Average Latency (usec)

Iteration 	Block	|	File O_DIRECT	|	File O_SYNC
	1.	155	|		139	|		119
	2.	155	|		133	|		118
	3.	148	|		130	|		116

For the first review, I kept the code isolated, I'll aggregate
some of the patches in next version.

For simplicity right now backend file naming is associated with
ctrl-id, for the next version we can get rid of the ctrl-id
dependency, use subsys-nqn to create directory hierarchy
under the "mount_path". With that change, it will be easier to implement
the persistent subsystem structure for file backed subsystem(s)
along with dynamic controller creation as long as the file system is
preserved and "mount_path" is intact.

Chaitanya Kulkarni (20):
  nvmet: add block device guard for smart-log
  nvmet: add a wrapper for ns handlers
  nvmet: add structure members for file I/O
  nvmet: initialize file handle and req fields
  nvmet: add NVMe I/O command handlers for file
  nvmet: add new members for sync file I/O
  nvmet: add sync I/O configfs attr for ns
  nvmet: configure file backed ns for sync IO
  nvmet: use workqueue for sync I/O
  nvmet: isolate id ctrl/ns field initialization
  nvmet: introduce new members for ns-mgmt
  nvmet: add a configfs entry for subsys mount path
  nvmet: initialize new ns and subsys members
  nvmet: add and restructure ns mgmt helpers
  fs: export kern_path_locked() to reuse the code
  nvmet: add ns-mgmt command handlers
  nvmet: override identify for file backed ns
  nvmet: allow host to configure sync vs direct IO
  nvmet: add check for ctrl processing paused status
  nvmet: use processing paused state for VWC

 drivers/nvme/target/admin-cmd.c | 423 +++++++++++++++++++++++++++++++++++++---
 drivers/nvme/target/configfs.c  | 105 +++++++++-
 drivers/nvme/target/core.c      | 168 ++++++++++++++--
 drivers/nvme/target/io-cmd.c    | 165 +++++++++++++++-
 drivers/nvme/target/nvmet.h     |  21 ++
 fs/namei.c                      |   1 +
 6 files changed, 838 insertions(+), 45 deletions(-)

--
2.14.1

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 01/20] nvmet: add block device guard for smart-log
  2018-04-18 18:59 [PATCH 00/19] nvmet: add support for file backed namespaces Chaitanya Kulkarni
@ 2018-04-18 18:59 ` Chaitanya Kulkarni
  2018-04-18 18:59 ` [PATCH 02/20] nvmet: add a wrapper for ns handlers Chaitanya Kulkarni
                   ` (19 subsequent siblings)
  20 siblings, 0 replies; 29+ messages in thread
From: Chaitanya Kulkarni @ 2018-04-18 18:59 UTC (permalink / raw)


Only use block device backed namespaces to compute target
side smart log. This is a preparation patch for implementing
file backed namespaces.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
---
 drivers/nvme/target/admin-cmd.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index 90dcdc40ac71..602995f7a1b3 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -45,6 +45,10 @@ static u16 nvmet_get_smart_log_nsid(struct nvmet_req *req,
 		return NVME_SC_INVALID_NS;
 	}
 
+	/* for file backed ns just return */
+	if (!ns->bdev)
+		goto out;
+
 	host_reads = part_stat_read(ns->bdev->bd_part, ios[READ]);
 	data_units_read = part_stat_read(ns->bdev->bd_part, sectors[READ]);
 	host_writes = part_stat_read(ns->bdev->bd_part, ios[WRITE]);
@@ -54,6 +58,7 @@ static u16 nvmet_get_smart_log_nsid(struct nvmet_req *req,
 	put_unaligned_le64(data_units_read, &slog->data_units_read[0]);
 	put_unaligned_le64(host_writes, &slog->host_writes[0]);
 	put_unaligned_le64(data_units_written, &slog->data_units_written[0]);
+out:
 	nvmet_put_namespace(ns);
 
 	return NVME_SC_SUCCESS;
@@ -71,6 +76,9 @@ static u16 nvmet_get_smart_log_all(struct nvmet_req *req,
 
 	rcu_read_lock();
 	list_for_each_entry_rcu(ns, &ctrl->subsys->namespaces, dev_link) {
+		/* for file backed ns just return */
+		if (!ns->bdev)
+			continue;
 		host_reads += part_stat_read(ns->bdev->bd_part, ios[READ]);
 		data_units_read +=
 			part_stat_read(ns->bdev->bd_part, sectors[READ]);
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 02/20] nvmet: add a wrapper for ns handlers
  2018-04-18 18:59 [PATCH 00/19] nvmet: add support for file backed namespaces Chaitanya Kulkarni
  2018-04-18 18:59 ` [PATCH 01/20] nvmet: add block device guard for smart-log Chaitanya Kulkarni
@ 2018-04-18 18:59 ` Chaitanya Kulkarni
  2018-04-18 18:59 ` [PATCH 03/20] nvmet: add structure members for file I/O Chaitanya Kulkarni
                   ` (18 subsequent siblings)
  20 siblings, 0 replies; 29+ messages in thread
From: Chaitanya Kulkarni @ 2018-04-18 18:59 UTC (permalink / raw)


This patch isolates the code for block device open and close.
This is a preparation patch for implementation of the
file backed namespaces.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
---
 drivers/nvme/target/core.c | 54 +++++++++++++++++++++++++++++++---------------
 1 file changed, 37 insertions(+), 17 deletions(-)

diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index a78029e4e5f4..5f032dc7077f 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -271,6 +271,37 @@ void nvmet_put_namespace(struct nvmet_ns *ns)
 	percpu_ref_put(&ns->ref);
 }
 
+static int nvmet_ns_enable_blk(struct nvmet_ns *ns)
+{
+	int blk_flags = FMODE_READ | FMODE_WRITE;
+
+	ns->bdev = blkdev_get_by_path(ns->device_path, blk_flags, NULL);
+	if (IS_ERR(ns->bdev)) {
+		pr_err("failed to open block device %s: (%ld)\n",
+				ns->device_path, PTR_ERR(ns->bdev));
+		ns->bdev = NULL;
+		return -1;
+	}
+	ns->size = i_size_read(ns->bdev->bd_inode);
+	ns->blksize_shift =
+		blksize_bits(bdev_logical_block_size(ns->bdev));
+
+	return 0;
+}
+
+static int nvmet_ns_dev_enable(struct nvmet_ns *ns)
+{
+	return nvmet_ns_enable_blk(ns);
+}
+
+static void nvmet_ns_dev_disable(struct nvmet_ns *ns)
+{
+	if (ns->bdev) {
+		blkdev_put(ns->bdev, FMODE_WRITE | FMODE_READ);
+		ns->bdev = NULL;
+	}
+}
+
 int nvmet_ns_enable(struct nvmet_ns *ns)
 {
 	struct nvmet_subsys *subsys = ns->subsys;
@@ -281,23 +312,14 @@ int nvmet_ns_enable(struct nvmet_ns *ns)
 	if (ns->enabled)
 		goto out_unlock;
 
-	ns->bdev = blkdev_get_by_path(ns->device_path, FMODE_READ | FMODE_WRITE,
-			NULL);
-	if (IS_ERR(ns->bdev)) {
-		pr_err("failed to open block device %s: (%ld)\n",
-		       ns->device_path, PTR_ERR(ns->bdev));
-		ret = PTR_ERR(ns->bdev);
-		ns->bdev = NULL;
+	ret = nvmet_ns_dev_enable(ns);
+	if (ret)
 		goto out_unlock;
-	}
-
-	ns->size = i_size_read(ns->bdev->bd_inode);
-	ns->blksize_shift = blksize_bits(bdev_logical_block_size(ns->bdev));
 
 	ret = percpu_ref_init(&ns->ref, nvmet_destroy_namespace,
 				0, GFP_KERNEL);
 	if (ret)
-		goto out_blkdev_put;
+		goto out_dev_put;
 
 	if (ns->nsid > subsys->max_nsid)
 		subsys->max_nsid = ns->nsid;
@@ -328,9 +350,8 @@ int nvmet_ns_enable(struct nvmet_ns *ns)
 out_unlock:
 	mutex_unlock(&subsys->lock);
 	return ret;
-out_blkdev_put:
-	blkdev_put(ns->bdev, FMODE_WRITE|FMODE_READ);
-	ns->bdev = NULL;
+out_dev_put:
+	nvmet_ns_dev_disable(ns);
 	goto out_unlock;
 }
 
@@ -366,8 +387,7 @@ void nvmet_ns_disable(struct nvmet_ns *ns)
 	list_for_each_entry(ctrl, &subsys->ctrls, subsys_entry)
 		nvmet_add_async_event(ctrl, NVME_AER_TYPE_NOTICE, 0, 0);
 
-	if (ns->bdev)
-		blkdev_put(ns->bdev, FMODE_WRITE|FMODE_READ);
+	nvmet_ns_dev_disable(ns);
 out_unlock:
 	mutex_unlock(&subsys->lock);
 }
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 03/20] nvmet: add structure members for file I/O
  2018-04-18 18:59 [PATCH 00/19] nvmet: add support for file backed namespaces Chaitanya Kulkarni
  2018-04-18 18:59 ` [PATCH 01/20] nvmet: add block device guard for smart-log Chaitanya Kulkarni
  2018-04-18 18:59 ` [PATCH 02/20] nvmet: add a wrapper for ns handlers Chaitanya Kulkarni
@ 2018-04-18 18:59 ` Chaitanya Kulkarni
  2018-04-24 17:18   ` Christoph Hellwig
  2018-04-18 18:59 ` [PATCH 04/20] nvmet: initialize file handle and req fields Chaitanya Kulkarni
                   ` (17 subsequent siblings)
  20 siblings, 1 reply; 29+ messages in thread
From: Chaitanya Kulkarni @ 2018-04-18 18:59 UTC (permalink / raw)


This patch adds new members to perform I/O operations on the
file backed namespaces.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
---
 drivers/nvme/target/nvmet.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index 417f6c0331cc..acc61ca97198 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -26,6 +26,7 @@
 #include <linux/configfs.h>
 #include <linux/rcupdate.h>
 #include <linux/blkdev.h>
+#include <linux/fs.h>
 
 #define NVMET_ASYNC_EVENTS		4
 #define NVMET_ERROR_LOG_SLOTS		128
@@ -43,6 +44,7 @@ struct nvmet_ns {
 	struct list_head	dev_link;
 	struct percpu_ref	ref;
 	struct block_device	*bdev;
+	struct file		*filp;
 	u32			nsid;
 	u32			blksize_shift;
 	loff_t			size;
@@ -222,6 +224,8 @@ struct nvmet_req {
 	struct scatterlist	*sg;
 	struct bio		inline_bio;
 	struct bio_vec		inline_bvec[NVMET_MAX_INLINE_BIOVEC];
+	struct kiocb            iocb;
+	struct bio_vec          *bvec;
 	int			sg_cnt;
 	/* data length as parsed from the command: */
 	size_t			data_len;
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 04/20] nvmet: initialize file handle and req fields
  2018-04-18 18:59 [PATCH 00/19] nvmet: add support for file backed namespaces Chaitanya Kulkarni
                   ` (2 preceding siblings ...)
  2018-04-18 18:59 ` [PATCH 03/20] nvmet: add structure members for file I/O Chaitanya Kulkarni
@ 2018-04-18 18:59 ` Chaitanya Kulkarni
  2018-04-24 17:22   ` Christoph Hellwig
  2018-04-18 18:59 ` [PATCH 05/20] nvmet: add NVMe I/O command handlers for file Chaitanya Kulkarni
                   ` (16 subsequent siblings)
  20 siblings, 1 reply; 29+ messages in thread
From: Chaitanya Kulkarni @ 2018-04-18 18:59 UTC (permalink / raw)


This patch adds support to manipulate file handle and initializes
the required fields for the file backed target ns.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
---
 drivers/nvme/target/core.c | 62 ++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 60 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index 5f032dc7077f..b7c51a5629bf 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -289,9 +289,61 @@ static int nvmet_ns_enable_blk(struct nvmet_ns *ns)
 	return 0;
 }
 
+int nvmet_vfs_stat(const char *filename, struct kstat *stat)
+{
+	int ret;
+	mm_segment_t old_fs = get_fs();
+
+	set_fs(KERNEL_DS);
+	ret = vfs_stat(filename, stat);
+	if (ret)
+		pr_err("vfs_stat failed %s: %d\n", filename, ret);
+	set_fs(old_fs);
+	return ret;
+}
+
+static int nvmet_ns_enable_file(struct nvmet_ns *ns)
+{
+	int ret;
+	int flags = O_RDWR | O_LARGEFILE | O_DIRECT;
+	struct kstat stat;
+
+	ns->filp = filp_open(ns->device_path, flags, 0);
+	if (!ns->filp || IS_ERR(ns->filp)) {
+		pr_err("failed to open file %s: (%ld)\n",
+				ns->device_path, PTR_ERR(ns->bdev));
+		ns->filp = NULL;
+		return -1;
+	}
+
+	ret = nvmet_vfs_stat(ns->device_path, &stat);
+	if (ret)
+		goto err;
+
+	ns->size = stat.size;
+	ns->blksize_shift = ns->filp->f_inode->i_blkbits;
+
+	return ret;
+err:
+	filp_close(ns->filp, NULL);
+	ns->size = ns->blksize_shift = 0;
+	ns->filp = NULL;
+
+	return ret;
+}
+
 static int nvmet_ns_dev_enable(struct nvmet_ns *ns)
 {
-	return nvmet_ns_enable_blk(ns);
+	int ret = 0;
+	int ret1 = 0;
+
+	ret = nvmet_ns_enable_blk(ns);
+	if (ret)
+		ret1 = nvmet_ns_enable_file(ns);
+
+	if (ret && ret1)
+		return -1;
+	return 0;
 }
 
 static void nvmet_ns_dev_disable(struct nvmet_ns *ns)
@@ -299,7 +351,11 @@ static void nvmet_ns_dev_disable(struct nvmet_ns *ns)
 	if (ns->bdev) {
 		blkdev_put(ns->bdev, FMODE_WRITE | FMODE_READ);
 		ns->bdev = NULL;
-	}
+	} else if (ns->filp) {
+		filp_close(ns->filp, NULL);
+		ns->filp = NULL;
+	} else
+		pr_err("invalid ns handle\n.");
 }
 
 int nvmet_ns_enable(struct nvmet_ns *ns)
@@ -533,6 +589,8 @@ bool nvmet_req_init(struct nvmet_req *req, struct nvmet_cq *cq,
 	req->transfer_len = 0;
 	req->rsp->status = 0;
 	req->ns = NULL;
+	req->bvec = NULL;
+	memset(&req->iocb, 0, sizeof(req->iocb));
 
 	/* no support for fused commands yet */
 	if (unlikely(flags & (NVME_CMD_FUSE_FIRST | NVME_CMD_FUSE_SECOND))) {
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 05/20] nvmet: add NVMe I/O command handlers for file
  2018-04-18 18:59 [PATCH 00/19] nvmet: add support for file backed namespaces Chaitanya Kulkarni
                   ` (3 preceding siblings ...)
  2018-04-18 18:59 ` [PATCH 04/20] nvmet: initialize file handle and req fields Chaitanya Kulkarni
@ 2018-04-18 18:59 ` Chaitanya Kulkarni
  2018-04-24 17:29   ` Christoph Hellwig
  2018-04-18 18:59 ` [PATCH 06/20] nvmet: add new members for sync file I/O Chaitanya Kulkarni
                   ` (15 subsequent siblings)
  20 siblings, 1 reply; 29+ messages in thread
From: Chaitanya Kulkarni @ 2018-04-18 18:59 UTC (permalink / raw)


This patch implements the target side NVMe IO command
support for the file backed namespaces.

For NVMe read and write command we use asynchronous direct I/O.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
---
 drivers/nvme/target/io-cmd.c | 146 +++++++++++++++++++++++++++++++++++++++++--
 drivers/nvme/target/nvmet.h  |   1 +
 2 files changed, 143 insertions(+), 4 deletions(-)

diff --git a/drivers/nvme/target/io-cmd.c b/drivers/nvme/target/io-cmd.c
index 28bbdff4a88b..7791a63e38ab 100644
--- a/drivers/nvme/target/io-cmd.c
+++ b/drivers/nvme/target/io-cmd.c
@@ -14,8 +14,11 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 #include <linux/blkdev.h>
 #include <linux/module.h>
+#include <linux/uio.h>
 #include "nvmet.h"
 
+#define NR_BVEC(req)	(req->data_len / PAGE_SIZE)
+
 static void nvmet_bio_done(struct bio *bio)
 {
 	struct nvmet_req *req = bio->bi_private;
@@ -89,6 +92,69 @@ static void nvmet_execute_rw(struct nvmet_req *req)
 	blk_poll(bdev_get_queue(req->ns->bdev), cookie);
 }
 
+static void nvmet_file_io_complete(struct kiocb *iocb, long ret, long ret2)
+{
+	struct nvmet_req *req = container_of(iocb, struct nvmet_req, iocb);
+
+	kfree(req->bvec);
+	req->bvec = NULL;
+	nvmet_req_complete(req, ret != req->data_len ?
+			NVME_SC_INTERNAL | NVME_SC_DNR : 0);
+}
+
+static void nvmet_execute_rw_file(struct nvmet_req *req)
+{
+	struct iov_iter iter;
+	struct sg_mapping_iter miter;
+	loff_t pos;
+	ssize_t len = 0, ret;
+	int ki_flags = IOCB_DIRECT;
+	int bv_cnt = 0, rw = READ;
+
+	if (req->cmd->rw.opcode == nvme_cmd_write) {
+		rw = WRITE;
+		if (req->cmd->rw.control & cpu_to_le16(NVME_RW_FUA))
+			ki_flags |= IOCB_DSYNC;
+	}
+
+	req->bvec = kmalloc_array(NR_BVEC(req), sizeof(struct bio_vec),
+			GFP_KERNEL);
+	if (!req->bvec)
+		goto out;
+
+	sg_miter_start(&miter, req->sg, req->sg_cnt, SG_MITER_FROM_SG);
+	while (sg_miter_next(&miter)) {
+		req->bvec[bv_cnt].bv_page = miter.page;
+		req->bvec[bv_cnt].bv_offset = miter.__offset;
+		req->bvec[bv_cnt].bv_len = miter.length;
+		len += req->bvec[bv_cnt].bv_len;
+		bv_cnt++;
+	}
+	sg_miter_stop(&miter);
+	if (len != req->data_len)
+		goto free;
+
+	iov_iter_bvec(&iter, ITER_BVEC | rw, req->bvec, bv_cnt, len);
+	pos = le64_to_cpu(req->cmd->rw.slba) * (1 << (req->ns->blksize_shift));
+	req->iocb.ki_pos = pos;
+	req->iocb.ki_filp = req->ns->filp;
+	req->iocb.ki_flags = ki_flags;
+	req->iocb.ki_complete = nvmet_file_io_complete;
+
+	if (rw == WRITE)
+		ret = call_write_iter(req->ns->filp, &req->iocb, &iter);
+	else
+		ret = call_read_iter(req->ns->filp, &req->iocb, &iter);
+
+	if (ret != -EIOCBQUEUED)
+		nvmet_file_io_complete(&req->iocb, ret, 0);
+	return;
+free:
+	kfree(req->bvec);
+out:
+	nvmet_req_complete(req, NVME_SC_INTERNAL | NVME_SC_DNR);
+}
+
 static void nvmet_execute_flush(struct nvmet_req *req)
 {
 	struct bio *bio = &req->inline_bio;
@@ -102,6 +168,13 @@ static void nvmet_execute_flush(struct nvmet_req *req)
 	submit_bio(bio);
 }
 
+static void nvmet_execute_flush_file(struct nvmet_req *req)
+{
+	int ret = vfs_fsync(req->ns->filp, 1);
+
+	nvmet_req_complete(req, ret);
+}
+
 static u16 nvmet_discard_range(struct nvmet_ns *ns,
 		struct nvme_dsm_range *range, struct bio **bio)
 {
@@ -163,6 +236,43 @@ static void nvmet_execute_dsm(struct nvmet_req *req)
 	}
 }
 
+static void nvmet_execute_discard_file(struct nvmet_req *req)
+{
+	struct nvme_dsm_range range;
+	int mode = FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE;
+	loff_t offset;
+	loff_t len;
+	int i, ret;
+
+	for (i = 0; i <= le32_to_cpu(req->cmd->dsm.nr); i++) {
+		if (nvmet_copy_from_sgl(req, i * sizeof(range), &range,
+					sizeof(range)))
+			break;
+		offset = le64_to_cpu(range.slba) << req->ns->blksize_shift;
+		len = le32_to_cpu(range.nlb) << req->ns->blksize_shift;
+		ret = vfs_fallocate(req->ns->filp, mode, offset, len);
+		if (ret)
+			break;
+	}
+
+	nvmet_req_complete(req, ret < 0 ? NVME_SC_INTERNAL | NVME_SC_DNR : 0);
+}
+
+static void nvmet_execute_dsm_file(struct nvmet_req *req)
+{
+	switch (le32_to_cpu(req->cmd->dsm.attributes)) {
+	case NVME_DSMGMT_AD:
+		nvmet_execute_discard_file(req);
+		return;
+	case NVME_DSMGMT_IDR:
+	case NVME_DSMGMT_IDW:
+	default:
+		/* Not supported yet */
+		nvmet_req_complete(req, 0);
+		return;
+	}
+}
+
 static void nvmet_execute_write_zeroes(struct nvmet_req *req)
 {
 	struct nvme_write_zeroes_cmd *write_zeroes = &req->cmd->write_zeroes;
@@ -189,6 +299,22 @@ static void nvmet_execute_write_zeroes(struct nvmet_req *req)
 	}
 }
 
+static void nvmet_execute_write_zeroes_file(struct nvmet_req *req)
+{
+	struct nvme_write_zeroes_cmd *write_zeroes = &req->cmd->write_zeroes;
+	loff_t offset;
+	loff_t len;
+	int mode = FALLOC_FL_ZERO_RANGE | FALLOC_FL_KEEP_SIZE;
+	int ret;
+
+	offset = le64_to_cpu(write_zeroes->slba) << req->ns->blksize_shift;
+	len = (((sector_t)le32_to_cpu(write_zeroes->length) + 1) <<
+			req->ns->blksize_shift);
+
+	ret = vfs_fallocate(req->ns->filp, mode, offset, len);
+	nvmet_req_complete(req, ret < 0 ? NVME_SC_INTERNAL | NVME_SC_DNR : 0);
+}
+
 u16 nvmet_parse_io_cmd(struct nvmet_req *req)
 {
 	struct nvme_command *cmd = req->cmd;
@@ -207,20 +333,32 @@ u16 nvmet_parse_io_cmd(struct nvmet_req *req)
 	switch (cmd->common.opcode) {
 	case nvme_cmd_read:
 	case nvme_cmd_write:
-		req->execute = nvmet_execute_rw;
+		if (req->ns->filp)
+			req->execute = nvmet_execute_rw_file;
+		else
+			req->execute = nvmet_execute_rw;
 		req->data_len = nvmet_rw_len(req);
 		return 0;
 	case nvme_cmd_flush:
-		req->execute = nvmet_execute_flush;
+		if (req->ns->filp)
+			req->execute = nvmet_execute_flush_file;
+		else
+			req->execute = nvmet_execute_flush;
 		req->data_len = 0;
 		return 0;
 	case nvme_cmd_dsm:
-		req->execute = nvmet_execute_dsm;
+		if (req->ns->filp)
+			req->execute = nvmet_execute_dsm_file;
+		else
+			req->execute = nvmet_execute_dsm;
 		req->data_len = (le32_to_cpu(cmd->dsm.nr) + 1) *
 			sizeof(struct nvme_dsm_range);
 		return 0;
 	case nvme_cmd_write_zeroes:
-		req->execute = nvmet_execute_write_zeroes;
+		if (req->ns->filp)
+			req->execute = nvmet_execute_write_zeroes_file;
+		else
+			req->execute = nvmet_execute_write_zeroes;
 		return 0;
 	default:
 		pr_err("unhandled cmd %d on qid %d\n", cmd->common.opcode,
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index acc61ca97198..21825b4a222a 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -27,6 +27,7 @@
 #include <linux/rcupdate.h>
 #include <linux/blkdev.h>
 #include <linux/fs.h>
+#include <linux/falloc.h>
 
 #define NVMET_ASYNC_EVENTS		4
 #define NVMET_ERROR_LOG_SLOTS		128
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 06/20] nvmet: add new members for sync file I/O
  2018-04-18 18:59 [PATCH 00/19] nvmet: add support for file backed namespaces Chaitanya Kulkarni
                   ` (4 preceding siblings ...)
  2018-04-18 18:59 ` [PATCH 05/20] nvmet: add NVMe I/O command handlers for file Chaitanya Kulkarni
@ 2018-04-18 18:59 ` Chaitanya Kulkarni
  2018-04-24 17:30   ` Christoph Hellwig
  2018-04-18 18:59 ` [PATCH 07/20] nvmet: add sync I/O configfs attr for ns Chaitanya Kulkarni
                   ` (14 subsequent siblings)
  20 siblings, 1 reply; 29+ messages in thread
From: Chaitanya Kulkarni @ 2018-04-18 18:59 UTC (permalink / raw)


This patch adds new structure members to implement O_SYNC
IO mode for file-backed ns. The user can optionally choose
to switch between O_SYNC and O_DIRECT with the configfs parameter
which we add in the next patch.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
---
 drivers/nvme/target/nvmet.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index 21825b4a222a..96aa5fc72467 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -53,9 +53,11 @@ struct nvmet_ns {
 	uuid_t			uuid;
 
 	bool			enabled;
+	bool			sync;
 	struct nvmet_subsys	*subsys;
 	const char		*device_path;
 
+	struct workqueue_struct *file_wq;
 	struct config_group	device_group;
 	struct config_group	group;
 
@@ -233,6 +235,7 @@ struct nvmet_req {
 	/* data length as parsed from the SGL descriptor: */
 	size_t			transfer_len;
 
+	struct work_struct	sync_work; /* file backed ns */
 	struct nvmet_port	*port;
 
 	void (*execute)(struct nvmet_req *req);
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 07/20] nvmet: add sync I/O configfs attr for ns
  2018-04-18 18:59 [PATCH 00/19] nvmet: add support for file backed namespaces Chaitanya Kulkarni
                   ` (5 preceding siblings ...)
  2018-04-18 18:59 ` [PATCH 06/20] nvmet: add new members for sync file I/O Chaitanya Kulkarni
@ 2018-04-18 18:59 ` Chaitanya Kulkarni
  2018-04-18 18:59 ` [PATCH 08/20] nvmet: configure file backed ns for sync IO Chaitanya Kulkarni
                   ` (13 subsequent siblings)
  20 siblings, 0 replies; 29+ messages in thread
From: Chaitanya Kulkarni @ 2018-04-18 18:59 UTC (permalink / raw)


This patch adds a configfs attribute to the ns to optionally
switch the mode between O_SYNC and O_DIRECT.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
---
 drivers/nvme/target/configfs.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
index e6b2d2af81b6..bc95be1276c6 100644
--- a/drivers/nvme/target/configfs.c
+++ b/drivers/nvme/target/configfs.c
@@ -406,11 +406,38 @@ static ssize_t nvmet_ns_enable_store(struct config_item *item,
 
 CONFIGFS_ATTR(nvmet_ns_, enable);
 
+static ssize_t nvmet_ns_sync_show(struct config_item *item, char *page)
+{
+	return sprintf(page, "%d\n", to_nvmet_ns(item)->sync);
+}
+
+static ssize_t nvmet_ns_sync_store(struct config_item *item,
+		const char *page, size_t count)
+{
+	struct nvmet_ns *ns = to_nvmet_ns(item);
+	bool sync;
+	int ret = 0;
+
+	if (strtobool(page, &sync))
+		return -EINVAL;
+
+	if (ns->filp && ns->sync != sync) {
+		nvmet_ns_disable(ns);
+		ns->sync = sync;
+		ret = nvmet_ns_enable(ns);
+	}
+
+	return ret ? ret : count;
+}
+
+CONFIGFS_ATTR(nvmet_ns_, sync);
+
 static struct configfs_attribute *nvmet_ns_attrs[] = {
 	&nvmet_ns_attr_device_path,
 	&nvmet_ns_attr_device_nguid,
 	&nvmet_ns_attr_device_uuid,
 	&nvmet_ns_attr_enable,
+	&nvmet_ns_attr_sync,
 	NULL,
 };
 
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 08/20] nvmet: configure file backed ns for sync IO
  2018-04-18 18:59 [PATCH 00/19] nvmet: add support for file backed namespaces Chaitanya Kulkarni
                   ` (6 preceding siblings ...)
  2018-04-18 18:59 ` [PATCH 07/20] nvmet: add sync I/O configfs attr for ns Chaitanya Kulkarni
@ 2018-04-18 18:59 ` Chaitanya Kulkarni
  2018-04-24 17:41   ` Christoph Hellwig
  2018-04-18 19:00 ` [PATCH 09/20] nvmet: use workqueue for sync I/O Chaitanya Kulkarni
                   ` (12 subsequent siblings)
  20 siblings, 1 reply; 29+ messages in thread
From: Chaitanya Kulkarni @ 2018-04-18 18:59 UTC (permalink / raw)


This patch allows file backed namespaces to use newly
introduced configfs attribute sync, when sync is set
we open the file with O_SYNC and use the work queue
to execute the NVMe read/write commands.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
---
 drivers/nvme/target/core.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index b7c51a5629bf..4d25038ed05c 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -305,9 +305,10 @@ int nvmet_vfs_stat(const char *filename, struct kstat *stat)
 static int nvmet_ns_enable_file(struct nvmet_ns *ns)
 {
 	int ret;
-	int flags = O_RDWR | O_LARGEFILE | O_DIRECT;
+	int flags = O_RDWR | O_LARGEFILE;
 	struct kstat stat;
 
+	flags |= ns->sync ? O_SYNC : O_DIRECT;
 	ns->filp = filp_open(ns->device_path, flags, 0);
 	if (!ns->filp || IS_ERR(ns->filp)) {
 		pr_err("failed to open file %s: (%ld)\n",
@@ -323,6 +324,15 @@ static int nvmet_ns_enable_file(struct nvmet_ns *ns)
 	ns->size = stat.size;
 	ns->blksize_shift = ns->filp->f_inode->i_blkbits;
 
+	if (ns->sync) {
+		ns->file_wq = alloc_workqueue("nvmet-file",
+				WQ_UNBOUND_MAX_ACTIVE | WQ_MEM_RECLAIM, 0);
+
+		if (!ns->file_wq) {
+			ret = -1;
+			goto err;
+		}
+	}
 	return ret;
 err:
 	filp_close(ns->filp, NULL);
@@ -352,6 +362,11 @@ static void nvmet_ns_dev_disable(struct nvmet_ns *ns)
 		blkdev_put(ns->bdev, FMODE_WRITE | FMODE_READ);
 		ns->bdev = NULL;
 	} else if (ns->filp) {
+		if (ns->sync) {
+			flush_workqueue(ns->file_wq);
+			destroy_workqueue(ns->file_wq);
+			ns->file_wq = NULL;
+		}
 		filp_close(ns->filp, NULL);
 		ns->filp = NULL;
 	} else
@@ -470,6 +485,7 @@ struct nvmet_ns *nvmet_ns_alloc(struct nvmet_subsys *subsys, u32 nsid)
 	ns->nsid = nsid;
 	ns->subsys = subsys;
 	uuid_gen(&ns->uuid);
+	ns->sync = false;
 
 	return ns;
 }
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 09/20] nvmet: use workqueue for sync I/O
  2018-04-18 18:59 [PATCH 00/19] nvmet: add support for file backed namespaces Chaitanya Kulkarni
                   ` (7 preceding siblings ...)
  2018-04-18 18:59 ` [PATCH 08/20] nvmet: configure file backed ns for sync IO Chaitanya Kulkarni
@ 2018-04-18 19:00 ` Chaitanya Kulkarni
  2018-04-18 19:00 ` [PATCH 10/20] nvmet: isolate id ctrl/ns field initialization Chaitanya Kulkarni
                   ` (11 subsequent siblings)
  20 siblings, 0 replies; 29+ messages in thread
From: Chaitanya Kulkarni @ 2018-04-18 19:00 UTC (permalink / raw)


This patch allows NVMe read/write command to use sync or direct
I/O flags as it is configured from the configfs.

Here when ns->sync is set we use workqueue to execute
synchronous I/O operation in different thread context than
the submission thread.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
---
 drivers/nvme/target/io-cmd.c | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/drivers/nvme/target/io-cmd.c b/drivers/nvme/target/io-cmd.c
index 7791a63e38ab..6d355b2dac56 100644
--- a/drivers/nvme/target/io-cmd.c
+++ b/drivers/nvme/target/io-cmd.c
@@ -108,7 +108,7 @@ static void nvmet_execute_rw_file(struct nvmet_req *req)
 	struct sg_mapping_iter miter;
 	loff_t pos;
 	ssize_t len = 0, ret;
-	int ki_flags = IOCB_DIRECT;
+	int ki_flags = req->ns->sync ? IOCB_SYNC : IOCB_DIRECT;
 	int bv_cnt = 0, rw = READ;
 
 	if (req->cmd->rw.opcode == nvme_cmd_write) {
@@ -155,6 +155,20 @@ static void nvmet_execute_rw_file(struct nvmet_req *req)
 	nvmet_req_complete(req, NVME_SC_INTERNAL | NVME_SC_DNR);
 }
 
+static void nvmet_sync_work_file(struct work_struct *work)
+{
+	struct nvmet_req *req;
+
+	req = container_of(work, struct nvmet_req, sync_work);
+
+	nvmet_execute_rw_file(req);
+}
+
+static void nvmet_execute_rw_sync_file(struct nvmet_req *req)
+{
+	queue_work(req->ns->file_wq, &req->sync_work);
+}
+
 static void nvmet_execute_flush(struct nvmet_req *req)
 {
 	struct bio *bio = &req->inline_bio;
@@ -333,9 +347,14 @@ u16 nvmet_parse_io_cmd(struct nvmet_req *req)
 	switch (cmd->common.opcode) {
 	case nvme_cmd_read:
 	case nvme_cmd_write:
-		if (req->ns->filp)
-			req->execute = nvmet_execute_rw_file;
-		else
+		if (req->ns->filp) {
+			if (req->ns->sync) {
+				req->execute = nvmet_execute_rw_sync_file;
+				INIT_WORK(&req->sync_work,
+						nvmet_sync_work_file);
+			} else
+				req->execute = nvmet_execute_rw_file;
+		} else
 			req->execute = nvmet_execute_rw;
 		req->data_len = nvmet_rw_len(req);
 		return 0;
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 10/20] nvmet: isolate id ctrl/ns field initialization
  2018-04-18 18:59 [PATCH 00/19] nvmet: add support for file backed namespaces Chaitanya Kulkarni
                   ` (8 preceding siblings ...)
  2018-04-18 19:00 ` [PATCH 09/20] nvmet: use workqueue for sync I/O Chaitanya Kulkarni
@ 2018-04-18 19:00 ` Chaitanya Kulkarni
  2018-04-18 19:00 ` [PATCH 11/20] nvmet: introduce new members for ns-mgmt Chaitanya Kulkarni
                   ` (10 subsequent siblings)
  20 siblings, 0 replies; 29+ messages in thread
From: Chaitanya Kulkarni @ 2018-04-18 19:00 UTC (permalink / raw)


This is a preparation patch to implement the ns-mgmt commands
for file-backed namespaces.

We isolate the identify-ctrl and identify-ns default fields
initialization code into helper functions.

File-backed namespaces can use this code and override
the necessary members of the respective data structures.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
---
 drivers/nvme/target/admin-cmd.c | 64 ++++++++++++++++++++++++-----------------
 1 file changed, 38 insertions(+), 26 deletions(-)

diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index 602995f7a1b3..97fea3f1d3e4 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -169,19 +169,11 @@ static void nvmet_execute_get_log_page(struct nvmet_req *req)
 	nvmet_req_complete(req, status);
 }
 
-static void nvmet_execute_identify_ctrl(struct nvmet_req *req)
+static void nvmet_identify_ctrl_init(struct nvmet_ctrl *ctrl,
+		struct nvme_id_ctrl *id)
 {
-	struct nvmet_ctrl *ctrl = req->sq->ctrl;
-	struct nvme_id_ctrl *id;
-	u16 status = 0;
 	const char model[] = "Linux";
 
-	id = kzalloc(sizeof(*id), GFP_KERNEL);
-	if (!id) {
-		status = NVME_SC_INTERNAL;
-		goto out;
-	}
-
 	/* XXX: figure out how to assign real vendors IDs. */
 	id->vid = 0;
 	id->ssvid = 0;
@@ -274,32 +266,31 @@ static void nvmet_execute_identify_ctrl(struct nvmet_req *req)
 	id->psd[0].max_power = cpu_to_le16(0x9c4);
 	id->psd[0].entry_lat = cpu_to_le32(0x10);
 	id->psd[0].exit_lat = cpu_to_le32(0x4);
-
-	status = nvmet_copy_to_sgl(req, 0, id, sizeof(*id));
-
-	kfree(id);
-out:
-	nvmet_req_complete(req, status);
 }
 
-static void nvmet_execute_identify_ns(struct nvmet_req *req)
+static void nvmet_execute_identify_ctrl(struct nvmet_req *req)
 {
-	struct nvmet_ns *ns;
-	struct nvme_id_ns *id;
+	struct nvmet_ctrl *ctrl = req->sq->ctrl;
+	struct nvme_id_ctrl *id;
 	u16 status = 0;
 
-	ns = nvmet_find_namespace(req->sq->ctrl, req->cmd->identify.nsid);
-	if (!ns) {
-		status = NVME_SC_INVALID_NS | NVME_SC_DNR;
-		goto out;
-	}
-
 	id = kzalloc(sizeof(*id), GFP_KERNEL);
 	if (!id) {
 		status = NVME_SC_INTERNAL;
-		goto out_put_ns;
+		goto out;
 	}
 
+	nvmet_identify_ctrl_init(ctrl, id);
+
+	status = nvmet_copy_to_sgl(req, 0, id, sizeof(*id));
+
+	kfree(id);
+out:
+	nvmet_req_complete(req, status);
+}
+
+static void nvmet_identify_ns_init(struct nvmet_ns *ns, struct nvme_id_ns *id)
+{
 	/*
 	 * nuse = ncap = nsze isn't always true, but we have no way to find
 	 * that out from the underlying device.
@@ -323,6 +314,27 @@ static void nvmet_execute_identify_ns(struct nvmet_req *req)
 	memcpy(&id->nguid, &ns->nguid, sizeof(uuid_le));
 
 	id->lbaf[0].ds = ns->blksize_shift;
+}
+
+static void nvmet_execute_identify_ns(struct nvmet_req *req)
+{
+	struct nvmet_ns *ns;
+	struct nvme_id_ns *id;
+	u16 status = 0;
+
+	ns = nvmet_find_namespace(req->sq->ctrl, req->cmd->identify.nsid);
+	if (!ns) {
+		status = NVME_SC_INVALID_NS | NVME_SC_DNR;
+		goto out;
+	}
+
+	id = kzalloc(sizeof(*id), GFP_KERNEL);
+	if (!id) {
+		status = NVME_SC_INTERNAL;
+		goto out_put_ns;
+	}
+
+	nvmet_identify_ns_init(ns, id);
 
 	status = nvmet_copy_to_sgl(req, 0, id, sizeof(*id));
 
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 11/20] nvmet: introduce new members for ns-mgmt
  2018-04-18 18:59 [PATCH 00/19] nvmet: add support for file backed namespaces Chaitanya Kulkarni
                   ` (9 preceding siblings ...)
  2018-04-18 19:00 ` [PATCH 10/20] nvmet: isolate id ctrl/ns field initialization Chaitanya Kulkarni
@ 2018-04-18 19:00 ` Chaitanya Kulkarni
  2018-04-18 19:00 ` [PATCH 12/20] nvmet: add a configfs entry for subsys mount path Chaitanya Kulkarni
                   ` (9 subsequent siblings)
  20 siblings, 0 replies; 29+ messages in thread
From: Chaitanya Kulkarni @ 2018-04-18 19:00 UTC (permalink / raw)


We update the target data structures to support ns-mgmt
commands. The new members for nvmet_ns are used to in the
create-ns command implementation. We use mount_path
of the nvmet_subsys to allow the user to initialize the
directory under which all the files for the respective
namespace(s) are created.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
---
 drivers/nvme/target/nvmet.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index 96aa5fc72467..b185a2add99b 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -57,6 +57,10 @@ struct nvmet_ns {
 	struct nvmet_subsys	*subsys;
 	const char		*device_path;
 
+	u64			nsze;
+	u64			ncap;
+	u64			nuse;
+	u8			flbas;
 	struct workqueue_struct *file_wq;
 	struct config_group	device_group;
 	struct config_group	group;
@@ -165,6 +169,8 @@ struct nvmet_subsys {
 
 	struct config_group	namespaces_group;
 	struct config_group	allowed_hosts_group;
+	char			*mount_path;
+	unsigned long		nsid_map[1024]; /* MAX LIMIT ? */
 };
 
 static inline struct nvmet_subsys *to_subsys(struct config_item *item)
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 12/20] nvmet: add a configfs entry for subsys mount path
  2018-04-18 18:59 [PATCH 00/19] nvmet: add support for file backed namespaces Chaitanya Kulkarni
                   ` (10 preceding siblings ...)
  2018-04-18 19:00 ` [PATCH 11/20] nvmet: introduce new members for ns-mgmt Chaitanya Kulkarni
@ 2018-04-18 19:00 ` Chaitanya Kulkarni
  2018-04-18 19:00 ` [PATCH 13/20] nvmet: initialize new ns and subsys members Chaitanya Kulkarni
                   ` (8 subsequent siblings)
  20 siblings, 0 replies; 29+ messages in thread
From: Chaitanya Kulkarni @ 2018-04-18 19:00 UTC (permalink / raw)


This patch adds new directory "fs" parallel to subsystems.
This new directory contains an additional attribute to
initialize the mount_path for the file backed subsystems.
This is actually a subsystem and uses all the existing
attributes from the subsystem.

The main difference is for file-backed namespaces we don't
allow the user to create namespaces on the target side, the user can
only create/delete namespaces from the host side with nvme
create-ns/delete-ns commands. So in the configfs, we don't provide
the namespaces directory with newly added "fs" directory.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
---
 drivers/nvme/target/configfs.c | 105 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 104 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
index bc95be1276c6..f571dffbecdc 100644
--- a/drivers/nvme/target/configfs.c
+++ b/drivers/nvme/target/configfs.c
@@ -21,6 +21,7 @@
 #include "nvmet.h"
 
 static const struct config_item_type nvmet_host_type;
+static const struct config_item_type nvmet_fs_type;
 static const struct config_item_type nvmet_subsys_type;
 
 /*
@@ -504,7 +505,8 @@ static int nvmet_port_subsys_allow_link(struct config_item *parent,
 	struct nvmet_subsys_link *link, *p;
 	int ret;
 
-	if (target->ci_type != &nvmet_subsys_type) {
+	if (!(target->ci_type == &nvmet_subsys_type ||
+				target->ci_type == &nvmet_fs_type)) {
 		pr_err("can only link subsystems into the subsystems dir.!\n");
 		return -EINVAL;
 	}
@@ -799,6 +801,101 @@ static const struct config_item_type nvmet_subsystems_type = {
 	.ct_owner		= THIS_MODULE,
 };
 
+
+/*
+ * File backed subsys attributes and operations.
+ */
+static ssize_t nvmet_fs_attr_mount_path_show(struct config_item *item,
+			char *page)
+{
+	struct nvmet_subsys *subsys = to_subsys(item);
+
+	return snprintf(page, PAGE_SIZE, "%s\n", subsys->mount_path);
+}
+
+static ssize_t nvmet_fs_attr_mount_path_store(struct config_item *item,
+		const char *page, size_t count)
+{
+	struct nvmet_subsys *subsys = to_subsys(item);
+	int ret = count;
+
+	mutex_lock(&subsys->lock);
+	kfree(subsys->mount_path);
+
+	subsys->mount_path = kstrdup(page, GFP_KERNEL);
+	if (!subsys->mount_path)
+		ret = -ENOMEM;
+
+	mutex_unlock(&subsys->lock);
+
+	return ret;
+}
+
+CONFIGFS_ATTR(nvmet_fs_, attr_mount_path);
+
+static struct configfs_attribute *nvmet_fs_attrs[] = {
+	&nvmet_fs_attr_attr_mount_path,
+	&nvmet_subsys_attr_attr_allow_any_host,
+	&nvmet_subsys_attr_attr_version,
+	&nvmet_subsys_attr_attr_serial,
+	NULL,
+};
+
+/*
+ * File backed subsys structures & folder operation functions below.
+ */
+static void nvmet_fs_release(struct config_item *item)
+{
+	struct nvmet_subsys *subsys = to_subsys(item);
+
+	nvmet_subsys_del_ctrls(subsys);
+
+	nvmet_subsys_put(subsys);
+}
+
+static struct configfs_item_operations nvmet_fs_item_ops = {
+	.release		= nvmet_fs_release,
+};
+
+static const struct config_item_type nvmet_fs_type = {
+	.ct_item_ops		= &nvmet_fs_item_ops,
+	.ct_attrs		= nvmet_fs_attrs,
+	.ct_owner		= THIS_MODULE,
+};
+
+static struct config_group *nvmet_fs_make(struct config_group *group,
+		const char *name)
+{
+	struct nvmet_subsys *subsys;
+
+	if (sysfs_streq(name, NVME_DISC_SUBSYS_NAME)) {
+		pr_err("can't create discovery subsystem through configfs\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	subsys = nvmet_subsys_alloc(name, NVME_NQN_NVME);
+	if (!subsys)
+		return ERR_PTR(-ENOMEM);
+
+	config_group_init_type_name(&subsys->group, name, &nvmet_fs_type);
+
+	config_group_init_type_name(&subsys->allowed_hosts_group,
+			"allowed_hosts", &nvmet_allowed_hosts_type);
+	configfs_add_default_group(&subsys->allowed_hosts_group,
+			&subsys->group);
+
+	return &subsys->group;
+}
+
+static struct configfs_group_operations nvmet_fss_group_ops = {
+	.make_group		= nvmet_fs_make,
+};
+
+static struct config_item_type nvmet_fss_type = {
+	.ct_group_ops		= &nvmet_fss_group_ops,
+	.ct_owner		= THIS_MODULE,
+};
+
 static ssize_t nvmet_referral_enable_show(struct config_item *item,
 		char *page)
 {
@@ -954,6 +1051,7 @@ static const struct config_item_type nvmet_ports_type = {
 };
 
 static struct config_group nvmet_subsystems_group;
+static struct config_group nvmet_fss_group;
 static struct config_group nvmet_ports_group;
 
 static void nvmet_host_release(struct config_item *item)
@@ -1017,6 +1115,11 @@ int __init nvmet_init_configfs(void)
 	config_group_init(&nvmet_configfs_subsystem.su_group);
 	mutex_init(&nvmet_configfs_subsystem.su_mutex);
 
+	config_group_init_type_name(&nvmet_fss_group,
+			"fs", &nvmet_fss_type);
+	configfs_add_default_group(&nvmet_fss_group,
+			&nvmet_configfs_subsystem.su_group);
+
 	config_group_init_type_name(&nvmet_subsystems_group,
 			"subsystems", &nvmet_subsystems_type);
 	configfs_add_default_group(&nvmet_subsystems_group,
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 13/20] nvmet: initialize new ns and subsys members
  2018-04-18 18:59 [PATCH 00/19] nvmet: add support for file backed namespaces Chaitanya Kulkarni
                   ` (11 preceding siblings ...)
  2018-04-18 19:00 ` [PATCH 12/20] nvmet: add a configfs entry for subsys mount path Chaitanya Kulkarni
@ 2018-04-18 19:00 ` Chaitanya Kulkarni
  2018-04-18 19:00 ` [PATCH 14/20] nvmet: add and restructure ns mgmt helpers Chaitanya Kulkarni
                   ` (7 subsequent siblings)
  20 siblings, 0 replies; 29+ messages in thread
From: Chaitanya Kulkarni @ 2018-04-18 19:00 UTC (permalink / raw)


We initialize newly introduce subsys and ns members.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
---
 drivers/nvme/target/core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index 4d25038ed05c..34a2b8903ec0 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -1091,6 +1091,8 @@ struct nvmet_subsys *nvmet_subsys_alloc(const char *subsysnqn,
 	INIT_LIST_HEAD(&subsys->namespaces);
 	INIT_LIST_HEAD(&subsys->ctrls);
 	INIT_LIST_HEAD(&subsys->hosts);
+	subsys->mount_path = NULL;
+	subsys->nsid_map[0] = 0;
 
 	return subsys;
 }
@@ -1103,6 +1105,7 @@ static void nvmet_subsys_free(struct kref *ref)
 	WARN_ON_ONCE(!list_empty(&subsys->namespaces));
 
 	kfree(subsys->subsysnqn);
+	kfree(subsys->mount_path);
 	kfree(subsys);
 }
 
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 14/20] nvmet: add and restructure ns mgmt helpers
  2018-04-18 18:59 [PATCH 00/19] nvmet: add support for file backed namespaces Chaitanya Kulkarni
                   ` (12 preceding siblings ...)
  2018-04-18 19:00 ` [PATCH 13/20] nvmet: initialize new ns and subsys members Chaitanya Kulkarni
@ 2018-04-18 19:00 ` Chaitanya Kulkarni
  2018-04-18 19:00 ` [PATCH 15/20] fs: export kern_path_locked() to reuse the code Chaitanya Kulkarni
                   ` (6 subsequent siblings)
  20 siblings, 0 replies; 29+ messages in thread
From: Chaitanya Kulkarni @ 2018-04-18 19:00 UTC (permalink / raw)


We add new helper functions for the namespace management
and move the existing helper functions in admin-cmd.c.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
---
 drivers/nvme/target/admin-cmd.c | 51 +++++++++++++++++++++++++++++++++++++++++
 drivers/nvme/target/core.c      | 13 -----------
 drivers/nvme/target/nvmet.h     |  1 +
 3 files changed, 52 insertions(+), 13 deletions(-)

diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index 97fea3f1d3e4..e696f1e45e3a 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -14,11 +14,62 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 #include <linux/module.h>
 #include <linux/rculist.h>
+#include <linux/namei.h>
+#include <linux/fs.h>
+#include <linux/statfs.h>
 
 #include <generated/utsrelease.h>
 #include <asm/unaligned.h>
 #include "nvmet.h"
 
+static u32 nvmet_get_next_nsid(struct nvmet_subsys *subsys)
+{
+	u32 nsid;
+
+	mutex_lock(&subsys->lock);
+	nsid = find_first_zero_bit(subsys->nsid_map,
+			sizeof(subsys->nsid_map) * 8);
+	set_bit(nsid, subsys->nsid_map);
+
+	mutex_unlock(&subsys->lock);
+
+	return nsid + 1;
+}
+
+static void nvmet_free_nsid(struct nvmet_subsys *subsys, int nsid)
+{
+	nsid -= 1;
+	mutex_lock(&subsys->lock);
+	clear_bit(nsid, subsys->nsid_map);
+	mutex_unlock(&subsys->lock);
+}
+
+int nvmet_vfs_stat(const char *filename, struct kstat *stat)
+{
+	int ret;
+	mm_segment_t old_fs = get_fs();
+
+	set_fs(KERNEL_DS);
+	ret = vfs_stat(filename, stat);
+	if (ret)
+		pr_err("vfs_stat failed %s: %d\n", filename, ret);
+	set_fs(old_fs);
+	return ret;
+}
+
+static int nvmet_vfs_statfs(const char *pathname, struct kstatfs *st)
+{
+	int ret;
+	struct path path;
+
+	kern_path(pathname, LOOKUP_DIRECTORY, &path);
+	ret = vfs_statfs(&path, st);
+	if (ret)
+		pr_err("vfs_statfs failed %d\n", ret);
+	path_put(&path);
+	return ret;
+}
+
 u32 nvmet_get_log_page_len(struct nvme_command *cmd)
 {
 	u32 len = le16_to_cpu(cmd->get_log_page.numdu);
diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index 34a2b8903ec0..0b0649dc705c 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -289,19 +289,6 @@ static int nvmet_ns_enable_blk(struct nvmet_ns *ns)
 	return 0;
 }
 
-int nvmet_vfs_stat(const char *filename, struct kstat *stat)
-{
-	int ret;
-	mm_segment_t old_fs = get_fs();
-
-	set_fs(KERNEL_DS);
-	ret = vfs_stat(filename, stat);
-	if (ret)
-		pr_err("vfs_stat failed %s: %d\n", filename, ret);
-	set_fs(old_fs);
-	return ret;
-}
-
 static int nvmet_ns_enable_file(struct nvmet_ns *ns)
 {
 	int ret;
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index b185a2add99b..a365dc44a10e 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -350,4 +350,5 @@ extern struct rw_semaphore nvmet_config_sem;
 bool nvmet_host_allowed(struct nvmet_req *req, struct nvmet_subsys *subsys,
 		const char *hostnqn);
 
+int nvmet_vfs_stat(const char *filename, struct kstat *stat);
 #endif /* _NVMET_H */
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 15/20] fs: export kern_path_locked() to reuse the code
  2018-04-18 18:59 [PATCH 00/19] nvmet: add support for file backed namespaces Chaitanya Kulkarni
                   ` (13 preceding siblings ...)
  2018-04-18 19:00 ` [PATCH 14/20] nvmet: add and restructure ns mgmt helpers Chaitanya Kulkarni
@ 2018-04-18 19:00 ` Chaitanya Kulkarni
  2018-04-18 19:00 ` [PATCH 16/20] nvmet: add ns-mgmt command handlers Chaitanya Kulkarni
                   ` (5 subsequent siblings)
  20 siblings, 0 replies; 29+ messages in thread
From: Chaitanya Kulkarni @ 2018-04-18 19:00 UTC (permalink / raw)


We export kern_path_locked to reuse the code.
We use this API when deleting the file when host
calls nvme-delete-ns command.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
---
 fs/namei.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/namei.c b/fs/namei.c
index cafa365eeb70..9a57e33175c4 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2398,6 +2398,7 @@ struct dentry *kern_path_locked(const char *name, struct path *path)
 	putname(filename);
 	return d;
 }
+EXPORT_SYMBOL(kern_path_locked);
 
 int kern_path(const char *name, unsigned int flags, struct path *path)
 {
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 16/20] nvmet: add ns-mgmt command handlers
  2018-04-18 18:59 [PATCH 00/19] nvmet: add support for file backed namespaces Chaitanya Kulkarni
                   ` (14 preceding siblings ...)
  2018-04-18 19:00 ` [PATCH 15/20] fs: export kern_path_locked() to reuse the code Chaitanya Kulkarni
@ 2018-04-18 19:00 ` Chaitanya Kulkarni
  2018-04-18 19:00 ` [PATCH 17/20] nvmet: override identify for file backed ns Chaitanya Kulkarni
                   ` (4 subsequent siblings)
  20 siblings, 0 replies; 29+ messages in thread
From: Chaitanya Kulkarni @ 2018-04-18 19:00 UTC (permalink / raw)


Add ns-mgmt command handlers for file-backed ns. We use
previously initialize target subsystems "mount_path"
to manage the file backed namespaces.

The host can issue nvme-create ns commands to create a file
backed namespace on the target side under mount_path.
e.g.

nvme create-ns /dev/nvme1 --nsze=204800 --ncap=204800 --flbas=9

The above command will create and initialize the namespace
on the target side. It will use --flbas value as a power of 2
exponent to determine the block size. The total size of the file
will be calculated as a combination of the flbas and nsze value.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
---
 drivers/nvme/target/admin-cmd.c | 169 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 169 insertions(+)

diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index e696f1e45e3a..b60477c50880 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -613,6 +613,166 @@ static void nvmet_execute_keep_alive(struct nvmet_req *req)
 	nvmet_req_complete(req, 0);
 }
 
+static int nvmet_create_ns_path_file(struct nvmet_req *req)
+{
+	struct nvmet_ns *ns = req->ns;
+	int ret = 0;
+	char *mount_point = req->sq->ctrl->subsys->mount_path;
+	int cntlid = req->sq->ctrl->cntlid;
+	char *ns_path;
+	int flags = O_RDWR | O_LARGEFILE | O_DIRECT | O_CREAT;
+	int alloc = FALLOC_FL_ZERO_RANGE;
+	int holes = FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE;
+	loff_t start;
+
+	ns_path = kzalloc(PATH_MAX, GFP_KERNEL);
+	if (!ns_path) {
+		ns->device_path = NULL;
+		return -ENOMEM;
+	}
+
+	snprintf(ns_path, PATH_MAX, "%s/nvme%dn%d", mount_point,
+			cntlid, ns->nsid);
+	ns->device_path = kstrdup(ns_path, GFP_KERNEL);
+
+	ns->filp = filp_open(ns->device_path, flags, 0);
+	if (!ns->filp || IS_ERR(ns->filp)) {
+		pr_err("failed to open file %s: (%ld)\n",
+				ns->device_path, PTR_ERR(ns->filp));
+		ns->filp = NULL;
+		ret = -1;
+		goto out;
+	}
+
+	ns->size = ns->nsze * (1 << ns->flbas);
+	ret = vfs_fallocate(ns->filp, alloc, 0, ns->size);
+	if (ret) {
+		pr_err("failed to allocate ns %s: size %llu (%d)\n",
+				ns->device_path, ns->size, ret);
+		goto close_file;
+	}
+
+	start = ns->ncap * (1 << ns->flbas);
+	ret = vfs_fallocate(ns->filp, holes, start, ns->size);
+	if (ret) {
+		pr_err("failed to punch holes ns %s: size %llu (%d)\n",
+				ns->device_path, ns->size, ret);
+		goto close_file;
+	}
+	/* everything is okay we can close the file now */
+close_file:
+	filp_close(req->ns->filp, NULL);
+out:
+	kfree(ns_path);
+	return ret;
+}
+
+static u16 nvmet_delete_ns_file_path(struct nvmet_req *req)
+{
+	struct nvmet_ns *ns = req->ns;
+	int ret;
+	u16 status = NVME_SC_SUCCESS;
+	struct dentry *ns_dentry;
+	struct path parent_path;
+
+	ns_dentry = kern_path_locked(ns->device_path, &parent_path);
+	if (IS_ERR(ns_dentry)) {
+		pr_err("failed to get dentry %ld\n",
+				PTR_ERR(ns_dentry));
+
+		status = NVME_SC_INVALID_NS | NVME_SC_DNR;
+		goto err;
+	}
+	ret = vfs_unlink(d_inode(parent_path.dentry),
+			ns_dentry, NULL);
+	if (ret) {
+		pr_err("vfs_unlink() failed %d\n", ret);
+		status = NVME_SC_INVALID_NS | NVME_SC_DNR;
+	}
+	dput(ns_dentry);
+err:
+	inode_unlock(d_inode(parent_path.dentry));
+	path_put(&parent_path);
+
+	return status;
+}
+
+static u16 nvmet_create_ns_file(struct nvmet_req *req)
+{
+	struct nvmet_subsys *subsys = req->sq->ctrl->subsys;
+	struct nvme_id_ns *id;
+
+	id = kmalloc(sizeof(struct nvme_id_ns), GFP_KERNEL);
+	if (!id)
+		goto out;
+
+	nvmet_copy_from_sgl(req, 0, id, sizeof(*id));
+	req->ns = nvmet_ns_alloc(subsys, nvmet_get_next_nsid(subsys));
+	if (!req->ns)
+		goto err;
+
+	req->ns->nsze = le64_to_cpu(id->nsze);
+	req->ns->ncap = le64_to_cpu(id->ncap);
+	req->ns->nuse = le64_to_cpu(id->nuse);
+	req->ns->flbas = id->flbas == 0 ? 9 : le64_to_cpu(id->flbas);
+
+	if (nvmet_create_ns_path_file(req))
+		goto ns_free;
+
+	if (nvmet_ns_enable(req->ns)) {
+		nvmet_delete_ns_file_path(req);
+		goto ns_free;
+	}
+
+	nvmet_set_result(req, req->ns->nsid);
+	return NVME_SC_SUCCESS;
+
+ns_free:
+	kfree(req->ns->device_path);
+	req->ns->device_path = NULL;
+	kfree(req->ns);
+	req->ns = NULL;
+err:
+	kfree(id);
+out:
+	return NVME_SC_INTERNAL | NVME_SC_DNR;
+}
+
+static u16 nvmet_delete_ns_file(struct nvmet_req *req)
+{
+	u16 status = NVME_SC_SUCCESS;
+
+	req->ns = nvmet_find_namespace(req->sq->ctrl, req->cmd->identify.nsid);
+	if (unlikely(!req->ns)) {
+		status = NVME_SC_INVALID_NS | NVME_SC_DNR;
+		goto out;
+	}
+
+	nvmet_free_nsid(req->sq->ctrl->subsys, req->ns->nsid);
+
+	nvmet_ns_disable(req->ns);
+
+	status = nvmet_delete_ns_file_path(req);
+	kfree(req->ns->device_path);
+	req->ns->device_path = NULL;
+	kfree(req->ns);
+	req->ns = NULL;
+out:
+	return status;
+}
+
+static void nvmet_execute_ns_mgmt_file(struct nvmet_req *req)
+{
+	u16 status;
+
+	if (req->cmd->common.cdw10[0] == 0)
+		status = nvmet_create_ns_file(req);
+	else
+		status = nvmet_delete_ns_file(req);
+
+	nvmet_req_complete(req, status);
+}
+
 u16 nvmet_parse_admin_cmd(struct nvmet_req *req)
 {
 	struct nvme_command *cmd = req->cmd;
@@ -673,6 +833,15 @@ u16 nvmet_parse_admin_cmd(struct nvmet_req *req)
 		req->execute = nvmet_execute_keep_alive;
 		req->data_len = 0;
 		return 0;
+	case nvme_admin_ns_mgmt:
+		if (req->sq->ctrl->subsys->mount_path) {
+			req->execute = nvmet_execute_ns_mgmt_file;
+			req->data_len = 0;
+			if (cmd->common.cdw10[0] == 0)
+				req->data_len = NVME_IDENTIFY_DATA_SIZE;
+			return 0;
+		}
+		/* fall thru */
 	}
 
 	pr_err("unhandled cmd %d on qid %d\n", cmd->common.opcode,
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 17/20] nvmet: override identify for file backed ns
  2018-04-18 18:59 [PATCH 00/19] nvmet: add support for file backed namespaces Chaitanya Kulkarni
                   ` (15 preceding siblings ...)
  2018-04-18 19:00 ` [PATCH 16/20] nvmet: add ns-mgmt command handlers Chaitanya Kulkarni
@ 2018-04-18 19:00 ` Chaitanya Kulkarni
  2018-04-18 19:00 ` [PATCH 18/20] nvmet: allow host to configure sync vs direct IO Chaitanya Kulkarni
                   ` (3 subsequent siblings)
  20 siblings, 0 replies; 29+ messages in thread
From: Chaitanya Kulkarni @ 2018-04-18 19:00 UTC (permalink / raw)


Update identify ctrl/ns to initialize file backed namespaces fields.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
---
 drivers/nvme/target/admin-cmd.c | 75 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 75 insertions(+)

diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index b60477c50880..19b93952bd16 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -319,11 +319,39 @@ static void nvmet_identify_ctrl_init(struct nvmet_ctrl *ctrl,
 	id->psd[0].exit_lat = cpu_to_le32(0x4);
 }
 
+static u16 nvmet_identify_ctrl_init_file(struct nvmet_req *req,
+		struct nvme_id_ctrl *id)
+{
+	struct nvmet_ctrl *ctrl = req->sq->ctrl;
+	struct nvmet_subsys *subsys = ctrl->subsys;
+	struct kstatfs fsstat;
+	__le64 tnvmcap;
+	__le64 unvmcap;
+	u16 status = NVME_SC_SUCCESS;
+
+	if (nvmet_vfs_statfs(subsys->mount_path, &fsstat)) {
+		status = NVME_SC_INTERNAL | NVME_SC_DNR;
+		goto out;
+	}
+
+	id->oacs = 1 << 3; /* we support namespace management */
+
+	tnvmcap = cpu_to_le64(fsstat.f_blocks * fsstat.f_bsize);
+	memcpy(id->tnvmcap, &tnvmcap, sizeof(tnvmcap));
+
+	unvmcap = cpu_to_le64(fsstat.f_bfree * fsstat.f_bsize);
+	memcpy(id->unvmcap, &tnvmcap, sizeof(unvmcap));
+
+out:
+	return status;
+}
+
 static void nvmet_execute_identify_ctrl(struct nvmet_req *req)
 {
 	struct nvmet_ctrl *ctrl = req->sq->ctrl;
 	struct nvme_id_ctrl *id;
 	u16 status = 0;
+	u16 file_status = 0;
 
 	id = kzalloc(sizeof(*id), GFP_KERNEL);
 	if (!id) {
@@ -333,8 +361,14 @@ static void nvmet_execute_identify_ctrl(struct nvmet_req *req)
 
 	nvmet_identify_ctrl_init(ctrl, id);
 
+	if (req->sq->ctrl->subsys->mount_path)
+		file_status = nvmet_identify_ctrl_init_file(req, id);
+
 	status = nvmet_copy_to_sgl(req, 0, id, sizeof(*id));
 
+	if (req->sq->ctrl->subsys->mount_path && file_status)
+		status = file_status;
+
 	kfree(id);
 out:
 	nvmet_req_complete(req, status);
@@ -367,11 +401,46 @@ static void nvmet_identify_ns_init(struct nvmet_ns *ns, struct nvme_id_ns *id)
 	id->lbaf[0].ds = ns->blksize_shift;
 }
 
+static int nvmet_identify_ns_file_stats(struct nvmet_ns *ns,
+		struct nvme_id_ns *id)
+{
+	u64 alloc_blk;
+	u64 blk_size = (1 << ns->blksize_shift);
+	struct kstat stat;
+	__le64 size;
+
+	if (nvmet_vfs_stat(ns->device_path, &stat))
+		return -1;
+
+	alloc_blk = DIV_ROUND_UP((stat.blocks * (1 << 9)), blk_size);
+
+	id->nsze = cpu_to_le64(DIV_ROUND_UP(stat.size, blk_size));
+	id->ncap = id->nuse = cpu_to_le64(alloc_blk);
+
+	memcpy(id->nvmcap, &size, sizeof(size));
+
+	return 0;
+}
+
+static int nvmet_identify_ns_init_file(struct nvmet_ns *ns,
+		struct nvme_id_ns *id)
+{
+	int ret;
+
+	ret = nvmet_identify_ns_file_stats(ns, id);
+	if (ret)
+		return NVME_SC_INVALID_NS;
+
+	id->nsfeat = 1 << 0;
+	return 0;
+}
+
 static void nvmet_execute_identify_ns(struct nvmet_req *req)
 {
 	struct nvmet_ns *ns;
 	struct nvme_id_ns *id;
 	u16 status = 0;
+	u16 file_status = 0;
 
 	ns = nvmet_find_namespace(req->sq->ctrl, req->cmd->identify.nsid);
 	if (!ns) {
@@ -387,8 +456,14 @@ static void nvmet_execute_identify_ns(struct nvmet_req *req)
 
 	nvmet_identify_ns_init(ns, id);
 
+	if (req->sq->ctrl->subsys->mount_path)
+		file_status = nvmet_identify_ns_init_file(ns, id);
+
 	status = nvmet_copy_to_sgl(req, 0, id, sizeof(*id));
 
+	if (req->sq->ctrl->subsys->mount_path && file_status)
+		status = file_status;
+
 	kfree(id);
 out_put_ns:
 	nvmet_put_namespace(ns);
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 18/20] nvmet: allow host to configure sync vs direct IO
  2018-04-18 18:59 [PATCH 00/19] nvmet: add support for file backed namespaces Chaitanya Kulkarni
                   ` (16 preceding siblings ...)
  2018-04-18 19:00 ` [PATCH 17/20] nvmet: override identify for file backed ns Chaitanya Kulkarni
@ 2018-04-18 19:00 ` Chaitanya Kulkarni
  2018-04-18 19:00 ` [PATCH 19/20] nvmet: add check for ctrl processing paused status Chaitanya Kulkarni
                   ` (2 subsequent siblings)
  20 siblings, 0 replies; 29+ messages in thread
From: Chaitanya Kulkarni @ 2018-04-18 19:00 UTC (permalink / raw)


Add support to switch between O_SYNC and O_DIRECT.
We use NVMe Feature Volatile write cache field to toggle between
the two modes.

This also replaces existing mechanism of using the previously
added sync configfs namespaces attribute and allow the host to
switch between the modes.

The newly introduced field vwc is set when host side issues
nvme-set-feature for configuring the volatile write cache and
passed to each namespace.

At the time of initialization of the namespace we use the
subsystem vwc to initialize namespace vwc, having vwc
field in the target ns will allow implementing per namespace
vwc configuration in future.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
---
 drivers/nvme/target/admin-cmd.c | 54 +++++++++++++++++++++++++++++++++++++++++
 drivers/nvme/target/configfs.c  | 27 ---------------------
 drivers/nvme/target/core.c      |  8 +++---
 drivers/nvme/target/io-cmd.c    |  4 +--
 drivers/nvme/target/nvmet.h     |  6 ++++-
 5 files changed, 65 insertions(+), 34 deletions(-)

diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index 19b93952bd16..b119a31eb6f9 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -836,6 +836,56 @@ static u16 nvmet_delete_ns_file(struct nvmet_req *req)
 	return status;
 }
 
+static void nvmet_set_features_vwc_file(struct nvmet_req *req)
+{
+	u16 status = NVME_SC_SUCCESS;
+	struct nvmet_ns *ns;
+	struct nvmet_subsys *subsys = req->sq->ctrl->subsys;
+
+	if (subsys->vwc != le32_to_cpu(req->cmd->features.dword11)) {
+		subsys->vwc = le32_to_cpu(req->cmd->features.dword11);
+		mutex_lock(&subsys->lock);
+		list_for_each_entry_rcu(ns, &subsys->namespaces, dev_link) {
+			percpu_ref_get(&ns->ref);
+			mutex_unlock(&subsys->lock);
+			if (ns->vwc != subsys->vwc) {
+				/* improve error handling here */
+				nvmet_ns_disable(ns);
+				ns->vwc = subsys->vwc;
+				nvmet_ns_enable(ns);
+			}
+			mutex_lock(&subsys->lock);
+			percpu_ref_put(&ns->ref);
+		}
+		mutex_unlock(&subsys->lock);
+	}
+
+	nvmet_req_complete(req, status);
+}
+
+static void nvmet_execute_set_features_file(struct nvmet_req *req)
+{
+	switch (le32_to_cpu(req->cmd->features.fid)) {
+	case NVME_FEAT_VOLATILE_WC:
+		nvmet_set_features_vwc_file(req);
+		break;
+	default:
+		nvmet_execute_set_features(req);
+	}
+}
+
+static void nvmet_execute_get_features_file(struct nvmet_req *req)
+{
+	switch (le32_to_cpu(req->cmd->features.fid)) {
+	case NVME_FEAT_VOLATILE_WC:
+		nvmet_set_result(req, req->sq->ctrl->subsys->vwc);
+		nvmet_req_complete(req, 0);
+		break;
+	default:
+		nvmet_execute_get_features(req);
+	}
+}
+
 static void nvmet_execute_ns_mgmt_file(struct nvmet_req *req)
 {
 	u16 status;
@@ -894,10 +944,14 @@ u16 nvmet_parse_admin_cmd(struct nvmet_req *req)
 		return 0;
 	case nvme_admin_set_features:
 		req->execute = nvmet_execute_set_features;
+		if (req->sq->ctrl->subsys->mount_path)
+			req->execute = nvmet_execute_set_features_file;
 		req->data_len = 0;
 		return 0;
 	case nvme_admin_get_features:
 		req->execute = nvmet_execute_get_features;
+		if (req->sq->ctrl->subsys->mount_path)
+			req->execute = nvmet_execute_get_features_file;
 		req->data_len = 0;
 		return 0;
 	case nvme_admin_async_event:
diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
index f571dffbecdc..152d180af94b 100644
--- a/drivers/nvme/target/configfs.c
+++ b/drivers/nvme/target/configfs.c
@@ -407,38 +407,11 @@ static ssize_t nvmet_ns_enable_store(struct config_item *item,
 
 CONFIGFS_ATTR(nvmet_ns_, enable);
 
-static ssize_t nvmet_ns_sync_show(struct config_item *item, char *page)
-{
-	return sprintf(page, "%d\n", to_nvmet_ns(item)->sync);
-}
-
-static ssize_t nvmet_ns_sync_store(struct config_item *item,
-		const char *page, size_t count)
-{
-	struct nvmet_ns *ns = to_nvmet_ns(item);
-	bool sync;
-	int ret = 0;
-
-	if (strtobool(page, &sync))
-		return -EINVAL;
-
-	if (ns->filp && ns->sync != sync) {
-		nvmet_ns_disable(ns);
-		ns->sync = sync;
-		ret = nvmet_ns_enable(ns);
-	}
-
-	return ret ? ret : count;
-}
-
-CONFIGFS_ATTR(nvmet_ns_, sync);
-
 static struct configfs_attribute *nvmet_ns_attrs[] = {
 	&nvmet_ns_attr_device_path,
 	&nvmet_ns_attr_device_nguid,
 	&nvmet_ns_attr_device_uuid,
 	&nvmet_ns_attr_enable,
-	&nvmet_ns_attr_sync,
 	NULL,
 };
 
diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index 0b0649dc705c..36f0dacec042 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -295,7 +295,7 @@ static int nvmet_ns_enable_file(struct nvmet_ns *ns)
 	int flags = O_RDWR | O_LARGEFILE;
 	struct kstat stat;
 
-	flags |= ns->sync ? O_SYNC : O_DIRECT;
+	flags |= ns->vwc ? O_SYNC : O_DIRECT;
 	ns->filp = filp_open(ns->device_path, flags, 0);
 	if (!ns->filp || IS_ERR(ns->filp)) {
 		pr_err("failed to open file %s: (%ld)\n",
@@ -311,7 +311,7 @@ static int nvmet_ns_enable_file(struct nvmet_ns *ns)
 	ns->size = stat.size;
 	ns->blksize_shift = ns->filp->f_inode->i_blkbits;
 
-	if (ns->sync) {
+	if (ns->vwc) {
 		ns->file_wq = alloc_workqueue("nvmet-file",
 				WQ_UNBOUND_MAX_ACTIVE | WQ_MEM_RECLAIM, 0);
 
@@ -349,7 +349,7 @@ static void nvmet_ns_dev_disable(struct nvmet_ns *ns)
 		blkdev_put(ns->bdev, FMODE_WRITE | FMODE_READ);
 		ns->bdev = NULL;
 	} else if (ns->filp) {
-		if (ns->sync) {
+		if (ns->vwc) {
 			flush_workqueue(ns->file_wq);
 			destroy_workqueue(ns->file_wq);
 			ns->file_wq = NULL;
@@ -472,7 +472,7 @@ struct nvmet_ns *nvmet_ns_alloc(struct nvmet_subsys *subsys, u32 nsid)
 	ns->nsid = nsid;
 	ns->subsys = subsys;
 	uuid_gen(&ns->uuid);
-	ns->sync = false;
+	ns->vwc = subsys->vwc;
 
 	return ns;
 }
diff --git a/drivers/nvme/target/io-cmd.c b/drivers/nvme/target/io-cmd.c
index 6d355b2dac56..96bb17353a29 100644
--- a/drivers/nvme/target/io-cmd.c
+++ b/drivers/nvme/target/io-cmd.c
@@ -108,7 +108,7 @@ static void nvmet_execute_rw_file(struct nvmet_req *req)
 	struct sg_mapping_iter miter;
 	loff_t pos;
 	ssize_t len = 0, ret;
-	int ki_flags = req->ns->sync ? IOCB_SYNC : IOCB_DIRECT;
+	int ki_flags = req->ns->vwc ? IOCB_SYNC : IOCB_DIRECT;
 	int bv_cnt = 0, rw = READ;
 
 	if (req->cmd->rw.opcode == nvme_cmd_write) {
@@ -348,7 +348,7 @@ u16 nvmet_parse_io_cmd(struct nvmet_req *req)
 	case nvme_cmd_read:
 	case nvme_cmd_write:
 		if (req->ns->filp) {
-			if (req->ns->sync) {
+			if (req->ns->vwc) {
 				req->execute = nvmet_execute_rw_sync_file;
 				INIT_WORK(&req->sync_work,
 						nvmet_sync_work_file);
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index a365dc44a10e..acb9c266573b 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -53,7 +53,6 @@ struct nvmet_ns {
 	uuid_t			uuid;
 
 	bool			enabled;
-	bool			sync;
 	struct nvmet_subsys	*subsys;
 	const char		*device_path;
 
@@ -61,6 +60,8 @@ struct nvmet_ns {
 	u64			ncap;
 	u64			nuse;
 	u8			flbas;
+	u8			vwc;
+
 	struct workqueue_struct *file_wq;
 	struct config_group	device_group;
 	struct config_group	group;
@@ -169,6 +170,9 @@ struct nvmet_subsys {
 
 	struct config_group	namespaces_group;
 	struct config_group	allowed_hosts_group;
+
+
+	u8			vwc;
 	char			*mount_path;
 	unsigned long		nsid_map[1024]; /* MAX LIMIT ? */
 };
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 19/20] nvmet: add check for ctrl processing paused status
  2018-04-18 18:59 [PATCH 00/19] nvmet: add support for file backed namespaces Chaitanya Kulkarni
                   ` (17 preceding siblings ...)
  2018-04-18 19:00 ` [PATCH 18/20] nvmet: allow host to configure sync vs direct IO Chaitanya Kulkarni
@ 2018-04-18 19:00 ` Chaitanya Kulkarni
  2018-04-18 19:00 ` [PATCH 20/20] nvmet: use processing paused state for VWC Chaitanya Kulkarni
  2018-04-24 17:33 ` [PATCH 00/19] nvmet: add support for file backed namespaces Christoph Hellwig
  20 siblings, 0 replies; 29+ messages in thread
From: Chaitanya Kulkarni @ 2018-04-18 19:00 UTC (permalink / raw)


New ctrl status bit processing paused is checked
under nvmet_check_ctrl_status() which acts as a guard for
admin and IO commands.

Right now from the spec there is no way to differentiate
transport commands and storage commands so we can allow only
allow transport commands. Consider a scenario where we want
transport commands to work but the storage commands are disabled.

This allows to set processing paused status where we want to
allow a subset of the commands to work.

The example of above scenario is when implementing the file backed
namespaces we allow host to set volatile write cache feature
when connected. In this scenario, we cannot handle any incoming
admin and IO commands as we enable/disable the VWC but we need
to handle keep alive command in order to preserve the connection.
Also here we want a controller to be in ready state.

The check for new bit NVME_CSTS_PP allows us to execute subset
commands and deny others when ctrl is enabled and in a ready
status.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
---
 drivers/nvme/target/core.c  | 50 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/nvme/target/nvmet.h |  2 ++
 2 files changed, 52 insertions(+)

diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index 36f0dacec042..0c5b69b638ed 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -802,6 +802,22 @@ u16 nvmet_check_ctrl_status(struct nvmet_req *req, struct nvme_command *cmd)
 		req->ns = NULL;
 		return NVME_SC_CMD_SEQ_ERROR | NVME_SC_DNR;
 	}
+
+	/*
+	 * We use processing paused state which allows subset of the commands
+	 * to work, e.g. we allow keep alive command when in a paused state.
+	 */
+
+	if (unlikely(req->sq->ctrl->csts & NVME_CSTS_PP)) {
+		switch (cmd->common.opcode) {
+		case nvme_admin_async_event:
+		case nvme_admin_keep_alive:
+		case nvme_admin_get_features:
+			break;
+		default:
+			return NVME_SC_CMD_SEQ_ERROR | NVME_SC_DNR;
+		}
+	}
 	return 0;
 }
 
@@ -989,6 +1005,40 @@ void nvmet_ctrl_put(struct nvmet_ctrl *ctrl)
 	kref_put(&ctrl->ref, nvmet_ctrl_free);
 }
 
+/*
+ * Right now from the spec there is no way to differentiate transport commands
+ * and storage commands. There could be a scenario where we want transport
+ * commands to work but the storage commands are disabled.
+ * We put ctrl in processing paused state where we want to allow
+ * subset of the commands to work. Please refer to the
+ * nvmet_check_ctrl_status().
+ */
+bool nvmet_ctrl_processing_pause(struct nvmet_ctrl *ctrl)
+{
+	bool ret = false;
+
+	mutex_lock(&ctrl->lock);
+	if (ctrl->cc & NVME_CC_ENABLE && ctrl->csts & NVME_CSTS_RDY) {
+		ctrl->csts |= NVME_CSTS_PP;
+		ret = true;
+	}
+	mutex_unlock(&ctrl->lock);
+	return ret;
+}
+
+bool nvmet_ctrl_processing_resume(struct nvmet_ctrl *ctrl)
+{
+	bool ret = false;
+
+	mutex_lock(&ctrl->lock);
+	if (ctrl->cc & NVME_CC_ENABLE && ctrl->csts & NVME_CSTS_RDY) {
+		ctrl->csts &= ~NVME_CSTS_PP;
+		ret = true;
+	}
+	mutex_unlock(&ctrl->lock);
+	return ret;
+}
+
 static void nvmet_fatal_error_handler(struct work_struct *work)
 {
 	struct nvmet_ctrl *ctrl =
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index acb9c266573b..bd9d5b8a6a30 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -297,6 +297,8 @@ void nvmet_sq_setup(struct nvmet_ctrl *ctrl, struct nvmet_sq *sq, u16 qid,
 void nvmet_sq_destroy(struct nvmet_sq *sq);
 int nvmet_sq_init(struct nvmet_sq *sq);
 
+bool nvmet_ctrl_processing_pause(struct nvmet_ctrl *ctrl);
+bool nvmet_ctrl_processing_resume(struct nvmet_ctrl *ctrl);
 void nvmet_ctrl_fatal_error(struct nvmet_ctrl *ctrl);
 
 void nvmet_update_cc(struct nvmet_ctrl *ctrl, u32 new);
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 20/20] nvmet: use processing paused state for VWC
  2018-04-18 18:59 [PATCH 00/19] nvmet: add support for file backed namespaces Chaitanya Kulkarni
                   ` (18 preceding siblings ...)
  2018-04-18 19:00 ` [PATCH 19/20] nvmet: add check for ctrl processing paused status Chaitanya Kulkarni
@ 2018-04-18 19:00 ` Chaitanya Kulkarni
  2018-04-24 17:33 ` [PATCH 00/19] nvmet: add support for file backed namespaces Christoph Hellwig
  20 siblings, 0 replies; 29+ messages in thread
From: Chaitanya Kulkarni @ 2018-04-18 19:00 UTC (permalink / raw)


We use the ctrl processing paused state when switching between
O_SYNC and O_DIRECT.

Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
---
 drivers/nvme/target/admin-cmd.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index b119a31eb6f9..f1d4005f92b0 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -842,6 +842,11 @@ static void nvmet_set_features_vwc_file(struct nvmet_req *req)
 	struct nvmet_ns *ns;
 	struct nvmet_subsys *subsys = req->sq->ctrl->subsys;
 
+	if (nvmet_ctrl_processing_pause(req->sq->ctrl) == false) {
+		status = NVME_SC_INTERNAL | NVME_SC_DNR;
+		goto out;
+	}
+
 	if (subsys->vwc != le32_to_cpu(req->cmd->features.dword11)) {
 		subsys->vwc = le32_to_cpu(req->cmd->features.dword11);
 		mutex_lock(&subsys->lock);
@@ -860,6 +865,9 @@ static void nvmet_set_features_vwc_file(struct nvmet_req *req)
 		mutex_unlock(&subsys->lock);
 	}
 
+	if (nvmet_ctrl_processing_resume(req->sq->ctrl) == false)
+		status = NVME_SC_INTERNAL | NVME_SC_DNR;
+out:
 	nvmet_req_complete(req, status);
 }
 
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 03/20] nvmet: add structure members for file I/O
  2018-04-18 18:59 ` [PATCH 03/20] nvmet: add structure members for file I/O Chaitanya Kulkarni
@ 2018-04-24 17:18   ` Christoph Hellwig
  0 siblings, 0 replies; 29+ messages in thread
From: Christoph Hellwig @ 2018-04-24 17:18 UTC (permalink / raw)


On Wed, Apr 18, 2018@02:59:54PM -0400, Chaitanya Kulkarni wrote:
> This patch adds new members to perform I/O operations on the
> file backed namespaces.

In general this should go along with actually adding the code.

> +	struct file		*filp;

Please call this file instead of filp.

>  	u32			nsid;
>  	u32			blksize_shift;
>  	loff_t			size;
> @@ -222,6 +224,8 @@ struct nvmet_req {
>  	struct scatterlist	*sg;
>  	struct bio		inline_bio;
>  	struct bio_vec		inline_bvec[NVMET_MAX_INLINE_BIOVEC];
> +	struct kiocb            iocb;
> +	struct bio_vec          *bvec;

Can this be unioned with any block devices fields?

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 04/20] nvmet: initialize file handle and req fields
  2018-04-18 18:59 ` [PATCH 04/20] nvmet: initialize file handle and req fields Chaitanya Kulkarni
@ 2018-04-24 17:22   ` Christoph Hellwig
  0 siblings, 0 replies; 29+ messages in thread
From: Christoph Hellwig @ 2018-04-24 17:22 UTC (permalink / raw)


> +static int nvmet_ns_enable_file(struct nvmet_ns *ns)
> +{
> +	int ret;
> +	int flags = O_RDWR | O_LARGEFILE | O_DIRECT;
> +	struct kstat stat;
> +
> +	ns->filp = filp_open(ns->device_path, flags, 0);
> +	if (!ns->filp || IS_ERR(ns->filp)) {
> +		pr_err("failed to open file %s: (%ld)\n",
> +				ns->device_path, PTR_ERR(ns->bdev));
> +		ns->filp = NULL;
> +		return -1;
> +	}
> +
> +	ret = nvmet_vfs_stat(ns->device_path, &stat);
> +	if (ret)
> +		goto err;

Just call vfs_getattr on file->f_path instead of this nvmet_vfs_stat
helper that isn't really needed.

> +	ns->blksize_shift = ns->filp->f_inode->i_blkbits;

Pleae use file_inode() to get an inode from the file instead of the
direct reference to accomodate overlayfs and other stackable file
systems.

> +
> +	return ret;
> +err:
> +	filp_close(ns->filp, NULL);

This should be a fput instead.

>  static int nvmet_ns_dev_enable(struct nvmet_ns *ns)
>  {
> -	return nvmet_ns_enable_blk(ns);
> +	int ret = 0;
> +	int ret1 = 0;
> +
> +	ret = nvmet_ns_enable_blk(ns);
> +	if (ret)
> +		ret1 = nvmet_ns_enable_file(ns);
> +
> +	if (ret && ret1)
> +		return -1;

I don't think there is any good reason to keep the blk open return
value, e.g. this could simpl be:

	ret = nvmet_ns_enable_blk(ns);
	if (ret)
		ret = nvmet_ns_enable_file(ns);

The file open return value is probably more useful in general.

>  }
>  
>  static void nvmet_ns_dev_disable(struct nvmet_ns *ns)
> @@ -299,7 +351,11 @@ static void nvmet_ns_dev_disable(struct nvmet_ns *ns)
>  	if (ns->bdev) {
>  		blkdev_put(ns->bdev, FMODE_WRITE | FMODE_READ);
>  		ns->bdev = NULL;
> -	}
> +	} else if (ns->filp) {
> +		filp_close(ns->filp, NULL);

fput again.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 05/20] nvmet: add NVMe I/O command handlers for file
  2018-04-18 18:59 ` [PATCH 05/20] nvmet: add NVMe I/O command handlers for file Chaitanya Kulkarni
@ 2018-04-24 17:29   ` Christoph Hellwig
  0 siblings, 0 replies; 29+ messages in thread
From: Christoph Hellwig @ 2018-04-24 17:29 UTC (permalink / raw)


> +#define NR_BVEC(req)	(req->data_len / PAGE_SIZE)

Don't we need to round up here instead of rounding down for
transfers that aren't a multiple of PAGE_SIZE?

Also given that there is a single user only it might make more sense
to just open code it.

> +static void nvmet_file_io_complete(struct kiocb *iocb, long ret, long ret2)
> +{
> +	struct nvmet_req *req = container_of(iocb, struct nvmet_req, iocb);
> +
> +	kfree(req->bvec);
> +	req->bvec = NULL;

There should be no need to set req->bvec to NULL there.

> +static void nvmet_execute_rw_file(struct nvmet_req *req)
> +{
> +	struct iov_iter iter;
> +	struct sg_mapping_iter miter;
> +	loff_t pos;
> +	ssize_t len = 0, ret;
> +	int ki_flags = IOCB_DIRECT;
> +	int bv_cnt = 0, rw = READ;

Maybe do a simple bool is_write instead of using the nasty
legacy READ/WRITE defines?

> +	req->bvec = kmalloc_array(NR_BVEC(req), sizeof(struct bio_vec),
> +			GFP_KERNEL);
> +	if (!req->bvec)
> +		goto out;

Any chance to reuse the inline_bvec for small requests?

> +
> +	sg_miter_start(&miter, req->sg, req->sg_cnt, SG_MITER_FROM_SG);
> +	while (sg_miter_next(&miter)) {
> +		req->bvec[bv_cnt].bv_page = miter.page;
> +		req->bvec[bv_cnt].bv_offset = miter.__offset;

>From scatterlist.h:

	/* these are internal states, keep away */
	unsigned int            __offset;       /* offset within page */

But given that the nvmet code allocated the scatterlist we already know
each element is only a page long at maximum.  So a simple for_each_sg
loop as for the block backend will probably do it.

> +	pos = le64_to_cpu(req->cmd->rw.slba) * (1 << (req->ns->blksize_shift));

The block backend does:

	sector = le64_to_cpu(req->cmd->rw.slba);
        sector <<= (req->ns->blksize_shift - 9);

which should be more efficient as it avoids the multiplication.

> +	if (rw == WRITE)
> +		ret = call_write_iter(req->ns->filp, &req->iocb, &iter);
> +	else
> +		ret = call_read_iter(req->ns->filp, &req->iocb, &iter);

Please call the read_iter/write_iter methods directly.

> +static void nvmet_execute_flush_file(struct nvmet_req *req)
> +{
> +	int ret = vfs_fsync(req->ns->filp, 1);
> +
> +	nvmet_req_complete(req, ret);
> +}

This will work ok but block the caller frontend thread for possibly
a long time.  Offloading this to a simple schedule_work might be
beneficial.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 06/20] nvmet: add new members for sync file I/O
  2018-04-18 18:59 ` [PATCH 06/20] nvmet: add new members for sync file I/O Chaitanya Kulkarni
@ 2018-04-24 17:30   ` Christoph Hellwig
  0 siblings, 0 replies; 29+ messages in thread
From: Christoph Hellwig @ 2018-04-24 17:30 UTC (permalink / raw)


On Wed, Apr 18, 2018@02:59:57PM -0400, Chaitanya Kulkarni wrote:
> This patch adds new structure members to implement O_SYNC
> IO mode for file-backed ns. The user can optionally choose
> to switch between O_SYNC and O_DIRECT with the configfs parameter
> which we add in the next patch.

Again please introduce the fields together with code using them.

O_SYNC and O_DIRECT can combined, which makes the description above
sounds rather strange.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 00/19] nvmet: add support for file backed namespaces
  2018-04-18 18:59 [PATCH 00/19] nvmet: add support for file backed namespaces Chaitanya Kulkarni
                   ` (19 preceding siblings ...)
  2018-04-18 19:00 ` [PATCH 20/20] nvmet: use processing paused state for VWC Chaitanya Kulkarni
@ 2018-04-24 17:33 ` Christoph Hellwig
  2018-04-26 16:53   ` chaitany kulkarni
  20 siblings, 1 reply; 29+ messages in thread
From: Christoph Hellwig @ 2018-04-24 17:33 UTC (permalink / raw)


Hi Chaitanya,

I've been through the patches that implement the simple O_DIRECT
file support.  I think all these can in fact be merged into a single
patch and would be easier to review.  Can you resend just that merged
patch for the next respin?  I think we want to review the buffered
I/O support and the namespace management support as incremental series
on top of that to keep things a little smaller and easier to review.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 08/20] nvmet: configure file backed ns for sync IO
  2018-04-18 18:59 ` [PATCH 08/20] nvmet: configure file backed ns for sync IO Chaitanya Kulkarni
@ 2018-04-24 17:41   ` Christoph Hellwig
  2018-05-04  2:36     ` chaitany kulkarni
  0 siblings, 1 reply; 29+ messages in thread
From: Christoph Hellwig @ 2018-04-24 17:41 UTC (permalink / raw)


On Wed, Apr 18, 2018@02:59:59PM -0400, Chaitanya Kulkarni wrote:
> This patch allows file backed namespaces to use newly
> introduced configfs attribute sync, when sync is set
> we open the file with O_SYNC and use the work queue
> to execute the NVMe read/write commands.

So this adds support for buffered I/O.  But why do you force
O_SYNC on everyone who uses buffered I/O?

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 00/19] nvmet: add support for file backed namespaces
  2018-04-24 17:33 ` [PATCH 00/19] nvmet: add support for file backed namespaces Christoph Hellwig
@ 2018-04-26 16:53   ` chaitany kulkarni
  0 siblings, 0 replies; 29+ messages in thread
From: chaitany kulkarni @ 2018-04-26 16:53 UTC (permalink / raw)


Thanks for the review comments.

I'll send the O_DIRECT single patch first with your review comments
incorporated.

On Tue, Apr 24, 2018@10:33 AM, Christoph Hellwig <hch@lst.de> wrote:
> Hi Chaitanya,
>
> I've been through the patches that implement the simple O_DIRECT
> file support.  I think all these can in fact be merged into a single
> patch and would be easier to review.  Can you resend just that merged
> patch for the next respin?  I think we want to review the buffered
> I/O support and the namespace management support as incremental series
> on top of that to keep things a little smaller and easier to review.
>
> _______________________________________________
> Linux-nvme mailing list
> Linux-nvme at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 08/20] nvmet: configure file backed ns for sync IO
  2018-04-24 17:41   ` Christoph Hellwig
@ 2018-05-04  2:36     ` chaitany kulkarni
  0 siblings, 0 replies; 29+ messages in thread
From: chaitany kulkarni @ 2018-05-04  2:36 UTC (permalink / raw)


You are right, we should not be forcing O_SYNC on the target side.

How about we rename the sync configfs parameter to buffer/cache/vwc?
(or any other preferable name).
We use buffer IO (NO O_* flags when for open and submitting IO) when
that parameter is set and use O_DIRECT when that parameter is not set.

Any thoughts?

On Tue, Apr 24, 2018@10:41 AM, Christoph Hellwig <hch@lst.de> wrote:
> On Wed, Apr 18, 2018@02:59:59PM -0400, Chaitanya Kulkarni wrote:
>> This patch allows file backed namespaces to use newly
>> introduced configfs attribute sync, when sync is set
>> we open the file with O_SYNC and use the work queue
>> to execute the NVMe read/write commands.
>
> So this adds support for buffered I/O.  But why do you force
> O_SYNC on everyone who uses buffered I/O?
>
> _______________________________________________
> Linux-nvme mailing list
> Linux-nvme at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2018-05-04  2:36 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-18 18:59 [PATCH 00/19] nvmet: add support for file backed namespaces Chaitanya Kulkarni
2018-04-18 18:59 ` [PATCH 01/20] nvmet: add block device guard for smart-log Chaitanya Kulkarni
2018-04-18 18:59 ` [PATCH 02/20] nvmet: add a wrapper for ns handlers Chaitanya Kulkarni
2018-04-18 18:59 ` [PATCH 03/20] nvmet: add structure members for file I/O Chaitanya Kulkarni
2018-04-24 17:18   ` Christoph Hellwig
2018-04-18 18:59 ` [PATCH 04/20] nvmet: initialize file handle and req fields Chaitanya Kulkarni
2018-04-24 17:22   ` Christoph Hellwig
2018-04-18 18:59 ` [PATCH 05/20] nvmet: add NVMe I/O command handlers for file Chaitanya Kulkarni
2018-04-24 17:29   ` Christoph Hellwig
2018-04-18 18:59 ` [PATCH 06/20] nvmet: add new members for sync file I/O Chaitanya Kulkarni
2018-04-24 17:30   ` Christoph Hellwig
2018-04-18 18:59 ` [PATCH 07/20] nvmet: add sync I/O configfs attr for ns Chaitanya Kulkarni
2018-04-18 18:59 ` [PATCH 08/20] nvmet: configure file backed ns for sync IO Chaitanya Kulkarni
2018-04-24 17:41   ` Christoph Hellwig
2018-05-04  2:36     ` chaitany kulkarni
2018-04-18 19:00 ` [PATCH 09/20] nvmet: use workqueue for sync I/O Chaitanya Kulkarni
2018-04-18 19:00 ` [PATCH 10/20] nvmet: isolate id ctrl/ns field initialization Chaitanya Kulkarni
2018-04-18 19:00 ` [PATCH 11/20] nvmet: introduce new members for ns-mgmt Chaitanya Kulkarni
2018-04-18 19:00 ` [PATCH 12/20] nvmet: add a configfs entry for subsys mount path Chaitanya Kulkarni
2018-04-18 19:00 ` [PATCH 13/20] nvmet: initialize new ns and subsys members Chaitanya Kulkarni
2018-04-18 19:00 ` [PATCH 14/20] nvmet: add and restructure ns mgmt helpers Chaitanya Kulkarni
2018-04-18 19:00 ` [PATCH 15/20] fs: export kern_path_locked() to reuse the code Chaitanya Kulkarni
2018-04-18 19:00 ` [PATCH 16/20] nvmet: add ns-mgmt command handlers Chaitanya Kulkarni
2018-04-18 19:00 ` [PATCH 17/20] nvmet: override identify for file backed ns Chaitanya Kulkarni
2018-04-18 19:00 ` [PATCH 18/20] nvmet: allow host to configure sync vs direct IO Chaitanya Kulkarni
2018-04-18 19:00 ` [PATCH 19/20] nvmet: add check for ctrl processing paused status Chaitanya Kulkarni
2018-04-18 19:00 ` [PATCH 20/20] nvmet: use processing paused state for VWC Chaitanya Kulkarni
2018-04-24 17:33 ` [PATCH 00/19] nvmet: add support for file backed namespaces Christoph Hellwig
2018-04-26 16:53   ` chaitany kulkarni

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.