[PATCH 00/14] RDMA/cxgb: Fine-tuning for several function implementations

[PATCH 00/14] RDMA/cxgb: Fine-tuning for several function implementations

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 8 Feb 2017 22:00:10 +0100

Several update suggestions were taken into account
from static source code analysis.

Markus Elfring (14):
  Use kcalloc() in cxio_create_qp()
  Rename jump labels in cxio_create_qp()
  Use kcalloc() in two functions
  Use common error handling code in recover_queues()
  Return an error code only as a constant in ep_open()
  Return an error code only as a constant in stag_open()
  Adjust three checks for null pointers
  Adjust construction of two error messages in recover_lost_dbs()
  Use common error handling code in recover_lost_dbs()
  Use kmalloc_array() in c4iw_id_table_alloc()
  Use kcalloc() in create_qp()
  Delete an unnecessary variable initialisation in create_qp()
  Rename jump labels in c4iw_create_qp()
  Rename jump labels in rdma_init()

 drivers/infiniband/hw/cxgb3/cxio_hal.c | 20 ++++----
 drivers/infiniband/hw/cxgb4/device.c   | 83 +++++++++++++++-------------------
 drivers/infiniband/hw/cxgb4/id_table.c |  5 +-
 drivers/infiniband/hw/cxgb4/qp.c       | 48 ++++++++++----------
 4 files changed, 75 insertions(+), 81 deletions(-)

-- 
2.11.1

[PATCH 01/14] RDMA/cxgb3: Use kcalloc() in cxio_create_qp()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 8 Feb 2017 11:37:13 +0100

* Multiplications for the size determination of memory allocations
  indicated that array data structures should be processed.
  Thus use the corresponding function "kcalloc".

  This issue was detected by using the Coccinelle software.

* Replace the specification of data structures by pointer dereferences
  to make the corresponding size determination a bit safer according to
  the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/infiniband/hw/cxgb3/cxio_hal.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/cxgb3/cxio_hal.c b/drivers/infiniband/hw/cxgb3/cxio_hal.c
index ada2e5009c86..aeb1dfa8bfe5 100644
--- a/drivers/infiniband/hw/cxgb3/cxio_hal.c
+++ b/drivers/infiniband/hw/cxgb3/cxio_hal.c
@@ -281,7 +281,7 @@ int cxio_create_qp(struct cxio_rdev *rdev_p, u32 kernel_domain,
 	if (!wq->qpid)
 		return -ENOMEM;
 
-	wq->rq = kzalloc(depth * sizeof(struct t3_swrq), GFP_KERNEL);
+	wq->rq = kcalloc(depth, sizeof(*wq->rq), GFP_KERNEL);
 	if (!wq->rq)
 		goto err1;
 
@@ -289,7 +289,7 @@ int cxio_create_qp(struct cxio_rdev *rdev_p, u32 kernel_domain,
 	if (!wq->rq_addr)
 		goto err2;
 
-	wq->sq = kzalloc(depth * sizeof(struct t3_swsq), GFP_KERNEL);
+	wq->sq = kcalloc(depth, sizeof(*wq->sq), GFP_KERNEL);
 	if (!wq->sq)
 		goto err3;
 
-- 
2.11.1

[PATCH 02/14] RDMA/cxgb3: Rename jump labels in cxio_create_qp()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 8 Feb 2017 14:28:25 +0100

Adjust jump labels according to the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/infiniband/hw/cxgb3/cxio_hal.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/infiniband/hw/cxgb3/cxio_hal.c b/drivers/infiniband/hw/cxgb3/cxio_hal.c
index aeb1dfa8bfe5..113f614b07d4 100644
--- a/drivers/infiniband/hw/cxgb3/cxio_hal.c
+++ b/drivers/infiniband/hw/cxgb3/cxio_hal.c
@@ -283,21 +283,21 @@ int cxio_create_qp(struct cxio_rdev *rdev_p, u32 kernel_domain,
 
 	wq->rq = kcalloc(depth, sizeof(*wq->rq), GFP_KERNEL);
 	if (!wq->rq)
-		goto err1;
+		goto put_pid;
 
 	wq->rq_addr = cxio_hal_rqtpool_alloc(rdev_p, rqsize);
 	if (!wq->rq_addr)
-		goto err2;
+		goto free_rq;
 
 	wq->sq = kcalloc(depth, sizeof(*wq->sq), GFP_KERNEL);
 	if (!wq->sq)
-		goto err3;
+		goto free_pool;
 
 	wq->queue = dma_alloc_coherent(&(rdev_p->rnic_info.pdev->dev),
 					     depth * sizeof(union t3_wr),
 					     &(wq->dma_addr), GFP_KERNEL);
 	if (!wq->queue)
-		goto err4;
+		goto free_sq;
 
 	memset(wq->queue, 0, depth * sizeof(union t3_wr));
 	dma_unmap_addr_set(wq, mapping, wq->dma_addr);
@@ -309,13 +309,13 @@ int cxio_create_qp(struct cxio_rdev *rdev_p, u32 kernel_domain,
 	PDBG("%s qpid 0x%x doorbell 0x%p udb 0x%llx\n", __func__,
 	     wq->qpid, wq->doorbell, (unsigned long long) wq->udb);
 	return 0;
-err4:
+free_sq:
 	kfree(wq->sq);
-err3:
+free_pool:
 	cxio_hal_rqtpool_free(rdev_p, wq->rq_addr, rqsize);
-err2:
+free_rq:
 	kfree(wq->rq);
-err1:
+put_pid:
 	put_qpid(rdev_p, wq->qpid, uctx);
 	return -ENOMEM;
 }
-- 
2.11.1

[PATCH 03/14] RDMA/cxgb4: Use kcalloc() in two functions

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 8 Feb 2017 15:40:16 +0100

Multiplications for the size determination of memory allocations
indicated that array data structures should be processed.
Thus use the corresponding function "kcalloc".

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/infiniband/hw/cxgb4/device.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/cxgb4/device.c b/drivers/infiniband/hw/cxgb4/device.c
index 4e4f1a732b01..444c28206dae 100644
--- a/drivers/infiniband/hw/cxgb4/device.c
+++ b/drivers/infiniband/hw/cxgb4/device.c
@@ -873,7 +873,7 @@ static int c4iw_rdev_open(struct c4iw_rdev *rdev)
 	rdev->status_page->cq_size = rdev->lldi.vr->cq.size;
 
 	if (c4iw_wr_log) {
-		rdev->wr_log = kzalloc((1 << c4iw_wr_log_size_order) *
+		rdev->wr_log = kcalloc(1 << c4iw_wr_log_size_order,
 				       sizeof(*rdev->wr_log), GFP_KERNEL);
 		if (rdev->wr_log) {
 			rdev->wr_log_size = 1 << c4iw_wr_log_size_order;
@@ -1466,7 +1466,7 @@ static void recover_queues(struct uld_ctx *ctx)
 	ctx->dev->db_state = RECOVERY;
 	idr_for_each(&ctx->dev->qpidr, count_qps, &count);
 
-	qp_list.qps = kzalloc(count * sizeof *qp_list.qps, GFP_ATOMIC);
+	qp_list.qps = kcalloc(count, sizeof(*qp_list.qps), GFP_ATOMIC);
 	if (!qp_list.qps) {
 		spin_unlock_irq(&ctx->dev->lock);
 		return;
-- 
2.11.1

[PATCH 04/14] RDMA/cxgb4: Use common error handling code in recover_queues()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 8 Feb 2017 15:51:58 +0100

Add a jump target so that a bit of exception handling can be better reused
at the end of this function.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/infiniband/hw/cxgb4/device.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/hw/cxgb4/device.c b/drivers/infiniband/hw/cxgb4/device.c
index 444c28206dae..46410c4a9afb 100644
--- a/drivers/infiniband/hw/cxgb4/device.c
+++ b/drivers/infiniband/hw/cxgb4/device.c
@@ -1467,10 +1467,9 @@ static void recover_queues(struct uld_ctx *ctx)
 	idr_for_each(&ctx->dev->qpidr, count_qps, &count);
 
 	qp_list.qps = kcalloc(count, sizeof(*qp_list.qps), GFP_ATOMIC);
-	if (!qp_list.qps) {
-		spin_unlock_irq(&ctx->dev->lock);
-		return;
-	}
+	if (!qp_list.qps)
+		goto unlock;
+
 	qp_list.idx = 0;
 
 	/* add and ref each qp so it doesn't get freed */
@@ -1488,6 +1487,7 @@ static void recover_queues(struct uld_ctx *ctx)
 	spin_lock_irq(&ctx->dev->lock);
 	WARN_ON(ctx->dev->db_state != RECOVERY);
 	ctx->dev->db_state = STOPPED;
+unlock:
 	spin_unlock_irq(&ctx->dev->lock);
 }
 
-- 
2.11.1

[PATCH 05/14] RDMA/cxgb4: Return an error code only as a constant in ep_open()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 8 Feb 2017 16:36:31 +0100

* Return an error code without storing it in an intermediate variable.

* Adjust jump targets according to the Linux coding style convention.

* Delete the local variable "ret" which became unnecessary with
  this refactoring.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/infiniband/hw/cxgb4/device.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/infiniband/hw/cxgb4/device.c b/drivers/infiniband/hw/cxgb4/device.c
index 46410c4a9afb..51801a7d4fb3 100644
--- a/drivers/infiniband/hw/cxgb4/device.c
+++ b/drivers/infiniband/hw/cxgb4/device.c
@@ -679,14 +679,12 @@ static int ep_release(struct inode *inode, struct file *file)
 static int ep_open(struct inode *inode, struct file *file)
 {
 	struct c4iw_debugfs_data *epd;
-	int ret = 0;
 	int count = 1;
 
 	epd = kmalloc(sizeof(*epd), GFP_KERNEL);
-	if (!epd) {
-		ret = -ENOMEM;
-		goto out;
-	}
+	if (!epd)
+		goto failure_indication;
+
 	epd->devp = inode->i_private;
 	epd->pos = 0;
 
@@ -698,10 +696,8 @@ static int ep_open(struct inode *inode, struct file *file)
 
 	epd->bufsize = count * 240;
 	epd->buf = vmalloc(epd->bufsize);
-	if (!epd->buf) {
-		ret = -ENOMEM;
-		goto err1;
-	}
+	if (!epd->buf)
+		goto free_epd;
 
 	spin_lock_irq(&epd->devp->lock);
 	idr_for_each(&epd->devp->hwtid_idr, dump_ep, epd);
@@ -710,11 +706,11 @@ static int ep_open(struct inode *inode, struct file *file)
 	spin_unlock_irq(&epd->devp->lock);
 
 	file->private_data = epd;
-	goto out;
-err1:
+	return 0;
+free_epd:
 	kfree(epd);
-out:
-	return ret;
+failure_indication:
+	return -ENOMEM;
 }
 
 static const struct file_operations ep_debugfs_fops = {
-- 
2.11.1

[PATCH 06/14] RDMA/cxgb4: Return an error code only as a constant in stag_open()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 8 Feb 2017 16:50:52 +0100

* Return an error code without storing it in an intermediate variable.

* Adjust jump targets according to the Linux coding style convention.

* Delete the local variable "ret" which became unnecessary with
  this refactoring.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/infiniband/hw/cxgb4/device.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/infiniband/hw/cxgb4/device.c b/drivers/infiniband/hw/cxgb4/device.c
index 51801a7d4fb3..90fa96abb5bc 100644
--- a/drivers/infiniband/hw/cxgb4/device.c
+++ b/drivers/infiniband/hw/cxgb4/device.c
@@ -433,14 +433,12 @@ static int stag_release(struct inode *inode, struct file *file)
 static int stag_open(struct inode *inode, struct file *file)
 {
 	struct c4iw_debugfs_data *stagd;
-	int ret = 0;
 	int count = 1;
 
 	stagd = kmalloc(sizeof *stagd, GFP_KERNEL);
-	if (!stagd) {
-		ret = -ENOMEM;
-		goto out;
-	}
+	if (!stagd)
+		goto failure_indication;
+
 	stagd->devp = inode->i_private;
 	stagd->pos = 0;
 
@@ -450,10 +448,8 @@ static int stag_open(struct inode *inode, struct file *file)
 
 	stagd->bufsize = count * 256;
 	stagd->buf = vmalloc(stagd->bufsize);
-	if (!stagd->buf) {
-		ret = -ENOMEM;
-		goto err1;
-	}
+	if (!stagd->buf)
+		goto free_stagd;
 
 	spin_lock_irq(&stagd->devp->lock);
 	idr_for_each(&stagd->devp->mmidr, dump_stag, stagd);
@@ -461,11 +457,11 @@ static int stag_open(struct inode *inode, struct file *file)
 
 	stagd->buf[stagd->pos++] = 0;
 	file->private_data = stagd;
-	goto out;
-err1:
+	return 0;
+free_stagd:
 	kfree(stagd);
-out:
-	return ret;
+failure_indication:
+	return -ENOMEM;
 }
 
 static const struct file_operations stag_debugfs_fops = {
-- 
2.11.1

[PATCH 07/14] RDMA/cxgb4: Adjust three checks for null pointers

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 8 Feb 2017 17:10:14 +0100
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The script "checkpatch.pl" pointed information out like the following.

Comparison to NULL could be written !…

Thus fix the affected source code places.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/infiniband/hw/cxgb4/device.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/hw/cxgb4/device.c b/drivers/infiniband/hw/cxgb4/device.c
index 90fa96abb5bc..7f30bfd71eb2 100644
--- a/drivers/infiniband/hw/cxgb4/device.c
+++ b/drivers/infiniband/hw/cxgb4/device.c
@@ -1126,10 +1126,10 @@ static inline int recv_rx_pkt(struct c4iw_dev *dev, const struct pkt_gl *gl,
 		goto out;
 
 	skb = copy_gl_to_skb_pkt(gl , rsp, dev->rdev.lldi.sge_pktshift);
-	if (skb == NULL)
+	if (!skb)
 		goto out;
 
-	if (c4iw_handlers[opcode] == NULL) {
+	if (!c4iw_handlers[opcode]) {
 		pr_info("%s no handler opcode 0x%x...\n", __func__,
 		       opcode);
 		kfree_skb(skb);
@@ -1149,7 +1149,7 @@ static int c4iw_uld_rx_handler(void *handle, const __be64 *rsp,
 	struct sk_buff *skb;
 	u8 opcode;
 
-	if (gl == NULL) {
+	if (!gl) {
 		/* omit RSS and rsp_ctrl at end of descriptor */
 		unsigned int len = 64 - sizeof(struct rsp_ctrl) - 8;
 
-- 
2.11.1

[PATCH 08/14] RDMA/cxgb4: Adjust construction of two error messages in recover_lost_dbs()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 8 Feb 2017 17:48:04 +0100

The script "checkpatch.pl" pointed information out like the following.

WARNING: quoted string split across lines

Thus adjust the error message constructions in this function.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/infiniband/hw/cxgb4/device.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/infiniband/hw/cxgb4/device.c b/drivers/infiniband/hw/cxgb4/device.c
index 7f30bfd71eb2..2931920d7c4c 100644
--- a/drivers/infiniband/hw/cxgb4/device.c
+++ b/drivers/infiniband/hw/cxgb4/device.c
@@ -1398,10 +1398,10 @@ static void recover_lost_dbs(struct uld_ctx *ctx, struct qp_list *qp_list)
 					  t4_sq_host_wq_pidx(&qp->wq),
 					  t4_sq_wq_size(&qp->wq));
 		if (ret) {
-			pr_err(MOD "%s: Fatal error - "
-			       "DB overflow recovery failed - "
-			       "error syncing SQ qid %u\n",
-			       pci_name(ctx->lldi.pdev), qp->wq.sq.qid);
+			pr_err(MOD "%s%sSQ qid %u\n",
+			       pci_name(ctx->lldi.pdev),
+			       ": Fatal error - DB overflow recovery failed - error syncing ",
+			       qp->wq.sq.qid);
 			spin_unlock(&qp->lock);
 			spin_unlock_irq(&qp->rhp->lock);
 			return;
@@ -1414,10 +1414,10 @@ static void recover_lost_dbs(struct uld_ctx *ctx, struct qp_list *qp_list)
 					  t4_rq_wq_size(&qp->wq));
 
 		if (ret) {
-			pr_err(MOD "%s: Fatal error - "
-			       "DB overflow recovery failed - "
-			       "error syncing RQ qid %u\n",
-			       pci_name(ctx->lldi.pdev), qp->wq.rq.qid);
+			pr_err(MOD "%s%sRQ qid %u\n",
+			       pci_name(ctx->lldi.pdev),
+			       ": Fatal error - DB overflow recovery failed - error syncing ",
+			       qp->wq.rq.qid);
 			spin_unlock(&qp->lock);
 			spin_unlock_irq(&qp->rhp->lock);
 			return;
-- 
2.11.1

[PATCH 09/14] RDMA/cxgb4: Use common error handling code in recover_lost_dbs()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 8 Feb 2017 18:08:08 +0100

Add a jump target so that a bit of exception handling can be better reused
from an in branch in this function.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/infiniband/hw/cxgb4/device.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/hw/cxgb4/device.c b/drivers/infiniband/hw/cxgb4/device.c
index 2931920d7c4c..86cf7026619f 100644
--- a/drivers/infiniband/hw/cxgb4/device.c
+++ b/drivers/infiniband/hw/cxgb4/device.c
@@ -1402,9 +1402,7 @@ static void recover_lost_dbs(struct uld_ctx *ctx, struct qp_list *qp_list)
 			       pci_name(ctx->lldi.pdev),
 			       ": Fatal error - DB overflow recovery failed - error syncing ",
 			       qp->wq.sq.qid);
-			spin_unlock(&qp->lock);
-			spin_unlock_irq(&qp->rhp->lock);
-			return;
+			goto unlock;
 		}
 		qp->wq.sq.wq_pidx_inc = 0;
 
@@ -1418,6 +1416,7 @@ static void recover_lost_dbs(struct uld_ctx *ctx, struct qp_list *qp_list)
 			       pci_name(ctx->lldi.pdev),
 			       ": Fatal error - DB overflow recovery failed - error syncing ",
 			       qp->wq.rq.qid);
+unlock:
 			spin_unlock(&qp->lock);
 			spin_unlock_irq(&qp->rhp->lock);
 			return;
-- 
2.11.1

[PATCH 10/14] RDMA/cxgb4: Use kmalloc_array() in c4iw_id_table_alloc()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 8 Feb 2017 20:03:24 +0100

* A multiplication for the size determination of a memory allocation
  indicated that an array data structure should be processed.
  Thus use the corresponding function "kmalloc_array".

  This issue was detected by using the Coccinelle software.

* Replace the specification of a data type by a pointer dereference
  to make the corresponding size determination a bit safer according to
  the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/infiniband/hw/cxgb4/id_table.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/cxgb4/id_table.c b/drivers/infiniband/hw/cxgb4/id_table.c
index 0161ae6ad629..51a0b7244e35 100644
--- a/drivers/infiniband/hw/cxgb4/id_table.c
+++ b/drivers/infiniband/hw/cxgb4/id_table.c
@@ -93,8 +93,9 @@ int c4iw_id_table_alloc(struct c4iw_id_table *alloc, u32 start, u32 num,
 		alloc->last = 0;
 	alloc->max  = num;
 	spin_lock_init(&alloc->lock);
-	alloc->table = kmalloc(BITS_TO_LONGS(num) * sizeof(long),
-				GFP_KERNEL);
+	alloc->table = kmalloc_array(BITS_TO_LONGS(num),
+				     sizeof(*alloc->table),
+				     GFP_KERNEL);
 	if (!alloc->table)
 		return -ENOMEM;
 
-- 
2.11.1

[PATCH 11/14] RDMA/cxgb4: Use kcalloc() in create_qp()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 8 Feb 2017 20:27:51 +0100

Multiplications for the size determination of memory allocations
indicated that array data structures should be processed.
Thus use the corresponding function "kcalloc".

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/infiniband/hw/cxgb4/qp.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/hw/cxgb4/qp.c b/drivers/infiniband/hw/cxgb4/qp.c
index d4fd2f5c8326..373d66a511a8 100644
--- a/drivers/infiniband/hw/cxgb4/qp.c
+++ b/drivers/infiniband/hw/cxgb4/qp.c
@@ -216,15 +216,17 @@ static int create_qp(struct c4iw_rdev *rdev, struct t4_wq *wq,
 	}
 
 	if (!user) {
-		wq->sq.sw_sq = kzalloc(wq->sq.size * sizeof *wq->sq.sw_sq,
-				 GFP_KERNEL);
+		wq->sq.sw_sq = kcalloc(wq->sq.size,
+				       sizeof(*wq->sq.sw_sq),
+				       GFP_KERNEL);
 		if (!wq->sq.sw_sq) {
 			ret = -ENOMEM;
 			goto free_rq_qid;
 		}
 
-		wq->rq.sw_rq = kzalloc(wq->rq.size * sizeof *wq->rq.sw_rq,
-				 GFP_KERNEL);
+		wq->rq.sw_rq = kcalloc(wq->rq.size,
+				       sizeof(*wq->rq.sw_rq),
+				       GFP_KERNEL);
 		if (!wq->rq.sw_rq) {
 			ret = -ENOMEM;
 			goto free_sw_sq;
-- 
2.11.1

[PATCH 12/14] RDMA/cxgb4: Delete an unnecessary variable initialisation in create_qp()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 8 Feb 2017 21:07:07 +0100

The local variable "ret" will be set to an appropriate value a bit later.
Thus omit the explicit initialisation at the beginning in this function.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/infiniband/hw/cxgb4/qp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/cxgb4/qp.c b/drivers/infiniband/hw/cxgb4/qp.c
index 373d66a511a8..ac63b1f70731 100644
--- a/drivers/infiniband/hw/cxgb4/qp.c
+++ b/drivers/infiniband/hw/cxgb4/qp.c
@@ -202,7 +202,7 @@ static int create_qp(struct c4iw_rdev *rdev, struct t4_wq *wq,
 	int wr_len;
 	struct c4iw_wr_wait wr_wait;
 	struct sk_buff *skb;
-	int ret = 0;
+	int ret;
 	int eqsize;
 
 	wq->sq.qid = c4iw_get_qpid(rdev, uctx);
-- 
2.11.1

[PATCH 13/14] RDMA/cxgb4: Rename jump labels in c4iw_create_qp()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 8 Feb 2017 21:12:26 +0100

Adjust jump labels according to the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/infiniband/hw/cxgb4/qp.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/infiniband/hw/cxgb4/qp.c b/drivers/infiniband/hw/cxgb4/qp.c
index ac63b1f70731..c278b1d968b0 100644
--- a/drivers/infiniband/hw/cxgb4/qp.c
+++ b/drivers/infiniband/hw/cxgb4/qp.c
@@ -1817,7 +1817,7 @@ struct ib_qp *c4iw_create_qp(struct ib_pd *pd, struct ib_qp_init_attr *attrs,
 	ret = create_qp(&rhp->rdev, &qhp->wq, &schp->cq, &rchp->cq,
 			ucontext ? &ucontext->uctx : &rhp->rdev.uctx);
 	if (ret)
-		goto err1;
+		goto free_qhp;
 
 	attrs->cap.max_recv_wr = rqsize - 1;
 	attrs->cap.max_send_wr = sqsize - 1;
@@ -1848,35 +1848,35 @@ struct ib_qp *c4iw_create_qp(struct ib_pd *pd, struct ib_qp_init_attr *attrs,
 
 	ret = insert_handle(rhp, &rhp->qpidr, qhp, qhp->wq.sq.qid);
 	if (ret)
-		goto err2;
+		goto destroy_qp;
 
 	if (udata) {
 		sq_key_mm = kmalloc(sizeof(*sq_key_mm), GFP_KERNEL);
 		if (!sq_key_mm) {
 			ret = -ENOMEM;
-			goto err3;
+			goto remove_handle;
 		}
 		rq_key_mm = kmalloc(sizeof(*rq_key_mm), GFP_KERNEL);
 		if (!rq_key_mm) {
 			ret = -ENOMEM;
-			goto err4;
+			goto free_sq_key;
 		}
 		sq_db_key_mm = kmalloc(sizeof(*sq_db_key_mm), GFP_KERNEL);
 		if (!sq_db_key_mm) {
 			ret = -ENOMEM;
-			goto err5;
+			goto free_rq_key;
 		}
 		rq_db_key_mm = kmalloc(sizeof(*rq_db_key_mm), GFP_KERNEL);
 		if (!rq_db_key_mm) {
 			ret = -ENOMEM;
-			goto err6;
+			goto free_sq_db_key;
 		}
 		if (t4_sq_onchip(&qhp->wq.sq)) {
 			ma_sync_key_mm = kmalloc(sizeof(*ma_sync_key_mm),
 						 GFP_KERNEL);
 			if (!ma_sync_key_mm) {
 				ret = -ENOMEM;
-				goto err7;
+				goto free_rq_db_key;
 			}
 			uresp.flags = C4IW_QPF_ONCHIP;
 		} else
@@ -1906,7 +1906,7 @@ struct ib_qp *c4iw_create_qp(struct ib_pd *pd, struct ib_qp_init_attr *attrs,
 		spin_unlock(&ucontext->mmap_lock);
 		ret = ib_copy_to_udata(udata, &uresp, sizeof uresp);
 		if (ret)
-			goto err8;
+			goto free_ma_sync_key;
 		sq_key_mm->key = uresp.sq_key;
 		sq_key_mm->addr = qhp->wq.sq.phys_addr;
 		sq_key_mm->len = PAGE_ALIGN(qhp->wq.sq.memsize);
@@ -1944,22 +1944,22 @@ struct ib_qp *c4iw_create_qp(struct ib_pd *pd, struct ib_qp_init_attr *attrs,
 	     attrs->cap.max_send_wr, qhp->wq.rq.qid, qhp->wq.rq.size,
 	     qhp->wq.rq.memsize, attrs->cap.max_recv_wr);
 	return &qhp->ibqp;
-err8:
+free_ma_sync_key:
 	kfree(ma_sync_key_mm);
-err7:
+free_rq_db_key:
 	kfree(rq_db_key_mm);
-err6:
+free_sq_db_key:
 	kfree(sq_db_key_mm);
-err5:
+free_rq_key:
 	kfree(rq_key_mm);
-err4:
+free_sq_key:
 	kfree(sq_key_mm);
-err3:
+remove_handle:
 	remove_handle(rhp, &rhp->qpidr, qhp->wq.sq.qid);
-err2:
+destroy_qp:
 	destroy_qp(&rhp->rdev, &qhp->wq,
 		   ucontext ? &ucontext->uctx : &rhp->rdev.uctx);
-err1:
+free_qhp:
 	kfree(qhp);
 	return ERR_PTR(ret);
 }
-- 
2.11.1

[PATCH 14/14] RDMA/cxgb4: Rename jump labels in rdma_init()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 8 Feb 2017 21:21:47 +0100

Adjust a jump label according to the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/infiniband/hw/cxgb4/qp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/cxgb4/qp.c b/drivers/infiniband/hw/cxgb4/qp.c
index c278b1d968b0..29b63eb110a6 100644
--- a/drivers/infiniband/hw/cxgb4/qp.c
+++ b/drivers/infiniband/hw/cxgb4/qp.c
@@ -1467,13 +1467,13 @@ static int rdma_init(struct c4iw_dev *rhp, struct c4iw_qp *qhp)
 
 	ret = c4iw_ofld_send(&rhp->rdev, skb);
 	if (ret)
-		goto err1;
+		goto free_ird;
 
 	ret = c4iw_wait_for_reply(&rhp->rdev, &qhp->ep->com.wr_wait,
 				  qhp->ep->hwtid, qhp->wq.sq.qid, __func__);
 	if (!ret)
 		goto out;
-err1:
+free_ird:
 	free_ird(rhp, qhp->attr.max_ird);
 out:
 	PDBG("%s ret %d\n", __func__, ret);
-- 
2.11.1

Re: [PATCH 02/14] RDMA/cxgb3: Rename jump labels in cxio_create_qp()

[Andy Shevchenko]

On Wed, Feb 8, 2017 at 11:12 PM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 8 Feb 2017 14:28:25 +0100
>
> Adjust jump labels according to the Linux coding style convention.

"jump label" might be confusing, "goto label" or "error path label"
sounds better.

What the benefit for working code?

>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/infiniband/hw/cxgb3/cxio_hal.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/infiniband/hw/cxgb3/cxio_hal.c b/drivers/infiniband/hw/cxgb3/cxio_hal.c
> index aeb1dfa8bfe5..113f614b07d4 100644
> --- a/drivers/infiniband/hw/cxgb3/cxio_hal.c
> +++ b/drivers/infiniband/hw/cxgb3/cxio_hal.c
> @@ -283,21 +283,21 @@ int cxio_create_qp(struct cxio_rdev *rdev_p, u32 kernel_domain,
>
>         wq->rq = kcalloc(depth, sizeof(*wq->rq), GFP_KERNEL);
>         if (!wq->rq)
> -               goto err1;
> +               goto put_pid;
>
>         wq->rq_addr = cxio_hal_rqtpool_alloc(rdev_p, rqsize);
>         if (!wq->rq_addr)
> -               goto err2;
> +               goto free_rq;
>
>         wq->sq = kcalloc(depth, sizeof(*wq->sq), GFP_KERNEL);
>         if (!wq->sq)
> -               goto err3;
> +               goto free_pool;
>
>         wq->queue = dma_alloc_coherent(&(rdev_p->rnic_info.pdev->dev),
>                                              depth * sizeof(union t3_wr),
>                                              &(wq->dma_addr), GFP_KERNEL);
>         if (!wq->queue)
> -               goto err4;
> +               goto free_sq;
>
>         memset(wq->queue, 0, depth * sizeof(union t3_wr));
>         dma_unmap_addr_set(wq, mapping, wq->dma_addr);
> @@ -309,13 +309,13 @@ int cxio_create_qp(struct cxio_rdev *rdev_p, u32 kernel_domain,
>         PDBG("%s qpid 0x%x doorbell 0x%p udb 0x%llx\n", __func__,
>              wq->qpid, wq->doorbell, (unsigned long long) wq->udb);
>         return 0;
> -err4:
> +free_sq:
>         kfree(wq->sq);
> -err3:
> +free_pool:
>         cxio_hal_rqtpool_free(rdev_p, wq->rq_addr, rqsize);
> -err2:
> +free_rq:
>         kfree(wq->rq);
> -err1:
> +put_pid:
>         put_qpid(rdev_p, wq->qpid, uctx);
>         return -ENOMEM;
>  }
> --
> 2.11.1
>



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 12/14] RDMA/cxgb4: Delete an unnecessary variable initialisation in create_qp()

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 1280 bytes --]

On Wed, Feb 08, 2017 at 10:22:28PM +0100, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 8 Feb 2017 21:07:07 +0100
>
> The local variable "ret" will be set to an appropriate value a bit later.
> Thus omit the explicit initialisation at the beginning in this function.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/infiniband/hw/cxgb4/qp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/infiniband/hw/cxgb4/qp.c b/drivers/infiniband/hw/cxgb4/qp.c
> index 373d66a511a8..ac63b1f70731 100644
> --- a/drivers/infiniband/hw/cxgb4/qp.c
> +++ b/drivers/infiniband/hw/cxgb4/qp.c
> @@ -202,7 +202,7 @@ static int create_qp(struct c4iw_rdev *rdev, struct t4_wq *wq,
>  	int wr_len;
>  	struct c4iw_wr_wait wr_wait;
>  	struct sk_buff *skb;
> -	int ret = 0;
> +	int ret;

The more clean approach will be to initialize this variable to -ENOMEM
and remove rest "ret = -ENOMEM" from the function.

>  	int eqsize;
>
>  	wq->sq.qid = c4iw_get_qpid(rdev, uctx);
> --
> 2.11.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 04/14] RDMA/cxgb4: Use common error handling code in recover_queues()

[Johannes Thumshirn]

On 02/08/2017 10:14 PM, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 8 Feb 2017 15:51:58 +0100
> 
> Add a jump target so that a bit of exception handling can be better reused
> at the end of this function.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/infiniband/hw/cxgb4/device.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/cxgb4/device.c b/drivers/infiniband/hw/cxgb4/device.c
> index 444c28206dae..46410c4a9afb 100644
> --- a/drivers/infiniband/hw/cxgb4/device.c
> +++ b/drivers/infiniband/hw/cxgb4/device.c
> @@ -1467,10 +1467,9 @@ static void recover_queues(struct uld_ctx *ctx)
>  	idr_for_each(&ctx->dev->qpidr, count_qps, &count);
>  
>  	qp_list.qps = kcalloc(count, sizeof(*qp_list.qps), GFP_ATOMIC);
> -	if (!qp_list.qps) {
> -		spin_unlock_irq(&ctx->dev->lock);
> -		return;
> -	}
> +	if (!qp_list.qps)
> +		goto unlock;
> +
>  	qp_list.idx = 0;
>  
>  	/* add and ref each qp so it doesn't get freed */
> @@ -1488,6 +1487,7 @@ static void recover_queues(struct uld_ctx *ctx)
>  	spin_lock_irq(&ctx->dev->lock);
>  	WARN_ON(ctx->dev->db_state != RECOVERY);
>  	ctx->dev->db_state = STOPPED;
> +unlock:
>  	spin_unlock_irq(&ctx->dev->lock);
>  }
>  
> 

This patch is rather pointless IMHO. Using goto labels only brings
benefits if you have more than one code path jumping to them this one
just confuses the readers.

Byte,
	Johannes
-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH 09/14] RDMA/cxgb4: Use common error handling code in recover_lost_dbs()

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 1517 bytes --]

On Wed, Feb 08, 2017 at 10:19:41PM +0100, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 8 Feb 2017 18:08:08 +0100
>
> Add a jump target so that a bit of exception handling can be better reused
> from an in branch in this function.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/infiniband/hw/cxgb4/device.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/infiniband/hw/cxgb4/device.c b/drivers/infiniband/hw/cxgb4/device.c
> index 2931920d7c4c..86cf7026619f 100644
> --- a/drivers/infiniband/hw/cxgb4/device.c
> +++ b/drivers/infiniband/hw/cxgb4/device.c
> @@ -1402,9 +1402,7 @@ static void recover_lost_dbs(struct uld_ctx *ctx, struct qp_list *qp_list)
>  			       pci_name(ctx->lldi.pdev),
>  			       ": Fatal error - DB overflow recovery failed - error syncing ",
>  			       qp->wq.sq.qid);
> -			spin_unlock(&qp->lock);
> -			spin_unlock_irq(&qp->rhp->lock);
> -			return;
> +			goto unlock;
>  		}
>  		qp->wq.sq.wq_pidx_inc = 0;
>
> @@ -1418,6 +1416,7 @@ static void recover_lost_dbs(struct uld_ctx *ctx, struct qp_list *qp_list)
>  			       pci_name(ctx->lldi.pdev),
>  			       ": Fatal error - DB overflow recovery failed - error syncing ",
>  			       qp->wq.rq.qid);
> +unlock:
>  			spin_unlock(&qp->lock);
>  			spin_unlock_irq(&qp->rhp->lock);
>  			return;


These patches are completely insane.
Goto label in the middle of the loop, just to exit from it.

> --
> 2.11.1
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 00/14] RDMA/cxgb: Fine-tuning for several function implementations

[Doug Ledford]

On Wed, 2017-02-08 at 22:10 +0100, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 8 Feb 2017 22:00:10 +0100
> 
> Several update suggestions were taken into account
> from static source code analysis.
> 
> Markus Elfring (14):
>   Use kcalloc() in cxio_create_qp()
>   Rename jump labels in cxio_create_qp()
>   Use kcalloc() in two functions
>   Use common error handling code in recover_queues()
>   Return an error code only as a constant in ep_open()
>   Return an error code only as a constant in stag_open()
>   Adjust three checks for null pointers
>   Adjust construction of two error messages in recover_lost_dbs()
>   Use common error handling code in recover_lost_dbs()
>   Use kmalloc_array() in c4iw_id_table_alloc()
>   Use kcalloc() in create_qp()
>   Delete an unnecessary variable initialisation in create_qp()
>   Rename jump labels in c4iw_create_qp()
>   Rename jump labels in rdma_init()
> 
>  drivers/infiniband/hw/cxgb3/cxio_hal.c | 20 ++++----
>  drivers/infiniband/hw/cxgb4/device.c   | 83 +++++++++++++++---------
> ----------
>  drivers/infiniband/hw/cxgb4/id_table.c |  5 +-
>  drivers/infiniband/hw/cxgb4/qp.c       | 48 ++++++++++----------
>  4 files changed, 75 insertions(+), 81 deletions(-)

I didn't see enough value in this series to bother picking it up.
 Steve, if there are any patches out of this series you actually want,
please collect them and forward them on to me.

-- 
Doug Ledford <dledford@redhat.com>
    GPG KeyID: B826A3330E572FDD
   
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

Re: [PATCH 00/14] RDMA/cxgb: Fine-tuning for several function implementations

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 1958 bytes --]

On Wed, Apr 05, 2017 at 02:16:14PM -0400, Doug Ledford wrote:
> On Wed, 2017-02-08 at 22:10 +0100, SF Markus Elfring wrote:
> > From: Markus Elfring <elfring@users.sourceforge.net>
> > Date: Wed, 8 Feb 2017 22:00:10 +0100
> >
> > Several update suggestions were taken into account
> > from static source code analysis.
> >
> > Markus Elfring (14):
> >   Use kcalloc() in cxio_create_qp()
> >   Rename jump labels in cxio_create_qp()
> >   Use kcalloc() in two functions
> >   Use common error handling code in recover_queues()
> >   Return an error code only as a constant in ep_open()
> >   Return an error code only as a constant in stag_open()
> >   Adjust three checks for null pointers
> >   Adjust construction of two error messages in recover_lost_dbs()
> >   Use common error handling code in recover_lost_dbs()
> >   Use kmalloc_array() in c4iw_id_table_alloc()
> >   Use kcalloc() in create_qp()
> >   Delete an unnecessary variable initialisation in create_qp()
> >   Rename jump labels in c4iw_create_qp()
> >   Rename jump labels in rdma_init()
> >
> >  drivers/infiniband/hw/cxgb3/cxio_hal.c | 20 ++++----
> >  drivers/infiniband/hw/cxgb4/device.c   | 83 +++++++++++++++---------
> > ----------
> >  drivers/infiniband/hw/cxgb4/id_table.c |  5 +-
> >  drivers/infiniband/hw/cxgb4/qp.c       | 48 ++++++++++----------
> >  4 files changed, 75 insertions(+), 81 deletions(-)
>
> I didn't see enough value in this series to bother picking it up.
>  Steve, if there are any patches out of this series you actually want,
> please collect them and forward them on to me.

Hi Doug,

Can you please use the same pattern for ALL Markus's patches?

It is too time consuming to review his changes and it gives a very little
actual value.

Thanks

>
> --
> Doug Ledford <dledford@redhat.com>
>     GPG KeyID: B826A3330E572FDD
>    
> Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]