nvmet: Back namespace with files

RFC PATCH New Keith Busch 5 participants 12 messages Mar 8, 2017 – Mar 13, 2017

Subsystems (4)

VFS NVMe exec and binfmt NVMe target

lore

Share & tools

lore

Keith Busch

author

2017-03-08 21:35 UTC #0 lore raw

RFC PATCH 1/1 nvmet: Back namespace with files

This RFC allows NVMeOF targets to be backed by files. Prior to this, only block device files were supported, but this patch will let you use any file, and continue working on block files if requested.

Backing with files has more flexibility on what a namespace can be, and it isn't difficult from here to add NVMe target support for caching, provisioning, namespace management, and all the features Linux filesystems provide.

This temporarily removes the admin smart log support, to be re-implemented later. It was incorrect anyway since the smart log is supposed to report device lifetime statistics, but we're just showing since hd_struct creation.

I only ported the loop target. The fc and rdma targets are a little more work, so this looked like a good stopping point to guage interest.

Signed-off-by Keith Busch <keith.busch at intel.com>

---
 drivers/nvme/target/admin-cmd.c   |  54 +-------------
 drivers/nvme/target/core.c        |  54 +++++++++-----
 drivers/nvme/target/fabrics-cmd.c |   6 +-
 drivers/nvme/target/io-cmd.c      | 153 ++++++++------------------------------
 drivers/nvme/target/loop.c        |  45 ++++++++---
 drivers/nvme/target/nvmet.h       |  10 +--
 fs/exec.c                         |  18 ++---
 fs/splice.c                       |  24 ++++--
 include/linux/fs.h                |   5 ++
 9 files changed, 136 insertions(+), 233 deletions(-)

drivers/nvme/target/admin-cmd.c +2 ~~-52~~

index 94e524f..2a8152d 100644
@@ -33,63 +33,13 @@ u32 nvmet_get_log_page_len(struct nvme_command *cmd)
 static u16 nvmet_get_smart_log_nsid(struct nvmet_req *req,
 		struct nvme_smart_log *slog)
 {
-	u16 status;
-	struct nvmet_ns *ns;
-	u64 host_reads, host_writes, data_units_read, data_units_written;
-
-	status = NVME_SC_SUCCESS;
-	ns = nvmet_find_namespace(req->sq->ctrl, req->cmd->get_log_page.nsid);
-	if (!ns) {
-		status = NVME_SC_INVALID_NS;
-		pr_err("nvmet : Could not find namespace id : %d\n",
-				le32_to_cpu(req->cmd->get_log_page.nsid));
-		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]);
-	data_units_written = part_stat_read(ns->bdev->bd_part, sectors[WRITE]);
-
-	put_unaligned_le64(host_reads, &slog->host_reads[0]);
-	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]);
-	nvmet_put_namespace(ns);
-out:
-	return status;
+	return NVME_SC_SUCCESS;
 }
 
 static u16 nvmet_get_smart_log_all(struct nvmet_req *req,
 		struct nvme_smart_log *slog)
 {
-	u16 status;
-	u64 host_reads = 0, host_writes = 0;
-	u64 data_units_read = 0, data_units_written = 0;
-	struct nvmet_ns *ns;
-	struct nvmet_ctrl *ctrl;
-
-	status = NVME_SC_SUCCESS;
-	ctrl = req->sq->ctrl;
-
-	rcu_read_lock();
-	list_for_each_entry_rcu(ns, &ctrl->subsys->namespaces, dev_link) {
-		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]);
-		data_units_written +=
-			part_stat_read(ns->bdev->bd_part, sectors[WRITE]);
-
-	}
-	rcu_read_unlock();
-
-	put_unaligned_le64(host_reads, &slog->host_reads[0]);
-	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]);
-
-	return status;
+	return NVME_SC_SUCCESS;
 }
 
 static u16 nvmet_get_smart_log(struct nvmet_req *req,

drivers/nvme/target/core.c +37 ~~-17~~

93 lines not on this page · raw message

drivers/nvme/target/fabrics-cmd.c +2 -4

index 8bd022af..8f0868f 100644
@@ -122,7 +122,7 @@ static void nvmet_execute_admin_connect(struct nvmet_req *req)
 	struct nvmet_ctrl *ctrl = NULL;
 	u16 status = 0;
 
-	d = kmap(sg_page(req->sg)) + req->sg->offset;
+	d = req->kvec[0].iov_base;
 
 	/* zero out initial completion result, assign values as needed */
 	req->rsp->result.u32 = 0;

@@ -158,7 +158,6 @@ static void nvmet_execute_admin_connect(struct nvmet_req *req)
 	req->rsp->result.u16 = cpu_to_le16(ctrl->cntlid);
 
 out:
-	kunmap(sg_page(req->sg));
 	nvmet_req_complete(req, status);
 }

@@ -170,7 +169,7 @@ static void nvmet_execute_io_connect(struct nvmet_req *req)
 	u16 qid = le16_to_cpu(c->qid);
 	u16 status = 0;
 
-	d = kmap(sg_page(req->sg)) + req->sg->offset;
+	d = req->kvec[0].iov_base;
 
 	/* zero out initial completion result, assign values as needed */
 	req->rsp->result.u32 = 0;

@@ -205,7 +204,6 @@ static void nvmet_execute_io_connect(struct nvmet_req *req)
 	pr_info("adding queue %d to ctrl %d.\n", qid, ctrl->cntlid);
 
 out:
-	kunmap(sg_page(req->sg));
 	nvmet_req_complete(req, status);
 	return;

drivers/nvme/target/io-cmd.c +29 ~~-124~~

202 lines not on this page · raw message

drivers/nvme/target/loop.c +33 ~~-12~~

83 lines not on this page · raw message

drivers/nvme/target/nvmet.h +3 -7

index 1370eee..e466636 100644
@@ -41,7 +41,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;

@@ -206,18 +206,14 @@ struct nvmet_fabrics_ops {
 	void (*delete_ctrl)(struct nvmet_ctrl *ctrl);
 };
 
-#define NVMET_MAX_INLINE_BIOVEC	8
-
 struct nvmet_req {
 	struct nvme_command	*cmd;
 	struct nvme_completion	*rsp;
 	struct nvmet_sq		*sq;
 	struct nvmet_cq		*cq;
 	struct nvmet_ns		*ns;
-	struct scatterlist	*sg;
-	struct bio		inline_bio;
-	struct bio_vec		inline_bvec[NVMET_MAX_INLINE_BIOVEC];
-	int			sg_cnt;
+	struct kvec		*kvec;
+	int			vlen;
 	size_t			data_len;
 
 	struct nvmet_port	*port;

fs/exec.c +7 ~~-11~~

index 698a860..cd45195 100644
@@ -859,21 +859,17 @@ struct file *open_exec(const char *name)
 }
 EXPORT_SYMBOL(open_exec);
 
+
 int kernel_read(struct file *file, loff_t offset,
 		char *addr, unsigned long count)
 {
-	mm_segment_t old_fs;
-	loff_t pos = offset;
-	int result;
-
-	old_fs = get_fs();
-	set_fs(get_ds());
-	/* The cast to a user pointer is valid due to the set_fs() */
-	result = vfs_read(file, (void __user *)addr, count, &pos);
-	set_fs(old_fs);
-	return result;
-}
+	struct kvec vec = {
+		.iov_base = addr,
+		.iov_len = count,
+	};
 
+	return kernel_readv(file, &vec, 1, offset);
+}
 EXPORT_SYMBOL(kernel_read);
 
 int kernel_read_file(struct file *file, void **buf, loff_t *size,

fs/splice.c +18 -6

index 4ef78aa..5ac63cc 100644
@@ -351,24 +351,24 @@ const struct pipe_buf_operations nosteal_pipe_buf_ops = {
 };
 EXPORT_SYMBOL(nosteal_pipe_buf_ops);
 
-static ssize_t kernel_readv(struct file *file, const struct kvec *vec,
+ssize_t kernel_readv(struct file *file, const struct kvec *vec,
 			    unsigned long vlen, loff_t offset)
 {
 	mm_segment_t old_fs;
-	loff_t pos = offset;
 	ssize_t res;
 
 	old_fs = get_fs();
 	set_fs(get_ds());
 	/* The cast to a user pointer is valid due to the set_fs() */
-	res = vfs_readv(file, (const struct iovec __user *)vec, vlen, &pos, 0);
+	res = vfs_readv(file, (const struct iovec __user *)vec, vlen, &offset, 0);
 	set_fs(old_fs);
 
 	return res;
 }
+EXPORT_SYMBOL(kernel_readv);
 
-ssize_t kernel_write(struct file *file, const char *buf, size_t count,
-			    loff_t pos)
+ssize_t kernel_writev(struct file *file, const struct kvec *vec,
+			    unsigned long vlen, loff_t offset)
 {
 	mm_segment_t old_fs;
 	ssize_t res;

@@ -376,11 +376,23 @@ ssize_t kernel_write(struct file *file, const char *buf, size_t count,
 	old_fs = get_fs();
 	set_fs(get_ds());
 	/* The cast to a user pointer is valid due to the set_fs() */
-	res = vfs_write(file, (__force const char __user *)buf, count, &pos);
+	res = vfs_writev(file, (const struct iovec __user *)vec, vlen, &offset, 0);
 	set_fs(old_fs);
 
 	return res;
 }
+EXPORT_SYMBOL(kernel_writev);
+
+ssize_t kernel_write(struct file *file, const char *buf, size_t count,
+			    loff_t pos)
+{
+	struct kvec vec = {
+		.iov_base = (void *)buf,
+		.iov_len = count,
+	};
+
+	return kernel_writev(file, &vec, 1, pos);
+}
 EXPORT_SYMBOL(kernel_write);
 
 static ssize_t default_file_splice_read(struct file *in, loff_t *ppos,

include/linux/fs.h +5 -0

index c930cbc..854eb68 100644
@@ -31,6 +31,7 @@
 #include <linux/workqueue.h>
 #include <linux/percpu-rwsem.h>
 #include <linux/delayed_call.h>
+#include <linux/uio.h>
 
 #include <asm/byteorder.h>
 #include <uapi/linux/fs.h>

@@ -2654,6 +2655,8 @@ static inline const char *kernel_read_file_id_str(enum kernel_read_file_id id)
 }
 
 extern int kernel_read(struct file *, loff_t, char *, unsigned long);
+extern ssize_t kernel_readv(struct file *file, const struct kvec *vec,
+			    unsigned long vlen, loff_t offset);
 extern int kernel_read_file(struct file *, void **, loff_t *, loff_t,
 			    enum kernel_read_file_id);
 extern int kernel_read_file_from_path(char *, void **, loff_t *, loff_t,

@@ -2661,6 +2664,8 @@ extern int kernel_read_file_from_path(char *, void **, loff_t *, loff_t,
 extern int kernel_read_file_from_fd(int, void **, loff_t *, loff_t,
 				    enum kernel_read_file_id);
 extern ssize_t kernel_write(struct file *, const char *, size_t, loff_t);
+extern ssize_t kernel_writev(struct file *file, const struct kvec *vec,
+			    unsigned long vlen, loff_t offset);
 extern ssize_t __kernel_write(struct file *, const char *, size_t, loff_t *);
 extern struct file * open_exec(const char *);

Signature · 1 line

2.5.5

Christoph Hellwig

maintainer

2017-03-08 21:35 UTC #1 lore raw

RFC PATCH 1/1 nvmet: Back namespace with files

Wrong kind of interface - this needs to use vfs_iter_read/write of type ITER_BVEC.

And before merging anything I'd really like to see how this is an improvement over simply using the loop device.

Keith Busch

author

2017-03-08 22:15 UTC #2 lore raw

RFC PATCH 1/1 nvmet: Back namespace with files

Quoted 2 lines · Christoph Hellwig

Wrong kind of interface - this needs to use vfs_iter_read/write of type ITER_BVEC.

Gotchya, that's what I was locking for. Thanks for the pointer.

Quoted 2 lines

And before merging anything I'd really like to see how this is an improvement over simply using the loop device.

This is a step toward getting namespace management. It's easy to create namespaces if they're files.

Christoph Hellwig

maintainer

2017-03-08 22:20 UTC #3 lore raw

RFC PATCH 1/1 nvmet: Back namespace with files

Quoted 2 lines · Keith Busch

This is a step toward getting namespace management. It's easy to create namespaces if they're files.

Given that namespace management is tied to controller IDs it's a fairly bad fit for fabrics, especially so with our dynamic controller model.

Keith Busch

author

2017-03-09 17:41 UTC #4 lore raw

RFC PATCH 1/1 nvmet: Back namespace with files

Quoted 2 lines · Christoph Hellwig

Given that namespace management is tied to controller IDs it's a fairly bad fit for fabrics, especially so with our dynamic controller model.

Hm, implementation details aside for a second, isn't namespace management more useful on fabrics than pci? It's like managing LUNs on a SAN, but with spec defined commands.

Christoph Hellwig

maintainer

2017-03-09 17:39 UTC #5 lore raw

RFC PATCH 1/1 nvmet: Back namespace with files

Quoted 7 lines · Keith Busch

Quoted 2 lines · Christoph Hellwig
Given that namespace management is tied to controller IDs it's a fairly bad fit for fabrics, especially so with our dynamic controller model.
Hm, implementation details aside for a second, isn't namespace management more useful on fabrics than pci? It's like managing LUNs on a SAN, but with spec defined commands.

It would be more useful, but for that it should operate on good persistent identifier, e.g. Host NQNs.

Sagi Grimberg

maintainer

2017-03-13 08:32 UTC #6 lore raw

RFC PATCH 1/1 nvmet: Back namespace with files

In reply to Keith Busch

Quoted 6 lines

Quoted 2 lines
Given that namespace management is tied to controller IDs it's a fairly bad fit for fabrics, especially so with our dynamic controller model.
Hm, implementation details aside for a second, isn't namespace management more useful on fabrics than pci? It's like managing LUNs on a SAN, but with spec defined commands.

Not exactly, usually namespace/lun provisioning is something that a given host does not typically do or even aware of. Unlike in PCIe, in fabrics, the host does not really own "the" subsystem, it just owns a virtual subsystem that the target exposed for it.

If we do support namespace management in the linux target, it'd need to be emulated somehow, obviously a host cannot simply add unprovisioned resources.

Keith Busch

author

2017-03-13 16:11 UTC #7 lore raw

RFC PATCH 1/1 nvmet: Back namespace with files

Quoted 4 lines · Sagi Grimberg

Quoted 3 lines
Hm, implementation details aside for a second, isn't namespace management more useful on fabrics than pci? It's like managing LUNs on a SAN, but with spec defined commands.

Quoted 8 lines

Not exactly, usually namespace/lun provisioning is something that a given host does not typically do or even aware of. Unlike in PCIe, in fabrics, the host does not really own "the" subsystem, it just owns a virtual subsystem that the target exposed for it.
If we do support namespace management in the linux target, it'd need to be emulated somehow, obviously a host cannot simply add unprovisioned resources.

Yes, the RFC was light on details, and I'm probably getting ahead of myself with the file-as-a-namespaces suggestion.

I'd like an NVMe subsystem to be provided a pool of storage that it may dynamically carve into namespaces and attach to connected hosts as needed. I also want hosts to do this using the spec defined Namespace Management commands. That isn't all that unusual from what I hear.

This doesn't really require a filesystem. Maybe LVM is more appropriate, but I think that'd require userspace to manage the LVs. Still, file backed namespaces has uses if only for testing purposes, like LIO's FILEIO.

Hannes Reinecke

2017-03-13 16:07 UTC #8 lore raw

RFC PATCH 1/1 nvmet: Back namespace with files

Quoted 17 lines · Keith Busch

Quoted 4 lines · Sagi Grimberg
Quoted 3 lines
Hm, implementation details aside for a second, isn't namespace management more useful on fabrics than pci? It's like managing LUNs on a SAN, but with spec defined commands.
Quoted 8 lines
Not exactly, usually namespace/lun provisioning is something that a given host does not typically do or even aware of. Unlike in PCIe, in fabrics, the host does not really own "the" subsystem, it just owns a virtual subsystem that the target exposed for it.
If we do support namespace management in the linux target, it'd need to be emulated somehow, obviously a host cannot simply add unprovisioned resources.
Yes, the RFC was light on details, and I'm probably getting ahead of myself with the file-as-a-namespaces suggestion.

Quoted 9 lines

I'd like an NVMe subsystem to be provided a pool of storage that it may dynamically carve into namespaces and attach to connected hosts as needed. I also want hosts to do this using the spec defined Namespace Management commands. That isn't all that unusual from what I hear.
This doesn't really require a filesystem. Maybe LVM is more appropriate, but I think that'd require userspace to manage the LVs. Still, file backed namespaces has uses if only for testing purposes, like LIO's FILEIO.

I'm not against this in principle, but can we make the selectable? IE not ripping out the original implementation, but rather have them both side-by-side and let the admin decide what he wants to use?

Cheers,

Hannes

Signature · 5 lines

Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare at suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: F. Imend?rffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG N?rnberg)

Christoph Hellwig

maintainer

2017-03-13 16:33 UTC #9 lore raw

RFC PATCH 1/1 nvmet: Back namespace with files

In reply to Keith Busch

Quoted 2 lines · Keith Busch

Yes, the RFC was light on details, and I'm probably getting ahead of myself with the file-as-a-namespaces suggestion.

No, it's a good idea. The big question is what we win with explicit support in nvmet over just using the loop device. I actually hacked up support earlier and it didn't show any performance advantage. That being said we now have the ITER_BVEC type and proper support for in-kernel aio, so the code might become simple enough to just take it.

J Freyensee

2017-03-13 18:00 UTC #10 lore raw

RFC PATCH 1/1 nvmet: Back namespace with files

Quoted 11 lines · Christoph Hellwig

Quoted 3 lines · Keith Busch
Yes, the RFC was light on details, and I'm probably getting ahead of myself with the file-as-a-namespaces suggestion.
No, it's a good idea.??The big question is what we win with explicit support in nvmet over just using the loop device.??I actually hacked up support earlier and it didn't show any performance advantage. That being said we now have the ITER_BVEC type and proper support for in-kernel aio, so the code might become simple enough to just take it.

If it's easier to setup and maintain and use that could suffice for a lack of performance advantage.

Quoted 5 lines

_______________________________________________
Linux-nvme mailing list
Linux-nvme at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Sagi Grimberg

maintainer

2017-03-08 23:24 UTC #11 lore raw

RFC PATCH 1/1 nvmet: Back namespace with files

In reply to Keith Busch

Keith,

Quoted 3 lines

This RFC allows NVMeOF targets to be backed by files. Prior to this, only block device files were supported, but this patch will let you use any file, and continue working on block files if requested.

I don't think this is the way to go. I really don't.

Quoted 4 lines

Backing with files has more flexibility on what a namespace can be, and it isn't difficult from here to add NVMe target support for caching, provisioning, namespace management, and all the features Linux filesystems provide.

As Christoph said, you need a *very* compelling justification why loop doesn't give you everything you need.

Quoted 2 lines

I only ported the loop target. The fc and rdma targets are a little more work, so this looked like a good stopping point to guage interest.

RDMA and FC is where sgls are actually important as they need dma mapping, which means that the kvec handling is something that needs to be pushed into the backend handling in io-cmd.c