NVMe over Fabrics: updates for req_op changes and misc fixes

NVMe over Fabrics: updates for req_op changes and misc fixes

[Christoph Hellwig]

Thanks Jens for pulling in the req_op changes from Mike!

This will require a few minor changes for the NVMe over Fabrics code, so
I've rebase the git tree on top of it, and merged all changes from review
and testing.  Below are the incremental changes vs the previous version,
which I plan to fold in before resending.  Nothing of this touches the
patches in the "general preparation for NVMe over Fabrics support" series,
which would be good to get in ASAP.

Also available as a git tree at:

	git://git.infradead.org/nvme-fabrics.git nvmf-all.2

Gitweb:

	http://git.infradead.org/nvme-fabrics.git/shortlog/refs/heads/nvmf-all.2

[PATCH 01/11] Revert "blk-mq: add blk_mq_alloc_request_hctx"

[Christoph Hellwig]

Just for book keeping. The next patch brings it back with some changes.

This reverts commit 98878e0f0b80520575b31147f7cff28e28d9a725.
---
 block/blk-mq.c         | 33 ---------------------------------
 include/linux/blk-mq.h |  2 --
 2 files changed, 35 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 049356d..13f4603 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -267,39 +267,6 @@ struct request *blk_mq_alloc_request(struct request_queue *q, int rw,
 }
 EXPORT_SYMBOL(blk_mq_alloc_request);
 
-struct request *blk_mq_alloc_request_hctx(struct request_queue *q, int rw,
-		unsigned int flags, unsigned int hctx_idx)
-{
-	struct blk_mq_hw_ctx *hctx;
-	struct blk_mq_ctx *ctx;
-	struct request *rq;
-	struct blk_mq_alloc_data alloc_data;
-	int ret;
-
-	ret = blk_queue_enter(q, flags & BLK_MQ_REQ_NOWAIT);
-	if (ret)
-		return ERR_PTR(ret);
-
-	hctx = q->queue_hw_ctx[hctx_idx];
-	ctx = __blk_mq_get_ctx(q, cpumask_first(hctx->cpumask));
-
-	blk_mq_set_alloc_data(&alloc_data, q, flags, ctx, hctx);
-
-	rq = __blk_mq_alloc_request(&alloc_data, rw);
-	if (!rq && !(flags & BLK_MQ_REQ_NOWAIT)) {
-		__blk_mq_run_hw_queue(hctx);
-
-		rq =  __blk_mq_alloc_request(&alloc_data, rw);
-	}
-	if (!rq) {
-		blk_queue_exit(q);
-		return ERR_PTR(-EWOULDBLOCK);
-	}
-
-	return rq;
-}
-EXPORT_SYMBOL(blk_mq_alloc_request_hctx);
-
 static void __blk_mq_free_request(struct blk_mq_hw_ctx *hctx,
 				  struct blk_mq_ctx *ctx, struct request *rq)
 {
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 9a5d581..faa7d5c2 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -198,8 +198,6 @@ enum {
 
 struct request *blk_mq_alloc_request(struct request_queue *q, int rw,
 		unsigned int flags);
-struct request *blk_mq_alloc_request_hctx(struct request_queue *q, int rw,
-		unsigned int flags, unsigned int hctx_idx);
 struct request *blk_mq_tag_to_rq(struct blk_mq_tags *tags, unsigned int tag);
 struct cpumask *blk_mq_tags_cpumask(struct blk_mq_tags *tags);
 
-- 
2.1.4

Re: [PATCH 01/11] Revert "blk-mq: add blk_mq_alloc_request_hctx"

[Jeff Moyer]

Christoph Hellwig <hch@lst.de> writes:

> Just for book keeping. The next patch brings it back with some changes.

That's a strange thing to do, and breaks bisectability.  Why not just
update the function?

-Jeff

>
> This reverts commit 98878e0f0b80520575b31147f7cff28e28d9a725.
> ---
>  block/blk-mq.c         | 33 ---------------------------------
>  include/linux/blk-mq.h |  2 --
>  2 files changed, 35 deletions(-)
>
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 049356d..13f4603 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -267,39 +267,6 @@ struct request *blk_mq_alloc_request(struct request_queue *q, int rw,
>  }
>  EXPORT_SYMBOL(blk_mq_alloc_request);
>  
> -struct request *blk_mq_alloc_request_hctx(struct request_queue *q, int rw,
> -		unsigned int flags, unsigned int hctx_idx)
> -{
> -	struct blk_mq_hw_ctx *hctx;
> -	struct blk_mq_ctx *ctx;
> -	struct request *rq;
> -	struct blk_mq_alloc_data alloc_data;
> -	int ret;
> -
> -	ret = blk_queue_enter(q, flags & BLK_MQ_REQ_NOWAIT);
> -	if (ret)
> -		return ERR_PTR(ret);
> -
> -	hctx = q->queue_hw_ctx[hctx_idx];
> -	ctx = __blk_mq_get_ctx(q, cpumask_first(hctx->cpumask));
> -
> -	blk_mq_set_alloc_data(&alloc_data, q, flags, ctx, hctx);
> -
> -	rq = __blk_mq_alloc_request(&alloc_data, rw);
> -	if (!rq && !(flags & BLK_MQ_REQ_NOWAIT)) {
> -		__blk_mq_run_hw_queue(hctx);
> -
> -		rq =  __blk_mq_alloc_request(&alloc_data, rw);
> -	}
> -	if (!rq) {
> -		blk_queue_exit(q);
> -		return ERR_PTR(-EWOULDBLOCK);
> -	}
> -
> -	return rq;
> -}
> -EXPORT_SYMBOL(blk_mq_alloc_request_hctx);
> -
>  static void __blk_mq_free_request(struct blk_mq_hw_ctx *hctx,
>  				  struct blk_mq_ctx *ctx, struct request *rq)
>  {
> diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
> index 9a5d581..faa7d5c2 100644
> --- a/include/linux/blk-mq.h
> +++ b/include/linux/blk-mq.h
> @@ -198,8 +198,6 @@ enum {
>  
>  struct request *blk_mq_alloc_request(struct request_queue *q, int rw,
>  		unsigned int flags);
> -struct request *blk_mq_alloc_request_hctx(struct request_queue *q, int rw,
> -		unsigned int flags, unsigned int hctx_idx);
>  struct request *blk_mq_tag_to_rq(struct blk_mq_tags *tags, unsigned int tag);
>  struct cpumask *blk_mq_tags_cpumask(struct blk_mq_tags *tags);

Re: [PATCH 01/11] Revert "blk-mq: add blk_mq_alloc_request_hctx"

[Christoph Hellwig]

On Wed, Jun 08, 2016 at 09:58:38AM -0400, Jeff Moyer wrote:
> Christoph Hellwig <hch@lst.de> writes:
> 
> > Just for book keeping. The next patch brings it back with some changes.
> 
> That's a strange thing to do, and breaks bisectability.  Why not just
> update the function?

Because it's just an updated to a not yet applied series.  The idea is
that Jens could just take patch 2 if we get a positive review or two for
it.

Re: [PATCH 01/11] Revert "blk-mq: add blk_mq_alloc_request_hctx"

[Jeff Moyer]

Christoph Hellwig <hch@lst.de> writes:

> On Wed, Jun 08, 2016 at 09:58:38AM -0400, Jeff Moyer wrote:
>> Christoph Hellwig <hch@lst.de> writes:
>> 
>> > Just for book keeping. The next patch brings it back with some changes.
>> 
>> That's a strange thing to do, and breaks bisectability.  Why not just
>> update the function?
>
> Because it's just an updated to a not yet applied series.  The idea is
> that Jens could just take patch 2 if we get a positive review or two for
> it.

Ah, gotcha.  Thanks for the clarification.

-Jeff

[PATCH 02/11] blk-mq: add blk_mq_alloc_request_hctx

[Christoph Hellwig]

From: Ming Lin <ming.l@ssi.samsung.com>

For some protocols like NVMe over Fabrics we need to be able to send
initialization commands to a specific queue.

Based on an earlier patch from Christoph Hellwig <hch@lst.de>.

Signed-off-by: Ming Lin <ming.l@ssi.samsung.com>
[hch: disallow sleeping allocation, req_op fixes]
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-mq.c         | 39 +++++++++++++++++++++++++++++++++++++++
 include/linux/blk-mq.h |  2 ++
 2 files changed, 41 insertions(+)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 13f4603..7aa60c4 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -267,6 +267,45 @@ struct request *blk_mq_alloc_request(struct request_queue *q, int rw,
 }
 EXPORT_SYMBOL(blk_mq_alloc_request);
 
+struct request *blk_mq_alloc_request_hctx(struct request_queue *q, int rw,
+		unsigned int flags, unsigned int hctx_idx)
+{
+	struct blk_mq_hw_ctx *hctx;
+	struct blk_mq_ctx *ctx;
+	struct request *rq;
+	struct blk_mq_alloc_data alloc_data;
+	int ret;
+
+	/*
+	 * If the tag allocator sleeps we could get an allocation for a
+	 * different hardware context.  No need to complicate the low level
+	 * allocator for this for the rare use case of a command tied to
+	 * a specific queue.
+	 */
+	if (WARN_ON_ONCE(!(flags & BLK_MQ_REQ_NOWAIT)))
+		return ERR_PTR(-EINVAL);
+
+	if (hctx_idx >= q->nr_hw_queues)
+		return ERR_PTR(-EIO);
+
+	ret = blk_queue_enter(q, true);
+	if (ret)
+		return ERR_PTR(ret);
+
+	hctx = q->queue_hw_ctx[hctx_idx];
+	ctx = __blk_mq_get_ctx(q, cpumask_first(hctx->cpumask));
+
+	blk_mq_set_alloc_data(&alloc_data, q, flags, ctx, hctx);
+	rq = __blk_mq_alloc_request(&alloc_data, rw, 0);
+	if (!rq) {
+		blk_queue_exit(q);
+		return ERR_PTR(-EWOULDBLOCK);
+	}
+
+	return rq;
+}
+EXPORT_SYMBOL_GPL(blk_mq_alloc_request_hctx);
+
 static void __blk_mq_free_request(struct blk_mq_hw_ctx *hctx,
 				  struct blk_mq_ctx *ctx, struct request *rq)
 {
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index faa7d5c2..e43bbff 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -198,6 +198,8 @@ enum {
 
 struct request *blk_mq_alloc_request(struct request_queue *q, int rw,
 		unsigned int flags);
+struct request *blk_mq_alloc_request_hctx(struct request_queue *q, int op,
+		unsigned int flags, unsigned int hctx_idx);
 struct request *blk_mq_tag_to_rq(struct blk_mq_tags *tags, unsigned int tag);
 struct cpumask *blk_mq_tags_cpumask(struct blk_mq_tags *tags);
 
-- 
2.1.4

Re: [PATCH 02/11] blk-mq: add blk_mq_alloc_request_hctx

[Sagi Grimberg]

Looks fine,

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

[PATCH 03/11] nvme_rdma: update for req_op changes

[Christoph Hellwig]

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/nvme/host/rdma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index 4edc912..e938ab6 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -1453,7 +1453,7 @@ static int nvme_rdma_queue_rq(struct blk_mq_hw_ctx *hctx,
 	ib_dma_sync_single_for_device(dev, sqe->dma,
 			sizeof(struct nvme_command), DMA_TO_DEVICE);
 
-	if (rq->cmd_type == REQ_TYPE_FS && (rq->cmd_flags & REQ_FLUSH))
+	if (rq->cmd_type == REQ_TYPE_FS && req_op(rq) == REQ_OP_FLUSH)
 		flush = true;
 	ret = nvme_rdma_post_send(queue, sqe, req->sge, req->num_sge,
 			req->need_inval ? &req->reg_wr.wr : NULL, flush);
-- 
2.1.4

Re: [PATCH 03/11] nvme_rdma: update for req_op changes

[Sagi Grimberg]

Looks fine,

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

Re: [PATCH 03/11] nvme_rdma: update for req_op changes

[Ming Lin]

Looks good.

Reviewed-by: Ming Lin <ming.l@samsung.com>

[PATCH 04/11] nvmet: update for req_op changes

[Christoph Hellwig]

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/nvme/target/io-cmd.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/nvme/target/io-cmd.c b/drivers/nvme/target/io-cmd.c
index 76dbf73..4cfd481 100644
--- a/drivers/nvme/target/io-cmd.c
+++ b/drivers/nvme/target/io-cmd.c
@@ -49,7 +49,7 @@ static void nvmet_execute_rw(struct nvmet_req *req)
 	struct bio *bio;
 	sector_t sector;
 	blk_qc_t cookie;
-	int rw, i;
+	int op, op_flags = 0, i;
 
 	if (!req->sg_cnt) {
 		nvmet_req_complete(req, 0);
@@ -57,12 +57,11 @@ static void nvmet_execute_rw(struct nvmet_req *req)
 	}
 
 	if (req->cmd->rw.opcode == nvme_cmd_write) {
+		op = REQ_OP_WRITE;
 		if (req->cmd->rw.control & cpu_to_le16(NVME_RW_FUA))
-			rw = WRITE_FUA;
-		else
-			rw = WRITE;
+			op_flags |= REQ_FUA;
 	} else {
-		rw = READ;
+		op = REQ_OP_READ;
 	}
 
 	sector = le64_to_cpu(req->cmd->rw.slba);
@@ -83,16 +82,17 @@ static void nvmet_execute_rw(struct nvmet_req *req)
 			bio = bio_alloc(GFP_KERNEL, min(sg_cnt, BIO_MAX_PAGES));
 			bio->bi_bdev = req->ns->bdev;
 			bio->bi_iter.bi_sector = sector;
+			bio_set_op_attrs(bio, op, op_flags);
 
 			bio_chain(bio, prev);
-			cookie = submit_bio(rw, prev);
+			cookie = submit_bio(prev);
 		}
 
 		sector += sg->length >> 9;
 		sg_cnt--;
 	}
 
-	cookie = submit_bio(rw, bio);
+	cookie = submit_bio(bio);
 
 	blk_poll(bdev_get_queue(req->ns->bdev), cookie);
 }
@@ -107,17 +107,18 @@ static void nvmet_execute_flush(struct nvmet_req *req)
 	bio->bi_bdev = req->ns->bdev;
 	bio->bi_private = req;
 	bio->bi_end_io = nvmet_bio_done;
+	bio->bi_rw = WRITE_FLUSH;
 
-	submit_bio(WRITE_FLUSH, bio);
+	submit_bio(bio);
 }
 
 static u16 nvmet_discard_range(struct nvmet_ns *ns,
-		struct nvme_dsm_range *range, int type, struct bio **bio)
+		struct nvme_dsm_range *range, struct bio **bio)
 {
 	if (__blkdev_issue_discard(ns->bdev,
 			le64_to_cpu(range->slba) << (ns->blksize_shift - 9),
 			le32_to_cpu(range->nlb) << (ns->blksize_shift - 9),
-			GFP_KERNEL, type, bio))
+			GFP_KERNEL, 0, bio))
 		return NVME_SC_INTERNAL | NVME_SC_DNR;
 	return 0;
 }
@@ -126,7 +127,7 @@ static void nvmet_execute_discard(struct nvmet_req *req)
 {
 	struct nvme_dsm_range range;
 	struct bio *bio = NULL;
-	int type = REQ_WRITE | REQ_DISCARD, i;
+	int i;
 	u16 status;
 
 	for (i = 0; i <= le32_to_cpu(req->cmd->dsm.nr); i++) {
@@ -135,7 +136,7 @@ static void nvmet_execute_discard(struct nvmet_req *req)
 		if (status)
 			break;
 
-		status = nvmet_discard_range(req->ns, &range, type, &bio);
+		status = nvmet_discard_range(req->ns, &range, &bio);
 		if (status)
 			break;
 	}
@@ -147,7 +148,7 @@ static void nvmet_execute_discard(struct nvmet_req *req)
 			bio->bi_error = -EIO;
 			bio_endio(bio);
 		} else {
-			submit_bio(type, bio);
+			submit_bio(bio);
 		}
 	} else {
 		nvmet_req_complete(req, status);
-- 
2.1.4

Re: [PATCH 04/11] nvmet: update for req_op changes

[Sagi Grimberg]

Looks fine,

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

Re: [PATCH 04/11] nvmet: update for req_op changes

[Christoph Hellwig]

FYI, this missed half of the fixups, here is the other half:

---
>From c5caa4c7c3ad5f9648e18f42687c0b63885f428d Mon Sep 17 00:00:00 2001
From: Christoph Hellwig <hch@lst.de>
Date: Tue, 14 Jun 2016 09:27:53 +0200
Subject: nvmet: more fixups for the op vs flags split

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/nvme/target/io-cmd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/target/io-cmd.c b/drivers/nvme/target/io-cmd.c
index 4cfd481..2cd069b6 100644
--- a/drivers/nvme/target/io-cmd.c
+++ b/drivers/nvme/target/io-cmd.c
@@ -73,6 +73,7 @@ static void nvmet_execute_rw(struct nvmet_req *req)
 	bio->bi_iter.bi_sector = sector;
 	bio->bi_private = req;
 	bio->bi_end_io = nvmet_bio_done;
+	bio_set_op_attrs(bio, op, op_flags);
 
 	for_each_sg(req->sg, sg, req->sg_cnt, i) {
 		while (bio_add_page(bio, sg_page(sg), sg->length, sg->offset)
@@ -107,7 +108,7 @@ static void nvmet_execute_flush(struct nvmet_req *req)
 	bio->bi_bdev = req->ns->bdev;
 	bio->bi_private = req;
 	bio->bi_end_io = nvmet_bio_done;
-	bio->bi_rw = WRITE_FLUSH;
+	bio_set_op_attrs(bio, REQ_OP_WRITE, WRITE_FLUSH);
 
 	submit_bio(bio);
 }
-- 
2.1.4

[PATCH 05/11] nvme-loop: set SGL flag on AER

[Christoph Hellwig]

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/nvme/target/loop.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/nvme/target/loop.c b/drivers/nvme/target/loop.c
index 08b4fbb..94e7829 100644
--- a/drivers/nvme/target/loop.c
+++ b/drivers/nvme/target/loop.c
@@ -209,6 +209,7 @@ static void nvme_loop_submit_async_event(struct nvme_ctrl *arg, int aer_idx)
 	memset(&iod->cmd, 0, sizeof(iod->cmd));
 	iod->cmd.common.opcode = nvme_admin_async_event;
 	iod->cmd.common.command_id = NVME_LOOP_AQ_BLKMQ_DEPTH;
+	iod->cmd.common.flags |= NVME_CMD_SGL_METABUF;
 
 	if (!nvmet_req_init(&iod->req, &queue->nvme_cq, &queue->nvme_sq,
 			&nvme_loop_ops)) {
-- 
2.1.4

[PATCH 06/11] nvme-rdma: set SGL flag on AER

[Christoph Hellwig]

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/nvme/host/rdma.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index e938ab6..d8574f4 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -1105,6 +1105,7 @@ static void nvme_rdma_submit_async_event(struct nvme_ctrl *arg, int aer_idx)
 	memset(cmd, 0, sizeof(*cmd));
 	cmd->common.opcode = nvme_admin_async_event;
 	cmd->common.command_id = NVME_RDMA_AQ_BLKMQ_DEPTH;
+	cmd->common.flags |= NVME_CMD_SGL_METABUF;
 	nvme_rdma_set_sg_null(cmd);
 
 	ib_dma_sync_single_for_device(dev, sqe->dma, sizeof(*cmd),
-- 
2.1.4

[PATCH 07/11] nvmet: fix 64-bit division in nvmet_set_features

[Christoph Hellwig]

As it makes 32-bit builds unhappy.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/nvme/target/admin-cmd.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index 240e323..2fac17a 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -298,6 +298,7 @@ static void nvmet_execute_set_features(struct nvmet_req *req)
 	struct nvmet_subsys *subsys = req->sq->ctrl->subsys;
 	u32 cdw10 = le32_to_cpu(req->cmd->common.cdw10[0]);
 	u64 val;
+	u32 val32;
 	u16 status = 0;
 
 	switch (cdw10 & 0xf) {
@@ -307,7 +308,8 @@ static void nvmet_execute_set_features(struct nvmet_req *req)
 		break;
 	case NVME_FEAT_KATO:
 		val = le64_to_cpu(req->cmd->prop_set.value);
-		req->sq->ctrl->kato = (u32)DIV_ROUND_UP(val & 0xffff, 1000);
+		val32 = val & 0xffff;
+		req->sq->ctrl->kato = DIV_ROUND_UP(val32, 1000);
 		nvmet_set_result(req, req->sq->ctrl->kato);
 		break;
 	default:
-- 
2.1.4

Re: [PATCH 07/11] nvmet: fix 64-bit division in nvmet_set_features

[Sagi Grimberg]

Looks fine,

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

[PATCH 08/11] nvmet: fix eno.cocci warnings

[Christoph Hellwig]

From: kbuild test robot <lkp@intel.com>

drivers/nvme/target/configfs.c:796:5-11: ERROR: allocation function on line 795 returns NULL not ERR_PTR on failure
drivers/nvme/target/configfs.c:739:5-11: ERROR: allocation function on line 738 returns NULL not ERR_PTR on failure

 The various basic memory allocation functions don't return ERR_PTR

Generated by: scripts/coccinelle/null/eno.cocci

CC: Christoph Hellwig <hch@lst.de>
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/nvme/target/configfs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
index aebe646..9bed302 100644
--- a/drivers/nvme/target/configfs.c
+++ b/drivers/nvme/target/configfs.c
@@ -736,7 +736,7 @@ static struct config_group *nvmet_referral_make(
 	struct nvmet_port *port;
 
 	port = kzalloc(sizeof(*port), GFP_KERNEL);
-	if (IS_ERR(port))
+	if (!port)
 		return ERR_CAST(port);
 
 	INIT_LIST_HEAD(&port->entry);
@@ -793,7 +793,7 @@ static struct config_group *nvmet_ports_make(struct config_group *group,
 		return ERR_PTR(-EINVAL);
 
 	port = kzalloc(sizeof(*port), GFP_KERNEL);
-	if (IS_ERR(port))
+	if (!port)
 		return ERR_CAST(port);
 
 	INIT_LIST_HEAD(&port->entry);
-- 
2.1.4

Re: [PATCH 08/11] nvmet: fix eno.cocci warnings

[Ming Lin]

Looks good.

Reviewed-by: Ming Lin <ming.l@samsung.com>

[PATCH 09/11] nvmet-rdma: check for a valid adrfam

[Christoph Hellwig]

From: Ming Lin <ming.l@samsung.com>

For now only ipv4 is supported.

Signed-off-by: Ming Lin <ming.l@samsung.com>
Acked-by: Sagi Grimberg <sagi@grimberg.me>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/nvme/target/rdma.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/nvme/target/rdma.c b/drivers/nvme/target/rdma.c
index fccb01d..c184ee5 100644
--- a/drivers/nvme/target/rdma.c
+++ b/drivers/nvme/target/rdma.c
@@ -1313,6 +1313,15 @@ static int nvmet_rdma_add_port(struct nvmet_port *port)
 	u16 port_in;
 	int ret;
 
+	switch (port->disc_addr.adrfam) {
+	case NVMF_ADDR_FAMILY_IP4:
+		break;
+	default:
+		pr_err("address family %d not supported\n",
+				port->disc_addr.adrfam);
+		return -EINVAL;
+	}
+
 	ret = kstrtou16(port->disc_addr.trsvcid, 0, &port_in);
 	if (ret)
 		return ret;
-- 
2.1.4

[PATCH 10/11] nvme: keep alive should use any queue

[Christoph Hellwig]

No need to specify a queue ID for admin commands.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/nvme/host/core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index dfd2fd7..86886c3 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -499,7 +499,8 @@ static int nvme_keep_alive(struct nvme_ctrl *ctrl)
 	memset(&c, 0, sizeof(c));
 	c.common.opcode = nvme_admin_keep_alive;
 
-	rq = nvme_alloc_request(ctrl->admin_q, &c, BLK_MQ_REQ_RESERVED, 0);
+	rq = nvme_alloc_request(ctrl->admin_q, &c, BLK_MQ_REQ_RESERVED,
+			NVME_QID_ANY);
 	if (IS_ERR(rq))
 		return PTR_ERR(rq);
 
-- 
2.1.4

Re: [PATCH 10/11] nvme: keep alive should use any queue

[Sagi Grimberg]

Looks fine,

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

[PATCH 11/11] nvme-fabrics: pass non-blocking flag to connect request allocation

[Christoph Hellwig]

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/nvme/host/fabrics.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
index ee4b7f1..225a732 100644
--- a/drivers/nvme/host/fabrics.c
+++ b/drivers/nvme/host/fabrics.c
@@ -378,11 +378,10 @@ int nvmf_connect_admin_queue(struct nvme_ctrl *ctrl)
 
 	ret = __nvme_submit_sync_cmd(ctrl->admin_q, &cmd, &cqe,
 			data, sizeof(*data), 0, NVME_QID_ANY, 1,
-			BLK_MQ_REQ_RESERVED);
+			BLK_MQ_REQ_RESERVED | BLK_MQ_REQ_NOWAIT);
 	if (ret) {
-		nvmf_log_connect_error(ctrl,
-					 ret, le32_to_cpu(cqe.result),
-					 &cmd, data);
+		nvmf_log_connect_error(ctrl, ret, le32_to_cpu(cqe.result),
+				       &cmd, data);
 		goto out_free_data;
 	}
 
@@ -437,12 +436,12 @@ int nvmf_connect_io_queue(struct nvme_ctrl *ctrl, u16 qid)
 	strncpy(data->hostnqn, ctrl->opts->host->nqn, NVMF_NQN_SIZE);
 
 	ret = __nvme_submit_sync_cmd(ctrl->connect_q, &cmd, &cqe,
-			data, sizeof(*data), 0, qid, 1, BLK_MQ_REQ_RESERVED);
-	if (ret)
-		nvmf_log_connect_error(ctrl,
-					 ret, le32_to_cpu(cqe.result),
-					 &cmd, data);
-
+			data, sizeof(*data), 0, qid, 1,
+			BLK_MQ_REQ_RESERVED | BLK_MQ_REQ_NOWAIT);
+	if (ret) {
+		nvmf_log_connect_error(ctrl, ret, le32_to_cpu(cqe.result),
+				       &cmd, data);
+	}
 	kfree(data);
 	return ret;
 }
-- 
2.1.4

Re: [PATCH 11/11] nvme-fabrics: pass non-blocking flag to connect request allocation

[Sagi Grimberg]

Looks fine,

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

Would be nice to reduce the number of arguments
passed to __nvme_submit_sync_cmd() at some point..

Re: [PATCH 11/11] nvme-fabrics: pass non-blocking flag to connect request allocation

[Christoph Hellwig]

On Wed, Jun 08, 2016 at 04:55:41PM +0300, Sagi Grimberg wrote:
> Looks fine,
>
> Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
>
> Would be nice to reduce the number of arguments
> passed to __nvme_submit_sync_cmd() at some point..

We're only adding them in our series..  But I agree that the number
of arguments is a bit of a mess, but I couldn't really come up with
something better.

Re: NVMe over Fabrics: updates for req_op changes and misc fixes

[Ming Lin]

For the series,

Tested-by: Ming Lin <ming.l@samsung.com>