[PATCH 0/9] nvme: add helpers and remove duplicate code

[PATCH 0/9] nvme: add helpers and remove duplicate code

[Chaitanya Kulkarni]

From: Chaitanya Kulkarni <kch@nvidia.com>

HI,

Instead of opencoding connect_q initialization/error handling and the
initialization of the admin tagset, add common helpers to remove the
ducplicate code in different transports.

-ck

Chaitanya Kulkarni (9):
  nvme-fc: add a helper to initialize connect_q
  nvme-rdma: use helper to initialize connect_q
  nvme-tcp: use helper to initialize connect_q
  nvme-loop: use helper to initialize connect_q
  nvme-fc: centralize admin tagset initialization
  nvme-pci: use helper for tagset initialization
  nvme-rdma: use helper for tagset initialization
  nvme-tcp: use helper for tagset initialization
  nvme-loop: use helper for tagset initialization

 drivers/nvme/host/fc.c     | 18 +++++-------------
 drivers/nvme/host/nvme.h   | 25 +++++++++++++++++++++++++
 drivers/nvme/host/pci.c    | 11 +++--------
 drivers/nvme/host/rdma.c   | 19 ++++++-------------
 drivers/nvme/host/tcp.c    | 18 +++++-------------
 drivers/nvme/target/loop.c | 19 ++++++-------------
 6 files changed, 50 insertions(+), 60 deletions(-)

-- 
2.29.0

[PATCH 1/9] nvme-fc: add a helper to initialize connect_q

[Chaitanya Kulkarni]

From: Chaitanya Kulkarni <kch@nvidia.com>

Add and use helper to remove duplicate code for fabrics connect_q
initialization from all the transports.

Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
---
 drivers/nvme/host/fc.c   | 6 ++----
 drivers/nvme/host/nvme.h | 8 ++++++++
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index 71b3108c22f0..23c896a138a3 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -2916,11 +2916,9 @@ nvme_fc_create_io_queues(struct nvme_fc_ctrl *ctrl)
 
 	ctrl->ctrl.tagset = &ctrl->tag_set;
 
-	ctrl->ctrl.connect_q = blk_mq_init_queue(&ctrl->tag_set);
-	if (IS_ERR(ctrl->ctrl.connect_q)) {
-		ret = PTR_ERR(ctrl->ctrl.connect_q);
+	ret = nvme_ctrl_init_connect_q(&(ctrl->ctrl));
+	if (ret)
 		goto out_free_tag_set;
-	}
 
 	ret = nvme_fc_create_hw_io_queues(ctrl, ctrl->ctrl.sqsize + 1);
 	if (ret)
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index a162f6c6da6e..f8658f984d64 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -894,6 +894,14 @@ static inline int nvme_update_zone_info(struct nvme_ns *ns, unsigned lbaf)
 }
 #endif
 
+static inline int nvme_ctrl_init_connect_q(struct nvme_ctrl *ctrl)
+{
+	ctrl->connect_q = blk_mq_init_queue(ctrl->tagset);
+	if (IS_ERR(ctrl->connect_q))
+		return PTR_ERR(ctrl->connect_q);
+	return 0;
+}
+
 static inline struct nvme_ns *nvme_get_ns_from_dev(struct device *dev)
 {
 	return dev_to_disk(dev)->private_data;
-- 
2.29.0

[PATCH 2/9] nvme-rdma: use helper to initialize connect_q

[Chaitanya Kulkarni]

From: Chaitanya Kulkarni <kch@nvidia.com>

Use helper to remove duplicate code for connect queue initialization
and error handling for all fabrics transports.

Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
---
 drivers/nvme/host/rdma.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index 850f84d204d0..d033294b063e 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -978,11 +978,10 @@ static int nvme_rdma_configure_io_queues(struct nvme_rdma_ctrl *ctrl, bool new)
 			goto out_free_io_queues;
 		}
 
-		ctrl->ctrl.connect_q = blk_mq_init_queue(&ctrl->tag_set);
-		if (IS_ERR(ctrl->ctrl.connect_q)) {
-			ret = PTR_ERR(ctrl->ctrl.connect_q);
+
+		ret = nvme_ctrl_init_connect_q(&(ctrl->ctrl));
+		if (ret)
 			goto out_free_tag_set;
-		}
 	}
 
 	ret = nvme_rdma_start_io_queues(ctrl);
-- 
2.29.0

[PATCH 3/9] nvme-tcp: use helper to initialize connect_q

[Chaitanya Kulkarni]

From: Chaitanya Kulkarni <kch@nvidia.com>

Use helper to remove duplicate code for connect queue initialization
and error handling for all fabrics transports.

Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
---
 drivers/nvme/host/tcp.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 4ceb28675fdf..6cbcc8b4daaf 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -1825,11 +1825,9 @@ static int nvme_tcp_configure_io_queues(struct nvme_ctrl *ctrl, bool new)
 			goto out_free_io_queues;
 		}
 
-		ctrl->connect_q = blk_mq_init_queue(ctrl->tagset);
-		if (IS_ERR(ctrl->connect_q)) {
-			ret = PTR_ERR(ctrl->connect_q);
+		ret = nvme_ctrl_init_connect_q(ctrl);
+		if (ret)
 			goto out_free_tag_set;
-		}
 	}
 
 	ret = nvme_tcp_start_io_queues(ctrl);
-- 
2.29.0

[PATCH 4/9] nvme-loop: use helper to initialize connect_q

[Chaitanya Kulkarni]

From: Chaitanya Kulkarni <kch@nvidia.com>

Use helper to remove duplicate code for connect queue initialization
and error handling for all fabrics transports.

Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
---
 drivers/nvme/target/loop.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/nvme/target/loop.c b/drivers/nvme/target/loop.c
index eb1094254c82..23f9d6f88804 100644
--- a/drivers/nvme/target/loop.c
+++ b/drivers/nvme/target/loop.c
@@ -543,11 +543,9 @@ static int nvme_loop_create_io_queues(struct nvme_loop_ctrl *ctrl)
 	if (ret)
 		goto out_destroy_queues;
 
-	ctrl->ctrl.connect_q = blk_mq_init_queue(&ctrl->tag_set);
-	if (IS_ERR(ctrl->ctrl.connect_q)) {
-		ret = PTR_ERR(ctrl->ctrl.connect_q);
+	ret = nvme_ctrl_init_connect_q(&(ctrl->ctrl));
+	if (ret)
 		goto out_free_tagset;
-	}
 
 	ret = nvme_loop_connect_io_queues(ctrl);
 	if (ret)
-- 
2.29.0

[PATCH 5/9] nvme-fc: centralize admin tagset initialization

[Chaitanya Kulkarni]

From: Chaitanya Kulkarni <kch@nvidia.com>

Add and use helper to remove duplicate code for admin tag initialization
for all transports.

Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
---
 drivers/nvme/host/fc.c   | 12 +++---------
 drivers/nvme/host/nvme.h | 17 +++++++++++++++++
 2 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index 23c896a138a3..ec2afedfa991 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -3529,18 +3529,12 @@ nvme_fc_init_ctrl(struct device *dev, struct nvmf_ctrl_options *opts,
 
 	nvme_fc_init_queue(ctrl, 0);
 
-	memset(&ctrl->admin_tag_set, 0, sizeof(ctrl->admin_tag_set));
-	ctrl->admin_tag_set.ops = &nvme_fc_admin_mq_ops;
-	ctrl->admin_tag_set.queue_depth = NVME_AQ_MQ_TAG_DEPTH;
-	ctrl->admin_tag_set.reserved_tags = NVMF_RESERVED_TAGS;
-	ctrl->admin_tag_set.numa_node = ctrl->ctrl.numa_node;
+	nvme_init_admin_tagset(&(ctrl->admin_tag_set), ctrl,
+			       ctrl->ctrl.numa_node, &nvme_fc_admin_mq_ops,
+			       BLK_MQ_F_NO_SCHED, NVMF_RESERVED_TAGS);
 	ctrl->admin_tag_set.cmd_size =
 		struct_size((struct nvme_fcp_op_w_sgl *)NULL, priv,
 			    ctrl->lport->ops->fcprqst_priv_sz);
-	ctrl->admin_tag_set.driver_data = ctrl;
-	ctrl->admin_tag_set.nr_hw_queues = 1;
-	ctrl->admin_tag_set.timeout = NVME_ADMIN_TIMEOUT;
-	ctrl->admin_tag_set.flags = BLK_MQ_F_NO_SCHED;
 
 	ret = blk_mq_alloc_tag_set(&ctrl->admin_tag_set);
 	if (ret)
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index f8658f984d64..6a3fd759415a 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -938,4 +938,21 @@ static inline bool nvme_multi_css(struct nvme_ctrl *ctrl)
 	return (ctrl->ctrl_config & NVME_CC_CSS_MASK) == NVME_CC_CSS_CSI;
 }
 
+static inline void nvme_init_admin_tagset(struct blk_mq_tag_set *set,
+					  void *driver_data, int numa_node,
+					  const struct blk_mq_ops *ops,
+					  unsigned int flags,
+					  unsigned int reserved_tags)
+{
+	memset(set, 0, sizeof(*set));
+	set->driver_data = driver_data;
+	set->numa_node = numa_node;
+	set->ops = ops;
+	set->flags = flags;
+	set->reserved_tags = reserved_tags;
+	set->queue_depth = NVME_AQ_MQ_TAG_DEPTH;
+	set->nr_hw_queues = 1;
+	set->timeout = NVME_ADMIN_TIMEOUT;
+}
+
 #endif /* _NVME_H */
-- 
2.29.0

[PATCH 6/9] nvme-pci: use helper for tagset initialization

[Chaitanya Kulkarni]

From: Chaitanya Kulkarni <kch@nvidia.com>

Use helper to remove duplicate code for admin tag initialization
for all transports.

Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
---
 drivers/nvme/host/pci.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 6a99ed680915..87bdccd85eff 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -1755,15 +1755,10 @@ static void nvme_dev_remove_admin(struct nvme_dev *dev)
 static int nvme_alloc_admin_tags(struct nvme_dev *dev)
 {
 	if (!dev->ctrl.admin_q) {
-		dev->admin_tagset.ops = &nvme_mq_admin_ops;
-		dev->admin_tagset.nr_hw_queues = 1;
-
-		dev->admin_tagset.queue_depth = NVME_AQ_MQ_TAG_DEPTH;
-		dev->admin_tagset.timeout = NVME_ADMIN_TIMEOUT;
-		dev->admin_tagset.numa_node = dev->ctrl.numa_node;
+		nvme_init_admin_tagset(&(dev->admin_tagset), dev,
+				       dev->ctrl.numa_node, &nvme_mq_admin_ops,
+				       BLK_MQ_F_NO_SCHED, 0);
 		dev->admin_tagset.cmd_size = sizeof(struct nvme_iod);
-		dev->admin_tagset.flags = BLK_MQ_F_NO_SCHED;
-		dev->admin_tagset.driver_data = dev;
 
 		if (blk_mq_alloc_tag_set(&dev->admin_tagset))
 			return -ENOMEM;
-- 
2.29.0

[PATCH 7/9] nvme-rdma: use helper for tagset initialization

[Chaitanya Kulkarni]

From: Chaitanya Kulkarni <kch@nvidia.com>

Use helper to remove duplicate code for admin tag initialization
for all transports.

Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
---
 drivers/nvme/host/rdma.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index d033294b063e..1195bb1d5b89 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -799,17 +799,11 @@ static struct blk_mq_tag_set *nvme_rdma_alloc_tagset(struct nvme_ctrl *nctrl,
 
 	if (admin) {
 		set = &ctrl->admin_tag_set;
-		memset(set, 0, sizeof(*set));
-		set->ops = &nvme_rdma_admin_mq_ops;
-		set->queue_depth = NVME_AQ_MQ_TAG_DEPTH;
-		set->reserved_tags = NVMF_RESERVED_TAGS;
-		set->numa_node = nctrl->numa_node;
+		nvme_init_admin_tagset(set, ctrl, nctrl->numa_node,
+				&nvme_rdma_admin_mq_ops, BLK_MQ_F_NO_SCHED,
+				NVMF_RESERVED_TAGS);
 		set->cmd_size = sizeof(struct nvme_rdma_request) +
 				NVME_RDMA_DATA_SGL_SIZE;
-		set->driver_data = ctrl;
-		set->nr_hw_queues = 1;
-		set->timeout = NVME_ADMIN_TIMEOUT;
-		set->flags = BLK_MQ_F_NO_SCHED;
 	} else {
 		set = &ctrl->tag_set;
 		memset(set, 0, sizeof(*set));
-- 
2.29.0

[PATCH 8/9] nvme-tcp: use helper for tagset initialization

[Chaitanya Kulkarni]

From: Chaitanya Kulkarni <kch@nvidia.com>

Use helper to remove duplicate code for admin tag initialization
for all transports.

Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
---
 drivers/nvme/host/tcp.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 6cbcc8b4daaf..e51ceada3bd9 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -1613,16 +1613,10 @@ static struct blk_mq_tag_set *nvme_tcp_alloc_tagset(struct nvme_ctrl *nctrl,
 
 	if (admin) {
 		set = &ctrl->admin_tag_set;
-		memset(set, 0, sizeof(*set));
-		set->ops = &nvme_tcp_admin_mq_ops;
-		set->queue_depth = NVME_AQ_MQ_TAG_DEPTH;
-		set->reserved_tags = NVMF_RESERVED_TAGS;
-		set->numa_node = nctrl->numa_node;
-		set->flags = BLK_MQ_F_BLOCKING;
+		nvme_init_admin_tagset(set, ctrl, nctrl->numa_node,
+				&nvme_tcp_admin_mq_ops, BLK_MQ_F_BLOCKING,
+				NVMF_RESERVED_TAGS);
 		set->cmd_size = sizeof(struct nvme_tcp_request);
-		set->driver_data = ctrl;
-		set->nr_hw_queues = 1;
-		set->timeout = NVME_ADMIN_TIMEOUT;
 	} else {
 		set = &ctrl->tag_set;
 		memset(set, 0, sizeof(*set));
-- 
2.29.0

[PATCH 9/9] nvme-loop: use helper for tagset initialization

[Chaitanya Kulkarni]

From: Chaitanya Kulkarni <kch@nvidia.com>

Use helper to remove duplicate code for admin tag initialization
for all transports.

Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
---
 drivers/nvme/target/loop.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/nvme/target/loop.c b/drivers/nvme/target/loop.c
index 23f9d6f88804..fdee038041f8 100644
--- a/drivers/nvme/target/loop.c
+++ b/drivers/nvme/target/loop.c
@@ -350,17 +350,12 @@ static int nvme_loop_configure_admin_queue(struct nvme_loop_ctrl *ctrl)
 {
 	int error;
 
-	memset(&ctrl->admin_tag_set, 0, sizeof(ctrl->admin_tag_set));
-	ctrl->admin_tag_set.ops = &nvme_loop_admin_mq_ops;
-	ctrl->admin_tag_set.queue_depth = NVME_AQ_MQ_TAG_DEPTH;
-	ctrl->admin_tag_set.reserved_tags = NVMF_RESERVED_TAGS;
-	ctrl->admin_tag_set.numa_node = ctrl->ctrl.numa_node;
+	nvme_init_admin_tagset(&(ctrl->admin_tag_set), ctrl,
+			       ctrl->ctrl.numa_node,
+			       &nvme_loop_admin_mq_ops, BLK_MQ_F_NO_SCHED,
+			       NVMF_RESERVED_TAGS);
 	ctrl->admin_tag_set.cmd_size = sizeof(struct nvme_loop_iod) +
 		NVME_INLINE_SG_CNT * sizeof(struct scatterlist);
-	ctrl->admin_tag_set.driver_data = ctrl;
-	ctrl->admin_tag_set.nr_hw_queues = 1;
-	ctrl->admin_tag_set.timeout = NVME_ADMIN_TIMEOUT;
-	ctrl->admin_tag_set.flags = BLK_MQ_F_NO_SCHED;
 
 	ctrl->queues[0].ctrl = ctrl;
 	error = nvmet_sq_init(&ctrl->queues[0].nvme_sq);
-- 
2.29.0

Re: [PATCH 1/9] nvme-fc: add a helper to initialize connect_q

[Chaitanya Kulkarni]

(+CC James james.smart@broadcom.com)

On 1/28/22 1:20 AM, Chaitanya Kulkarni wrote:
> From: Chaitanya Kulkarni <kch@nvidia.com>
> 
> Add and use helper to remove duplicate code for fabrics connect_q
> initialization from all the transports.
> 
> Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
> ---
>   drivers/nvme/host/fc.c   | 6 ++----
>   drivers/nvme/host/nvme.h | 8 ++++++++
>   2 files changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
> index 71b3108c22f0..23c896a138a3 100644
> --- a/drivers/nvme/host/fc.c
> +++ b/drivers/nvme/host/fc.c
> @@ -2916,11 +2916,9 @@ nvme_fc_create_io_queues(struct nvme_fc_ctrl *ctrl)
>   
>   	ctrl->ctrl.tagset = &ctrl->tag_set;
>   
> -	ctrl->ctrl.connect_q = blk_mq_init_queue(&ctrl->tag_set);
> -	if (IS_ERR(ctrl->ctrl.connect_q)) {
> -		ret = PTR_ERR(ctrl->ctrl.connect_q);
> +	ret = nvme_ctrl_init_connect_q(&(ctrl->ctrl));
> +	if (ret)
>   		goto out_free_tag_set;
> -	}
>   
>   	ret = nvme_fc_create_hw_io_queues(ctrl, ctrl->ctrl.sqsize + 1);
>   	if (ret)
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index a162f6c6da6e..f8658f984d64 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -894,6 +894,14 @@ static inline int nvme_update_zone_info(struct nvme_ns *ns, unsigned lbaf)
>   }
>   #endif
>   
> +static inline int nvme_ctrl_init_connect_q(struct nvme_ctrl *ctrl)
> +{
> +	ctrl->connect_q = blk_mq_init_queue(ctrl->tagset);
> +	if (IS_ERR(ctrl->connect_q))
> +		return PTR_ERR(ctrl->connect_q);
> +	return 0;
> +}
> +
>   static inline struct nvme_ns *nvme_get_ns_from_dev(struct device *dev)
>   {
>   	return dev_to_disk(dev)->private_data;
> 

Re: [PATCH 5/9] nvme-fc: centralize admin tagset initialization

[Chaitanya Kulkarni]

(+CC James james.smart@broadcom.com)

On 1/28/22 1:20 AM, Chaitanya Kulkarni wrote:
> From: Chaitanya Kulkarni <kch@nvidia.com>
> 
> Add and use helper to remove duplicate code for admin tag initialization
> for all transports.
> 
> Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
> ---
>   drivers/nvme/host/fc.c   | 12 +++---------
>   drivers/nvme/host/nvme.h | 17 +++++++++++++++++
>   2 files changed, 20 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
> index 23c896a138a3..ec2afedfa991 100644
> --- a/drivers/nvme/host/fc.c
> +++ b/drivers/nvme/host/fc.c
> @@ -3529,18 +3529,12 @@ nvme_fc_init_ctrl(struct device *dev, struct nvmf_ctrl_options *opts,
>   
>   	nvme_fc_init_queue(ctrl, 0);
>   
> -	memset(&ctrl->admin_tag_set, 0, sizeof(ctrl->admin_tag_set));
> -	ctrl->admin_tag_set.ops = &nvme_fc_admin_mq_ops;
> -	ctrl->admin_tag_set.queue_depth = NVME_AQ_MQ_TAG_DEPTH;
> -	ctrl->admin_tag_set.reserved_tags = NVMF_RESERVED_TAGS;
> -	ctrl->admin_tag_set.numa_node = ctrl->ctrl.numa_node;
> +	nvme_init_admin_tagset(&(ctrl->admin_tag_set), ctrl,
> +			       ctrl->ctrl.numa_node, &nvme_fc_admin_mq_ops,
> +			       BLK_MQ_F_NO_SCHED, NVMF_RESERVED_TAGS);
>   	ctrl->admin_tag_set.cmd_size =
>   		struct_size((struct nvme_fcp_op_w_sgl *)NULL, priv,
>   			    ctrl->lport->ops->fcprqst_priv_sz);
> -	ctrl->admin_tag_set.driver_data = ctrl;
> -	ctrl->admin_tag_set.nr_hw_queues = 1;
> -	ctrl->admin_tag_set.timeout = NVME_ADMIN_TIMEOUT;
> -	ctrl->admin_tag_set.flags = BLK_MQ_F_NO_SCHED;
>   
>   	ret = blk_mq_alloc_tag_set(&ctrl->admin_tag_set);
>   	if (ret)
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index f8658f984d64..6a3fd759415a 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -938,4 +938,21 @@ static inline bool nvme_multi_css(struct nvme_ctrl *ctrl)
>   	return (ctrl->ctrl_config & NVME_CC_CSS_MASK) == NVME_CC_CSS_CSI;
>   }
>   
> +static inline void nvme_init_admin_tagset(struct blk_mq_tag_set *set,
> +					  void *driver_data, int numa_node,
> +					  const struct blk_mq_ops *ops,
> +					  unsigned int flags,
> +					  unsigned int reserved_tags)
> +{
> +	memset(set, 0, sizeof(*set));
> +	set->driver_data = driver_data;
> +	set->numa_node = numa_node;
> +	set->ops = ops;
> +	set->flags = flags;
> +	set->reserved_tags = reserved_tags;
> +	set->queue_depth = NVME_AQ_MQ_TAG_DEPTH;
> +	set->nr_hw_queues = 1;
> +	set->timeout = NVME_ADMIN_TIMEOUT;
> +}
> +
>   #endif /* _NVME_H */
> 

Re: [PATCH 1/9] nvme-fc: add a helper to initialize connect_q

[Sagi Grimberg]

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

Re: [PATCH 2/9] nvme-rdma: use helper to initialize connect_q

[Sagi Grimberg]

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

Re: [PATCH 3/9] nvme-tcp: use helper to initialize connect_q

[Sagi Grimberg]

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

Re: [PATCH 4/9] nvme-loop: use helper to initialize connect_q

[Sagi Grimberg]

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

Re: [PATCH 5/9] nvme-fc: centralize admin tagset initialization

[Sagi Grimberg]

On 1/28/22 11:20, Chaitanya Kulkarni wrote:
> From: Chaitanya Kulkarni <kch@nvidia.com>
> 
> Add and use helper to remove duplicate code for admin tag initialization
> for all transports.
> 
> Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
> ---
>   drivers/nvme/host/fc.c   | 12 +++---------
>   drivers/nvme/host/nvme.h | 17 +++++++++++++++++
>   2 files changed, 20 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
> index 23c896a138a3..ec2afedfa991 100644
> --- a/drivers/nvme/host/fc.c
> +++ b/drivers/nvme/host/fc.c
> @@ -3529,18 +3529,12 @@ nvme_fc_init_ctrl(struct device *dev, struct nvmf_ctrl_options *opts,
>   
>   	nvme_fc_init_queue(ctrl, 0);
>   
> -	memset(&ctrl->admin_tag_set, 0, sizeof(ctrl->admin_tag_set));
> -	ctrl->admin_tag_set.ops = &nvme_fc_admin_mq_ops;
> -	ctrl->admin_tag_set.queue_depth = NVME_AQ_MQ_TAG_DEPTH;
> -	ctrl->admin_tag_set.reserved_tags = NVMF_RESERVED_TAGS;
> -	ctrl->admin_tag_set.numa_node = ctrl->ctrl.numa_node;
> +	nvme_init_admin_tagset(&(ctrl->admin_tag_set), ctrl,
> +			       ctrl->ctrl.numa_node, &nvme_fc_admin_mq_ops,
> +			       BLK_MQ_F_NO_SCHED, NVMF_RESERVED_TAGS);
>   	ctrl->admin_tag_set.cmd_size =
>   		struct_size((struct nvme_fcp_op_w_sgl *)NULL, priv,
>   			    ctrl->lport->ops->fcprqst_priv_sz);
> -	ctrl->admin_tag_set.driver_data = ctrl;
> -	ctrl->admin_tag_set.nr_hw_queues = 1;
> -	ctrl->admin_tag_set.timeout = NVME_ADMIN_TIMEOUT;
> -	ctrl->admin_tag_set.flags = BLK_MQ_F_NO_SCHED;
>   
>   	ret = blk_mq_alloc_tag_set(&ctrl->admin_tag_set);
>   	if (ret)
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index f8658f984d64..6a3fd759415a 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -938,4 +938,21 @@ static inline bool nvme_multi_css(struct nvme_ctrl *ctrl)
>   	return (ctrl->ctrl_config & NVME_CC_CSS_MASK) == NVME_CC_CSS_CSI;
>   }
>   
> +static inline void nvme_init_admin_tagset(struct blk_mq_tag_set *set,
> +					  void *driver_data, int numa_node,
> +					  const struct blk_mq_ops *ops,
> +					  unsigned int flags,
> +					  unsigned int reserved_tags)
> +{
> +	memset(set, 0, sizeof(*set));
> +	set->driver_data = driver_data;
> +	set->numa_node = numa_node;
> +	set->ops = ops;
> +	set->flags = flags;
> +	set->reserved_tags = reserved_tags;
> +	set->queue_depth = NVME_AQ_MQ_TAG_DEPTH;
> +	set->nr_hw_queues = 1;
> +	set->timeout = NVME_ADMIN_TIMEOUT;
> +}

Not sure what this buys us... helper that takes 6 arguments and
sets 2 duplicated values?

Re: [PATCH 1/9] nvme-fc: add a helper to initialize connect_q

[Christoph Hellwig]

On Fri, Jan 28, 2022 at 01:20:07AM -0800, Chaitanya Kulkarni wrote:
> +static inline int nvme_ctrl_init_connect_q(struct nvme_ctrl *ctrl)
> +{
> +	ctrl->connect_q = blk_mq_init_queue(ctrl->tagset);
> +	if (IS_ERR(ctrl->connect_q))
> +		return PTR_ERR(ctrl->connect_q);
> +	return 0;
> +}

This should be out of line in fabrics.c.

Also I'd prefer one patch to switch everyone to the helper in this
case.

Re: [PATCH 5/9] nvme-fc: centralize admin tagset initialization

[Christoph Hellwig]

On Fri, Jan 28, 2022 at 01:20:11AM -0800, Chaitanya Kulkarni wrote:
> +	nvme_init_admin_tagset(&(ctrl->admin_tag_set), ctrl,

No need for the braceses.

> +			       ctrl->ctrl.numa_node, &nvme_fc_admin_mq_ops,
> +			       BLK_MQ_F_NO_SCHED, NVMF_RESERVED_TAGS);

But do we need all these args?  We should move the admin_tag_sert to the
generic nvme_ctrl and just pass that and the ops.  Everything else
should be same (I hope it currently is, otherwise we need to fix) or
easily derivable (e.g. the check if it is fabrics).

Then mve the helper out of line please and convert all callers in one
patch.

Re: [PATCH 5/9] nvme-fc: centralize admin tagset initialization

[Chaitanya Kulkarni]

On 2/1/22 12:11 AM, Christoph Hellwig wrote:
> External email: Use caution opening links or attachments
> 
> 
> On Fri, Jan 28, 2022 at 01:20:11AM -0800, Chaitanya Kulkarni wrote:
>> +     nvme_init_admin_tagset(&(ctrl->admin_tag_set), ctrl,
> 
> No need for the braceses.
> 
>> +                            ctrl->ctrl.numa_node, &nvme_fc_admin_mq_ops,
>> +                            BLK_MQ_F_NO_SCHED, NVMF_RESERVED_TAGS);
> 
> But do we need all these args?  We should move the admin_tag_sert to the
> generic nvme_ctrl and just pass that and the ops.  Everything else
> should be same (I hope it currently is, otherwise we need to fix) or
> easily derivable (e.g. the check if it is fabrics).
> 
> Then mve the helper out of line please and convert all callers in one
> patch.
> 

Okay, will send out v2 with above comments.

Re: [PATCH 5/9] nvme-fc: centralize admin tagset initialization

[Chaitanya Kulkarni]

Sagi,

>> +static inline void nvme_init_admin_tagset(struct blk_mq_tag_set *set,
>> +                                       void *driver_data, int numa_node,
>> +                                       const struct blk_mq_ops *ops,
>> +                                       unsigned int flags,
>> +                                       unsigned int reserved_tags)
>> +{
>> +     memset(set, 0, sizeof(*set));
>> +     set->driver_data = driver_data;
>> +     set->numa_node = numa_node;
>> +     set->ops = ops;
>> +     set->flags = flags;
>> +     set->reserved_tags = reserved_tags;
>> +     set->queue_depth = NVME_AQ_MQ_TAG_DEPTH;
>> +     set->nr_hw_queues = 1;
>> +     set->timeout = NVME_ADMIN_TIMEOUT;
>> +}
> 
> Not sure what this buys us... helper that takes 6 arguments and
> sets 2 duplicated values?

It just that code is duplicated all over the place and most fabrics
controller use common values for admin tagset such as queue depth,
nr_hw_queues, and timeout.

-ck