Skip to contentSkip to conversation

nvmet: add simple file backed ns support

PATCH Superseded Chaitanya Kulkarni 3 participants 4 messages May 23, 2018 – May 25, 2018

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

PATCH v7 1/1 nvmet: add simple file backed ns support

This patch adds simple file backed namespace support for NVMeOF target.

In the 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) via target side configfs interface.

For admin smart log command on the target side, we only use block device backed namespaces to compute target side smart log.

The new file io-cmd-file.c is responsible for handling the code for I/O commands when ns is file backed. Also, we introduce mempools based slow path using sync I/Os for file backed ns to ensure forward progress under reclaim.

With introduction of the new handle type (file) we also restructure the code for block device I/O commands where we rename the io-cmd.c -> io->cmd-bdev.c, use "nvmet_bdev_" prefix to
I/O command handler function and introduce
nvmet_[bdev | file]_ns_[enable | disable]() variants for block device and file in respective files.

Signed-off-by Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>

---

Changes Since V6 :- (Codebase :- 4.17.0-rc6)

1. Apply Christoph's cleanup changes.
2. Make a slight change in the cleanup patch, a call to completion routine from nvmet_file_execute_rw() now uses error value only when ret < 0 (error case) otherwise total_len. Without this change ret will hold the value return for the last I/O, this will fail for multiple iterations of the successful sync I/O case where we need to pass total I/O size (total_len) for completion routine.

Changes since V5:- (Codebase :- 4.17.0-rc5)

1. Rename io-cmd.c to io-cmd-bdev.c, and add "nvmet_bdev" prefix to I/O command handlers in the same file.
2. Move bdev and file ns enable|disable functions to io-cmd-bdev.c io-cmd-file.c
3. Change the kmem_cache identifier from "nvmet-fs" -> "nvmet-bvec".
4. Remove error print for nvmet_parse_io_cmd() for invalid handle type.
5. Trim the file header for io-cmd-file.c.
6. Remove typedefs for function pointers.
7. Restructure the file I/O code and aggregate slow and fast path into one function.
8. Rename NVMET_MAX_BVEC -> NVMET_MAX_MPOOL_BVEC.
9. Move nvmet_rw_len() to nvmet.h and keep it inline.
10. Define NVMET_MPOOL_MIN_OBJ directly.
11. Rename ns->cachep -> ns->bvec_cache and ns->mempool -> ns->bvec_pool.
12. Only issue mempool backed sync I/O when
nr_bvec > NVMET_MAX_MPOOL_BVEC.

Changes since V4:-

1. Adjust the code for 4.17.0-rc5.
2. Change name of nvmet_parse_io_cmd -> nvmet_parse_io_cmd_blkdev() in io-cmd.c.
3. Isolate the file backed ns I/O command handlers to separate file io-cmd-file.c.
4. Integrate Christoph's relative patch to use anonymous unions and file I/O command parse cleanup.
5. Introduce per namespace mempool to ensure forward progress under reclaim for each namespace.
6. Introduce centralize I/O command parse function to parse I/O commands based on the ns handle (device and file).
7. Introduce slow path for read/write commands, here we iterate through bvec allocated from mempool to complete file read/write handling.
8. Restructure nvmet_execute_rw_file() to handle new allocations and slow path.

Changes since V3:-

1. Fix the comment in the smart-log target code.
2. Change suffix from blk to blkdev.
3. Get rid of flags variable in nvmet_ns_enable_[blkdev|file]().
4. Skip the error print when path is not block device for nvmet_ns_enable_blkkdev().
5. Return PTR_ERR if blkdev open fails.
6. Remove NULL check after filp_open() and return PTR_ERR on filp_open() error.
7. Use AT_STATX_FORCE_SYNC and STATX_SIZE for vfs_getattr().
8. Separate line for each assignment.
9. Remove new lines before return.
10. Remove nvmet_ns_dev_enable() and call nvmet_ns_enable_[blkdev|file]() to enable ns handle from nvmet_ns_enable().
11. Use workqueue for discard and write zeroes.
12. Separate the block device and file parsing commands.
13. Move #include <linux/file.h> core.c (needed for fput) and

    #include <linux/falloc.h> io-cmd.c from nvmet.h.

14. Unionize file and block device handles.
15. Only initialize iocb for file IO.
16. Add WARN_ON_ONCE() in nvmet_ns_dev_disable().

Note :- Replacing kmalloc_array() with bvec_alloc() is not done in this
version, as discussion is in progress.

Changes since V2:-

1. Adjust this patch for 4.17.0-rc4.

Changes since V1:-

1. Replace filp to file, filp_close to fput, call_[read|write]_iter() to [read|write]_iter().
2. Add code for the offloading flush operation for file-backed ns.
3. Aggregate the patches for O_DIRECT implementation.
4. Use inline bvec for smaller I/Os for file-backed ns.
5. Use file_inode() instead of open coding.
6. Get rid of bvec calculation macro and use DIV_ROUND_UP for the number of bvec calculation.
7. Avoid multiplication on the IO code patch.
8. Remove the req->bvec = NULL in IO completion callback.
9. Remove the sg mapping iterator and use simple for_each_sg_page() to iterate the pages.

---
 drivers/nvme/target/Makefile                    |   4 +-
 drivers/nvme/target/admin-cmd.c                 |   8 +
 drivers/nvme/target/core.c                      |  52 ++--
 drivers/nvme/target/{io-cmd.c => io-cmd-bdev.c} |  66 ++++--
 drivers/nvme/target/io-cmd-file.c               | 300 ++++++++++++++++++++++++
 drivers/nvme/target/nvmet.h                     |  28 ++-
 6 files changed, 415 insertions(+), 43 deletions(-)
 rename drivers/nvme/target/{io-cmd.c => io-cmd-bdev.c} (77%)
 create mode 100644 drivers/nvme/target/io-cmd-file.c
drivers/nvme/target/Makefile +2 -2
index 4882501..8118c93 100644
@@ -6,8 +6,8 @@ obj-$(CONFIG_NVME_TARGET_RDMA)		+= nvmet-rdma.o
 obj-$(CONFIG_NVME_TARGET_FC)		+= nvmet-fc.o
 obj-$(CONFIG_NVME_TARGET_FCLOOP)	+= nvme-fcloop.o
 
-nvmet-y		+= core.o configfs.o admin-cmd.o io-cmd.o fabrics-cmd.o \
-			discovery.o
+nvmet-y		+= core.o configfs.o admin-cmd.o fabrics-cmd.o \
+			discovery.o io-cmd-file.o io-cmd-bdev.o
 nvme-loop-y	+= loop.o
 nvmet-rdma-y	+= rdma.o
 nvmet-fc-y	+= fc.o
drivers/nvme/target/admin-cmd.c +8 -0
index 5e0e9fc..68a3457 100644
@@ -45,6 +45,10 @@ static u16 nvmet_get_smart_log_nsid(struct nvmet_req *req,
 		return NVME_SC_INVALID_NS;
 	}
 
+	/* we don't have the right data for file backed ns */
+	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) {
+		/* we don't have the right data for file backed ns */
+		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]);
drivers/nvme/target/core.c +35 -17

87 lines not on this page · raw message

drivers/nvme/target/io-cmd-bdev.c renamed from drivers/nvme/target/io-cmd.c +44 -22

145 lines not on this page · raw message

drivers/nvme/target/io-cmd-file.c new file +300 -0

300 lines not on this page · raw message

drivers/nvme/target/nvmet.h +26 -2
index 15fd84a..2d09afc 100644
@@ -43,6 +43,7 @@ struct nvmet_ns {
 	struct list_head	dev_link;
 	struct percpu_ref	ref;
 	struct block_device	*bdev;
+	struct file		*file;
 	u32			nsid;
 	u32			blksize_shift;
 	loff_t			size;
@@ -57,6 +58,8 @@ struct nvmet_ns {
 	struct config_group	group;
 
 	struct completion	disable_done;
+	mempool_t		*bvec_pool;
+	struct kmem_cache	*bvec_cache;
 };
 
 static inline struct nvmet_ns *to_nvmet_ns(struct config_item *item)
@@ -222,8 +225,18 @@ struct nvmet_req {
 	struct nvmet_cq		*cq;
 	struct nvmet_ns		*ns;
 	struct scatterlist	*sg;
-	struct bio		inline_bio;
 	struct bio_vec		inline_bvec[NVMET_MAX_INLINE_BIOVEC];
+	union {
+		struct {
+			struct bio      inline_bio;
+		} b;
+		struct {
+			bool			mpool_alloc;
+			struct kiocb            iocb;
+			struct bio_vec          *bvec;
+			struct work_struct      work;
+		} f;
+	};
 	int			sg_cnt;
 	/* data length as parsed from the command: */
 	size_t			data_len;
@@ -263,7 +276,8 @@ struct nvmet_async_event {
 };
 
 u16 nvmet_parse_connect_cmd(struct nvmet_req *req);
-u16 nvmet_parse_io_cmd(struct nvmet_req *req);
+u16 nvmet_bdev_parse_io_cmd(struct nvmet_req *req);
+u16 nvmet_file_parse_io_cmd(struct nvmet_req *req);
 u16 nvmet_parse_admin_cmd(struct nvmet_req *req);
 u16 nvmet_parse_discovery_cmd(struct nvmet_req *req);
 u16 nvmet_parse_fabrics_cmd(struct nvmet_req *req);
@@ -338,4 +352,14 @@ extern struct rw_semaphore nvmet_config_sem;
 bool nvmet_host_allowed(struct nvmet_req *req, struct nvmet_subsys *subsys,
 		const char *hostnqn);
 
+int nvmet_bdev_ns_enable(struct nvmet_ns *ns);
+int nvmet_file_ns_enable(struct nvmet_ns *ns);
+void nvmet_bdev_ns_disable(struct nvmet_ns *ns);
+void nvmet_file_ns_disable(struct nvmet_ns *ns);
+
+static inline u32 nvmet_rw_len(struct nvmet_req *req)
+{
+	return ((u32)le16_to_cpu(req->cmd->rw.length) + 1) <<
+			req->ns->blksize_shift;
+}
 #endif /* _NVMET_H */
Signature · 1 line
2.9.5
#1 lore raw

PATCH v7 1/1 nvmet: add simple file backed ns support

drivers/nvme/target/io-cmd-file.c line 166

Quoted 2 lines
drivers/nvme/target/io-cmd-file.c new file +2 -0
+		if (unlikely(is_sync) &&
+			(nr_bvec - 1 == 0 || bv_cnt == NVMET_MAX_MPOOL_BVEC)) {

Nitpick (can be fixed when applying): Either align to the condition or by two tabs, not by one tab as the code inside the block. E.g. in this case I'd do:

		if (unlikely(is_sync) &&
		    (nr_bvec - 1 == 0 || bv_cnt == NVMET_MAX_MPOOL_BVEC)) {

Otherwise looks good,

Reviewed-by Christoph Hellwig <hch at lst.de>
#2 lore raw

PATCH v7 1/1 nvmet: add simple file backed ns support

[CK] It really looked weird?when I aligned it to the start, I guess I need to get used to it. BTW, I have got the caching patch ready, I'll send shortly.

From: Christoph Hellwig <[email protected]>
Sent: Tuesday, May 22, 2018 11:10:03 PM
To: Chaitanya Kulkarni
Cc: linux-nvme at lists.infradead.org; hch at lst.de; keith.busch at intel.com; sagi at grimberg.me; Damien Le Moal
Subject: Re: [PATCH V7] nvmet: add simple file backed ns support
?

Quoted 2 lines

+???????????? if (unlikely(is_sync) &&
+???????????????????? (nr_bvec - 1 == 0 || bv_cnt == NVMET_MAX_MPOOL_BVEC)) {

Nitpick (can be fixed when applying):? Either align to the condition or by two tabs, not by one tab as the code inside the block.? E.g. in this case I'd do:

??????????????? if (unlikely(is_sync) &&
??????????????????? (nr_bvec - 1 == 0 || bv_cnt == NVMET_MAX_MPOOL_BVEC)) {

Otherwise looks good,

Reviewed-by Christoph Hellwig <hch at lst.de>