nvmet: add simple file backed ns support

This patch adds simple file backed namespace support for 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) 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 for file backed ns.

---

Changes since V4:-

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

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 discusison 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      |   2 +-
 drivers/nvme/target/core.c        | 130 +++++++++++++---
 drivers/nvme/target/io-cmd-file.c | 312 ++++++++++++++++++++++++++++++++++++++
 drivers/nvme/target/io-cmd.c      |  25 +--
 drivers/nvme/target/nvmet.h       |  21 ++-
 5 files changed, 452 insertions(+), 38 deletions(-)
 create mode 100644 drivers/nvme/target/io-cmd-file.c

index 4882501..f5dfe65 100644
@@ -7,7 +7,7 @@ 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
+			discovery.o io-cmd-file.o
 nvme-loop-y	+= loop.o
 nvmet-rdma-y	+= rdma.o
 nvmet-fc-y	+= fc.o

index cd23441..8e6d7a6 100644
@@ -23,11 +23,11 @@ static void nvmet_bio_done(struct bio *bio)
 	nvmet_req_complete(req,
 		bio->bi_status ? NVME_SC_INTERNAL | NVME_SC_DNR : 0);
 
-	if (bio != &req->inline_bio)
+	if (bio != &req->b.inline_bio)
 		bio_put(bio);
 }
 
-static inline u32 nvmet_rw_len(struct nvmet_req *req)
+inline u32 nvmet_rw_len(struct nvmet_req *req)
 {
 	return ((u32)le16_to_cpu(req->cmd->rw.length) + 1) <<
 			req->ns->blksize_shift;

@@ -36,7 +36,7 @@ static inline u32 nvmet_rw_len(struct nvmet_req *req)
 static void nvmet_execute_rw(struct nvmet_req *req)
 {
 	int sg_cnt = req->sg_cnt;
-	struct bio *bio = &req->inline_bio;
+	struct bio *bio = &req->b.inline_bio;
 	struct scatterlist *sg;
 	sector_t sector;
 	blk_qc_t cookie;

@@ -91,7 +91,7 @@ static void nvmet_execute_rw(struct nvmet_req *req)
 
 static void nvmet_execute_flush(struct nvmet_req *req)
 {
-	struct bio *bio = &req->inline_bio;
+	struct bio *bio = &req->b.inline_bio;
 
 	bio_init(bio, req->inline_bvec, ARRAY_SIZE(req->inline_bvec));
 	bio_set_dev(bio, req->ns->bdev);

@@ -189,20 +189,9 @@ static void nvmet_execute_write_zeroes(struct nvmet_req *req)
 	}
 }
 
-u16 nvmet_parse_io_cmd(struct nvmet_req *req)
+u16 nvmet_parse_io_cmd_blkdev(struct nvmet_req *req)
 {
 	struct nvme_command *cmd = req->cmd;
-	u16 ret;
-
-	ret = nvmet_check_ctrl_status(req, cmd);
-	if (unlikely(ret)) {
-		req->ns = NULL;
-		return ret;
-	}
-
-	req->ns = nvmet_find_namespace(req->sq->ctrl, cmd->rw.nsid);
-	if (unlikely(!req->ns))
-		return NVME_SC_INVALID_NS | NVME_SC_DNR;
 
 	switch (cmd->common.opcode) {
 	case nvme_cmd_read:

@@ -223,8 +212,8 @@ u16 nvmet_parse_io_cmd(struct nvmet_req *req)
 		req->execute = nvmet_execute_write_zeroes;
 		return 0;
 	default:
-		pr_err("unhandled cmd %d on qid %d\n", cmd->common.opcode,
-		       req->sq->qid);
+		pr_err("unhandled cmd for blkdev ns %d on qid %d\n",
+				cmd->common.opcode, req->sq->qid);
 		return NVME_SC_INVALID_OPCODE | NVME_SC_DNR;
 	}
 }

index 15fd84a..215a206 100644
@@ -29,6 +29,8 @@
 
 #define NVMET_ASYNC_EVENTS		4
 #define NVMET_ERROR_LOG_SLOTS		128
+#define NVMET_MAX_BVEC			16
+#define NVMET_MPOOL_MIN_OBJ		NVMET_MAX_BVEC
 
 /* Helper Macros when NVMe error is NVME_SC_CONNECT_INVALID_PARAM
  * The 16 bit shift is to set IATTR bit to 1, which means offending

@@ -43,6 +45,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 +60,8 @@ struct nvmet_ns {
 	struct config_group	group;
 
 	struct completion	disable_done;
+	mempool_t		*mempool;
+	struct kmem_cache	*cachep;
 };
 
 static inline struct nvmet_ns *to_nvmet_ns(struct config_item *item)

@@ -222,8 +227,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 {
+			struct kiocb            iocb;
+			struct bio_vec          *bvec;
+			struct work_struct      work;
+			struct completion	io_complete;
+		} f;
+	};
 	int			sg_cnt;
 	/* data length as parsed from the command: */
 	size_t			data_len;

@@ -263,7 +278,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_parse_io_cmd_blkdev(struct nvmet_req *req);
+u16 nvmet_parse_io_cmd_file(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 +354,5 @@ extern struct rw_semaphore nvmet_config_sem;
 bool nvmet_host_allowed(struct nvmet_req *req, struct nvmet_subsys *subsys,
 		const char *hostnqn);
 
+inline u32 nvmet_rw_len(struct nvmet_req *req);
 #endif /* _NVMET_H */

2.9.5

Please use ~ 73 lines for the changelog to instead of line wrapping earlier.

	if (req->ns->file)
		return nvmet_parse_io_cmd_file(req);
	else
		return nvmet_parse_io_cmd_blkdev(req);

You can remove this whole boilerplate given that you have a SPDX tag.

	if (nr_bvec > NVMET_MAX_INLINE_BIOVEC) {
		req->f.bvec = kmalloc_array(nr_bvec, sizeof(struct bio_vec),
				GFP_KERNEL);
	} else {
		req->f.bvec = req->inline_bvec;
	}

	/* fallback under memory pressure */
	if (unlikely(!req->f.bvec)) {
		req->f.bvec = mempool_alloc(req->ns->mempool, GFP_KERNEL);
		if (nr_bvec > NVMET_MAX_MEMPOOL_BVEC)
			is_sync = true;
	}

Useful tip for vi users: http://vim.wikia.com/wiki/Chop_long_lines

The same method can combine shorter lines.

Hi Christoph/Sagi,

Thanks for the review.

I've fixed all the comments, except two, please have a look inline.

From: Christoph Hellwig <[email protected]>
Sent: Thursday, May 17, 2018 5:47 AM
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 V5] nvmet: add simple file backed ns support
?

Please use ~ 73 lines for the changelog to instead of line wrapping earlier.

what about nvmet-bvec as the name?

[CK] Also, while we are at it should I change the name of the? following functions?currently present?in the io-cmd.c and add suffix "bdev" ?

1.?nvmet_execute_write_zeroes
2. nvmet_execute_dsm
3. nvmet_execute_discard
4. nvmet_discard_range
5 .nvmet_discard_range
6. nvmet_execute_rw

You can remove this whole boilerplate given that you have a SPDX tag.

No need for all but the outer braces.

Instead of using your own completion, the synchronous case should just not set a ki_complete callback at all, which means the file system will handle the I/O synchronously for you.

That probably means you can share the actual low-level routine for doing I/O, just don't set ki_complete in the is_sync case, and have some sort of outer loop chunking up into nr_bvec sized chunks

This seems to always hit the slow path.? How about this instead:

??????? if (nr_bvec > NVMET_MAX_INLINE_BIOVEC) {
??????????????? req->f.bvec = kmalloc_array(nr_bvec, sizeof(struct bio_vec), ??????????????????????????????? GFP_KERNEL);
??????? } else {
??????????????? req->f.bvec = req->inline_bvec;
??????? }

??????? /* fallback under memory pressure */
??????? if (unlikely(!req->f.bvec)) {
??????????????? req->f.bvec = mempool_alloc(req->ns->mempool, GFP_KERNEL); ??????????????? if (nr_bvec > NVMET_MAX_MEMPOOL_BVEC) ??????????????????????? is_sync = true;

[CK] Here we only issue sync I/O when nr_bvec > NVMET_MAX_MEMPOOL_BVEC, when nr_bvec <=?NVMET_MAX_MEMPOOL_BVEC and we still allocate from mempool which will result in?async I/O. We have to make this allocation info (mempool vs kmalloc_arrat) accessible in the?callback function by?adding a new member to the struct nvmet_req anonymous union.
Instead of adding a new member, since we are under memory pressure here can we set is_sync unconditionally when we allocate from the mempool and force sync I/O? ?
? ? ? ? }

}

Also I think NVMET_MAX_BVEC should be NVMET_MAX_MEMPOOL_BVEC and the actual I/O functions really don't need most of the arguments as far as I can tell.? mempool_alloc with a GFP_KERNEL argument can't fail it will just block forever, so no need to chek the return value there.

I'd keep this as an inline and just move it to nvmet.h

I'd name these bvec_pool an bvec_cache.

Sounds good to me. And please use bdev everywhere instead of blk or blkdev.

nvmet_bdev_*

Sounds good.

I don't think that makes sense. A lot of I/Os under memory pressure are probably small, and there is no need to slow them down for no good reason.