Skip to contentSkip to conversation

nvmet: add support for file backed namespaces

PATCH New Chaitanya Kulkarni 3 participants 29 messages Apr 18, 2018 – May 4, 2018

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

cover nvmet: add support for file backed namespaces

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

#1 lore raw

PATCH 1/20 nvmet: add block device guard for smart-log

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(+)
drivers/nvme/target/admin-cmd.c +8 -0
index 90dcdc40ac71..602995f7a1b3 100644
@@ -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]);
Signature · 1 line
2.14.1
#2 lore raw

PATCH 2/20 nvmet: add a wrapper for ns handlers

In reply to Chaitanya Kulkarni

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(-)
drivers/nvme/target/core.c +37 -17
index a78029e4e5f4..5f032dc7077f 100644
@@ -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);
 }
Signature · 1 line
2.14.1
#3 lore raw

PATCH 3/20 nvmet: add structure members for file I/O

In reply to Chaitanya Kulkarni

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(+)
drivers/nvme/target/nvmet.h +4 -0
index 417f6c0331cc..acc61ca97198 100644
@@ -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;
Signature · 1 line
2.14.1
#4 lore raw

PATCH 3/20 nvmet: add structure members for file I/O

Quoted 2 lines · Chaitanya Kulkarni

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.

drivers/nvme/target/nvmet.h struct nvmet_ns { · line 47

Quoted 1 line
drivers/nvme/target/nvmet.h +1 -0
+	struct file		*filp;

Please call this file instead of filp.

drivers/nvme/target/nvmet.h struct nvmet_req { · line 228

Quoted 9 lines
drivers/nvme/target/nvmet.h +2 -0
 	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?

#5 lore raw

PATCH 4/20 nvmet: initialize file handle and req fields

In reply to Chaitanya Kulkarni

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(-)
drivers/nvme/target/core.c +60 -2
index 5f032dc7077f..b7c51a5629bf 100644
@@ -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))) {
Signature · 1 line
2.14.1
#6 lore raw

PATCH 4/20 nvmet: initialize file handle and req fields

Quoted 7 lines
drivers/nvme/target/core.c +7 -0
+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);

drivers/nvme/target/core.c static int nvmet_ns_enable_blk(struct nvmet_ns *ns) · line 321

Quoted 10 lines
drivers/nvme/target/core.c +10 -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.

drivers/nvme/target/core.c static int nvmet_ns_enable_blk(struct nvmet_ns *ns) · line 324

Quoted 1 line
drivers/nvme/target/core.c +1 -0
+	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.

drivers/nvme/target/core.c static int nvmet_ns_enable_blk(struct nvmet_ns *ns) · line 328

Quoted 4 lines
drivers/nvme/target/core.c +4 -0
+
+	return ret;
+err:
+	filp_close(ns->filp, NULL);

This should be a fput instead.

drivers/nvme/target/core.c static int nvmet_ns_enable_blk(struct nvmet_ns *ns) · line 345

Quoted 12 lines

static int nvmet_ns_dev_enable(struct nvmet_ns *ns) {

drivers/nvme/target/core.c +9 -1
-	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.

drivers/nvme/target/core.c static void nvmet_ns_dev_disable(struct nvmet_ns *ns) · line 355

Quoted 10 lines

}

static void nvmet_ns_dev_disable(struct nvmet_ns *ns)

drivers/nvme/target/core.c +2 -1
@@ -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.

#7 lore raw

PATCH 5/20 nvmet: add NVMe I/O command handlers for file

In reply to Chaitanya Kulkarni

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(-)
drivers/nvme/target/io-cmd.c +142 -4
index 28bbdff4a88b..7791a63e38ab 100644
@@ -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,
drivers/nvme/target/nvmet.h +1 -0
index acc61ca97198..21825b4a222a 100644
@@ -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
Signature · 1 line
2.14.1
#8 lore raw

PATCH 5/20 nvmet: add NVMe I/O command handlers for file

Quoted 1 line

+#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.

drivers/nvme/target/io-cmd.c static void nvmet_execute_rw(struct nvmet_req *req) · line 100

Quoted 6 lines
drivers/nvme/target/io-cmd.c +6 -0
+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.

drivers/nvme/target/io-cmd.c static void nvmet_execute_rw(struct nvmet_req *req) · line 112

Quoted 8 lines
drivers/nvme/target/io-cmd.c +8 -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;

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

drivers/nvme/target/io-cmd.c static void nvmet_execute_rw(struct nvmet_req *req) · line 123

Quoted 4 lines
drivers/nvme/target/io-cmd.c +4 -0
+	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?

Quoted 7 lines
drivers/nvme/target/io-cmd.c +5 -0
+
+	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.

drivers/nvme/target/io-cmd.c static void nvmet_execute_rw(struct nvmet_req *req) · line 138

Quoted 1 line
drivers/nvme/target/io-cmd.c +1 -0
+	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.

drivers/nvme/target/io-cmd.c static void nvmet_execute_rw(struct nvmet_req *req) · line 147

Quoted 4 lines
drivers/nvme/target/io-cmd.c +4 -0
+	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.

drivers/nvme/target/io-cmd.c static void nvmet_execute_flush(struct nvmet_req *req) · line 176

Quoted 6 lines
drivers/nvme/target/io-cmd.c +6 -0
+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.

#9 lore raw

PATCH 6/20 nvmet: add new members for sync file I/O

In reply to Chaitanya Kulkarni

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(+)
drivers/nvme/target/nvmet.h +3 -0
index 21825b4a222a..96aa5fc72467 100644
@@ -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);
Signature · 1 line
2.14.1
#10 lore raw

PATCH 6/20 nvmet: add new members for sync file I/O

Quoted 4 lines · Chaitanya Kulkarni

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.

#11 lore raw

PATCH 7/20 nvmet: add sync I/O configfs attr for ns

In reply to Chaitanya Kulkarni

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(+)
drivers/nvme/target/configfs.c +27 -0
index e6b2d2af81b6..bc95be1276c6 100644
@@ -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,
 };
 
Signature · 1 line
2.14.1
#12 lore raw

PATCH 8/20 nvmet: configure file backed ns for sync IO

In reply to Chaitanya Kulkarni

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(-)
drivers/nvme/target/core.c +17 -1
index b7c51a5629bf..4d25038ed05c 100644
@@ -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;
 }
Signature · 1 line
2.14.1
#13 lore raw

PATCH 8/20 nvmet: configure file backed ns for sync IO

Quoted 4 lines · Chaitanya Kulkarni

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?

#14 lore raw

PATCH 8/20 nvmet: configure file backed ns for sync IO

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?

Quoted 13 lines · Christoph Hellwig
Quoted 4 lines · Chaitanya Kulkarni

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

#15 lore raw

PATCH 9/20 nvmet: use workqueue for sync I/O

In reply to Chaitanya Kulkarni

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(-)
drivers/nvme/target/io-cmd.c +23 -4
index 7791a63e38ab..6d355b2dac56 100644
@@ -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;
Signature · 1 line
2.14.1
#16 lore raw

PATCH 10/20 nvmet: isolate id ctrl/ns field initialization

In reply to Chaitanya Kulkarni

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(-)
drivers/nvme/target/admin-cmd.c +38 -26
index 602995f7a1b3..97fea3f1d3e4 100644
@@ -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));
 
Signature · 1 line
2.14.1
#17 lore raw

PATCH 11/20 nvmet: introduce new members for ns-mgmt

In reply to Chaitanya Kulkarni

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(+)
drivers/nvme/target/nvmet.h +6 -0
index 96aa5fc72467..b185a2add99b 100644
@@ -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)
Signature · 1 line
2.14.1
#18 lore raw

PATCH 12/20 nvmet: add a configfs entry for subsys mount path

In reply to Chaitanya Kulkarni

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(-)
drivers/nvme/target/configfs.c +104 -1
index bc95be1276c6..f571dffbecdc 100644
@@ -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,
Signature · 1 line
2.14.1
#19 lore raw

PATCH 13/20 nvmet: initialize new ns and subsys members

In reply to Chaitanya Kulkarni

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(+)
drivers/nvme/target/core.c +3 -0
index 4d25038ed05c..34a2b8903ec0 100644
@@ -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);
 }
 
Signature · 1 line
2.14.1
#20 lore raw

PATCH 14/20 nvmet: add and restructure ns mgmt helpers

In reply to Chaitanya Kulkarni

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(-)
drivers/nvme/target/admin-cmd.c +51 -0
index 97fea3f1d3e4..e696f1e45e3a 100644
@@ -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);
drivers/nvme/target/core.c +0 -13
index 34a2b8903ec0..0b0649dc705c 100644
@@ -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;
drivers/nvme/target/nvmet.h +1 -0
index b185a2add99b..a365dc44a10e 100644
@@ -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 */
Signature · 1 line
2.14.1
#21 lore raw

PATCH 15/20 fs: export kern_path_locked() to reuse the code

In reply to Chaitanya Kulkarni

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(+)
fs/namei.c +1 -0
index cafa365eeb70..9a57e33175c4 100644
@@ -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)
 {
Signature · 1 line
2.14.1
#22 lore raw

PATCH 16/20 nvmet: add ns-mgmt command handlers

In reply to Chaitanya Kulkarni

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(+)
drivers/nvme/target/admin-cmd.c +169 -0
index e696f1e45e3a..b60477c50880 100644
@@ -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,
Signature · 1 line
2.14.1
#23 lore raw

PATCH 17/20 nvmet: override identify for file backed ns

In reply to Chaitanya Kulkarni

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(+)
drivers/nvme/target/admin-cmd.c +75 -0
index b60477c50880..19b93952bd16 100644
@@ -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);
Signature · 1 line
2.14.1
#24 lore raw

PATCH 18/20 nvmet: allow host to configure sync vs direct IO

In reply to Chaitanya Kulkarni

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(-)
drivers/nvme/target/admin-cmd.c +54 -0
index 19b93952bd16..b119a31eb6f9 100644
@@ -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:
drivers/nvme/target/configfs.c +0 -27
index f571dffbecdc..152d180af94b 100644
@@ -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,
 };
 
drivers/nvme/target/core.c +4 -4
index 0b0649dc705c..36f0dacec042 100644
@@ -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;
 }
drivers/nvme/target/io-cmd.c +2 -2
index 6d355b2dac56..96bb17353a29 100644
@@ -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);
drivers/nvme/target/nvmet.h +5 -1
index a365dc44a10e..acb9c266573b 100644
@@ -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 ? */
 };
Signature · 1 line
2.14.1
#25 lore raw

PATCH 19/20 nvmet: add check for ctrl processing paused status

In reply to Chaitanya Kulkarni

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(+)
drivers/nvme/target/core.c +50 -0
index 36f0dacec042..0c5b69b638ed 100644
@@ -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 =
drivers/nvme/target/nvmet.h +2 -0
index acb9c266573b..bd9d5b8a6a30 100644
@@ -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);
Signature · 1 line
2.14.1
#26 lore raw

PATCH 20/20 nvmet: use processing paused state for VWC

In reply to Chaitanya Kulkarni

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(+)
drivers/nvme/target/admin-cmd.c +8 -0
index b119a31eb6f9..f1d4005f92b0 100644
@@ -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);
 }
 
Signature · 1 line
2.14.1
#27 lore raw

cover nvmet: add support for file backed namespaces

In reply to Chaitanya Kulkarni

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.

#28 lore raw

cover nvmet: add support for file backed namespaces

Thanks for the review comments.

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

Quoted 13 lines · Christoph Hellwig

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